Messages in this thread Patch in this message |  | | Date | Wed, 06 May 2026 23:18:34 +0200 | | From | "Arnd Bergmann" <> | | Subject | Re: [PATCH v25 5/7] firmware: drivers: imx: adds miscdev |
| |
On Thu, Jan 22, 2026, at 12:49, Pankaj Gupta wrote:
> +/* IOCTL definitions. */ > + > +struct se_ioctl_setup_iobuf { > + void __user *user_buf; > + __u32 length; > + __u32 flags; > + __u64 ele_addr; > +}; > + > +struct se_ioctl_cmd_snd_rcv_rsp_info { > + __u32 __user *tx_buf; > + int tx_buf_sz; > + __u32 __user *rx_buf; > + int rx_buf_sz;
These just showed up in linux-next and triggered warnings in my (still private) uapi checks:
./usr/include/linux/se_ioctl.h:22:15: error: padding struct to align 'ele_addr' [-Werror=padded] 22 | __u64 ele_addr; | ^~~~~~~~
./usr/include/linux/se_ioctl.h:45:16: error: padding struct to align 'rx_buf' [-Werror=padded] 45 | __u32 *rx_buf; | ^~~~~~ ./usr/include/linux/se_ioctl.h:47:1: error: padding struct size to alignment boundary with 4 bytes [-Werror=padded] 47 | };
The problem here is the use of indirect pointers, which are nor recommended in ABI structures because of the implied padding and the need for compat mode handlers, see Documentation/driver-api/ioctl.rst.
I think the fixup below should address all of this, but I have not reviewed the driver in detail to see if thats's all.
I also noticed that the __user annotations are inconsistent, so please also run this through 'make C=1' and fix up the warnings you get.
Arnd
diff --git a/drivers/firmware/imx/se_ctrl.c b/drivers/firmware/imx/se_ctrl.c index 2ba0a6988a39..d0e3d981d1e0 100644 --- a/drivers/firmware/imx/se_ctrl.c +++ b/drivers/firmware/imx/se_ctrl.c @@ -384,7 +384,7 @@ static int add_b_desc_to_pending_list(void *shared_ptr_with_pos, return -ENOMEM; b_desc->shared_buf_ptr = shared_ptr_with_pos; - b_desc->usr_buf_ptr = io->user_buf; + b_desc->usr_buf_ptr = u64_to_user_ptr(io->user_buf); b_desc->size = io->length; if (io->flags & SE_IO_BUF_FLAGS_IS_INPUT) { @@ -526,13 +526,13 @@ static int se_ioctl_cmd_snd_rcv_rsp_handler(struct se_if_device_ctx *dev_ctx, } if (cmd_snd_rcv_rsp_info.tx_buf_sz < SE_MU_HDR_SZ) { - dev_err(priv->dev, "%s: User buffer too small(%d < %d)", + dev_err(priv->dev, "%s: User buffer too small(%lld < %d)", dev_ctx->devname, cmd_snd_rcv_rsp_info.tx_buf_sz, SE_MU_HDR_SZ); se_ioctl_cmd_snd_rcv_cleanup(dev_ctx, uarg, &cmd_snd_rcv_rsp_info); return -ENOSPC; } - err = se_chk_tx_msg_hdr(priv, (struct se_msg_hdr *)cmd_snd_rcv_rsp_info.tx_buf); + err = se_chk_tx_msg_hdr(priv, u64_to_user_ptr(cmd_snd_rcv_rsp_info.tx_buf)); if (err) { se_ioctl_cmd_snd_rcv_cleanup(dev_ctx, uarg, &cmd_snd_rcv_rsp_info); return err; @@ -546,7 +546,7 @@ static int se_ioctl_cmd_snd_rcv_rsp_handler(struct se_if_device_ctx *dev_ctx, } struct se_api_msg *tx_msg __free(kfree) = - memdup_user(cmd_snd_rcv_rsp_info.tx_buf, + memdup_user(u64_to_user_ptr(cmd_snd_rcv_rsp_info.tx_buf), cmd_snd_rcv_rsp_info.tx_buf_sz); if (IS_ERR(tx_msg)) { err = PTR_ERR(tx_msg); @@ -593,8 +593,8 @@ static int se_ioctl_cmd_snd_rcv_rsp_handler(struct se_if_device_ctx *dev_ctx, print_hex_dump_debug("to user ", DUMP_PREFIX_OFFSET, 4, 4, rx_msg, cmd_snd_rcv_rsp_info.rx_buf_sz, false); - if (copy_to_user(cmd_snd_rcv_rsp_info.rx_buf, rx_msg, - cmd_snd_rcv_rsp_info.rx_buf_sz)) { + if (copy_to_user(u64_to_user_ptr(cmd_snd_rcv_rsp_info.rx_buf), + rx_msg, cmd_snd_rcv_rsp_info.rx_buf_sz)) { dev_err(priv->dev, "%s: Failed to copy to user.", dev_ctx->devname); err = -EFAULT; } @@ -655,7 +655,7 @@ static int se_ioctl_setup_iobuf_handler(struct se_if_device_ctx *dev_ctx, return -EFAULT; } - dev_dbg(dev_ctx->priv->dev, "%s: io [buf: %p(%d) flag: %x].", dev_ctx->devname, + dev_dbg(dev_ctx->priv->dev, "%s: io [buf: %llx(%d) flag: %x].", dev_ctx->devname, io.user_buf, io.length, io.flags); if (io.length == 0 || !io.user_buf) { @@ -696,7 +696,8 @@ static int se_ioctl_setup_iobuf_handler(struct se_if_device_ctx *dev_ctx, * buffer is input: * copy data from user space to this allocated buffer. */ - if (copy_from_user(shared_mem->ptr + pos, io.user_buf, io.length)) { + if (copy_from_user(shared_mem->ptr + pos, + u64_to_user_ptr(io.user_buf), io.length)) { dev_err(dev_ctx->priv->dev, "%s: Failed copy data to shared memory.", dev_ctx->devname); diff --git a/include/uapi/linux/se_ioctl.h b/include/uapi/linux/se_ioctl.h index 0c948bdc8c26..9fb81cb72b94 100644 --- a/include/uapi/linux/se_ioctl.h +++ b/include/uapi/linux/se_ioctl.h @@ -16,7 +16,7 @@ /* IOCTL definitions. */ struct se_ioctl_setup_iobuf { - void __user *user_buf; + __u64 user_buf; __u32 length; __u32 flags; __u64 ele_addr; @@ -40,10 +40,10 @@ struct se_ioctl_get_if_info { }; struct se_ioctl_cmd_snd_rcv_rsp_info { - __u32 __user *tx_buf; - int tx_buf_sz; - __u32 __user *rx_buf; - int rx_buf_sz; + __u64 tx_buf; + __u64 tx_buf_sz; + __u64 rx_buf; + __u64 rx_buf_sz; }; struct se_ioctl_get_soc_info {
|  |