media: media-request: Fix crash if memory allocation fails
Syzbot reports a NULL-ptr deref in the kref_put() call:
BUG: KASAN: null-ptr-deref in media_request_put drivers/media/mc/mc-request.c:81 [inline]
kref_put include/linux/kref.h:64 [inline]
media_request_put drivers/media/mc/mc-request.c:81 [inline]
media_request_close+0x4d/0x170 drivers/media/mc/mc-request.c:89
__fput+0x2ed/0x750 fs/file_table.c:281
task_work_run+0x147/0x1d0 kernel/task_work.c:123
tracehook_notify_resume include/linux/tracehook.h:188 [inline]
exit_to_usermode_loop arch/x86/entry/common.c:165 [inline]
prepare_exit_to_usermode+0x48e/0x600 arch/x86/entry/common.c:196
What led to this crash was an injected memory allocation failure in
media_request_alloc():
FAULT_INJECTION: forcing a failure.
name failslab, interval 1, probability 0, space 0, times 0
should_failslab+0x5/0x20
kmem_cache_alloc_trace+0x57/0x300
? anon_inode_getfile+0xe5/0x170
media_request_alloc+0x339/0x440
media_device_request_alloc+0x94/0xc0
media_device_ioctl+0x1fb/0x330
? do_vfs_ioctl+0x6ea/0x1a00
? media_ioctl+0x101/0x120
? __media_device_usb_init+0x430/0x430
? media_poll+0x110/0x110
__se_sys_ioctl+0xf9/0x160
do_syscall_64+0xf3/0x1b0
When that allocation fails, filp->private_data is left uninitialized
which media_request_close() does not expect and crashes.
To avoid this, reorder media_request_alloc() such that
allocating the struct file happens as the last step thus
media_request_close() will no longer get called for a partially created
media request.
Reported-by: syzbot+6bed2d543cf7e48b822b@syzkaller.appspotmail.com
Cc: stable@vger.kernel.org
Signed-off-by: Tuomas Tynkkynen <tuomas.tynkkynen@iki.fi>
Fixes: 10905d70d7
("media: media-request: implement media requests")
Reviewed-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
This commit is contained in:
parent
7b98128828
commit
e30cc79cc8
@ -296,9 +296,18 @@ int media_request_alloc(struct media_device *mdev, int *alloc_fd)
|
||||
if (WARN_ON(!mdev->ops->req_alloc ^ !mdev->ops->req_free))
|
||||
return -ENOMEM;
|
||||
|
||||
if (mdev->ops->req_alloc)
|
||||
req = mdev->ops->req_alloc(mdev);
|
||||
else
|
||||
req = kzalloc(sizeof(*req), GFP_KERNEL);
|
||||
if (!req)
|
||||
return -ENOMEM;
|
||||
|
||||
fd = get_unused_fd_flags(O_CLOEXEC);
|
||||
if (fd < 0)
|
||||
return fd;
|
||||
if (fd < 0) {
|
||||
ret = fd;
|
||||
goto err_free_req;
|
||||
}
|
||||
|
||||
filp = anon_inode_getfile("request", &request_fops, NULL, O_CLOEXEC);
|
||||
if (IS_ERR(filp)) {
|
||||
@ -306,15 +315,6 @@ int media_request_alloc(struct media_device *mdev, int *alloc_fd)
|
||||
goto err_put_fd;
|
||||
}
|
||||
|
||||
if (mdev->ops->req_alloc)
|
||||
req = mdev->ops->req_alloc(mdev);
|
||||
else
|
||||
req = kzalloc(sizeof(*req), GFP_KERNEL);
|
||||
if (!req) {
|
||||
ret = -ENOMEM;
|
||||
goto err_fput;
|
||||
}
|
||||
|
||||
filp->private_data = req;
|
||||
req->mdev = mdev;
|
||||
req->state = MEDIA_REQUEST_STATE_IDLE;
|
||||
@ -336,12 +336,15 @@ int media_request_alloc(struct media_device *mdev, int *alloc_fd)
|
||||
|
||||
return 0;
|
||||
|
||||
err_fput:
|
||||
fput(filp);
|
||||
|
||||
err_put_fd:
|
||||
put_unused_fd(fd);
|
||||
|
||||
err_free_req:
|
||||
if (mdev->ops->req_free)
|
||||
mdev->ops->req_free(req);
|
||||
else
|
||||
kfree(req);
|
||||
|
||||
return ret;
|
||||
}
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user