On Sat, 25 Jul 2015 16:36:50 +0200 Benjamin Randazzo
<benjamin@xxxxxxxxxxx> wrote:
> In drivers/md/md.c get_bitmap_file() uses kmalloc() for creating a
> mdu_bitmap_file_t called "file".
>
> 5769 file = kmalloc(sizeof(*file), GFP_NOIO);
> 5770 if (!file)
> 5771 return -ENOMEM;
>
> This structure is copied to user space at the end of the function.
>
> 5786 if (err == 0 &&
> 5787 copy_to_user(arg, file, sizeof(*file)))
> 5788 err = -EFAULT
>
> But if bitmap is disabled only the first byte of "file" is initialized
> with zero, so it's possible to read some bytes (up to 4095) of kernel
> space memory from user space. This is an information leak.
>
> 5775 /* bitmap disabled, zero the first byte and copy out */
> 5776 if (!mddev->bitmap_info.file)
> 5777 file->pathname[0] = '\0';
>
> Signed-off-by: Benjamin Randazzo <benjamin@xxxxxxxxxxx>
> ---
> drivers/md/md.c | 21 ++++++++++-----------
> 1 file changed, 10 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 80879dc..382bdbc 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -5766,22 +5766,21 @@ static int get_bitmap_file(struct mddev *mddev, void __user * arg)
> char *ptr;
> int err;
>
> - file = kmalloc(sizeof(*file), GFP_NOIO);
> + file = kzalloc(sizeof(*file), GFP_NOIO);
> if (!file)
> return -ENOMEM;
>
> err = 0;
> spin_lock(&mddev->lock);
> - /* bitmap disabled, zero the first byte and copy out */
> - if (!mddev->bitmap_info.file)
> - file->pathname[0] = '\0';
> - else if ((ptr = file_path(mddev->bitmap_info.file,
> - file->pathname, sizeof(file->pathname))),
> - IS_ERR(ptr))
> - err = PTR_ERR(ptr);
> - else
> - memmove(file->pathname, ptr,
> - sizeof(file->pathname)-(ptr-file->pathname));
> + /* bitmap enabled */
> + if (mddev->bitmap_info.file) {
> + if ((ptr = file_path(mddev->bitmap_info.file, file->pathname,
> + sizeof(file->pathname))), IS_ERR(ptr))
> + err = PTR_ERR(ptr);
> + else
> + memmove(file->pathname, ptr,
> + sizeof(file->pathname)-(ptr-file->pathname));
> + }
> spin_unlock(&mddev->lock);
>
> if (err == 0 &&
Thanks.
I re-arranged the code a little bit more as there is no longer any
excuse for having the "ptr = file_path()" assignment inside the
condition of the 'if'.
Applied. Thanks,
NeilBrown
--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
