Re: [PATCH 2/3] dm-ioctl: fix unsafe use of dm_ioctl.data_size

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Dec 06 2012 at  4:22pm -0500,
Alasdair G Kergon <agk@xxxxxxxxxx> wrote:

> I don't think we need to support the situation when the value changes during
> the copying of the parameters, so how about something more like this instead?
> 
> Alasdair
> 
> 
> From: Alasdair G Kergon <agk@xxxxxxxxxx>
> 
> Abort dm ioctl processing if userspace changes the data_size parameter
> after we validated it but before we finished copying the data buffer
> from userspace.
> 
> The dm ioctl parameters are processed in the following sequence:
>  1. ctl_ioctl() calls copy_params();
>  2. copy_params() makes a first copy of the fixed-sized portion of the
>     userspace parameters into the local variable "tmp";
>  3. copy_params() then validates tmp.data_size and allocates a new
>     structure big enough to hold the complete data and copies the whole
>     userspace buffer there;
>  4. ctl_ioctl() reads userspace data the second time and copies the whole
>     buffer into the pointer "param";
>  5. ctl_ioctl() reads param->data_size without any validation and stores it
>     in the variable "input_param_size";
>  6. "input_param_size" is further used as the authoritative size of the
>     kernel buffer.
> 
> The problem is that userspace code could change the contents of user
> memory between steps 2 and 4.  In particular, the data_size parameter
> can be changed to an invalid value after the kernel has validated it.
> This lets userspace force the kernel to access invalid kernel memory.
> 
> The fix is to ensure that the size has not changed at step 4.
> 
> This patch shouldn't have a security impact because CAP_SYS_ADMIN is
> required to run this code, but it should be fixed anyway.
> 
> Reported-by: Mikulas Patocka <mpatocka@xxxxxxxxxx>
> Signed-off-by: Alasdair G Kergon <agk@xxxxxxxxxx>
> Cc: stable@xxxxxxxxxx

Looks good.

Acked-by: Mike Snitzer <snitzer@xxxxxxxxxx>

--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel


[Index of Archives]     [DM Crypt]     [Fedora Desktop]     [ATA RAID]     [Fedora Marketing]     [Fedora Packaging]     [Fedora SELinux]     [Yosemite Discussion]     [KDE Users]     [Fedora Docs]

  Powered by Linux