[dm-devel] [PATCH 2/3] dm-ioctl: fix unsafe use of dm_ioctl.data_size
Mike Snitzer
snitzer at redhat.com
Thu Dec 6 23:30:28 UTC 2012
On Thu, Dec 06 2012 at 4:22pm -0500,
Alasdair G Kergon <agk at redhat.com> 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 at redhat.com>
>
> 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 at redhat.com>
> Signed-off-by: Alasdair G Kergon <agk at redhat.com>
> Cc: stable at kernel.org
Looks good.
Acked-by: Mike Snitzer <snitzer at redhat.com>
More information about the dm-devel
mailing list