On Tue, 10 Oct 2017, gregkh@xxxxxxxxxxxxxxxxxxx wrote:
>
> The patch below does not apply to the 4.9-stable tree.
> If someone wants it applied there, or to any other stable or longterm
> tree, then please email the backport, including the original git commit
> id to <stable@xxxxxxxxxxxxxxx>.
>
> thanks,
>
> greg k-h
Hi
This patch fixes a feature introduced in the kernel 4.13, so it doesn't
need to be applied to older kernels.
Mikulas
> ------------------ original commit in Linus's tree ------------------
>
> >From 62e082430ea4bb5b28909ca4375bb683931e22aa Mon Sep 17 00:00:00 2001
> From: Mikulas Patocka <mpatocka@xxxxxxxxxx>
> Date: Wed, 20 Sep 2017 07:29:49 -0400
> Subject: [PATCH] dm ioctl: fix alignment of event number in the device list
>
> The size of struct dm_name_list is different on 32-bit and 64-bit
> kernels (so "(nl + 1)" differs between 32-bit and 64-bit kernels).
>
> This mismatch caused some harmless difference in padding when using 32-bit
> or 64-bit kernel. Commit 23d70c5e52dd ("dm ioctl: report event number in
> DM_LIST_DEVICES") added reporting event number in the output of
> DM_LIST_DEVICES_CMD. This difference in padding makes it impossible for
> userspace to determine the location of the event number (the location
> would be different when running on 32-bit and 64-bit kernels).
>
> Fix the padding by using offsetof(struct dm_name_list, name) instead of
> sizeof(struct dm_name_list) to determine the location of entries.
>
> Also, the ioctl version number is incremented to 37 so that userspace
> can use the version number to determine that the event number is present
> and correctly located.
>
> In addition, a global event is now raised when a DM device is created,
> removed, renamed or when table is swapped, so that the user can monitor
> for device changes.
>
> Reported-by: Eugene Syromiatnikov <esyr@xxxxxxxxxx>
> Fixes: 23d70c5e52dd ("dm ioctl: report event number in DM_LIST_DEVICES")
> Cc: stable@xxxxxxxxxxxxxxx # 4.13
> Signed-off-by: Mikulas Patocka <mpatocka@xxxxxxxxxx>
> Signed-off-by: Mike Snitzer <snitzer@xxxxxxxxxx>
>
> diff --git a/drivers/md/dm-core.h b/drivers/md/dm-core.h
> index 24eddbdf2ab4..203144762f36 100644
> --- a/drivers/md/dm-core.h
> +++ b/drivers/md/dm-core.h
> @@ -149,5 +149,6 @@ static inline bool dm_message_test_buffer_overflow(char *result, unsigned maxlen
>
> extern atomic_t dm_global_event_nr;
> extern wait_queue_head_t dm_global_eventq;
> +void dm_issue_global_event(void);
>
> #endif
> diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
> index 8756a6850431..e52676fa9832 100644
> --- a/drivers/md/dm-ioctl.c
> +++ b/drivers/md/dm-ioctl.c
> @@ -477,9 +477,13 @@ static int remove_all(struct file *filp, struct dm_ioctl *param, size_t param_si
> * Round up the ptr to an 8-byte boundary.
> */
> #define ALIGN_MASK 7
> +static inline size_t align_val(size_t val)
> +{
> + return (val + ALIGN_MASK) & ~ALIGN_MASK;
> +}
> static inline void *align_ptr(void *ptr)
> {
> - return (void *) (((size_t) (ptr + ALIGN_MASK)) & ~ALIGN_MASK);
> + return (void *)align_val((size_t)ptr);
> }
>
> /*
> @@ -505,7 +509,7 @@ static int list_devices(struct file *filp, struct dm_ioctl *param, size_t param_
> struct hash_cell *hc;
> size_t len, needed = 0;
> struct gendisk *disk;
> - struct dm_name_list *nl, *old_nl = NULL;
> + struct dm_name_list *orig_nl, *nl, *old_nl = NULL;
> uint32_t *event_nr;
>
> down_write(&_hash_lock);
> @@ -516,17 +520,15 @@ static int list_devices(struct file *filp, struct dm_ioctl *param, size_t param_
> */
> for (i = 0; i < NUM_BUCKETS; i++) {
> list_for_each_entry (hc, _name_buckets + i, name_list) {
> - needed += sizeof(struct dm_name_list);
> - needed += strlen(hc->name) + 1;
> - needed += ALIGN_MASK;
> - needed += (sizeof(uint32_t) + ALIGN_MASK) & ~ALIGN_MASK;
> + needed += align_val(offsetof(struct dm_name_list, name) + strlen(hc->name) + 1);
> + needed += align_val(sizeof(uint32_t));
> }
> }
>
> /*
> * Grab our output buffer.
> */
> - nl = get_result_buffer(param, param_size, &len);
> + nl = orig_nl = get_result_buffer(param, param_size, &len);
> if (len < needed) {
> param->flags |= DM_BUFFER_FULL_FLAG;
> goto out;
> @@ -549,11 +551,16 @@ static int list_devices(struct file *filp, struct dm_ioctl *param, size_t param_
> strcpy(nl->name, hc->name);
>
> old_nl = nl;
> - event_nr = align_ptr(((void *) (nl + 1)) + strlen(hc->name) + 1);
> + event_nr = align_ptr(nl->name + strlen(hc->name) + 1);
> *event_nr = dm_get_event_nr(hc->md);
> nl = align_ptr(event_nr + 1);
> }
> }
> + /*
> + * If mismatch happens, security may be compromised due to buffer
> + * overflow, so it's better to crash.
> + */
> + BUG_ON((char *)nl - (char *)orig_nl != needed);
>
> out:
> up_write(&_hash_lock);
> @@ -1621,7 +1628,8 @@ static int target_message(struct file *filp, struct dm_ioctl *param, size_t para
> * which has a variable size, is not used by the function processing
> * the ioctl.
> */
> -#define IOCTL_FLAGS_NO_PARAMS 1
> +#define IOCTL_FLAGS_NO_PARAMS 1
> +#define IOCTL_FLAGS_ISSUE_GLOBAL_EVENT 2
>
> /*-----------------------------------------------------------------
> * Implementation of open/close/ioctl on the special char
> @@ -1635,12 +1643,12 @@ static ioctl_fn lookup_ioctl(unsigned int cmd, int *ioctl_flags)
> ioctl_fn fn;
> } _ioctls[] = {
> {DM_VERSION_CMD, 0, NULL}, /* version is dealt with elsewhere */
> - {DM_REMOVE_ALL_CMD, IOCTL_FLAGS_NO_PARAMS, remove_all},
> + {DM_REMOVE_ALL_CMD, IOCTL_FLAGS_NO_PARAMS | IOCTL_FLAGS_ISSUE_GLOBAL_EVENT, remove_all},
> {DM_LIST_DEVICES_CMD, 0, list_devices},
>
> - {DM_DEV_CREATE_CMD, IOCTL_FLAGS_NO_PARAMS, dev_create},
> - {DM_DEV_REMOVE_CMD, IOCTL_FLAGS_NO_PARAMS, dev_remove},
> - {DM_DEV_RENAME_CMD, 0, dev_rename},
> + {DM_DEV_CREATE_CMD, IOCTL_FLAGS_NO_PARAMS | IOCTL_FLAGS_ISSUE_GLOBAL_EVENT, dev_create},
> + {DM_DEV_REMOVE_CMD, IOCTL_FLAGS_NO_PARAMS | IOCTL_FLAGS_ISSUE_GLOBAL_EVENT, dev_remove},
> + {DM_DEV_RENAME_CMD, IOCTL_FLAGS_ISSUE_GLOBAL_EVENT, dev_rename},
> {DM_DEV_SUSPEND_CMD, IOCTL_FLAGS_NO_PARAMS, dev_suspend},
> {DM_DEV_STATUS_CMD, IOCTL_FLAGS_NO_PARAMS, dev_status},
> {DM_DEV_WAIT_CMD, 0, dev_wait},
> @@ -1869,6 +1877,9 @@ static int ctl_ioctl(struct file *file, uint command, struct dm_ioctl __user *us
> unlikely(ioctl_flags & IOCTL_FLAGS_NO_PARAMS))
> DMERR("ioctl %d tried to output some data but has IOCTL_FLAGS_NO_PARAMS set", cmd);
>
> + if (!r && ioctl_flags & IOCTL_FLAGS_ISSUE_GLOBAL_EVENT)
> + dm_issue_global_event();
> +
> /*
> * Copy the results back to userland.
> */
> diff --git a/drivers/md/dm.c b/drivers/md/dm.c
> index 6e54145969c5..4be85324f44d 100644
> --- a/drivers/md/dm.c
> +++ b/drivers/md/dm.c
> @@ -52,6 +52,12 @@ static struct workqueue_struct *deferred_remove_workqueue;
> atomic_t dm_global_event_nr = ATOMIC_INIT(0);
> DECLARE_WAIT_QUEUE_HEAD(dm_global_eventq);
>
> +void dm_issue_global_event(void)
> +{
> + atomic_inc(&dm_global_event_nr);
> + wake_up(&dm_global_eventq);
> +}
> +
> /*
> * One of these is allocated per bio.
> */
> @@ -1865,9 +1871,8 @@ static void event_callback(void *context)
> dm_send_uevents(&uevents, &disk_to_dev(md->disk)->kobj);
>
> atomic_inc(&md->event_nr);
> - atomic_inc(&dm_global_event_nr);
> wake_up(&md->eventq);
> - wake_up(&dm_global_eventq);
> + dm_issue_global_event();
> }
>
> /*
> @@ -2283,6 +2288,7 @@ struct dm_table *dm_swap_table(struct mapped_device *md, struct dm_table *table)
> }
>
> map = __bind(md, table, &limits);
> + dm_issue_global_event();
>
> out:
> mutex_unlock(&md->suspend_lock);
> diff --git a/include/uapi/linux/dm-ioctl.h b/include/uapi/linux/dm-ioctl.h
> index 412c06a624c8..ccaea525340b 100644
> --- a/include/uapi/linux/dm-ioctl.h
> +++ b/include/uapi/linux/dm-ioctl.h
> @@ -269,9 +269,9 @@ enum {
> #define DM_DEV_SET_GEOMETRY _IOWR(DM_IOCTL, DM_DEV_SET_GEOMETRY_CMD, struct dm_ioctl)
>
> #define DM_VERSION_MAJOR 4
> -#define DM_VERSION_MINOR 36
> +#define DM_VERSION_MINOR 37
> #define DM_VERSION_PATCHLEVEL 0
> -#define DM_VERSION_EXTRA "-ioctl (2017-06-09)"
> +#define DM_VERSION_EXTRA "-ioctl (2017-09-20)"
>
> /* Status bits */
> #define DM_READONLY_FLAG (1 << 0) /* In/Out */
>