Skip to content

Commit 0e2ee70

Browse files
Thadeu Lima de Souza CascardoHans Verkuil
Thadeu Lima de Souza Cascardo
authored and
Hans Verkuil
committed
media: uvcvideo: Mark invalid entities with id UVC_INVALID_ENTITY_ID
Per UVC 1.1+ specification 3.7.2, units and terminals must have a non-zero unique ID. ``` Each Unit and Terminal within the video function is assigned a unique identification number, the Unit ID (UID) or Terminal ID (TID), contained in the bUnitID or bTerminalID field of the descriptor. The value 0x00 is reserved for undefined ID, ``` If we add a new entity with id 0 or a duplicated ID, it will be marked as UVC_INVALID_ENTITY_ID. In a previous attempt commit 3dd075f ("media: uvcvideo: Require entities to have a non-zero unique ID"), we ignored all the invalid units, this broke a lot of non-compatible cameras. Hopefully we are more lucky this time. This also prevents some syzkaller reproducers from triggering warnings due to a chain of entities referring to themselves. In one particular case, an Output Unit is connected to an Input Unit, both with the same ID of 1. But when looking up for the source ID of the Output Unit, that same entity is found instead of the input entity, which leads to such warnings. In another case, a backward chain was considered finished as the source ID was 0. Later on, that entity was found, but its pads were not valid. Here is a sample stack trace for one of those cases. [ 20.650953] usb 1-1: new high-speed USB device number 2 using dummy_hcd [ 20.830206] usb 1-1: Using ep0 maxpacket: 8 [ 20.833501] usb 1-1: config 0 descriptor?? [ 21.038518] usb 1-1: string descriptor 0 read error: -71 [ 21.038893] usb 1-1: Found UVC 0.00 device <unnamed> (2833:0201) [ 21.039299] uvcvideo 1-1:0.0: Entity type for entity Output 1 was not initialized! [ 21.041583] uvcvideo 1-1:0.0: Entity type for entity Input 1 was not initialized! [ 21.042218] ------------[ cut here ]------------ [ 21.042536] WARNING: CPU: 0 PID: 9 at drivers/media/mc/mc-entity.c:1147 media_create_pad_link+0x2c4/0x2e0 [ 21.043195] Modules linked in: [ 21.043535] CPU: 0 UID: 0 PID: 9 Comm: kworker/0:1 Not tainted 6.11.0-rc7-00030-g3480e43aeccf #444 [ 21.044101] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.15.0-1 04/01/2014 [ 21.044639] Workqueue: usb_hub_wq hub_event [ 21.045100] RIP: 0010:media_create_pad_link+0x2c4/0x2e0 [ 21.045508] Code: fe e8 20 01 00 00 b8 f4 ff ff ff 48 83 c4 30 5b 41 5c 41 5d 41 5e 41 5f 5d c3 cc cc cc cc 0f 0b eb e9 0f 0b eb 0a 0f 0b eb 06 <0f> 0b eb 02 0f 0b b8 ea ff ff ff eb d4 66 2e 0f 1f 84 00 00 00 00 [ 21.046801] RSP: 0018:ffffc9000004b318 EFLAGS: 00010246 [ 21.047227] RAX: ffff888004e5d458 RBX: 0000000000000000 RCX: ffffffff818fccf1 [ 21.047719] RDX: 000000000000007b RSI: 0000000000000000 RDI: ffff888004313290 [ 21.048241] RBP: ffff888004313290 R08: 0001ffffffffffff R09: 0000000000000000 [ 21.048701] R10: 0000000000000013 R11: 0001888004313290 R12: 0000000000000003 [ 21.049138] R13: ffff888004313080 R14: ffff888004313080 R15: 0000000000000000 [ 21.049648] FS: 0000000000000000(0000) GS:ffff88803ec00000(0000) knlGS:0000000000000000 [ 21.050271] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 21.050688] CR2: 0000592cc27635b0 CR3: 000000000431c000 CR4: 0000000000750ef0 [ 21.051136] PKRU: 55555554 [ 21.051331] Call Trace: [ 21.051480] <TASK> [ 21.051611] ? __warn+0xc4/0x210 [ 21.051861] ? media_create_pad_link+0x2c4/0x2e0 [ 21.052252] ? report_bug+0x11b/0x1a0 [ 21.052540] ? trace_hardirqs_on+0x31/0x40 [ 21.052901] ? handle_bug+0x3d/0x70 [ 21.053197] ? exc_invalid_op+0x1a/0x50 [ 21.053511] ? asm_exc_invalid_op+0x1a/0x20 [ 21.053924] ? media_create_pad_link+0x91/0x2e0 [ 21.054364] ? media_create_pad_link+0x2c4/0x2e0 [ 21.054834] ? media_create_pad_link+0x91/0x2e0 [ 21.055131] ? _raw_spin_unlock+0x1e/0x40 [ 21.055441] ? __v4l2_device_register_subdev+0x202/0x210 [ 21.055837] uvc_mc_register_entities+0x358/0x400 [ 21.056144] uvc_register_chains+0x1fd/0x290 [ 21.056413] uvc_probe+0x380e/0x3dc0 [ 21.056676] ? __lock_acquire+0x5aa/0x26e0 [ 21.056946] ? find_held_lock+0x33/0xa0 [ 21.057196] ? kernfs_activate+0x70/0x80 [ 21.057533] ? usb_match_dynamic_id+0x1b/0x70 [ 21.057811] ? find_held_lock+0x33/0xa0 [ 21.058047] ? usb_match_dynamic_id+0x55/0x70 [ 21.058330] ? lock_release+0x124/0x260 [ 21.058657] ? usb_match_one_id_intf+0xa2/0x100 [ 21.058997] usb_probe_interface+0x1ba/0x330 [ 21.059399] really_probe+0x1ba/0x4c0 [ 21.059662] __driver_probe_device+0xb2/0x180 [ 21.059944] driver_probe_device+0x5a/0x100 [ 21.060170] __device_attach_driver+0xe9/0x160 [ 21.060427] ? __pfx___device_attach_driver+0x10/0x10 [ 21.060872] bus_for_each_drv+0xa9/0x100 [ 21.061312] __device_attach+0xed/0x190 [ 21.061812] device_initial_probe+0xe/0x20 [ 21.062229] bus_probe_device+0x4d/0xd0 [ 21.062590] device_add+0x308/0x590 [ 21.062912] usb_set_configuration+0x7b6/0xaf0 [ 21.063403] usb_generic_driver_probe+0x36/0x80 [ 21.063714] usb_probe_device+0x7b/0x130 [ 21.063936] really_probe+0x1ba/0x4c0 [ 21.064111] __driver_probe_device+0xb2/0x180 [ 21.064577] driver_probe_device+0x5a/0x100 [ 21.065019] __device_attach_driver+0xe9/0x160 [ 21.065403] ? __pfx___device_attach_driver+0x10/0x10 [ 21.065820] bus_for_each_drv+0xa9/0x100 [ 21.066094] __device_attach+0xed/0x190 [ 21.066535] device_initial_probe+0xe/0x20 [ 21.066992] bus_probe_device+0x4d/0xd0 [ 21.067250] device_add+0x308/0x590 [ 21.067501] usb_new_device+0x347/0x610 [ 21.067817] hub_event+0x156b/0x1e30 [ 21.068060] ? process_scheduled_works+0x48b/0xaf0 [ 21.068337] process_scheduled_works+0x5a3/0xaf0 [ 21.068668] worker_thread+0x3cf/0x560 [ 21.068932] ? kthread+0x109/0x1b0 [ 21.069133] kthread+0x197/0x1b0 [ 21.069343] ? __pfx_worker_thread+0x10/0x10 [ 21.069598] ? __pfx_kthread+0x10/0x10 [ 21.069908] ret_from_fork+0x32/0x40 [ 21.070169] ? __pfx_kthread+0x10/0x10 [ 21.070424] ret_from_fork_asm+0x1a/0x30 [ 21.070737] </TASK> Reported-by: syzbot+0584f746fde3d52b4675@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=0584f746fde3d52b4675 Reported-by: syzbot+dd320d114deb3f5bb79b@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=dd320d114deb3f5bb79b Reported-by: Youngjun Lee <yjjuny.lee@samsung.com> Fixes: a3fbc2e ("media: mc-entity.c: use WARN_ON, validate link pads") Cc: stable@vger.kernel.org Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@igalia.com> Co-developed-by: Ricardo Ribalda <ribalda@chromium.org> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Hans de Goede <hansg@kernel.org> Signed-off-by: Hans de Goede <hansg@kernel.org> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Signed-off-by: Hans Verkuil <hverkuil+cisco@kernel.org>
1 parent 0f99b8b commit 0e2ee70

2 files changed

Lines changed: 48 additions & 27 deletions

File tree

‎drivers/media/usb/uvc/uvc_driver.c‎

Lines changed: 46 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,9 @@ struct uvc_entity *uvc_entity_by_id(struct uvc_device *dev, int id)
137137
{
138138
struct uvc_entity *entity;
139139

140+
if (id == UVC_INVALID_ENTITY_ID)
141+
return NULL;
142+
140143
list_for_each_entry(entity, &dev->entities, list) {
141144
if (entity->id == id)
142145
return entity;
@@ -791,14 +794,27 @@ static const u8 uvc_media_transport_input_guid[16] =
791794
UVC_GUID_UVC_MEDIA_TRANSPORT_INPUT;
792795
static const u8 uvc_processing_guid[16] = UVC_GUID_UVC_PROCESSING;
793796

794-
static struct uvc_entity *uvc_alloc_entity(u16 type, u16 id,
795-
unsigned int num_pads, unsigned int extra_size)
797+
static struct uvc_entity *uvc_alloc_new_entity(struct uvc_device *dev, u16 type,
798+
u16 id, unsigned int num_pads,
799+
unsigned int extra_size)
796800
{
797801
struct uvc_entity *entity;
798802
unsigned int num_inputs;
799803
unsigned int size;
800804
unsigned int i;
801805

806+
/* Per UVC 1.1+ spec 3.7.2, the ID should be non-zero. */
807+
if (id == 0) {
808+
dev_err(&dev->intf->dev, "Found Unit with invalid ID 0\n");
809+
id = UVC_INVALID_ENTITY_ID;
810+
}
811+
812+
/* Per UVC 1.1+ spec 3.7.2, the ID is unique. */
813+
if (uvc_entity_by_id(dev, id)) {
814+
dev_err(&dev->intf->dev, "Found multiple Units with ID %u\n", id);
815+
id = UVC_INVALID_ENTITY_ID;
816+
}
817+
802818
extra_size = roundup(extra_size, sizeof(*entity->pads));
803819
if (num_pads)
804820
num_inputs = type & UVC_TERM_OUTPUT ? num_pads : num_pads - 1;
@@ -808,7 +824,7 @@ static struct uvc_entity *uvc_alloc_entity(u16 type, u16 id,
808824
+ num_inputs;
809825
entity = kzalloc(size, GFP_KERNEL);
810826
if (entity == NULL)
811-
return NULL;
827+
return ERR_PTR(-ENOMEM);
812828

813829
entity->id = id;
814830
entity->type = type;
@@ -920,10 +936,10 @@ static int uvc_parse_vendor_control(struct uvc_device *dev,
920936
break;
921937
}
922938

923-
unit = uvc_alloc_entity(UVC_VC_EXTENSION_UNIT, buffer[3],
924-
p + 1, 2*n);
925-
if (unit == NULL)
926-
return -ENOMEM;
939+
unit = uvc_alloc_new_entity(dev, UVC_VC_EXTENSION_UNIT,
940+
buffer[3], p + 1, 2 * n);
941+
if (IS_ERR(unit))
942+
return PTR_ERR(unit);
927943

928944
memcpy(unit->guid, &buffer[4], 16);
929945
unit->extension.bNumControls = buffer[20];
@@ -1032,10 +1048,10 @@ static int uvc_parse_standard_control(struct uvc_device *dev,
10321048
return -EINVAL;
10331049
}
10341050

1035-
term = uvc_alloc_entity(type | UVC_TERM_INPUT, buffer[3],
1036-
1, n + p);
1037-
if (term == NULL)
1038-
return -ENOMEM;
1051+
term = uvc_alloc_new_entity(dev, type | UVC_TERM_INPUT,
1052+
buffer[3], 1, n + p);
1053+
if (IS_ERR(term))
1054+
return PTR_ERR(term);
10391055

10401056
if (UVC_ENTITY_TYPE(term) == UVC_ITT_CAMERA) {
10411057
term->camera.bControlSize = n;
@@ -1091,10 +1107,10 @@ static int uvc_parse_standard_control(struct uvc_device *dev,
10911107
return 0;
10921108
}
10931109

1094-
term = uvc_alloc_entity(type | UVC_TERM_OUTPUT, buffer[3],
1095-
1, 0);
1096-
if (term == NULL)
1097-
return -ENOMEM;
1110+
term = uvc_alloc_new_entity(dev, type | UVC_TERM_OUTPUT,
1111+
buffer[3], 1, 0);
1112+
if (IS_ERR(term))
1113+
return PTR_ERR(term);
10981114

10991115
memcpy(term->baSourceID, &buffer[7], 1);
11001116

@@ -1113,9 +1129,10 @@ static int uvc_parse_standard_control(struct uvc_device *dev,
11131129
return -EINVAL;
11141130
}
11151131

1116-
unit = uvc_alloc_entity(buffer[2], buffer[3], p + 1, 0);
1117-
if (unit == NULL)
1118-
return -ENOMEM;
1132+
unit = uvc_alloc_new_entity(dev, buffer[2], buffer[3],
1133+
p + 1, 0);
1134+
if (IS_ERR(unit))
1135+
return PTR_ERR(unit);
11191136

11201137
memcpy(unit->baSourceID, &buffer[5], p);
11211138

@@ -1135,9 +1152,9 @@ static int uvc_parse_standard_control(struct uvc_device *dev,
11351152
return -EINVAL;
11361153
}
11371154

1138-
unit = uvc_alloc_entity(buffer[2], buffer[3], 2, n);
1139-
if (unit == NULL)
1140-
return -ENOMEM;
1155+
unit = uvc_alloc_new_entity(dev, buffer[2], buffer[3], 2, n);
1156+
if (IS_ERR(unit))
1157+
return PTR_ERR(unit);
11411158

11421159
memcpy(unit->baSourceID, &buffer[4], 1);
11431160
unit->processing.wMaxMultiplier =
@@ -1164,9 +1181,10 @@ static int uvc_parse_standard_control(struct uvc_device *dev,
11641181
return -EINVAL;
11651182
}
11661183

1167-
unit = uvc_alloc_entity(buffer[2], buffer[3], p + 1, n);
1168-
if (unit == NULL)
1169-
return -ENOMEM;
1184+
unit = uvc_alloc_new_entity(dev, buffer[2], buffer[3],
1185+
p + 1, n);
1186+
if (IS_ERR(unit))
1187+
return PTR_ERR(unit);
11701188

11711189
memcpy(unit->guid, &buffer[4], 16);
11721190
unit->extension.bNumControls = buffer[20];
@@ -1311,9 +1329,10 @@ static int uvc_gpio_parse(struct uvc_device *dev)
13111329
return dev_err_probe(&dev->intf->dev, irq,
13121330
"No IRQ for privacy GPIO\n");
13131331

1314-
unit = uvc_alloc_entity(UVC_EXT_GPIO_UNIT, UVC_EXT_GPIO_UNIT_ID, 0, 1);
1315-
if (!unit)
1316-
return -ENOMEM;
1332+
unit = uvc_alloc_new_entity(dev, UVC_EXT_GPIO_UNIT,
1333+
UVC_EXT_GPIO_UNIT_ID, 0, 1);
1334+
if (IS_ERR(unit))
1335+
return PTR_ERR(unit);
13171336

13181337
unit->gpio.gpio_privacy = gpio_privacy;
13191338
unit->gpio.irq = irq;

‎drivers/media/usb/uvc/uvcvideo.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,8 @@
4141
#define UVC_EXT_GPIO_UNIT 0x7ffe
4242
#define UVC_EXT_GPIO_UNIT_ID 0x100
4343

44+
#define UVC_INVALID_ENTITY_ID 0xffff
45+
4446
/* ------------------------------------------------------------------------
4547
* Driver specific constants.
4648
*/

0 commit comments

Comments
 (0)