Re: Possible null pointer dereference in adutux.ko

[Anton Volkov]

On 15.08.2017 16:20, Oliver Neukum wrote:
> Am Dienstag, den 15.08.2017, 15:59 +0300 schrieb Anton Volkov:
> > Hello.
> >
> > While searching for races in the Linux kernel I've come across
> > "drivers/usb/misc/adutux.ko" module. Here is a question that I came up
> > with while analyzing results. Lines are given using the info from Linux
> > v4.12.
> >
> > Consider the following case:
> >
> > Thread 1:                   Thread 2:
> > adu_release
> > ->adu_release_internal      adu_disconnect
> >       <READ &dev->udev->dev>    dev->udev = NULL
> >       (adutux.c: line 298)      (adutux.c: line 771)
> >                                 usb_deregister_dev
> >
> > Comments in the source code point at the possibility of adu_release()
> > being called separately from adu_disconnect(). adu_release() and
> > adu_disconnect() acquire different mutexes, so they are not protected
> > from one another. If adu_disconnect() changes dev->udev before its value
> > is read in adu_release_internal() there will be a NULL pointer
> > dereference on a read attempt. Is this case feasible from your point of
> > view?
> >
> > Thank you for your time.
>
> Hi,
>
> your analysis seems correct to me. In fact it looks like
>
> 66d4bc30d128e7c7ac4cf64aa78cb76e971cec5b
> USB: adutux: remove custom debug macro
>
> more or less broke disconnect on this driver
> (the URBs can also finish after dev->udev = NULL)
>
> Do you want to do a fix or do you want me to do it?
>
> 	Regards
> 		Oliver

Hello, Oliver.

I am not sure about the best way to solve this problem. If you have any 
ideas about it then it would probably be better if you could handle the 
fix. Or if you share the ideas I can prepare a patch.

Regards,
Anton

-- Anton Volkov
Linux Verification Center, ISPRAS
web: http://linuxtesting.org
e-mail: avolkov@xxxxxxxxx