History

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.

Mikulas Patocka committed 9 years ago

Commit 62e08241 parent e19b2054 files · +35 −17Browse files

Unified · Split

4 files
  1. drivers/md/dm-core.h +1 −0
  2. drivers/md/dm-ioctl.c +24 −13
  3. drivers/md/dm.c +8 −2
  4. include/uapi/linux/dm-ioctl.h +2 −2
drivers/md/dm-core.h+1−0
@@ -149,5 +149,6 @@ static inline bool dm_message_test_buffer_overflow(char *result, unsigned maxlen · 148 unmodified lines
149149
150150 extern atomic_t dm_global_event_nr;
151151 extern wait_queue_head_t dm_global_eventq;
152+void dm_issue_global_event(void);
152153
153154 #endif
drivers/md/dm-ioctl.c+24−13
@@ -477,9 +477,13 @@ static int remove_all(struct file *filp, struct dm_ioctl *param, size_t param_si · 476 unmodified lines
477477 * Round up the ptr to an 8-byte boundary.
478478 */
479479 #define ALIGN_MASK 7
480+static inline size_t align_val(size_t val)
481+{
482+ return (val + ALIGN_MASK) & ~ALIGN_MASK;
483+}
480484 static inline void *align_ptr(void *ptr)
481485 {
482− return (void *) (((size_t) (ptr + ALIGN_MASK)) & ~ALIGN_MASK);
486+ return (void *)align_val((size_t)ptr);
483487 }
484488
485489 /*
@@ -505,7 +509,7 @@ static int list_devices(struct file *filp, struct dm_ioctl *param, size_t param_ · 19 unmodified lines
505509 struct hash_cell *hc;
506510 size_t len, needed = 0;
507511 struct gendisk *disk;
508− struct dm_name_list *nl, *old_nl = NULL;
512+ struct dm_name_list *orig_nl, *nl, *old_nl = NULL;
509513 uint32_t *event_nr;
510514
511515 down_write(&_hash_lock);
@@ -516,17 +520,15 @@ static int list_devices(struct file *filp, struct dm_ioctl *param, size_t param_ · 4 unmodified lines
516520 */
517521 for (i = 0; i < NUM_BUCKETS; i++) {
518522 list_for_each_entry (hc, _name_buckets + i, name_list) {
519− needed += sizeof(struct dm_name_list);
520− needed += strlen(hc->name) + 1;
521− needed += ALIGN_MASK;
522− needed += (sizeof(uint32_t) + ALIGN_MASK) & ~ALIGN_MASK;
523+ needed += align_val(offsetof(struct dm_name_list, name) + strlen(hc->name) + 1);
524+ needed += align_val(sizeof(uint32_t));
523525 }
524526 }
525527
526528 /*
527529 * Grab our output buffer.
528530 */
529− nl = get_result_buffer(param, param_size, &len);
531+ nl = orig_nl = get_result_buffer(param, param_size, &len);
530532 if (len < needed) {
531533 param->flags |= DM_BUFFER_FULL_FLAG;
532534 goto out;
@@ -549,11 +551,16 @@ static int list_devices(struct file *filp, struct dm_ioctl *param, size_t param_ · 16 unmodified lines
549551 strcpy(nl->name, hc->name);
550552
551553 old_nl = nl;
552− event_nr = align_ptr(((void *) (nl + 1)) + strlen(hc->name) + 1);
554+ event_nr = align_ptr(nl->name + strlen(hc->name) + 1);
553555 *event_nr = dm_get_event_nr(hc->md);
554556 nl = align_ptr(event_nr + 1);
555557 }
556558 }
559+ /*
560+ * If mismatch happens, security may be compromised due to buffer
561+ * overflow, so it's better to crash.
562+ */
563+ BUG_ON((char *)nl - (char *)orig_nl != needed);
557564
558565 out:
559566 up_write(&_hash_lock);
@@ -1621,7 +1628,8 @@ static int target_message(struct file *filp, struct dm_ioctl *param, size_t para · 1061 unmodified lines
16211628 * which has a variable size, is not used by the function processing
16221629 * the ioctl.
16231630 */
1624−#define IOCTL_FLAGS_NO_PARAMS 1
1631+#define IOCTL_FLAGS_NO_PARAMS 1
1632+#define IOCTL_FLAGS_ISSUE_GLOBAL_EVENT 2
16251633
16261634 /*-----------------------------------------------------------------
16271635 * Implementation of open/close/ioctl on the special char
@@ -1635,12 +1643,12 @@ static ioctl_fn lookup_ioctl(unsigned int cmd, int *ioctl_flags) · 7 unmodified lines
16351643 ioctl_fn fn;
16361644 } _ioctls[] = {
16371645 {DM_VERSION_CMD, 0, NULL}, /* version is dealt with elsewhere */
1638− {DM_REMOVE_ALL_CMD, IOCTL_FLAGS_NO_PARAMS, remove_all},
1646+ {DM_REMOVE_ALL_CMD, IOCTL_FLAGS_NO_PARAMS | IOCTL_FLAGS_ISSUE_GLOBAL_EVENT, remove_all},
16391647 {DM_LIST_DEVICES_CMD, 0, list_devices},
16401648
1641− {DM_DEV_CREATE_CMD, IOCTL_FLAGS_NO_PARAMS, dev_create},
1642− {DM_DEV_REMOVE_CMD, IOCTL_FLAGS_NO_PARAMS, dev_remove},
1643− {DM_DEV_RENAME_CMD, 0, dev_rename},
1649+ {DM_DEV_CREATE_CMD, IOCTL_FLAGS_NO_PARAMS | IOCTL_FLAGS_ISSUE_GLOBAL_EVENT, dev_create},
1650+ {DM_DEV_REMOVE_CMD, IOCTL_FLAGS_NO_PARAMS | IOCTL_FLAGS_ISSUE_GLOBAL_EVENT, dev_remove},
1651+ {DM_DEV_RENAME_CMD, IOCTL_FLAGS_ISSUE_GLOBAL_EVENT, dev_rename},
16441652 {DM_DEV_SUSPEND_CMD, IOCTL_FLAGS_NO_PARAMS, dev_suspend},
16451653 {DM_DEV_STATUS_CMD, IOCTL_FLAGS_NO_PARAMS, dev_status},
16461654 {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 · 222 unmodified lines
18691877 unlikely(ioctl_flags & IOCTL_FLAGS_NO_PARAMS))
18701878 DMERR("ioctl %d tried to output some data but has IOCTL_FLAGS_NO_PARAMS set", cmd);
18711879
1880+ if (!r && ioctl_flags & IOCTL_FLAGS_ISSUE_GLOBAL_EVENT)
1881+ dm_issue_global_event();
1882+
18721883 /*
18731884 * Copy the results back to userland.
18741885 */
drivers/md/dm.c+8−2
@@ -52,6 +52,12 @@ static struct workqueue_struct *deferred_remove_workqueue; · 51 unmodified lines
5252 atomic_t dm_global_event_nr = ATOMIC_INIT(0);
5353 DECLARE_WAIT_QUEUE_HEAD(dm_global_eventq);
5454
55+void dm_issue_global_event(void)
56+{
57+ atomic_inc(&dm_global_event_nr);
58+ wake_up(&dm_global_eventq);
59+}
60+
5561 /*
5662 * One of these is allocated per bio.
5763 */
@@ -1865,9 +1871,8 @@ static void event_callback(void *context) · 1807 unmodified lines
18651871 dm_send_uevents(&uevents, &disk_to_dev(md->disk)->kobj);
18661872
18671873 atomic_inc(&md->event_nr);
1868− atomic_inc(&dm_global_event_nr);
18691874 wake_up(&md->eventq);
1870− wake_up(&dm_global_eventq);
1875+ dm_issue_global_event();
18711876 }
18721877
18731878 /*
@@ -2283,6 +2288,7 @@ struct dm_table *dm_swap_table(struct mapped_device *md, struct dm_table *table) · 409 unmodified lines
22832288 }
22842289
22852290 map = __bind(md, table, &limits);
2291+ dm_issue_global_event();
22862292
22872293 out:
22882294 mutex_unlock(&md->suspend_lock);
include/uapi/linux/dm-ioctl.h+2−2
@@ -269,9 +269,9 @@ enum { · 268 unmodified lines
269269 #define DM_DEV_SET_GEOMETRY _IOWR(DM_IOCTL, DM_DEV_SET_GEOMETRY_CMD, struct dm_ioctl)
270270
271271 #define DM_VERSION_MAJOR 4
272−#define DM_VERSION_MINOR 36
272+#define DM_VERSION_MINOR 37
273273 #define DM_VERSION_PATCHLEVEL 0
274−#define DM_VERSION_EXTRA "-ioctl (2017-06-09)"
274+#define DM_VERSION_EXTRA "-ioctl (2017-09-20)"
275275
276276 /* Status bits */
277277 #define DM_READONLY_FLAG (1 << 0) /* In/Out */