On 14/11/2023 17:47, Akram Ahmad wrote:
The mmc_ioc_cmd and mmc_ioc_multi_cmd structs are used to hold information and data about an MMC ioctl.
The PCuABI uses 129-bit capabilities as user pointers, which means that the __u64 type must be replaced with the __kernel_uintptr_t type, which is large enough to hold capabilities, yet will remain 64-bit on other architectures. Additional modifications have been made to copy routines.
Adding capabilities results in the sizes of the structs to grow when applications are rebuilt against this updated kernel header, which breaks compat64 applications which have not been built against the new headers.
This is rather confusing as so far it seems to be about the struct itself, but really it is about the command value, which is currently inferred from the struct size. Might be useful to spell that out.
This is fixed by hard-coding the values used in the command definitions, as compat structs are not defined in this uAPI header.
Signed-off-by: Akram Ahmad Akram.Ahmad@arm.com
drivers/mmc/core/block.c | 8 ++++---- include/uapi/linux/mmc/ioctl.h | 16 ++++++++++++---- 2 files changed, 16 insertions(+), 8 deletions(-)
diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c index 3333c4917028..547941096c0e 100644 --- a/drivers/mmc/core/block.c +++ b/drivers/mmc/core/block.c @@ -454,7 +454,7 @@ static int get_mmc_ioc_cmd_from_compat64(struct mmc_ioc_cmd *native_cmd, native_cmd->data_timeout_ns = compat_cmd.data_timeout_ns; native_cmd->cmd_timeout_ms = compat_cmd.cmd_timeout_ms; native_cmd->__pad = compat_cmd.__pad;
- native_cmd->data_ptr = compat_cmd.data_ptr;
- native_cmd->data_ptr = (__kernel_uintptr_t)compat_ptr(compat_cmd.data_ptr);
return 0; } @@ -464,7 +464,7 @@ static int copy_mmc_ioc_cmd_from_user(struct mmc_ioc_cmd *to, void * __user src) if (in_compat64()) return get_mmc_ioc_cmd_from_compat64(to, src);
- if (copy_from_user(to, src, sizeof(*to)))
- if (copy_from_user_with_ptr(to, src, sizeof(*to))) return -EFAULT; return 0;
} @@ -497,7 +497,7 @@ static struct mmc_blk_ioc_data *mmc_blk_ioctl_copy_from_user( return idata; }
- idata->buf = memdup_user(uaddr_to_user_ptr(idata->ic.data_ptr),
- idata->buf = memdup_user((void __user *)idata->ic.data_ptr, idata->buf_bytes); if (IS_ERR(idata->buf)) { err = PTR_ERR(idata->buf);
@@ -525,7 +525,7 @@ static int mmc_blk_ioctl_copy_to_user(struct mmc_ioc_cmd __user *ic_ptr, return -EFAULT; if (!idata->ic.write_flag) {
if (copy_to_user(uaddr_to_user_ptr(ic->data_ptr),
if (copy_to_user((void __user *)ic->data_ptr, idata->buf, idata->buf_bytes)) return -EFAULT;diff --git a/include/uapi/linux/mmc/ioctl.h b/include/uapi/linux/mmc/ioctl.h index e7401ade6822..3ed175d27d7b 100644 --- a/include/uapi/linux/mmc/ioctl.h +++ b/include/uapi/linux/mmc/ioctl.h @@ -2,6 +2,9 @@ #ifndef LINUX_MMC_IOCTL_H #define LINUX_MMC_IOCTL_H +#ifndef __KERNEL__ +#include <stdint.h> +#endif #include <linux/types.h> #include <linux/major.h> @@ -46,9 +49,9 @@ struct mmc_ioc_cmd { __u32 __pad; /* DAT buffer */
- __u64 data_ptr;
- __kernel_uintptr_t data_ptr;
}; -#define mmc_ioc_cmd_set_data(ic, ptr) ic.data_ptr = (__u64)(unsigned long) ptr +#define mmc_ioc_cmd_set_data(ic, ptr) ic.data_ptr = (uintptr_t) ptr /**
- struct mmc_ioc_multi_cmd - multi command information
@@ -61,13 +64,18 @@ struct mmc_ioc_multi_cmd { struct mmc_ioc_cmd cmds[]; }; -#define MMC_IOC_CMD _IOWR(MMC_BLOCK_MAJOR, 0, struct mmc_ioc_cmd) +/*
- As the size of the struct has changed with the use of __kernel_uintptr_t,
"changed in PCuABI", to be specific.
- it is necessary to hard-code the size of the compat64 struct so that the
"compat64" is very specific, but in fact this size is the same in any ABI except PCuABI.
- correct ioctl number is assigned. This also applies to MMC_IOC_MULTI_CMD.
I think it might be good to say explicitly that the goal is for the value to remain unchanged - one could argue which value is the "correct" one in PCuABI.
- */
+#define MMC_IOC_CMD _IOC(_IOC_READ|_IOC_WRITE, MMC_BLOCK_MAJOR, 0, 72) /*
- MMC_IOC_MULTI_CMD: Used to send an array of MMC commands described by
- the structure mmc_ioc_multi_cmd. The MMC driver will issue all
- commands in array in sequence to card.
*/ -#define MMC_IOC_MULTI_CMD _IOWR(MMC_BLOCK_MAJOR, 1, struct mmc_ioc_multi_cmd) +#define MMC_IOC_MULTI_CMD _IOC(_IOC_READ|_IOC_WRITE, MMC_BLOCK_MAJOR, 1, 8)
There's no need to change this definition, since struct mmc_ioc_multi_cmd contains an integer. The flexible array member at the end is just syntactic sugar, it doesn't actually add anything to the struct.
Kevin
/*
- Since this ioctl is only meant to enhance (and not replace) normal access
- to the mmc bus device, an upper data transfer limit of MMC_IOC_MAX_BYTES