Re: [syzbot] UBSAN: array-index-out-of-bounds in pvr2_i2c_core_init

[Pavel Skripkin <paskripkin@gmail.com>]

[-- Attachment #1.1.1: Type: text/plain, Size: 1167 bytes --]

On 4/14/22 23:14, syzbot wrote:
> Hello,
> 
> syzbot has tested the proposed patch but the reproducer is still triggering an issue:
> WARNING in pvr2_hdw_create
> 
> pvrusb2: Hardware description: OnAir USB2 Hybrid USB tuner
> ------------[ cut here ]------------
> WARNING: CPU: 1 PID: 3607 at kernel/workqueue.c:3066 __flush_work+0x926/0xb10 kernel/workqueue.c:3066
> Modules linked in:
> CPU: 1 PID: 3607 Comm: kworker/1:4 Not tainted 5.18.0-rc2-syzkaller-00187-g115acbb56978-dirty #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> Workqueue: usb_hub_wq hub_event
> RIP: 0010:__flush_work+0x926/0xb10 kernel/workqueue.c:3066
> Code: ff 41 89 c4 89 8d 88 fe ff ff e8 d5 2b 77 00 48 0f ba 2b 03 e9 6a fa ff ff e8 c6 8f 2b 00 0f 0b e9 5a fc ff ff e8 ba 8f 2b 00 <0f> 0b 45 31 f6 e9 4b fc ff ff e8 eb 28 77 00 e9 3a fb ff ff e8 a1
> RSP: 0018:ffffc90003d1ee00 EFLAGS: 00010293
> 

Not bad guess.

Moving work initialization upper, since rewriting error handling is boring


#syz test:
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master




With regards,
Pavel Skripkin

[-- Attachment #1.1.2: ph --]
[-- Type: text/plain, Size: 1141 bytes --]

diff --git a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c
index cd7b118d5929..f3a38f106212 100644
--- a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c
+++ b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c
@@ -2569,6 +2569,11 @@ struct pvr2_hdw *pvr2_hdw_create(struct usb_interface *intf,
 	} while (0);
 	mutex_unlock(&pvr2_unit_mtx);
 
+	INIT_WORK(&hdw->workpoll,pvr2_hdw_worker_poll);
+
+	if (hdw->unit_number == -1)
+		goto fail;
+
 	cnt1 = 0;
 	cnt2 = scnprintf(hdw->name+cnt1,sizeof(hdw->name)-cnt1,"pvrusb2");
 	cnt1 += cnt2;
@@ -2580,8 +2585,6 @@ struct pvr2_hdw *pvr2_hdw_create(struct usb_interface *intf,
 	if (cnt1 >= sizeof(hdw->name)) cnt1 = sizeof(hdw->name)-1;
 	hdw->name[cnt1] = 0;
 
-	INIT_WORK(&hdw->workpoll,pvr2_hdw_worker_poll);
-
 	pvr2_trace(PVR2_TRACE_INIT,"Driver unit number is %d, name is %s",
 		   hdw->unit_number,hdw->name);
 
@@ -2600,7 +2603,7 @@ struct pvr2_hdw *pvr2_hdw_create(struct usb_interface *intf,
 	mutex_init(&hdw->big_lock_mutex);
 
 	return hdw;
- fail:
+fail:
 	if (hdw) {
 		del_timer_sync(&hdw->quiescent_timer);
 		del_timer_sync(&hdw->decoder_stabilization_timer);

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]