Hi Mirea, On 29 April 2017 at 17:58, Mirea, Bogdan-Stefan <Bogdan-Stefan_Mirea@xxxxxxxxxx> wrote: > Hi Baolin, > > On April 29, 2017 at 5:14, Baolin Wang <baolin.wang@xxxxxxxxxx> wrote: >> Hi, >> >> On 28 April 2017 at 21:26, Bogdan Mirea <Bogdan-Stefan_mirea@xxxxxxxxxx> wrote: >> > In the current implementation we get a Kernel Oops when calling rmmod >> > for g_serial driver. This happens because the function that creates >> > the kthread (gs_console_setup) which should be called at driver >> > probing is never called for ttyGS* resulting in a null >> > info->console_thread pointer. >> > >> > Noticed this behaviour on Renesas Rcar-H3 Salvator-X (R8A7795). >> > >> > Signed-off-by: Bogdan Mirea <Bogdan-Stefan_mirea@xxxxxxxxxx> >> > --- >> > drivers/usb/gadget/function/u_serial.c | 3 ++- >> > 1 file changed, 2 insertions(+), 1 deletion(-) >> > >> > diff --git a/drivers/usb/gadget/function/u_serial.c >> > b/drivers/usb/gadget/function/u_serial.c >> > index e0cd1e4..7283ff5 100644 >> > --- a/drivers/usb/gadget/function/u_serial.c >> > +++ b/drivers/usb/gadget/function/u_serial.c >> > @@ -1256,7 +1256,8 @@ static void gserial_console_exit(void) >> > struct gscons_info *info = &gscons_info; >> > >> > unregister_console(&gserial_cons); >> > - kthread_stop(info->console_thread); >> > + if (info->console_thread && !IS_ERR(info->console_thread)) >> > + kthread_stop(info->console_thread); >> > gs_buf_free(&info->con_buf); >> > } >> >> Which kernel version did you test on? Since this issue had been fixed by below patch: >> https://www.spinics.net/lists/linux-usb/msg149581.html (usb: gadget: >> serial: fix possible Oops caused by calling kthread_stop(NULL)) >> >> -- >> Baolin.wang >> Best Regards > > Thank you for your input! > I am using Kernel v4.9 and in this Kernel version the patch[1] is not > present, I'm sorry for not checking v4.10. > But this patch[1] only covers the case of a NULL pointer and doesn't > include the case of a bad pointer value that should also be checked > with IS_ERR(info->console_thread). > > This check is needed since the gserial setup callback function > (gs_console_setup()) is only freeing the info->con_buf in the case of > kthread_create failure which will result in bad info->console_thread > pointer so this will also fail on rmmod if the bad pointer value is not > checked in the gserial_console_exit() function before calling > kthread_stop(). > > I think cherry-picking the patch[1] and create a new one that is also > checking this bad pointer value case is the best approach. Agreed. -- 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
