On Sat, 25 Jul 2015 16:36:50 +0200 Benjamin Randazzo <[email protected]> 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 <[email protected]> > --- > 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-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
