| Message ID | 20230601212456.1533-5-demi@invisiblethingslab.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Mike Snitzer |
| Headers | show |
| Series | Several device-mapper fixes | expand |
Hi Demi,
kernel test robot noticed the following build warnings:
[auto build test WARNING on device-mapper-dm/for-next]
[also build test WARNING on linus/master v6.4-rc4 next-20230602]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Demi-Marie-Obenour/device-mapper-Check-that-target-specs-are-sufficiently-aligned/20230602-052741
base: https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git for-next
patch link: https://lore.kernel.org/r/20230601212456.1533-5-demi%40invisiblethingslab.com
patch subject: [PATCH 4/6] device-mapper: Avoid double-fetch of version
config: x86_64-randconfig-c032-20230531 (https://download.01.org/0day-ci/archive/20230603/202306031511.xIeQ4BQz-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202306031511.xIeQ4BQz-lkp@intel.com/
cocci warnings: (new ones prefixed by >>)
>> drivers/md/dm-ioctl.c:1900:42-48: ERROR: application of sizeof to pointer
On Sat, Jun 03, 2023 at 03:40:09PM +0800, kernel test robot wrote: > Hi Demi, > > kernel test robot noticed the following build warnings: > > [auto build test WARNING on device-mapper-dm/for-next] > [also build test WARNING on linus/master v6.4-rc4 next-20230602] > [If your patch is applied to the wrong git tree, kindly drop us a note. > And when submitting patch, we suggest to use '--base' as documented in > https://git-scm.com/docs/git-format-patch#_base_tree_information] > > url: https://github.com/intel-lab-lkp/linux/commits/Demi-Marie-Obenour/device-mapper-Check-that-target-specs-are-sufficiently-aligned/20230602-052741 > base: https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git for-next > patch link: https://lore.kernel.org/r/20230601212456.1533-5-demi%40invisiblethingslab.com > patch subject: [PATCH 4/6] device-mapper: Avoid double-fetch of version > config: x86_64-randconfig-c032-20230531 (https://download.01.org/0day-ci/archive/20230603/202306031511.xIeQ4BQz-lkp@intel.com/config) > compiler: gcc-12 (Debian 12.2.0-14) 12.2.0 > > If you fix the issue, kindly add following tag where applicable > | Reported-by: kernel test robot <lkp@intel.com> > | Closes: https://lore.kernel.org/oe-kbuild-all/202306031511.xIeQ4BQz-lkp@intel.com/ > > cocci warnings: (new ones prefixed by >>) > >> drivers/md/dm-ioctl.c:1900:42-48: ERROR: application of sizeof to pointer Ugh, silly mistake: I changed an array to a pointer but did not change the sizeof. Will send a v2 with the fix.
diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c index da6ca26b51d0953df380582bb3a51c2ec22c27cb..fd46b249f6f856c49752063fc49d720e95df0525 100644 --- a/drivers/md/dm-ioctl.c +++ b/drivers/md/dm-ioctl.c @@ -1873,12 +1873,13 @@ static ioctl_fn lookup_ioctl(unsigned int cmd, int *ioctl_flags) * As well as checking the version compatibility this always * 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; + uint32_t *version = kernel_params->version; - if (copy_from_user(version, user->version, sizeof(version))) + if (copy_from_user(version, user->version, sizeof(user->version))) return -EFAULT; if ((version[0] != DM_VERSION_MAJOR) || @@ -1922,7 +1923,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); unsigned int noio_flag; - if (copy_from_user(param_kernel, user, minimum_data_size)) + /* Version has been copied from userspace already, 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; if (param_kernel->data_size < minimum_data_size) { @@ -2034,7 +2038,7 @@ static int ctl_ioctl(struct file *file, uint command, struct dm_ioctl __user *us * Check the interface version passed in. This also * writes out the kernel's interface version. */ - r = check_version(cmd, user); + r = check_version(cmd, user, ¶m_kernel); if (r) return r;
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. Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Cc: stable@vger.kernel.org Signed-off-by: Demi Marie Obenour <demi@invisiblethingslab.com> --- drivers/md/dm-ioctl.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)