dm ioctl: Avoid double-fetch of version
[ Upstream commit 249bed821b4db6d95a99160f7d6d236ea5fe6362 ] The version is fetched once in check_version(), which then does some validation and then overwrites the version in userspace with the API version supported by the kernel. copy_params() then fetches the version from userspace *again*, and this time no validation is done. The result is that the kernel's version number is completely controllable by userspace, provided that userspace can win a race condition. Fix this flaw by not copying the version back to the kernel the second time. This is not exploitable as the version is not further used in the kernel. However, it could become a problem if future patches start relying on the version field. Cc: stable@vger.kernel.org Signed-off-by: Demi Marie Obenour <demi@invisiblethingslab.com> Signed-off-by: Mike Snitzer <snitzer@kernel.org> Signed-off-by: Sasha Levin <sashal@kernel.org>
This commit is contained in:
parent
2798779419
commit
d5eb0375d7
1 changed files with 21 additions and 12 deletions
|
|
@ -1811,30 +1811,36 @@ static ioctl_fn lookup_ioctl(unsigned int cmd, int *ioctl_flags)
|
||||||
* As well as checking the version compatibility this always
|
* As well as checking the version compatibility this always
|
||||||
* copies the kernel interface version out.
|
* copies the kernel interface version out.
|
||||||
*/
|
*/
|
||||||
static int check_version(unsigned int cmd, struct dm_ioctl __user *user)
|
static int check_version(unsigned int cmd, struct dm_ioctl __user *user,
|
||||||
|
struct dm_ioctl *kernel_params)
|
||||||
{
|
{
|
||||||
uint32_t version[3];
|
|
||||||
int r = 0;
|
int r = 0;
|
||||||
|
|
||||||
if (copy_from_user(version, user->version, sizeof(version)))
|
/* Make certain version is first member of dm_ioctl struct */
|
||||||
|
BUILD_BUG_ON(offsetof(struct dm_ioctl, version) != 0);
|
||||||
|
|
||||||
|
if (copy_from_user(kernel_params->version, user->version, sizeof(kernel_params->version)))
|
||||||
return -EFAULT;
|
return -EFAULT;
|
||||||
|
|
||||||
if ((version[0] != DM_VERSION_MAJOR) ||
|
if ((kernel_params->version[0] != DM_VERSION_MAJOR) ||
|
||||||
(version[1] > DM_VERSION_MINOR)) {
|
(kernel_params->version[1] > DM_VERSION_MINOR)) {
|
||||||
DMERR("ioctl interface mismatch: kernel(%u.%u.%u), user(%u.%u.%u), cmd(%d)",
|
DMERR("ioctl interface mismatch: kernel(%u.%u.%u), user(%u.%u.%u), cmd(%d)",
|
||||||
DM_VERSION_MAJOR, DM_VERSION_MINOR,
|
DM_VERSION_MAJOR, DM_VERSION_MINOR,
|
||||||
DM_VERSION_PATCHLEVEL,
|
DM_VERSION_PATCHLEVEL,
|
||||||
version[0], version[1], version[2], cmd);
|
kernel_params->version[0],
|
||||||
|
kernel_params->version[1],
|
||||||
|
kernel_params->version[2],
|
||||||
|
cmd);
|
||||||
r = -EINVAL;
|
r = -EINVAL;
|
||||||
}
|
}
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* Fill in the kernel version.
|
* Fill in the kernel version.
|
||||||
*/
|
*/
|
||||||
version[0] = DM_VERSION_MAJOR;
|
kernel_params->version[0] = DM_VERSION_MAJOR;
|
||||||
version[1] = DM_VERSION_MINOR;
|
kernel_params->version[1] = DM_VERSION_MINOR;
|
||||||
version[2] = DM_VERSION_PATCHLEVEL;
|
kernel_params->version[2] = DM_VERSION_PATCHLEVEL;
|
||||||
if (copy_to_user(user->version, version, sizeof(version)))
|
if (copy_to_user(user->version, kernel_params->version, sizeof(kernel_params->version)))
|
||||||
return -EFAULT;
|
return -EFAULT;
|
||||||
|
|
||||||
return r;
|
return r;
|
||||||
|
|
@ -1860,7 +1866,10 @@ static int copy_params(struct dm_ioctl __user *user, struct dm_ioctl *param_kern
|
||||||
const size_t minimum_data_size = offsetof(struct dm_ioctl, data);
|
const size_t minimum_data_size = offsetof(struct dm_ioctl, data);
|
||||||
unsigned int noio_flag;
|
unsigned int noio_flag;
|
||||||
|
|
||||||
if (copy_from_user(param_kernel, user, minimum_data_size))
|
/* check_version() already copied version from userspace, avoid TOCTOU */
|
||||||
|
if (copy_from_user((char *)param_kernel + sizeof(param_kernel->version),
|
||||||
|
(char __user *)user + sizeof(param_kernel->version),
|
||||||
|
minimum_data_size - sizeof(param_kernel->version)))
|
||||||
return -EFAULT;
|
return -EFAULT;
|
||||||
|
|
||||||
if (param_kernel->data_size < minimum_data_size) {
|
if (param_kernel->data_size < minimum_data_size) {
|
||||||
|
|
@ -1972,7 +1981,7 @@ static int ctl_ioctl(struct file *file, uint command, struct dm_ioctl __user *us
|
||||||
* Check the interface version passed in. This also
|
* Check the interface version passed in. This also
|
||||||
* writes out the kernel's interface version.
|
* writes out the kernel's interface version.
|
||||||
*/
|
*/
|
||||||
r = check_version(cmd, user);
|
r = check_version(cmd, user, ¶m_kernel);
|
||||||
if (r)
|
if (r)
|
||||||
return r;
|
return r;
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue