On 29 June 2017 at 07:01, Stephen Warren <swarren@xxxxxxxxxxxxx> wrote: > > > Baolin, > > I have a question about the following commit: > > 511a36d2f357 "usb: gadget: Add the gserial port checking in gs_start_tx()" > >> When usb gadget is set gadget serial function, it will be crash in below >> situation. >> >> It will clean the 'port->port_usb' pointer in gserial_disconnect() >> function >> when usb link is inactive, but it will release lock for disabling the >> endpoints >> in this function. Druing the lock release period, it maybe complete one >> request >> to issue gs_write_complete()--->gs_start_tx() function, but the >> 'port->port_usb' >> pointer had been set NULL, thus it will be crash in gs_start_tx() >> function. >> >> This patch adds the 'port->port_usb' pointer checking in gs_start_tx() >> function >> to avoid this situation. > > > (also archived at) > https://www.spinics.net/lists/linux-usb/msg143313.html > https://patchwork.kernel.org/patch/9207037/ > > In kernel 4.4, I'm seeing a rare NULL pointer dereference in > gs_write_room(). I believe this happens pretty early during system boot, > likely soon after the gadget configuration has been created, or soon after > the host PC enumerates the device running the gadget serial driver. Full > stack trace below, or soon after resume from system suspend. I wonder if you > think this might be the same issue you fixed? Also, your fix was only > applied to function gs_start_tx(). Do you think the same issue might also > apply to other functions in the same driver, and hence we might need to add > the same check to other functions like gs_start_tx() or similar? > > I will try applying your patch to see if it fixes my issue. However, the > issue is very hard to reproduce, so it will be difficult to tell if the > solution works:-( I have not seen this problem on my platform, so I am not sure if it is the same issue. In gs_write_room(), the 'port->port_usb' has been checked, maybe the 'port' pointer was set to NULL? I think you can check this or check if this issue was fixed on latest kernel? > >> [ 132.757406] Unable to handle kernel NULL pointer dereference at virtual >> address 00000170 > > ... >> >> [ 132.757428] CPU: 5 PID: 53 Comm: kworker/u12:1 Not tainted 4.4.38-tegra >> #1 > > ... >> >> [ 132.757439] Workqueue: events_unbound flush_to_ldisc > > ... >> >> [ 132.757447] PC is at _raw_spin_lock_irqsave+0x20/0x54 >> [ 132.757452] LR is at gs_write_room+0x1c/0x7c > > ... >> >> [ 132.757481] Process kworker/u12:1 (pid: 53, stack limit = >> 0xffffffc1eb6a8020) >> [ 132.757482] Call trace: >> [ 132.757484] [<ffffffc000b2bb70>] _raw_spin_lock_irqsave+0x20/0x54 >> [ 132.757487] [<ffffffc000466af4>] tty_write_room+0x1c/0x2c >> [ 132.757489] [<ffffffc0004634a0>] __process_echoes+0x24/0x25c >> [ 132.757491] [<ffffffc000466208>] n_tty_receive_buf_common+0x1ec/0xa24 >> [ 132.757493] [<ffffffc000466a50>] n_tty_receive_buf2+0x10/0x18 >> [ 132.757495] [<ffffffc000469554>] flush_to_ldisc+0xe4/0x154 >> [ 132.757499] [<ffffffc0000bb9d4>] process_one_work+0x154/0x434 >> [ 132.757501] [<ffffffc0000bbde8>] worker_thread+0x134/0x40c >> [ 132.757503] [<ffffffc0000c1678>] kthread+0xe0/0xf4 >> [ 132.757506] [<ffffffc000084f90>] ret_from_fork+0x10/0x40 > > > Thanks for any insight at all! -- Baolin.wang Best Regards -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
