| From e30cc79cc80fd919b697a15c5000d9f57487de8e Mon Sep 17 00:00:00 2001 |
| From: Tuomas Tynkkynen <tuomas.tynkkynen@iki.fi> |
| Date: Sun, 21 Jun 2020 13:30:40 +0200 |
| Subject: media: media-request: Fix crash if memory allocation fails |
| |
| From: Tuomas Tynkkynen <tuomas.tynkkynen@iki.fi> |
| |
| commit e30cc79cc80fd919b697a15c5000d9f57487de8e upstream. |
| |
| 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: 10905d70d788 ("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> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| |
| --- |
| drivers/media/mc/mc-request.c | 31 +++++++++++++++++-------------- |
| 1 file changed, 17 insertions(+), 14 deletions(-) |
| |
| --- a/drivers/media/mc/mc-request.c |
| +++ b/drivers/media/mc/mc-request.c |
| @@ -296,9 +296,18 @@ int media_request_alloc(struct media_dev |
| 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_dev |
| 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_dev |
| |
| 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; |
| } |
| |