Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1751573AbdHONih (ORCPT <rfc822;w@1wt.eu>);
        Tue, 15 Aug 2017 09:38:37 -0400
Received: from mail.ispras.ru ([83.149.199.45]:46642 "EHLO mail.ispras.ru"
        rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
        id S1750997AbdHONig (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
        Tue, 15 Aug 2017 09:38:36 -0400
Subject: Re: Possible null pointer dereference in adutux.ko
To: Oliver Neukum <oneukum@suse.com>, johan@kernel.org,
        gregkh@linuxfoundation.org, wsa-dev@sang-engineering.com
Cc: Alexey Khoroshilov <khoroshilov@ispras.ru>,
        ldv-project@linuxtesting.org, linux-kernel@vger.kernel.org,
        linux-usb@vger.kernel.org
References: <fc719c9c-db6a-9254-2d6d-918946c5a18d@ispras.ru>
 <1502803217.6606.3.camel@suse.com>
From: Anton Volkov <avolkov@ispras.ru>
Message-ID: <72416d59-5d6d-b499-0373-489e846b2643@ispras.ru>
Date: Tue, 15 Aug 2017 16:38:34 +0300
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
 Thunderbird/52.2.1
MIME-Version: 1.0
In-Reply-To: <1502803217.6606.3.camel@suse.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 1805
Lines: 57

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