On Thu, Mar 11 2021 at 1:26pm -0500,
Mikulas Patocka <mpatocka@xxxxxxxxxx> wrote:
> When LVM needs to find a device with a particular UUID it needs to ask for
> UUID for each device. This patch returns UUID directly in the list of
> devices, so that LVM doesn't have to query all the devices with an ioctl.
> The UUID is returned if the flag DM_UUID_FLAG is set in the parameters.
>
> Returning UUID is done in backward-compatible way. There's one unused
> 32-bit word value after the event number. This patch sets the bit
> DM_NAME_LIST_FLAG_HAS_UUID if UUID is present and
> DM_NAME_LIST_FLAG_DOESNT_HAVE_UUID if it isn't (if none of these bits is
> set, then we have an old kernel that doesn't support returning UUIDs). The
> UUID is stored after this word. The 'next' value is updated to point after
> the UUID, so that old version of libdevmapper will skip the UUID without
> attempting to interpret it.
>
> Signed-off-by: Mikulas Patocka <mpatocka@xxxxxxxxxx>
>
> ---
> drivers/md/dm-ioctl.c | 20 +++++++++++++++++---
> include/uapi/linux/dm-ioctl.h | 7 +++++++
> 2 files changed, 24 insertions(+), 3 deletions(-)
>
> Index: linux-2.6/drivers/md/dm-ioctl.c
> ===================================================================
> --- linux-2.6.orig/drivers/md/dm-ioctl.c 2021-03-09 21:04:07.000000000 +0100
> +++ linux-2.6/drivers/md/dm-ioctl.c 2021-03-11 18:53:58.000000000 +0100
> @@ -558,7 +558,9 @@ static int list_devices(struct file *fil
> for (n = rb_first(&name_rb_tree); n; n = rb_next(n)) {
> hc = container_of(n, struct hash_cell, name_node);
> needed += align_val(offsetof(struct dm_name_list, name) + strlen(hc->name) + 1);
> - needed += align_val(sizeof(uint32_t));
> + needed += align_val(sizeof(uint32_t) * 2);
> + if (param->flags & DM_UUID_FLAG && hc->uuid)
> + needed += align_val(strlen(hc->uuid) + 1);
> }
>
> /*
> @@ -577,6 +579,7 @@ static int list_devices(struct file *fil
> * Now loop through filling out the names.
> */
> for (n = rb_first(&name_rb_tree); n; n = rb_next(n)) {
> + void *uuid_ptr;
> hc = container_of(n, struct hash_cell, name_node);
> if (old_nl)
> old_nl->next = (uint32_t) ((void *) nl -
> @@ -588,8 +591,19 @@ static int list_devices(struct file *fil
>
> old_nl = nl;
> event_nr = align_ptr(nl->name + strlen(hc->name) + 1);
> - *event_nr = dm_get_event_nr(hc->md);
> - nl = align_ptr(event_nr + 1);
> + event_nr[0] = dm_get_event_nr(hc->md);
> + event_nr[1] = 0;
> + uuid_ptr = align_ptr(event_nr + 2);
> + if (param->flags & DM_UUID_FLAG) {
> + if (hc->uuid) {
> + event_nr[1] |= DM_NAME_LIST_FLAG_HAS_UUID;
> + strcpy(uuid_ptr, hc->uuid);
> + uuid_ptr = align_ptr(uuid_ptr + strlen(hc->uuid) + 1);
> + } else {
> + event_nr[1] |= DM_NAME_LIST_FLAG_DOESNT_HAVE_UUID;
> + }
> + }
> + nl = uuid_ptr;
> }
> /*
> * If mismatch happens, security may be compromised due to buffer
> Index: linux-2.6/include/uapi/linux/dm-ioctl.h
> ===================================================================
> --- linux-2.6.orig/include/uapi/linux/dm-ioctl.h 2021-03-09 12:20:23.000000000 +0100
> +++ linux-2.6/include/uapi/linux/dm-ioctl.h 2021-03-11 18:42:14.000000000 +0100
> @@ -193,8 +193,15 @@ struct dm_name_list {
> __u32 next; /* offset to the next record from
> the _start_ of this */
> char name[0];
> +
> + /* uint32_t event_nr; */
> + /* uint32_t flags; */
> + /* char uuid[0]; */
> };
If extra padding is being leveraged here (from the __u32 next), why not
at least explicitly add the members and then pad out the balance of that
__u32? I'm not liking the usage of phantom struct members.. e.g.
the games played with accessing them.
Mike
>
> +#define DM_NAME_LIST_FLAG_HAS_UUID 1
> +#define DM_NAME_LIST_FLAG_DOESNT_HAVE_UUID 2
> +
> /*
> * Used to retrieve the target versions
> */
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/dm-devel
