"Badhri Jagan Sridharan (Gerrit)" Hi, <noreply-gerritcodereview-MIY7KvT_1CRPvjnb_n9qpw@xxxxxxxxxx> writes: > Hello Felipe Balbi, > > I'd like you to do a code review. Please visit > > https://android-review.googlesource.com/183164 > > to review the following change. > > Change subject: usb: gadget: configfs: Fix interfaces array NULL-termination > ...................................................................... > > usb: gadget: configfs: Fix interfaces array NULL-termination > > memset() to 0 interfaces array before reusing > usb_configuration structure. > > This commit fix bug: > > ln -s functions/acm.1 configs/c.1 > ln -s functions/acm.2 configs/c.1 > ln -s functions/acm.3 configs/c.1 > echo "UDC name" > UDC > echo "" > UDC > rm configs/c.1/acm.* > rmdir functions/* > mkdir functions/ecm.usb0 > ln -s functions/ecm.usb0 configs/c.1 > echo "UDC name" > UDC > > [ 82.220969] Unable to handle kernel NULL pointer dereference at virtual > address 00000000 > [ 82.229009] pgd = c0004000 > [ 82.231698] [00000000] *pgd=00000000 > [ 82.235260] Internal error: Oops: 17 [#1] PREEMPT SMP ARM > [ 82.240638] Modules linked in: > [ 82.243681] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.0.0-rc2 #39 > [ 82.249926] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree) > [ 82.256003] task: c07cd2f0 ti: c07c8000 task.ti: c07c8000 > [ 82.261393] PC is at composite_setup+0xe3c/0x1674 > [ 82.266073] LR is at composite_setup+0xf20/0x1674 > [ 82.270760] pc : [<c03510d4>] lr : [<c03511b8>] psr: 600001d3 > [ 82.270760] sp : c07c9df0 ip : c0806448 fp : ed8c9c9c > [ 82.282216] r10: 00000001 r9 : 00000000 r8 : edaae918 > [ 82.287425] r7 : ed551cc0 r6 : 00007fff r5 : 00000000 r4 : ed799634 > [ 82.293934] r3 : 00000003 r2 : 00010002 r1 : edaae918 r0 : 0000002e > [ 82.300446] Flags: nZCv IRQs off FIQs off Mode SVC_32 ISA ARM > Segment kernel > [ 82.307910] Control: 10c5387d Table: 6bc1804a DAC: 00000015 > [ 82.313638] Process swapper/0 (pid: 0, stack limit = 0xc07c8210) > [ 82.319627] Stack: (0xc07c9df0 to 0xc07ca000) > [ 82.323969] 9de0: 00000000 c06e65f4 > 00000000 c07c9f68 > [ 82.332130] 9e00: 00000067 c07c59ac 000003f7 edaae918 ed8c9c98 ed799690 > eca2f140 200001d3 > [ 82.340289] 9e20: ee79a2d8 c07c9e88 c07c5304 ffff55db 00010002 edaae810 > edaae860 eda96d50 > [ 82.348448] 9e40: 00000009 ee264510 00000007 c07ca444 edaae860 c0340890 > c0827a40 ffff55e0 > [ 82.356607] 9e60: c0827a40 eda96e40 ee264510 edaae810 00000000 edaae860 > 00000007 c07ca444 > [ 82.364766] 9e80: edaae860 c0354170 c03407dc c033db4c edaae810 00000000 > 00000000 00000010 > [ 82.372925] 9ea0: 00000032 c0341670 00000000 00000000 00000001 eda96e00 > 00000000 00000000 > [ 82.381084] 9ec0: 00000000 00000032 c0803a23 ee1aa840 00000001 c005d54c > 249e2450 00000000 > [ 82.389244] 9ee0: 200001d3 ee1aa840 ee1aa8a0 ed84f4c0 00000000 c07c9f68 > 00000067 c07c59ac > [ 82.397403] 9f00: 00000000 c005d688 ee1aa840 ee1aa8a0 c07db4b4 c006009c > 00000032 00000000 > [ 82.405562] 9f20: 00000001 c005ce20 c07c59ac c005cf34 f002000c c07ca780 > c07c9f68 00000057 > [ 82.413722] 9f40: f0020000 413fc090 00000001 c00086b4 c000f804 60000053 > ffffffff c07c9f9c > [ 82.421880] 9f60: c0803a20 c0011fc0 00000000 00000000 c07c9fb8 c001bee0 > c07ca4f0 c057004c > [ 82.430040] 9f80: c07ca4fc c0803a20 c0803a20 413fc090 00000001 00000000 > 01000000 c07c9fb0 > [ 82.438199] 9fa0: c000f800 c000f804 60000053 ffffffff 00000000 c0050e70 > c0803bc0 c0783bd8 > [ 82.446358] 9fc0: ffffffff ffffffff c0783664 00000000 00000000 c07b13e8 > 00000000 c0803e54 > [ 82.454517] 9fe0: c07ca480 c07b13e4 c07ce40c 4000406a 00000000 40008074 > 00000000 00000000 > [ 82.462689] [<c03510d4>] (composite_setup) from [<c0340890>] > (s3c_hsotg_complete_setup+0xb4/0x418) > [ 82.471626] [<c0340890>] (s3c_hsotg_complete_setup) from [<c0354170>] > (usb_gadget_giveback_request+0xc/0x10) > [ 82.481429] [<c0354170>] (usb_gadget_giveback_request) from [<c033db4c>] > (s3c_hsotg_complete_request+0xcc/0x12c) > [ 82.491583] [<c033db4c>] (s3c_hsotg_complete_request) from [<c0341670>] > (s3c_hsotg_irq+0x4fc/0x558) > [ 82.500614] [<c0341670>] (s3c_hsotg_irq) from [<c005d54c>] > (handle_irq_event_percpu+0x50/0x150) > [ 82.509291] [<c005d54c>] (handle_irq_event_percpu) from [<c005d688>] > (handle_irq_event+0x3c/0x5c) > [ 82.518145] [<c005d688>] (handle_irq_event) from [<c006009c>] > (handle_fasteoi_irq+0xd4/0x18c) > [ 82.526650] [<c006009c>] (handle_fasteoi_irq) from [<c005ce20>] > (generic_handle_irq+0x20/0x30) > [ 82.535242] [<c005ce20>] (generic_handle_irq) from [<c005cf34>] > (__handle_domain_irq+0x6c/0xdc) > [ 82.543923] [<c005cf34>] (__handle_domain_irq) from [<c00086b4>] > (gic_handle_irq+0x2c/0x6c) > [ 82.552256] [<c00086b4>] (gic_handle_irq) from [<c0011fc0>] > (__irq_svc+0x40/0x74) > [ 82.559716] Exception stack(0xc07c9f68 to 0xc07c9fb0) > [ 82.564753] 9f60: 00000000 00000000 c07c9fb8 c001bee0 > c07ca4f0 c057004c > [ 82.572913] 9f80: c07ca4fc c0803a20 c0803a20 413fc090 00000001 00000000 > 01000000 c07c9fb0 > [ 82.581069] 9fa0: c000f800 c000f804 60000053 ffffffff > [ 82.586113] [<c0011fc0>] (__irq_svc) from [<c000f804>] > (arch_cpu_idle+0x30/0x3c) > [ 82.593491] [<c000f804>] (arch_cpu_idle) from [<c0050e70>] > (cpu_startup_entry+0x128/0x1a4) > [ 82.601740] [<c0050e70>] (cpu_startup_entry) from [<c0783bd8>] > (start_kernel+0x350/0x3bc) > [ 82.609890] Code: 0a000002 e3530005 05975010 15975008 (e5953000) > [ 82.615965] ---[ end trace f57d5f599a5f1bfa ]--- > > Most of kernel code assume that interface array in > struct usb_configuration is NULL terminated. > > When gadget is composed with configfs configuration > structure may be reused for different functions set. > > This bug happens because purge_configs_funcs() sets > only next_interface_id to 0. Interface array still > contains pointers to already freed interfaces. If in > second try we add less interfaces than earlier we > may access unallocated memory when trying to get > interface descriptors. > > Signed-off-by: Krzysztof Opasiak <k.opasiak@xxxxxxxxxxx> > Cc: <stable@xxxxxxxxxxxxxxx> # 3.10+ > Signed-off-by: Felipe Balbi <balbi@xxxxxx> > (cherry picked from commit 3f1694ac7562fd9f2aee85c2b6d1f93424f06dcb) > --- > M drivers/usb/gadget/configfs.c > 1 file changed, 1 insertion(+), 0 deletions(-) > > > > diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c > index d458b21..52ddfec 100644 > --- a/drivers/usb/gadget/configfs.c > +++ b/drivers/usb/gadget/configfs.c > @@ -1333,6 +1333,7 @@ > } > } > c->next_interface_id = 0; > + memset(c->interface, 0, sizeof(c->interface)); > c->superspeed = 0; > c->highspeed = 0; > c->fullspeed = 0; > > -- > To view, visit https://android-review.googlesource.com/183164 > To unsubscribe, visit https://android-review.googlesource.com/settings > > Gerrit-MessageType: newchange > Gerrit-Change-Id: I7c2d5928aca93f4ad8593db1d5fe61277d08e173 > Gerrit-PatchSet: 1 > Gerrit-Project: kernel/mediatek > Gerrit-Branch: android-mtk-3.18 > Gerrit-Owner: Badhri Jagan Sridharan <badhri@xxxxxxxxxx> > Gerrit-Reviewer: Felipe Balbi <balbi@xxxxxx> You _must_ fix your Gerrit settings. I'm definitely _NOT_ going to be revieweing whatever patches you're adding to your tree. Specially so since it's on a 3.18 kernel which is getting close to one year old. I've received 3 or 4 of these emails already and it's getting a bit annoying, please fix your Gerrit to, at least, ignore patches you're cherry-picking from stable trees. -- balbi
Attachment:
signature.asc
Description: PGP signature
