From b0816027543f2bce3be9d43d7994428bb4d6d0d4 Mon Sep 17 00:00:00 2001 From: Xiaoming Xiang Date: Thu, 10 Apr 2025 07:10:20 +0000 Subject: [PATCH] fuzz: nvidia-oot: fix syzkaller fuzz test crash fix cdi and fusa kmd module crash Jira CAMERASW-30661 Change-Id: I78b49a5f0d65f7c69ff1f1a8b746c4091d4adeb7 Signed-off-by: Xiaoming Xiang Reviewed-on: https://git-master.nvidia.com/r/c/linux-nv-oot/+/3337853 Reviewed-by: Ming Chang (SW-TEGRA) GVS: buildbot_gerritrpt Reviewed-by: Patrick Young Reviewed-by: Frank Chen --- .../camera/fusa-capture/capture-isp-channel.c | 45 ++++++++++++++++--- .../tegra/camera/fusa-capture/capture-isp.c | 29 ++++++++++-- .../camera/fusa-capture/capture-vi-channel.c | 36 ++++++++++----- .../tegra/camera/fusa-capture/capture-vi.c | 9 ++-- drivers/media/platform/tegra/cdi/cdi_dev.c | 10 ++++- drivers/media/platform/tegra/cdi/cdi_mgr.c | 45 ++++++++++++++----- drivers/media/platform/tegra/isc/isc_mgr.c | 10 ++--- 7 files changed, 143 insertions(+), 41 deletions(-) diff --git a/drivers/media/platform/tegra/camera/fusa-capture/capture-isp-channel.c b/drivers/media/platform/tegra/camera/fusa-capture/capture-isp-channel.c index 9420b555..3a694d4e 100644 --- a/drivers/media/platform/tegra/camera/fusa-capture/capture-isp-channel.c +++ b/drivers/media/platform/tegra/camera/fusa-capture/capture-isp-channel.c @@ -253,6 +253,12 @@ static int isp_channel_open( mutex_unlock(&chdrv_lock); return -ENODEV; } + + if (chan_drv->isp_capture_pdev == NULL) { + mutex_unlock(&chdrv_lock); + return -ENODEV; + } + mutex_unlock(&chdrv_lock); chan = kzalloc(sizeof(*chan), GFP_KERNEL); @@ -436,6 +442,11 @@ static long isp_channel_ioctl( return -EINVAL; } + if (unlikely(ptr == NULL)) { + pr_err("%s: invalid argument user pointer\n", __func__); + return -EINVAL; + } + switch (_IOC_NR(cmd)) { case _IOC_NR(ISP_CAPTURE_SETUP): { struct isp_capture_setup setup = {}; @@ -641,6 +652,11 @@ int isp_channel_drv_register( { struct isp_channel_drv *chan_drv; unsigned int i; + int err = 0; + struct device *dev; + + if (unlikely(ndev == NULL)) + return -ENOMEM; chan_drv = kzalloc(offsetof(struct isp_channel_drv, channels[max_isp_channels]), GFP_KERNEL); @@ -657,25 +673,44 @@ int isp_channel_drv_register( if (chdrv_ != NULL) { dev_warn(chan_drv->dev, "%s: dev is busy\n", __func__); mutex_unlock(&chdrv_lock); - kfree(chan_drv); - return -EBUSY; + err = -EBUSY; + goto error; } chdrv_ = chan_drv; mutex_unlock(&chdrv_lock); if (isp_channel_major < 0) { pr_err("%s: Invalid major number for ISP channel\n", __func__); - return -EINVAL; + err = -EINVAL; + goto error; } for (i = 0; i < chan_drv->num_channels; i++) { dev_t devt = MKDEV(isp_channel_major, i); - - device_create(isp_channel_class, &chan_drv->isp_capture_pdev->dev, devt, NULL, + dev = device_create(isp_channel_class, &chan_drv->isp_capture_pdev->dev, devt, NULL, "capture-isp-channel%u", i); + if (IS_ERR(dev)) { + pr_err("%s: Failed to create device\n", __func__); + err = PTR_ERR(dev); + goto error_destroy; + } } return 0; + +error_destroy: + while (i--) { + dev_t devt = MKDEV(isp_channel_major, i); + + device_destroy(isp_channel_class, devt); + } + +error: + mutex_lock(&chdrv_lock); + chdrv_ = NULL; + mutex_unlock(&chdrv_lock); + kfree(chan_drv); + return err; } EXPORT_SYMBOL(isp_channel_drv_register); diff --git a/drivers/media/platform/tegra/camera/fusa-capture/capture-isp.c b/drivers/media/platform/tegra/camera/fusa-capture/capture-isp.c index e98315db..652ef365 100644 --- a/drivers/media/platform/tegra/camera/fusa-capture/capture-isp.c +++ b/drivers/media/platform/tegra/camera/fusa-capture/capture-isp.c @@ -1584,18 +1584,21 @@ void isp_get_nvhost_device( platform_get_drvdata(chan->isp_capture_pdev); if (setup == NULL) { - dev_err(chan->isp_dev, "%s: Invalid ISP capture request\n", __func__); + pr_err("%s: Invalid ISP capture request\n", __func__); return; } isp_inst = setup->isp_unit; if (isp_inst >= MAX_ISP_UNITS) { - dev_err(chan->isp_dev, - "%s: ISP unit index is out of bound\n", __func__); + pr_err("%s: ISP unit index is out of bound\n", __func__); return; } + if (info->isp_pdevices[isp_inst] == NULL) { + pr_err("%s:ISP devices[%u] is NULL\n", __func__, isp_inst); + return; + } chan->isp_dev = &info->isp_pdevices[isp_inst]->dev; chan->ndev = info->isp_pdevices[isp_inst]; } @@ -3435,7 +3438,18 @@ static int capture_isp_probe(struct platform_device *pdev) uint32_t i; int err = 0; struct tegra_capture_isp_data *info; - struct device *dev = &pdev->dev; + struct device *dev; + + if (pdev == NULL) { + pr_err("%s: Invalid platform device\n", __func__); + return -EINVAL; + } + + dev = &pdev->dev; + if (dev == NULL) { + pr_err("%s: Invalid device\n", __func__); + return -EINVAL; + } dev_dbg(dev, "%s:tegra-camrtc-capture-isp probe\n", __func__); @@ -3445,6 +3459,11 @@ static int capture_isp_probe(struct platform_device *pdev) info->num_isp_devices = 0; + if (dev->of_node == NULL) { + dev_err(dev, "No device tree node found\n"); + return -EINVAL; + } + err = of_property_read_u32(dev->of_node, "nvidia,isp-max-channels", &info->max_isp_channels); if (err < 0) { @@ -3463,6 +3482,8 @@ static int capture_isp_probe(struct platform_device *pdev) } } + memset(info->isp_pdevices, 0, sizeof(info->isp_pdevices)); + i = 0U; do { struct device_node *node; diff --git a/drivers/media/platform/tegra/camera/fusa-capture/capture-vi-channel.c b/drivers/media/platform/tegra/camera/fusa-capture/capture-vi-channel.c index 9f32dd51..e36c665d 100644 --- a/drivers/media/platform/tegra/camera/fusa-capture/capture-vi-channel.c +++ b/drivers/media/platform/tegra/camera/fusa-capture/capture-vi-channel.c @@ -262,7 +262,7 @@ struct tegra_vi_channel *vi_channel_open_ex( } mutex_unlock(&chdrv_lock); - chan = kzalloc(sizeof(*chan), GFP_KERNEL); + chan = kzalloc(sizeof(*chan), GFP_KERNEL | GFP_NOWAIT); if (unlikely(chan == NULL)) return ERR_PTR(-ENOMEM); @@ -598,6 +598,7 @@ static long vi_channel_ioctl( switch (_IOC_NR(cmd)) { case _IOC_NR(VI_CAPTURE_SETUP): { struct vi_capture_setup setup = {}; + struct capture_buffer_table *buffer_ctx; if (copy_from_user(&setup, ptr, sizeof(setup))) break; @@ -621,10 +622,10 @@ static long vi_channel_ioctl( return -EFAULT; } - capture->buf_ctx = create_buffer_table(chan->dev); - if (capture->buf_ctx == NULL) { + buffer_ctx = create_buffer_table(chan->dev); + if (buffer_ctx == NULL) { dev_err(chan->dev, "vi buffer setup failed"); - break; + return -ENOMEM; } /* pin the capture descriptor ring buffer */ @@ -633,8 +634,8 @@ static long vi_channel_ioctl( if (err < 0) { dev_err(chan->dev, "%s: memory setup failed\n", __func__); - destroy_buffer_table(capture->buf_ctx); - capture->buf_ctx = NULL; + destroy_buffer_table(buffer_ctx); + buffer_ctx = NULL; return -EFAULT; } @@ -645,8 +646,8 @@ static long vi_channel_ioctl( "%s: descriptor buffer is too small for given queue depth\n", __func__); capture_common_unpin_memory(&capture->requests); - destroy_buffer_table(capture->buf_ctx); - capture->buf_ctx = NULL; + destroy_buffer_table(buffer_ctx); + buffer_ctx = NULL; return -ENOMEM; } @@ -655,10 +656,12 @@ static long vi_channel_ioctl( if (err < 0) { dev_err(chan->dev, "vi capture setup failed\n"); capture_common_unpin_memory(&capture->requests); - destroy_buffer_table(capture->buf_ctx); - capture->buf_ctx = NULL; + destroy_buffer_table(buffer_ctx); + buffer_ctx = NULL; return err; } + + capture->buf_ctx = buffer_ctx; break; } @@ -694,8 +697,12 @@ static long vi_channel_ioctl( for (i = 0; i < capture->queue_depth; i++) vi_capture_request_unpin(chan, i); capture_common_unpin_memory(&capture->requests); - destroy_buffer_table(capture->buf_ctx); - capture->buf_ctx = NULL; + + if (capture->buf_ctx != NULL) { + destroy_buffer_table(capture->buf_ctx); + capture->buf_ctx = NULL; + } + vfree(capture->unpins_list); capture->unpins_list = NULL; } @@ -814,6 +821,11 @@ static long vi_channel_ioctl( if (copy_from_user(&req, ptr, sizeof(req)) != 0U) break; + if (capture->buf_ctx == NULL) { + dev_err(chan->dev, "vi buffer setup not done\n"); + break; + } + err = capture_buffer_request( capture->buf_ctx, req.mem, req.flag); if (err < 0) diff --git a/drivers/media/platform/tegra/camera/fusa-capture/capture-vi.c b/drivers/media/platform/tegra/camera/fusa-capture/capture-vi.c index 899e42b4..e410bc4c 100644 --- a/drivers/media/platform/tegra/camera/fusa-capture/capture-vi.c +++ b/drivers/media/platform/tegra/camera/fusa-capture/capture-vi.c @@ -1624,6 +1624,7 @@ int vi_capture_control_message_from_user( void *msg_cpy; struct CAPTURE_CONTROL_MSG *resp_msg; int err = 0; + size_t copy_size = 0; if (chan == NULL) { dev_err(NULL, "%s: NULL VI channel received\n", __func__); @@ -1655,16 +1656,18 @@ int vi_capture_control_message_from_user( msg_ptr = (const void __user *)(uintptr_t)msg->ptr; response = (void __user *)(uintptr_t)msg->response; - msg_cpy = kzalloc(msg->size, GFP_KERNEL); + msg_cpy = kzalloc(sizeof(struct CAPTURE_CONTROL_MSG), GFP_KERNEL); if (unlikely(msg_cpy == NULL)) return -ENOMEM; - err = copy_from_user(msg_cpy, msg_ptr, msg->size) ? -EFAULT : 0; + copy_size = min_t(size_t, msg->size, sizeof(struct CAPTURE_CONTROL_MSG)); + + err = copy_from_user(msg_cpy, msg_ptr, copy_size) ? -EFAULT : 0; if (err < 0) goto fail; - err = vi_capture_control_send_message(chan, msg_cpy, msg->size); + err = vi_capture_control_send_message(chan, msg_cpy, copy_size); if (err < 0) goto fail; diff --git a/drivers/media/platform/tegra/cdi/cdi_dev.c b/drivers/media/platform/tegra/cdi/cdi_dev.c index da180365..eb924b68 100644 --- a/drivers/media/platform/tegra/cdi/cdi_dev.c +++ b/drivers/media/platform/tegra/cdi/cdi_dev.c @@ -181,6 +181,13 @@ int cdi_dev_raw_rd( if (!offset_len) offset_len = info->reg_len; + if (offset_len > 2) { + dev_err(info->dev, "%s: offset_len %u exceeds maximum supported length of 2\n", + __func__, offset_len); + mutex_unlock(&info->mutex); + return -EINVAL; + } + if (offset_len == 2) { data[0] = (u8)((offset >> 8) & 0xff); data[1] = (u8)(offset & 0xff); @@ -258,6 +265,7 @@ int cdi_dev_raw_wr( (unsigned int)((size % MAX_MSG_SIZE) ? 1 : 0), &num_msgs)) { dev_err(info->dev, "%s: calculate the num_msgs due to an overflow\n", __func__); + mutex_unlock(&info->mutex); return -ENOMEM; } @@ -369,7 +377,7 @@ static int cdi_dev_get_package( return -EINVAL; } - if (!info->rw_pkg.size) { + if (!info->rw_pkg.size || info->rw_pkg.size > MAX_MSG_SIZE) { dev_err(info->dev, "%s invalid package size %d\n", __func__, info->rw_pkg.size); return -EINVAL; diff --git a/drivers/media/platform/tegra/cdi/cdi_mgr.c b/drivers/media/platform/tegra/cdi/cdi_mgr.c index dec7e12a..864b9b30 100644 --- a/drivers/media/platform/tegra/cdi/cdi_mgr.c +++ b/drivers/media/platform/tegra/cdi/cdi_mgr.c @@ -513,15 +513,38 @@ static int __cdi_create_dev( }; int err = 0; + if (cdi_mgr == NULL || new_dev == NULL) { + pr_err("%s: invalid dev params\n", __func__); + return -EINVAL; + } - if (new_dev->addr >= 0x80 || new_dev->drv_name[0] == '\0' || - (new_dev->val_bits != 8 && new_dev->val_bits != 16) || - (new_dev->reg_bits != 0 && new_dev->reg_bits != 8 && - new_dev->reg_bits != 16)) { + if (new_dev->addr >= 0x80) { dev_err(cdi_mgr->dev, - "%s: invalid cdi dev params: %s %x %d %d\n", - __func__, new_dev->drv_name, new_dev->addr, - new_dev->reg_bits, new_dev->val_bits); + "%s: invalid cdi dev addr: %x\n", __func__, new_dev->addr); + return -EINVAL; + } + + if (new_dev->drv_name[0] == '\0') { + dev_err(cdi_mgr->dev, + "%s: invalid cdi dev name: %s\n", __func__, new_dev->drv_name); + return -EINVAL; + } + + if (new_dev->val_bits != 8 && new_dev->val_bits != 16) { + dev_err(cdi_mgr->dev, + "%s: invalid cdi dev val_bits: %d\n", __func__, new_dev->val_bits); + return -EINVAL; + } + + if (new_dev->reg_bits != 0 && new_dev->reg_bits != 8 && + new_dev->reg_bits != 16) { + dev_err(cdi_mgr->dev, + "%s: invalid cdi dev reg_bits: %d\n", __func__, new_dev->reg_bits); + return -EINVAL; + } + + if (cdi_mgr->adap == NULL) { + dev_err(cdi_mgr->dev, "%s: i2c adapter not found\n", __func__); return -EINVAL; } @@ -558,11 +581,11 @@ static int __cdi_create_dev( brd.addr = cdi_dev->cfg.addr; brd.platform_data = &cdi_dev->pdata; cdi_dev->client = i2c_new_client_device(cdi_mgr->adap, &brd); - if (!cdi_dev->client) { + if (IS_ERR(cdi_dev->client)) { + err = PTR_ERR(cdi_dev->client); dev_err(cdi_mgr->dev, - "%s cannot allocate client: %s bus %d, %x\n", __func__, - cdi_dev->pdata.drv_name, cdi_mgr->adap->nr, brd.addr); - err = -EINVAL; + "%s cannot allocate client: %s bus %d, %x, err: %d\n", __func__, + cdi_dev->pdata.drv_name, cdi_mgr->adap->nr, brd.addr, err); goto dev_create_err; } diff --git a/drivers/media/platform/tegra/isc/isc_mgr.c b/drivers/media/platform/tegra/isc/isc_mgr.c index 7756858b..17284a9c 100644 --- a/drivers/media/platform/tegra/isc/isc_mgr.c +++ b/drivers/media/platform/tegra/isc/isc_mgr.c @@ -1,5 +1,5 @@ // SPDX-License-Identifier: GPL-2.0-only -// SPDX-FileCopyrightText: Copyright (c) 2015-2024 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-FileCopyrightText: Copyright (c) 2015-2025 NVIDIA CORPORATION & AFFILIATES. All rights reserved. #include @@ -305,11 +305,11 @@ static int __isc_create_dev( brd.addr = isc_dev->cfg.addr; brd.platform_data = &isc_dev->pdata; isc_dev->client = i2c_new_client_device(isc_mgr->adap, &brd); - if (!isc_dev->client) { + if (IS_ERR(isc_dev->client)) { + err = PTR_ERR(isc_dev->client); dev_err(isc_mgr->dev, - "%s cannot allocate client: %s bus %d, %x\n", __func__, - isc_dev->pdata.drv_name, isc_mgr->adap->nr, brd.addr); - err = -EINVAL; + "%s cannot allocate client: %s bus %d, %x, err: %d\n", __func__, + isc_dev->pdata.drv_name, isc_mgr->adap->nr, brd.addr, err); goto dev_create_err; }