Skip to content

Commit

Permalink
dm ioctl: fix misbehavior if list_versions races with module loading
Browse files Browse the repository at this point in the history
commit 4fe1ec9 upstream.

__list_versions will first estimate the required space using the
"dm_target_iterate(list_version_get_needed, &needed)" call and then will
fill the space using the "dm_target_iterate(list_version_get_info,
&iter_info)" call. Each of these calls locks the targets using the
"down_read(&_lock)" and "up_read(&_lock)" calls, however between the first
and second "dm_target_iterate" there is no lock held and the target
modules can be loaded at this point, so the second "dm_target_iterate"
call may need more space than what was the first "dm_target_iterate"
returned.

The code tries to handle this overflow (see the beginning of
list_version_get_info), however this handling is incorrect.

The code sets "param->data_size = param->data_start + needed" and
"iter_info.end = (char *)vers+len" - "needed" is the size returned by the
first dm_target_iterate call; "len" is the size of the buffer allocated by
userspace.

"len" may be greater than "needed"; in this case, the code will write up
to "len" bytes into the buffer, however param->data_size is set to
"needed", so it may write data past the param->data_size value. The ioctl
interface copies only up to param->data_size into userspace, thus part of
the result will be truncated.

Fix this bug by setting "iter_info.end = (char *)vers + needed;" - this
guarantees that the second "dm_target_iterate" call will write only up to
the "needed" buffer and it will exit with "DM_BUFFER_FULL_FLAG" if it
overflows the "needed" space - in this case, userspace will allocate a
larger buffer and retry.

Note that there is also a bug in list_version_get_needed - we need to add
"strlen(tt->name) + 1" to the needed size, not "strlen(tt->name)".

Cc: stable@vger.kernel.org
Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
Signed-off-by: Mike Snitzer <snitzer@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  • Loading branch information
Mikulas Patocka authored and gregkh committed Nov 26, 2022
1 parent 2b5f885 commit 5398b8e
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions drivers/md/dm-ioctl.c
Expand Up @@ -655,7 +655,7 @@ static void list_version_get_needed(struct target_type *tt, void *needed_param)
size_t *needed = needed_param;

*needed += sizeof(struct dm_target_versions);
*needed += strlen(tt->name);
*needed += strlen(tt->name) + 1;
*needed += ALIGN_MASK;
}

Expand Down Expand Up @@ -720,7 +720,7 @@ static int __list_versions(struct dm_ioctl *param, size_t param_size, const char
iter_info.old_vers = NULL;
iter_info.vers = vers;
iter_info.flags = 0;
iter_info.end = (char *)vers+len;
iter_info.end = (char *)vers + needed;

/*
* Now loop through filling out the names & versions.
Expand Down

0 comments on commit 5398b8e

Please sign in to comment.