linux-kernel - Re: Possible null pointer dereference in adutux.ko

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <72416d59-5d6d-b499-0373-489e846b2643@ispras.ru>
Date:   Tue, 15 Aug 2017 16:38:34 +0300
From:   Anton Volkov <avolkov@...ras.ru>
To:     Oliver Neukum <oneukum@...e.com>, johan@...nel.org,
        gregkh@...uxfoundation.org, wsa-dev@...g-engineering.com
Cc:     Alexey Khoroshilov <khoroshilov@...ras.ru>,
        ldv-project@...uxtesting.org, linux-kernel@...r.kernel.org,
        linux-usb@...r.kernel.org
Subject: Re: Possible null pointer dereference in adutux.ko

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@...ras.ru