[PATCH] media: usb: hackrf: add null ptr check in hackrf_ctrl_msg

[PATCH] media: usb: hackrf: add null ptr check in hackrf_ctrl_msg

[Mingxuan Xiang]

If the user yanks out the cable before closing the file,
dev->udev would be set to NULL therefore causing a null-ptr-deref
in hackrf_ctrl_msg issued by hackrf_stop_streaming.

This patch adds a check in hackrf_ctrl_msg before using
dev->udev.

Found by modified syzkaller.

BUG: KASAN: null-ptr-deref in hackrf_ctrl_msg+0x6d/0x180 drivers/media/usb/hackrf/hackrf.c:195
Read of size 4 at addr 0000000000000000 by task syz-executor/579
Call Trace:
 <TASK>
 __dump_stack lib/dump_stack.c:88 [inline]
 dump_stack_lvl+0x5e/0x7c lib/dump_stack.c:106
 print_report.cold+0x49a/0x6bb mm/kasan/report.c:436
 kasan_report+0xa8/0x130 mm/kasan/report.c:495
 hackrf_ctrl_msg+0x6d/0x180 drivers/media/usb/hackrf/hackrf.c:195
 hackrf_stop_streaming+0x45/0x140 drivers/media/usb/hackrf/hackrf.c:869
 __vb2_queue_cancel+0x5c/0x550 drivers/media/common/videobuf2/videobuf2-core.c:1992
 vb2_core_streamoff+0x2f/0xb0 drivers/media/common/videobuf2/videobuf2-core.c:2149
 __vb2_cleanup_fileio+0x3e/0xa0 drivers/media/common/videobuf2/videobuf2-core.c:2710
 vb2_core_queue_release+0x1a/0x50 drivers/media/common/videobuf2/videobuf2-core.c:2430
 vb2_queue_release drivers/media/common/videobuf2/videobuf2-v4l2.c:947 [inline]
 _vb2_fop_release+0x110/0x140 drivers/media/common/videobuf2/videobuf2-v4l2.c:1132
 v4l2_release+0x1b9/0x1e0 drivers/media/v4l2-core/v4l2-dev.c:459
 __fput+0x12d/0x4b0 fs/file_table.c:320
 task_work_run+0xa8/0xf0 kernel/task_work.c:177
 resume_user_mode_work include/linux/resume_user_mode.h:49 [inline]
 exit_to_user_mode_loop kernel/entry/common.c:169 [inline]
 exit_to_user_mode_prepare+0x123/0x130 kernel/entry/common.c:201
 __syscall_exit_to_user_mode_work kernel/entry/common.c:283 [inline]
 syscall_exit_to_user_mode+0x22/0x50 kernel/entry/common.c:294
 do_syscall_64+0x48/0x90 arch/x86/entry/common.c:86
 entry_SYSCALL_64_after_hwframe+0x63/0xcd

Signed-off-by: Mingxuan Xiang <mx_xiang@hust.edu.cn>
---
 drivers/media/usb/hackrf/hackrf.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/media/usb/hackrf/hackrf.c b/drivers/media/usb/hackrf/hackrf.c
index 9c0ecd5f056c..9588b8aa6e98 100644
--- a/drivers/media/usb/hackrf/hackrf.c
+++ b/drivers/media/usb/hackrf/hackrf.c
@@ -186,6 +186,11 @@ static int hackrf_ctrl_msg(struct hackrf_dev *dev, u8 request, u16 value,
 	unsigned int pipe;
 	u8 requesttype;
 
+	if (!dev->udev) {
+		pr_err("udev is null in %s\n", __func__);
+		ret = -EINVAL;
+		goto err;
+	}
 	switch (request) {
 	case CMD_SET_TRANSCEIVER_MODE:
 	case CMD_SET_FREQ:
-- 
2.43.0

Re: [PATCH] media: usb: hackrf: add null ptr check in hackrf_ctrl_msg

[Hans Verkuil]

On 22/01/2024 12:22, Mingxuan Xiang wrote:
> If the user yanks out the cable before closing the file,
> dev->udev would be set to NULL therefore causing a null-ptr-deref
> in hackrf_ctrl_msg issued by hackrf_stop_streaming.
> 
> This patch adds a check in hackrf_ctrl_msg before using
> dev->udev.
> 
> Found by modified syzkaller.
> 
> BUG: KASAN: null-ptr-deref in hackrf_ctrl_msg+0x6d/0x180 drivers/media/usb/hackrf/hackrf.c:195
> Read of size 4 at addr 0000000000000000 by task syz-executor/579
> Call Trace:
>  <TASK>
>  __dump_stack lib/dump_stack.c:88 [inline]
>  dump_stack_lvl+0x5e/0x7c lib/dump_stack.c:106
>  print_report.cold+0x49a/0x6bb mm/kasan/report.c:436
>  kasan_report+0xa8/0x130 mm/kasan/report.c:495
>  hackrf_ctrl_msg+0x6d/0x180 drivers/media/usb/hackrf/hackrf.c:195
>  hackrf_stop_streaming+0x45/0x140 drivers/media/usb/hackrf/hackrf.c:869
>  __vb2_queue_cancel+0x5c/0x550 drivers/media/common/videobuf2/videobuf2-core.c:1992
>  vb2_core_streamoff+0x2f/0xb0 drivers/media/common/videobuf2/videobuf2-core.c:2149
>  __vb2_cleanup_fileio+0x3e/0xa0 drivers/media/common/videobuf2/videobuf2-core.c:2710
>  vb2_core_queue_release+0x1a/0x50 drivers/media/common/videobuf2/videobuf2-core.c:2430
>  vb2_queue_release drivers/media/common/videobuf2/videobuf2-v4l2.c:947 [inline]
>  _vb2_fop_release+0x110/0x140 drivers/media/common/videobuf2/videobuf2-v4l2.c:1132
>  v4l2_release+0x1b9/0x1e0 drivers/media/v4l2-core/v4l2-dev.c:459
>  __fput+0x12d/0x4b0 fs/file_table.c:320
>  task_work_run+0xa8/0xf0 kernel/task_work.c:177
>  resume_user_mode_work include/linux/resume_user_mode.h:49 [inline]
>  exit_to_user_mode_loop kernel/entry/common.c:169 [inline]
>  exit_to_user_mode_prepare+0x123/0x130 kernel/entry/common.c:201
>  __syscall_exit_to_user_mode_work kernel/entry/common.c:283 [inline]
>  syscall_exit_to_user_mode+0x22/0x50 kernel/entry/common.c:294
>  do_syscall_64+0x48/0x90 arch/x86/entry/common.c:86
>  entry_SYSCALL_64_after_hwframe+0x63/0xcd
> 
> Signed-off-by: Mingxuan Xiang <mx_xiang@hust.edu.cn>
> ---
>  drivers/media/usb/hackrf/hackrf.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/media/usb/hackrf/hackrf.c b/drivers/media/usb/hackrf/hackrf.c
> index 9c0ecd5f056c..9588b8aa6e98 100644
> --- a/drivers/media/usb/hackrf/hackrf.c
> +++ b/drivers/media/usb/hackrf/hackrf.c
> @@ -186,6 +186,11 @@ static int hackrf_ctrl_msg(struct hackrf_dev *dev, u8 request, u16 value,
>  	unsigned int pipe;
>  	u8 requesttype;
>  
> +	if (!dev->udev) {
> +		pr_err("udev is null in %s\n", __func__);
> +		ret = -EINVAL;
> +		goto err;
> +	}

Just replace this with:

	if (!dev->udev)
		return -ENODEV;

-ENODEV is the correct error code for this situation, and the error
message is overkill here, I think.

Regards,

	Hans

>  	switch (request) {
>  	case CMD_SET_TRANSCEIVER_MODE:
>  	case CMD_SET_FREQ: