[PATCH AUTOSEL 6.1 1/6] i3c: mipi-i3c-hci: Fix out of bounds access in hci_dma_irq_handler
From: Jarkko Nikula jarkko.nikula@linux.intel.com
[ Upstream commit 45a832f989e520095429589d5b01b0c65da9b574 ]
Do not loop over ring headers in hci_dma_irq_handler() that are not allocated and enabled in hci_dma_init(). Otherwise out of bounds access will occur from rings->headers[i] access when i >= number of allocated ring headers.
Signed-off-by: Jarkko Nikula jarkko.nikula@linux.intel.com Link: https://lore.kernel.org/r/20230921055704.1087277-5-jarkko.nikula@linux.intel... Signed-off-by: Alexandre Belloni alexandre.belloni@bootlin.com Signed-off-by: Sasha Levin sashal@kernel.org --- drivers/i3c/master/mipi-i3c-hci/dma.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/i3c/master/mipi-i3c-hci/dma.c b/drivers/i3c/master/mipi-i3c-hci/dma.c index 2990ac9eaade7..71b5dbe45c45c 100644 --- a/drivers/i3c/master/mipi-i3c-hci/dma.c +++ b/drivers/i3c/master/mipi-i3c-hci/dma.c @@ -734,7 +734,7 @@ static bool hci_dma_irq_handler(struct i3c_hci *hci, unsigned int mask) unsigned int i; bool handled = false;
- for (i = 0; mask && i < 8; i++) { + for (i = 0; mask && i < rings->total; i++) { struct hci_rh_data *rh; u32 status;
From: Wolfram Sang wsa+renesas@sang-engineering.com
[ Upstream commit 6af79f7fe748fe6a3c5c3a63d7f35981a82c2769 ]
Yang Yingliang reported a memleak: ===
I got memory leak as follows when doing fault injection test:
unreferenced object 0xffff888014aec078 (size 8): comm "xrun", pid 356, jiffies 4294910619 (age 16.332s) hex dump (first 8 bytes): 31 2d 30 30 31 63 00 00 1-001c.. backtrace: [<00000000eb56c0a9>] __kmalloc_track_caller+0x1a6/0x300 [<000000000b220ea3>] kvasprintf+0xad/0x140 [<00000000b83203e5>] kvasprintf_const+0x62/0x190 [<000000002a5eab37>] kobject_set_name_vargs+0x56/0x140 [<00000000300ac279>] dev_set_name+0xb0/0xe0 [<00000000b66ebd6f>] i2c_new_client_device+0x7e4/0x9a0
If device_register() returns error in i2c_new_client_device(), the name allocated by i2c_dev_set_name() need be freed. As comment of device_register() says, it should use put_device() to give up the reference in the error path.
=== I think this solution is less intrusive and more robust than he originally proposed solutions, though.
Reported-by: Yang Yingliang yangyingliang@huawei.com Closes: http://patchwork.ozlabs.org/project/linux-i2c/patch/20221124085448.3620240-1... Signed-off-by: Wolfram Sang wsa+renesas@sang-engineering.com Signed-off-by: Wolfram Sang wsa@kernel.org Signed-off-by: Sasha Levin sashal@kernel.org --- drivers/i2c/i2c-core-base.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c index 7539b0740351d..5e3976ba52650 100644 --- a/drivers/i2c/i2c-core-base.c +++ b/drivers/i2c/i2c-core-base.c @@ -916,8 +916,9 @@ int i2c_dev_irq_from_resources(const struct resource *resources, struct i2c_client * i2c_new_client_device(struct i2c_adapter *adap, struct i2c_board_info const *info) { - struct i2c_client *client; - int status; + struct i2c_client *client; + bool need_put = false; + int status;
client = kzalloc(sizeof *client, GFP_KERNEL); if (!client) @@ -955,7 +956,6 @@ i2c_new_client_device(struct i2c_adapter *adap, struct i2c_board_info const *inf client->dev.fwnode = info->fwnode;
device_enable_async_suspend(&client->dev); - i2c_dev_set_name(adap, client, info);
if (info->swnode) { status = device_add_software_node(&client->dev, info->swnode); @@ -967,6 +967,7 @@ i2c_new_client_device(struct i2c_adapter *adap, struct i2c_board_info const *inf } }
+ i2c_dev_set_name(adap, client, info); status = device_register(&client->dev); if (status) goto out_remove_swnode; @@ -978,6 +979,7 @@ i2c_new_client_device(struct i2c_adapter *adap, struct i2c_board_info const *inf
out_remove_swnode: device_remove_software_node(&client->dev); + need_put = true; out_err_put_of_node: of_node_put(info->of_node); out_err: @@ -985,7 +987,10 @@ i2c_new_client_device(struct i2c_adapter *adap, struct i2c_board_info const *inf "Failed to register i2c client %s at 0x%02x (%d)\n", client->name, client->addr, status); out_err_silent: - kfree(client); + if (need_put) + put_device(&client->dev); + else + kfree(client); return ERR_PTR(status); } EXPORT_SYMBOL_GPL(i2c_new_client_device);
From: Axel Lin axel.lin@ingics.com
[ Upstream commit 5ac61d26b8baff5b2e5a9f3dc1ef63297e4b53e7 ]
Make sure we don't OOPS in case clock-frequency is set to 0 in a DT. The variable set here is later used as a divisor.
Signed-off-by: Axel Lin axel.lin@ingics.com Acked-by: Boris Brezillon boris.brezillon@free-electrons.com Signed-off-by: Wolfram Sang wsa@kernel.org Signed-off-by: Sasha Levin sashal@kernel.org --- drivers/i2c/busses/i2c-sun6i-p2wi.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/drivers/i2c/busses/i2c-sun6i-p2wi.c b/drivers/i2c/busses/i2c-sun6i-p2wi.c index 9e3483f507ff5..f2ed13b551088 100644 --- a/drivers/i2c/busses/i2c-sun6i-p2wi.c +++ b/drivers/i2c/busses/i2c-sun6i-p2wi.c @@ -201,6 +201,11 @@ static int p2wi_probe(struct platform_device *pdev) return -EINVAL; }
+ if (clk_freq == 0) { + dev_err(dev, "clock-frequency is set to 0 in DT\n"); + return -EINVAL; + } + if (of_get_child_count(np) > 1) { dev_err(dev, "P2WI only supports one slave device\n"); return -EINVAL;
From: zhenwei pi pizhenwei@bytedance.com
[ Upstream commit fafb51a67fb883eb2dde352539df939a251851be ]
The following codes have an implicit conversion from size_t to u32: (u32)max_size = (size_t)virtio_max_dma_size(vdev);
This may lead overflow, Ex (size_t)4G -> (u32)0. Once virtio_max_dma_size() has a larger size than U32_MAX, use U32_MAX instead.
Signed-off-by: zhenwei pi pizhenwei@bytedance.com Message-Id: 20230904061045.510460-1-pizhenwei@bytedance.com Signed-off-by: Michael S. Tsirkin mst@redhat.com Signed-off-by: Sasha Levin sashal@kernel.org --- drivers/block/virtio_blk.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c index a7697027ce43b..efa5535a8e1d8 100644 --- a/drivers/block/virtio_blk.c +++ b/drivers/block/virtio_blk.c @@ -900,6 +900,7 @@ static int virtblk_probe(struct virtio_device *vdev) u16 min_io_size; u8 physical_block_exp, alignment_offset; unsigned int queue_depth; + size_t max_dma_size;
if (!vdev->config->get) { dev_err(&vdev->dev, "%s failure: config access disabled\n", @@ -998,7 +999,8 @@ static int virtblk_probe(struct virtio_device *vdev) /* No real sector limit. */ blk_queue_max_hw_sectors(q, -1U);
- max_size = virtio_max_dma_size(vdev); + max_dma_size = virtio_max_dma_size(vdev); + max_size = max_dma_size > U32_MAX ? U32_MAX : max_dma_size;
/* Host can optionally specify maximum segment size and number of * segments. */
From: Si-Wei Liu si-wei.liu@oracle.com
[ Upstream commit bc91df5c70ac720eca18bd1f4a288f2582713d3e ]
Using .compat_reset op from the previous patch, the buggy .reset behaviour can be kept as-is on older userspace apps, which don't ack the IOTLB_PERSIST backend feature. As this compatibility quirk is limited to those drivers that used to be buggy in the past, it won't affect change the behaviour or affect ABI on the setups with API compliant driver.
The separation of .compat_reset from the regular .reset allows vhost-vdpa able to know which driver had broken behaviour before, so it can apply the corresponding compatibility quirk to the individual driver whenever needed. Compared to overloading the existing .reset with flags, .compat_reset won't cause any extra burden to the implementation of every compliant driver.
[mst: squashed in two fixup commits]
Message-Id: 1697880319-4937-6-git-send-email-si-wei.liu@oracle.com Message-Id: 1698102863-21122-1-git-send-email-si-wei.liu@oracle.com Reported-by: Dragos Tatulea dtatulea@nvidia.com Tested-by: Dragos Tatulea dtatulea@nvidia.com Message-Id: 1698275594-19204-1-git-send-email-si-wei.liu@oracle.com Reported-by: Lei Yang leiyang@redhat.com Signed-off-by: Si-Wei Liu si-wei.liu@oracle.com Signed-off-by: Michael S. Tsirkin mst@redhat.com Tested-by: Lei Yang leiyang@redhat.com Signed-off-by: Sasha Levin sashal@kernel.org --- drivers/vhost/vdpa.c | 20 ++++++++++++++++---- drivers/virtio/virtio_vdpa.c | 2 +- include/linux/vdpa.h | 7 +++++-- 3 files changed, 22 insertions(+), 7 deletions(-)
diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c index 31a156669a531..9e60a1d3b8166 100644 --- a/drivers/vhost/vdpa.c +++ b/drivers/vhost/vdpa.c @@ -210,13 +210,24 @@ static void vhost_vdpa_unsetup_vq_irq(struct vhost_vdpa *v, u16 qid) irq_bypass_unregister_producer(&vq->call_ctx.producer); }
-static int vhost_vdpa_reset(struct vhost_vdpa *v) +static int _compat_vdpa_reset(struct vhost_vdpa *v) { struct vdpa_device *vdpa = v->vdpa; + u32 flags = 0;
- v->in_batch = 0; + if (v->vdev.vqs) { + flags |= !vhost_backend_has_feature(v->vdev.vqs[0], + VHOST_BACKEND_F_IOTLB_PERSIST) ? + VDPA_RESET_F_CLEAN_MAP : 0; + } + + return vdpa_reset(vdpa, flags); +}
- return vdpa_reset(vdpa); +static int vhost_vdpa_reset(struct vhost_vdpa *v) +{ + v->in_batch = 0; + return _compat_vdpa_reset(v); }
static long vhost_vdpa_get_device_id(struct vhost_vdpa *v, u8 __user *argp) @@ -273,7 +284,7 @@ static long vhost_vdpa_set_status(struct vhost_vdpa *v, u8 __user *statusp) vhost_vdpa_unsetup_vq_irq(v, i);
if (status == 0) { - ret = vdpa_reset(vdpa); + ret = _compat_vdpa_reset(v); if (ret) return ret; } else @@ -1202,6 +1213,7 @@ static void vhost_vdpa_cleanup(struct vhost_vdpa *v) vhost_vdpa_free_domain(v); vhost_dev_cleanup(&v->vdev); kfree(v->vdev.vqs); + v->vdev.vqs = NULL; }
static int vhost_vdpa_open(struct inode *inode, struct file *filep) diff --git a/drivers/virtio/virtio_vdpa.c b/drivers/virtio/virtio_vdpa.c index 9670cc79371d8..73d052b4fe4d2 100644 --- a/drivers/virtio/virtio_vdpa.c +++ b/drivers/virtio/virtio_vdpa.c @@ -99,7 +99,7 @@ static void virtio_vdpa_reset(struct virtio_device *vdev) { struct vdpa_device *vdpa = vd_get_vdpa(vdev);
- vdpa_reset(vdpa); + vdpa_reset(vdpa, 0); }
static bool virtio_vdpa_notify(struct virtqueue *vq) diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h index 6d0f5e4e82c25..2737f951fbd8f 100644 --- a/include/linux/vdpa.h +++ b/include/linux/vdpa.h @@ -427,14 +427,17 @@ static inline struct device *vdpa_get_dma_dev(struct vdpa_device *vdev) return vdev->dma_dev; }
-static inline int vdpa_reset(struct vdpa_device *vdev) +static inline int vdpa_reset(struct vdpa_device *vdev, u32 flags) { const struct vdpa_config_ops *ops = vdev->config; int ret;
down_write(&vdev->cf_lock); vdev->features_valid = false; - ret = ops->reset(vdev); + if (ops->compat_reset && flags) + ret = ops->compat_reset(vdev, flags); + else + ret = ops->reset(vdev); up_write(&vdev->cf_lock); return ret; }
From: Billy Tsai billy_tsai@aspeedtech.com
[ Upstream commit b53e9758a31c683fc8615df930262192ed5f034b ]
The `i3c_master_bus_init` function may attach the I2C devices before the I3C bus initialization. In this flow, the DAT `alloc_entry`` will be used before the DAT `init`. Additionally, if the `i3c_master_bus_init` fails, the DAT `cleanup` will execute before the device is detached, which will execue DAT `free_entry` function. The above scenario can cause the driver to use DAT_data when it is NULL.
Signed-off-by: Billy Tsai billy_tsai@aspeedtech.com Link: https://lore.kernel.org/r/20231023080237.560936-1-billy_tsai@aspeedtech.com Signed-off-by: Alexandre Belloni alexandre.belloni@bootlin.com Signed-off-by: Sasha Levin sashal@kernel.org --- drivers/i3c/master/mipi-i3c-hci/dat_v1.c | 29 ++++++++++++++++-------- 1 file changed, 19 insertions(+), 10 deletions(-)
diff --git a/drivers/i3c/master/mipi-i3c-hci/dat_v1.c b/drivers/i3c/master/mipi-i3c-hci/dat_v1.c index 97bb49ff5b53b..47b9b4d4ed3fc 100644 --- a/drivers/i3c/master/mipi-i3c-hci/dat_v1.c +++ b/drivers/i3c/master/mipi-i3c-hci/dat_v1.c @@ -64,15 +64,17 @@ static int hci_dat_v1_init(struct i3c_hci *hci) return -EOPNOTSUPP; }
- /* use a bitmap for faster free slot search */ - hci->DAT_data = bitmap_zalloc(hci->DAT_entries, GFP_KERNEL); - if (!hci->DAT_data) - return -ENOMEM; - - /* clear them */ - for (dat_idx = 0; dat_idx < hci->DAT_entries; dat_idx++) { - dat_w0_write(dat_idx, 0); - dat_w1_write(dat_idx, 0); + if (!hci->DAT_data) { + /* use a bitmap for faster free slot search */ + hci->DAT_data = bitmap_zalloc(hci->DAT_entries, GFP_KERNEL); + if (!hci->DAT_data) + return -ENOMEM; + + /* clear them */ + for (dat_idx = 0; dat_idx < hci->DAT_entries; dat_idx++) { + dat_w0_write(dat_idx, 0); + dat_w1_write(dat_idx, 0); + } }
return 0; @@ -87,7 +89,13 @@ static void hci_dat_v1_cleanup(struct i3c_hci *hci) static int hci_dat_v1_alloc_entry(struct i3c_hci *hci) { unsigned int dat_idx; + int ret;
+ if (!hci->DAT_data) { + ret = hci_dat_v1_init(hci); + if (ret) + return ret; + } dat_idx = find_first_zero_bit(hci->DAT_data, hci->DAT_entries); if (dat_idx >= hci->DAT_entries) return -ENOENT; @@ -103,7 +111,8 @@ static void hci_dat_v1_free_entry(struct i3c_hci *hci, unsigned int dat_idx) { dat_w0_write(dat_idx, 0); dat_w1_write(dat_idx, 0); - __clear_bit(dat_idx, hci->DAT_data); + if (hci->DAT_data) + __clear_bit(dat_idx, hci->DAT_data); }
static void hci_dat_v1_set_dynamic_addr(struct i3c_hci *hci,
linux-stable-mirror@lists.linaro.org
-
Sasha Levin