This series contains several miscellaneous fixes to input validation in
device-mapper. The only potentially controversial commits should be the
last two, which prevent creating devices named ".", "..", or "control".
The other patches fix input validation problems that have existed since
at least the beginning of git history.
Demi Marie Obenour (6):
device-mapper: Check that target specs are sufficiently aligned
device-mapper: Avoid pointer arithmetic overflow
device-mapper: structs and parameter strings must not overlap
device-mapper: Avoid double-fetch of version
device-mapper: Refuse to create device named "control"
device-mapper: "." and ".." are not valid symlink names
drivers/md/dm-ioctl.c | 75 ++++++++++++++++++++++++++++++++++++-------
1 file changed, 63 insertions(+), 12 deletions(-)
--
Sincerely,
Demi Marie Obenour (she/her/hers)
Invisible Things Lab
Using either of these is going to greatly confuse userspace, as they are
not valid symlink names and so creating the usual /dev/mapper/NAME
symlink will not be possible. As creating a device with either of these
names is almost certainly a userspace bug, just error out.
Signed-off-by: Demi Marie Obenour <[email protected]>
---
drivers/md/dm-ioctl.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
index b12592bcb4b2b8513f5da6208fb545203534d7ff..adf0c4becc743e4ad59e1d6b0ef108ddd56f207d 100644
--- a/drivers/md/dm-ioctl.c
+++ b/drivers/md/dm-ioctl.c
@@ -771,8 +771,10 @@ static int check_name(const char *name)
return -EINVAL;
}
- if (strcmp(name, DM_CONTROL_NODE) == 0) {
- DMERR("device name cannot be \"%s\"", DM_CONTROL_NODE);
+ if (strcmp(name, DM_CONTROL_NODE) == 0 ||
+ strcmp(name, ".") == 0 ||
+ strcmp(name, "..") == 0) {
+ DMERR("device name cannot be \"%s\", \".\", or \"..\"", DM_CONTROL_NODE);
return -EINVAL;
}
--
Sincerely,
Demi Marie Obenour (she/her/hers)
Invisible Things Lab
The NUL terminator for each target parameter string must precede the
following 'struct dm_target_spec'. Otherwise, dm_split_args() might
corrupt this struct. Furthermore, the first 'struct dm_target_spec'
must come after the 'struct dm_ioctl', as if it overlaps too much
dm_split_args() could corrupt the 'struct dm_ioctl'.
Signed-off-by: Demi Marie Obenour <[email protected]>
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: [email protected]
---
drivers/md/dm-ioctl.c | 28 +++++++++++++++++++++-------
1 file changed, 21 insertions(+), 7 deletions(-)
diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
index 64e8f16d344c47057de5e2d29e3d63202197dca0..da6ca26b51d0953df380582bb3a51c2ec22c27cb 100644
--- a/drivers/md/dm-ioctl.c
+++ b/drivers/md/dm-ioctl.c
@@ -1391,7 +1391,7 @@ static inline fmode_t get_mode(struct dm_ioctl *param)
return mode;
}
-static int next_target(struct dm_target_spec *last, uint32_t next, void *end,
+static int next_target(struct dm_target_spec *last, uint32_t next, const char *end,
struct dm_target_spec **spec, char **target_params)
{
static_assert(_Alignof(struct dm_target_spec) <= 8,
@@ -1404,7 +1404,7 @@ static int next_target(struct dm_target_spec *last, uint32_t next, void *end,
* sizeof(struct dm_target_spec) or more, as otherwise *last was
* out of bounds already.
*/
- size_t remaining = (char *)end - (char *)last;
+ size_t remaining = end - (char *)last;
/*
* There must be room for both the next target spec and the
@@ -1423,10 +1423,7 @@ static int next_target(struct dm_target_spec *last, uint32_t next, void *end,
*spec = (struct dm_target_spec *) ((unsigned char *) last + next);
*target_params = (char *) (*spec + 1);
- if (*spec < (last + 1))
- return -EINVAL;
-
- return invalid_str(*target_params, end);
+ return 0;
}
static int populate_table(struct dm_table *table,
@@ -1436,8 +1433,9 @@ static int populate_table(struct dm_table *table,
unsigned int i = 0;
struct dm_target_spec *spec = (struct dm_target_spec *) param;
uint32_t next = param->data_start;
- void *end = (void *) param + param_size;
+ const char *const end = (const char *) param + param_size;
char *target_params;
+ size_t min_size = sizeof(struct dm_ioctl);
if (!param->target_count) {
DMERR("%s: no targets specified", __func__);
@@ -1445,6 +1443,13 @@ static int populate_table(struct dm_table *table,
}
for (i = 0; i < param->target_count; i++) {
+ const char *nul_terminator;
+
+ if (next < min_size) {
+ DMERR("%s: next target spec (offset %u) overlaps %s",
+ __func__, next, i ? "previous target" : "'struct dm_ioctl'");
+ return -EINVAL;
+ }
r = next_target(spec, next, end, &spec, &target_params);
if (r) {
@@ -1452,6 +1457,15 @@ static int populate_table(struct dm_table *table,
return r;
}
+ nul_terminator = memchr(target_params, 0, (size_t)(end - target_params));
+ if (nul_terminator == NULL) {
+ DMERR("%s: target parameters not NUL-terminated", __func__);
+ return -EINVAL;
+ }
+
+ /* Add 1 for NUL terminator */
+ min_size = (size_t)(nul_terminator - (const char *)spec) + 1;
+
r = dm_table_add_target(table, spec->target_type,
(sector_t) spec->sector_start,
(sector_t) spec->length,
--
Sincerely,
Demi Marie Obenour (she/her/hers)
Invisible Things Lab
Otherwise subsequent code will dereference a misaligned
`struct dm_target_spec *`, which is undefined behavior.
Signed-off-by: Demi Marie Obenour <[email protected]>
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: [email protected]
---
drivers/md/dm-ioctl.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
index cc77cf3d410921432eb0c62cdede7d55b9aa674a..34fa74c6a70db8aa67aaba3f6a2fc4f38ef736bc 100644
--- a/drivers/md/dm-ioctl.c
+++ b/drivers/md/dm-ioctl.c
@@ -1394,6 +1394,13 @@ static inline fmode_t get_mode(struct dm_ioctl *param)
static int next_target(struct dm_target_spec *last, uint32_t next, void *end,
struct dm_target_spec **spec, char **target_params)
{
+ static_assert(_Alignof(struct dm_target_spec) <= 8,
+ "struct dm_target_spec has excessive alignment requirements");
+ if (next % 8) {
+ DMERR("Next target spec (offset %u) is not 8-byte aligned", next);
+ return -EINVAL;
+ }
+
*spec = (struct dm_target_spec *) ((unsigned char *) last + next);
*target_params = (char *) (*spec + 1);
--
Sincerely,
Demi Marie Obenour (she/her/hers)
Invisible Things Lab
This series contains several miscellaneous fixes to input validation in
device-mapper. The only potentially controversial commits should be the
last two, which prevent creating devices named ".", "..", or "control".
The other patches fix input validation problems that have existed since
at least the beginning of git history.
Changes since v1:
- Fix silly mistake (using sizeof() on a pointer) caught by 0day bot.
Demi Marie Obenour (6):
device-mapper: Check that target specs are sufficiently aligned
device-mapper: Avoid pointer arithmetic overflow
device-mapper: structs and parameter strings must not overlap
device-mapper: Avoid double-fetch of version
device-mapper: Refuse to create device named "control"
device-mapper: "." and ".." are not valid symlink names
drivers/md/dm-ioctl.c | 91 ++++++++++++++++++++++++++++++++++---------
1 file changed, 72 insertions(+), 19 deletions(-)
--
Sincerely,
Demi Marie Obenour (she/her/hers)
Invisible Things Lab
Typical userspace setups create a symlink under /dev/mapper with the
name of the device, but /dev/mapper/control is reserved for the control
device. Therefore, trying to create such a device is almost certain to
be a userspace bug.
Signed-off-by: Demi Marie Obenour <[email protected]>
---
drivers/md/dm-ioctl.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
index 7510afe237d979a5ee71afe87a20d49f631de1aa..5b647ab044e44b0c9d0961b5a336b41f50408f88 100644
--- a/drivers/md/dm-ioctl.c
+++ b/drivers/md/dm-ioctl.c
@@ -767,7 +767,12 @@ static int get_target_version(struct file *filp, struct dm_ioctl *param, size_t
static int check_name(const char *name)
{
if (strchr(name, '/')) {
- DMERR("invalid device name");
+ DMERR("device name cannot contain '/'");
+ return -EINVAL;
+ }
+
+ if (strcmp(name, DM_CONTROL_NODE) == 0) {
+ DMERR("device name cannot be \"%s\"", DM_CONTROL_NODE);
return -EINVAL;
}
--
Sincerely,
Demi Marie Obenour (she/her/hers)
Invisible Things Lab
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: [email protected]
Signed-off-by: Demi Marie Obenour <[email protected]>
---
drivers/md/dm-ioctl.c | 30 ++++++++++++++++++------------
1 file changed, 18 insertions(+), 12 deletions(-)
diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
index da6ca26b51d0953df380582bb3a51c2ec22c27cb..7510afe237d979a5ee71afe87a20d49f631de1aa 100644
--- a/drivers/md/dm-ioctl.c
+++ b/drivers/md/dm-ioctl.c
@@ -1873,30 +1873,33 @@ 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;
- if (copy_from_user(version, user->version, sizeof(version)))
+ if (copy_from_user(kernel_params->version, user->version, sizeof(kernel_params->version)))
return -EFAULT;
- if ((version[0] != DM_VERSION_MAJOR) ||
- (version[1] > DM_VERSION_MINOR)) {
+ if ((kernel_params->version[0] != DM_VERSION_MAJOR) ||
+ (kernel_params->version[1] > DM_VERSION_MINOR)) {
DMERR("ioctl interface mismatch: kernel(%u.%u.%u), user(%u.%u.%u), cmd(%d)",
DM_VERSION_MAJOR, DM_VERSION_MINOR,
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;
}
/*
* Fill in the kernel version.
*/
- version[0] = DM_VERSION_MAJOR;
- version[1] = DM_VERSION_MINOR;
- version[2] = DM_VERSION_PATCHLEVEL;
- if (copy_to_user(user->version, version, sizeof(version)))
+ kernel_params->version[0] = DM_VERSION_MAJOR;
+ kernel_params->version[1] = DM_VERSION_MINOR;
+ kernel_params->version[2] = DM_VERSION_PATCHLEVEL;
+ if (copy_to_user(user->version, kernel_params->version, sizeof(kernel_params->version)))
return -EFAULT;
return r;
@@ -1922,7 +1925,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 +2040,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;
--
Sincerely,
Demi Marie Obenour (she/her/hers)
Invisible Things Lab
On Sat, Jun 03 2023 at 10:52P -0400,
Demi Marie Obenour <[email protected]> wrote:
> 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: [email protected]
> Signed-off-by: Demi Marie Obenour <[email protected]>
> ---
> drivers/md/dm-ioctl.c | 30 ++++++++++++++++++------------
> 1 file changed, 18 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
> index da6ca26b51d0953df380582bb3a51c2ec22c27cb..7510afe237d979a5ee71afe87a20d49f631de1aa 100644
> --- a/drivers/md/dm-ioctl.c
> +++ b/drivers/md/dm-ioctl.c
> @@ -1873,30 +1873,33 @@ 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;
>
> - if (copy_from_user(version, user->version, sizeof(version)))
> + if (copy_from_user(kernel_params->version, user->version, sizeof(kernel_params->version)))
> return -EFAULT;
>
> - if ((version[0] != DM_VERSION_MAJOR) ||
> - (version[1] > DM_VERSION_MINOR)) {
> + if ((kernel_params->version[0] != DM_VERSION_MAJOR) ||
> + (kernel_params->version[1] > DM_VERSION_MINOR)) {
> DMERR("ioctl interface mismatch: kernel(%u.%u.%u), user(%u.%u.%u), cmd(%d)",
> DM_VERSION_MAJOR, DM_VERSION_MINOR,
> 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;
> }
>
> /*
> * Fill in the kernel version.
> */
> - version[0] = DM_VERSION_MAJOR;
> - version[1] = DM_VERSION_MINOR;
> - version[2] = DM_VERSION_PATCHLEVEL;
> - if (copy_to_user(user->version, version, sizeof(version)))
> + kernel_params->version[0] = DM_VERSION_MAJOR;
> + kernel_params->version[1] = DM_VERSION_MINOR;
> + kernel_params->version[2] = DM_VERSION_PATCHLEVEL;
> + if (copy_to_user(user->version, kernel_params->version, sizeof(kernel_params->version)))
> return -EFAULT;
>
> return r;
> @@ -1922,7 +1925,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 +2040,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;
>
I picked this patch up for 6.5, please see:
https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=dm-6.5&id=c71878e9982075eab2e9f6dc5a09ba7b60ac1e24
But FYI, I folded these changes in:
diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
index 526464904fc1..b58a15e212a3 100644
--- a/drivers/md/dm-ioctl.c
+++ b/drivers/md/dm-ioctl.c
@@ -1838,6 +1838,9 @@ static int check_version(unsigned int cmd, struct dm_ioctl __user *user,
{
int r = 0;
+ /* 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;
@@ -1885,7 +1888,7 @@ 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;
- /* Version has been copied from userspace already, avoid TOCTOU */
+ /* 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)))
On Thu, Jun 22, 2023 at 12:20:40PM -0400, Mike Snitzer wrote:
> On Sat, Jun 03 2023 at 10:52P -0400,
> Demi Marie Obenour <[email protected]> wrote:
>
> > 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: [email protected]
> > Signed-off-by: Demi Marie Obenour <[email protected]>
> > ---
> > drivers/md/dm-ioctl.c | 30 ++++++++++++++++++------------
> > 1 file changed, 18 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
> > index da6ca26b51d0953df380582bb3a51c2ec22c27cb..7510afe237d979a5ee71afe87a20d49f631de1aa 100644
> > --- a/drivers/md/dm-ioctl.c
> > +++ b/drivers/md/dm-ioctl.c
> > @@ -1873,30 +1873,33 @@ 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;
> >
> > - if (copy_from_user(version, user->version, sizeof(version)))
> > + if (copy_from_user(kernel_params->version, user->version, sizeof(kernel_params->version)))
> > return -EFAULT;
> >
> > - if ((version[0] != DM_VERSION_MAJOR) ||
> > - (version[1] > DM_VERSION_MINOR)) {
> > + if ((kernel_params->version[0] != DM_VERSION_MAJOR) ||
> > + (kernel_params->version[1] > DM_VERSION_MINOR)) {
> > DMERR("ioctl interface mismatch: kernel(%u.%u.%u), user(%u.%u.%u), cmd(%d)",
> > DM_VERSION_MAJOR, DM_VERSION_MINOR,
> > 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;
> > }
> >
> > /*
> > * Fill in the kernel version.
> > */
> > - version[0] = DM_VERSION_MAJOR;
> > - version[1] = DM_VERSION_MINOR;
> > - version[2] = DM_VERSION_PATCHLEVEL;
> > - if (copy_to_user(user->version, version, sizeof(version)))
> > + kernel_params->version[0] = DM_VERSION_MAJOR;
> > + kernel_params->version[1] = DM_VERSION_MINOR;
> > + kernel_params->version[2] = DM_VERSION_PATCHLEVEL;
> > + if (copy_to_user(user->version, kernel_params->version, sizeof(kernel_params->version)))
> > return -EFAULT;
> >
> > return r;
> > @@ -1922,7 +1925,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 +2040,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;
> >
>
> I picked this patch up for 6.5, please see:
> https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=dm-6.5&id=c71878e9982075eab2e9f6dc5a09ba7b60ac1e24
That’s great, thanks!
> But FYI, I folded these changes in:
>
> diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
> index 526464904fc1..b58a15e212a3 100644
> --- a/drivers/md/dm-ioctl.c
> +++ b/drivers/md/dm-ioctl.c
> @@ -1838,6 +1838,9 @@ static int check_version(unsigned int cmd, struct dm_ioctl __user *user,
> {
> int r = 0;
>
> + /* 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;
>
> @@ -1885,7 +1888,7 @@ 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;
>
> - /* Version has been copied from userspace already, avoid TOCTOU */
> + /* 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)))
Those changes indeed make the code better, thanks for including them!
--
Sincerely,
Demi Marie Obenour (she/her/hers)
Invisible Things Lab