Linux kernel and device drivers for NXP i.MX platforms
 help / color / mirror / Atom feed
* [bug report] firmware: drivers: imx: adds miscdev
@ 2026-05-07  6:35 Dan Carpenter
  0 siblings, 0 replies; 3+ messages in thread
From: Dan Carpenter @ 2026-05-07  6:35 UTC (permalink / raw)
  To: Pankaj Gupta; +Cc: imx

Hello Pankaj Gupta,

Commit 4de71839142b ("firmware: drivers: imx: adds miscdev") from Jan
22, 2026 (linux-next), leads to the following Smatch static checker
warning:

	drivers/firmware/imx/se_ctrl.c:490 init_device_context()
	warn: '&dev_ctx->link' not removed from list

drivers/firmware/imx/se_ctrl.c
    461 static int init_device_context(struct se_if_priv *priv, int ch_id,
    462                                struct se_if_device_ctx **new_dev_ctx)
    463 {
    464         struct se_if_device_ctx *dev_ctx;
    465         int ret = 0;
    466 
    467         dev_ctx = kzalloc(sizeof(*dev_ctx), GFP_KERNEL);
    468 
    469         if (!dev_ctx)
    470                 return -ENOMEM;
    471 
    472         dev_ctx->devname = kasprintf(GFP_KERNEL, "%s0_ch%d",
    473                                      get_se_if_name(priv->if_defs->se_if_type),
    474                                      ch_id);
    475         if (!dev_ctx->devname) {
    476                 kfree(dev_ctx);
    477                 return -ENOMEM;
    478         }
    479 
    480         mutex_init(&dev_ctx->fops_lock);
    481         dev_ctx->priv = priv;
    482         *new_dev_ctx = dev_ctx;
    483 
    484         list_add_tail(&dev_ctx->link, &priv->dev_ctx_list);
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
This adds dev_ctx to the list

    485         priv->active_devctx_count++;
    486 
    487         ret = init_se_shared_mem(dev_ctx);
    488         if (ret < 0) {
    489                 kfree(dev_ctx->devname);
--> 490                 kfree(dev_ctx);

But it's still on the list when we free it so it leads to a use after
free.

    491                 *new_dev_ctx = NULL;
    492         }
    493 
    494         return ret;
    495 }

This email is a free service from the Smatch-CI project [smatch.sf.net].

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [bug report] firmware: drivers: imx: adds miscdev
@ 2026-05-07  6:59 Dan Carpenter
  0 siblings, 0 replies; 3+ messages in thread
From: Dan Carpenter @ 2026-05-07  6:59 UTC (permalink / raw)
  To: Pankaj Gupta; +Cc: imx

Hello Pankaj Gupta,

Commit 4de71839142b ("firmware: drivers: imx: adds miscdev") from Jan
22, 2026 (linux-next), leads to the following Smatch static checker
warning:

	drivers/firmware/imx/se_ctrl.c:681 se_ioctl_setup_iobuf_handler()
	warn: integer overflow "io.length + 7"

drivers/firmware/imx/se_ctrl.c
    644 static int se_ioctl_setup_iobuf_handler(struct se_if_device_ctx *dev_ctx,
    645                                         void __user *uarg)
    646 {
    647         struct se_shared_mem *shared_mem = NULL;
    648         struct se_ioctl_setup_iobuf io = {0};
    649         int err = 0;
    650         u32 pos;
    651 
    652         if (copy_from_user(&io, uarg, sizeof(io))) {
    653                 dev_err(dev_ctx->priv->dev, "%s: Failed copy iobuf config from user.",
    654                         dev_ctx->devname);
    655                 return -EFAULT;
    656         }
    657 
    658         dev_dbg(dev_ctx->priv->dev, "%s: io [buf: %p(%d) flag: %x].", dev_ctx->devname,
    659                 io.user_buf, io.length, io.flags);
    660 
    661         if (io.length == 0 || !io.user_buf) {
    662                 /*
    663                  * Accept NULL pointers since some buffers are optional
    664                  * in FW commands. In this case we should return 0 as
    665                  * pointer to be embedded into the message.
    666                  * Skip all data copy part of code below.
    667                  */
    668                 io.ele_addr = 0;
    669                 goto copy;
    670         }
    671 
    672         /* No specific requirement for this buffer. */
    673         shared_mem = &dev_ctx->se_shared_mem_mgmt.non_secure_mem;
    674 
    675         /* Check there is enough space in the shared memory. */
    676         dev_dbg(dev_ctx->priv->dev, "%s: req_size = %d, max_size= %d, curr_pos = %d",
    677                 dev_ctx->devname, round_up(io.length, 8u), shared_mem->size,
    678                 shared_mem->pos);
    679 
    680         if (shared_mem->size < shared_mem->pos ||
--> 681             round_up(io.length, 8u) > (shared_mem->size - shared_mem->pos)) {

If io.length is >= U32_MAX - 7 then round_up() is zero.

    682                 dev_err(dev_ctx->priv->dev, "%s: Not enough space in shared memory.",
    683                         dev_ctx->devname);
    684                 return -ENOMEM;
    685         }
    686 
    687         /* Allocate space in shared memory. 8 bytes aligned. */
    688         pos = shared_mem->pos;
    689         shared_mem->pos += round_up(io.length, 8u);
    690         io.ele_addr = (u64)shared_mem->dma_addr + pos;
    691 
    692         memset(shared_mem->ptr + pos, 0, io.length);
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
And this memset() will crash.

    693         if ((io.flags & SE_IO_BUF_FLAGS_IS_INPUT) ||
    694             (io.flags & SE_IO_BUF_FLAGS_IS_IN_OUT)) {
    695                 /*
    696                  * buffer is input:
    697                  * copy data from user space to this allocated buffer.
    698                  */
    699                 if (copy_from_user(shared_mem->ptr + pos, io.user_buf, io.length)) {
    700                         dev_err(dev_ctx->priv->dev,
    701                                 "%s: Failed copy data to shared memory.",
    702                                 dev_ctx->devname);
    703                         return -EFAULT;
    704                 }
    705         }
    706 
    707         err = add_b_desc_to_pending_list(shared_mem->ptr + pos, &io, dev_ctx);
    708         if (err < 0)
    709                 dev_err(dev_ctx->priv->dev, "%s: Failed to allocate/link b_desc.",
    710                         dev_ctx->devname);
    711 
    712 copy:
    713         /* Provide the EdgeLock Enclave address to user space only if success.*/
    714         if (copy_to_user(uarg, &io, sizeof(io))) {
    715                 dev_err(dev_ctx->priv->dev, "%s: Failed to copy iobuff setup to user.",
    716                         dev_ctx->devname);
    717                 err = -EFAULT;
    718         }
    719 
    720         return err;
    721 }

This email is a free service from the Smatch-CI project [smatch.sf.net].

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [bug report] firmware: drivers: imx: adds miscdev
@ 2026-05-07  7:07 Dan Carpenter
  0 siblings, 0 replies; 3+ messages in thread
From: Dan Carpenter @ 2026-05-07  7:07 UTC (permalink / raw)
  To: Pankaj Gupta; +Cc: imx

Hello Pankaj Gupta,

Commit 4de71839142b ("firmware: drivers: imx: adds miscdev") from Jan
22, 2026 (linux-next), leads to the following Smatch static checker
warning:

	drivers/firmware/imx/se_ctrl.c:826 se_if_fops_read()
	warn: userbuf overflow? is 'priv->cmd_receiver_clbk_hdl.rx_msg_sz' <= 'size'

drivers/firmware/imx/se_ctrl.c
    789 static ssize_t se_if_fops_read(struct file *fp, char __user *buf, size_t size,
                                                                          ^^^^^^^^^^^
This function copies memory to the user but it doesn't take into
consideration this size variable which is the size of the buffer
in user space.

    790                                loff_t *ppos)
    791 {
    792         struct se_if_device_ctx *dev_ctx = fp->private_data;
    793         struct se_if_priv *priv = dev_ctx->priv;
    794         int err;
    795 
    796         dev_dbg(priv->dev, "%s: read to buf %p(%zu), ppos=%lld.", dev_ctx->devname,
    797                 buf, size, ((ppos) ? *ppos : 0));
    798 
    799         scoped_cond_guard(mutex_intr, return -EBUSY, &dev_ctx->fops_lock) {
    800                 if (dev_ctx != priv->cmd_receiver_clbk_hdl.dev_ctx) {
    801                         err = -EINVAL;
    802                         goto exit;
    803                 }
    804 
    805                 err = ele_msg_rcv(dev_ctx, &priv->cmd_receiver_clbk_hdl);
    806                 if (err < 0) {
    807                         dev_err(priv->dev,
    808                                 "%s: Er[0x%x]: Signal Interrupted. Current act-dev-ctx count: %d.",
    809                                 dev_ctx->devname, err, dev_ctx->priv->active_devctx_count);
    810                         goto exit;
    811                 }
    812 
    813                 /* We may need to copy the output data to user before
    814                  * delivering the completion message.
    815                  */
    816                 err = se_dev_ctx_cpy_out_data(dev_ctx);
    817                 if (err < 0)
    818                         goto exit;
    819 
    820                 /* Copy data from the buffer */
    821                 print_hex_dump_debug("to user ", DUMP_PREFIX_OFFSET, 4, 4,
    822                                      priv->cmd_receiver_clbk_hdl.rx_msg,
    823                                      priv->cmd_receiver_clbk_hdl.rx_msg_sz,
    824                                      false);
    825 
--> 826                 if (copy_to_user(buf, priv->cmd_receiver_clbk_hdl.rx_msg,
    827                                  priv->cmd_receiver_clbk_hdl.rx_msg_sz)) {
                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
We should only copy size bytes at most.



    828                         dev_err(priv->dev, "%s: Failed to copy to user.",
    829                                 dev_ctx->devname);

Don't print an error message if copy_to/from_user fails.  Returning
-EFAULT and crashing the program is communication enough.  No need
to fill /var/log/messages with spam.

    830                         err = -EFAULT;
    831                 } else {
    832                         err = priv->cmd_receiver_clbk_hdl.rx_msg_sz;
    833                 }
    834 exit:
    835                 priv->cmd_receiver_clbk_hdl.rx_msg_sz = 0;
    836 
    837                 se_dev_ctx_shared_mem_cleanup(dev_ctx);
    838 
    839                 return err;
    840         }
    841 }

This email is a free service from the Smatch-CI project [smatch.sf.net].

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-05-07  7:07 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-07  7:07 [bug report] firmware: drivers: imx: adds miscdev Dan Carpenter
  -- strict thread matches above, loose matches on Subject: below --
2026-05-07  6:59 Dan Carpenter
2026-05-07  6:35 Dan Carpenter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox