| From 0fd85b3ab396e16f1fb1170441a74e02822f92b7 Mon Sep 17 00:00:00 2001 |
| From: Sasha Levin <sashal@kernel.org> |
| Date: Sat, 19 Mar 2022 11:22:22 +0100 |
| Subject: media: uvcvideo: Fix missing check to determine if element is found |
| in list |
| |
| From: Xiaomeng Tong <xiam0nd.tong@gmail.com> |
| |
| [ Upstream commit 261f33388c29f6f3c12a724e6d89172b7f6d5996 ] |
| |
| The list iterator will point to a bogus position containing HEAD if |
| the list is empty or the element is not found in list. This case |
| should be checked before any use of the iterator, otherwise it will |
| lead to a invalid memory access. The missing check here is before |
| "pin = iterm->id;", just add check here to fix the security bug. |
| |
| In addition, the list iterator value will *always* be set and non-NULL |
| by list_for_each_entry(), so it is incorrect to assume that the iterator |
| value will be NULL if the element is not found in list, considering |
| the (mis)use here: "if (iterm == NULL". |
| |
| Use a new value 'it' as the list iterator, while use the old value |
| 'iterm' as a dedicated pointer to point to the found element, which |
| 1. can fix this bug, due to 'iterm' is NULL only if it's not found. |
| 2. do not need to change all the uses of 'iterm' after the loop. |
| 3. can also limit the scope of the list iterator 'it' *only inside* |
| the traversal loop by simply declaring 'it' inside the loop in the |
| future, as usage of the iterator outside of the list_for_each_entry |
| is considered harmful. https://lkml.org/lkml/2022/2/17/1032 |
| |
| Fixes: d5e90b7a6cd1c ("[media] uvcvideo: Move to video_ioctl2") |
| Signed-off-by: Xiaomeng Tong <xiam0nd.tong@gmail.com> |
| Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> |
| Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org> |
| Signed-off-by: Sasha Levin <sashal@kernel.org> |
| --- |
| drivers/media/usb/uvc/uvc_v4l2.c | 20 +++++++++++--------- |
| 1 file changed, 11 insertions(+), 9 deletions(-) |
| |
| diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c |
| index 2b1e06e825f0..53d81ef9a4be 100644 |
| --- a/drivers/media/usb/uvc/uvc_v4l2.c |
| +++ b/drivers/media/usb/uvc/uvc_v4l2.c |
| @@ -846,29 +846,31 @@ static int uvc_ioctl_enum_input(struct file *file, void *fh, |
| struct uvc_video_chain *chain = handle->chain; |
| const struct uvc_entity *selector = chain->selector; |
| struct uvc_entity *iterm = NULL; |
| + struct uvc_entity *it; |
| u32 index = input->index; |
| - int pin = 0; |
| |
| if (selector == NULL || |
| (chain->dev->quirks & UVC_QUIRK_IGNORE_SELECTOR_UNIT)) { |
| if (index != 0) |
| return -EINVAL; |
| - list_for_each_entry(iterm, &chain->entities, chain) { |
| - if (UVC_ENTITY_IS_ITERM(iterm)) |
| + list_for_each_entry(it, &chain->entities, chain) { |
| + if (UVC_ENTITY_IS_ITERM(it)) { |
| + iterm = it; |
| break; |
| + } |
| } |
| - pin = iterm->id; |
| } else if (index < selector->bNrInPins) { |
| - pin = selector->baSourceID[index]; |
| - list_for_each_entry(iterm, &chain->entities, chain) { |
| - if (!UVC_ENTITY_IS_ITERM(iterm)) |
| + list_for_each_entry(it, &chain->entities, chain) { |
| + if (!UVC_ENTITY_IS_ITERM(it)) |
| continue; |
| - if (iterm->id == pin) |
| + if (it->id == selector->baSourceID[index]) { |
| + iterm = it; |
| break; |
| + } |
| } |
| } |
| |
| - if (iterm == NULL || iterm->id != pin) |
| + if (iterm == NULL) |
| return -EINVAL; |
| |
| memset(input, 0, sizeof(*input)); |
| -- |
| 2.35.1 |
| |