netdev - Re: [PATCH net] rxrpc: Fix client call connect/disconnect race

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] [day] [month] [year] [list]

Message-Id: <20190308.112633.1040548315975715311.davem@davemloft.net>
Date:   Fri, 08 Mar 2019 11:26:33 -0800 (PST)
From:   David Miller <davem@...emloft.net>
To:     dhowells@...hat.com
Cc:     netdev@...r.kernel.org, marc.dionne@...istor.com,
        linux-afs@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net] rxrpc: Fix client call connect/disconnect race

From: David Howells <dhowells@...hat.com>
Date: Fri, 08 Mar 2019 12:48:39 +0000

> rxrpc_disconnect_client_call() reads the call's connection ID protocol
> value (call->cid) as part of that function's variable declarations.  This
> is bad because it's not inside the locked section and so may race with
> someone granting use of the channel to the call.
> 
> This manifests as an assertion failure (see below) where the call in the
> presumed channel (0 because call->cid wasn't set when we read it) doesn't
> match the call attached to the channel we were actually granted (if 1, 2 or
> 3).
> 
> Fix this by moving the read and dependent calculations inside of the
> channel_lock section.  Also, only set the channel number and pointer
> variables if cid is not zero (ie. unset).
> 
> This problem can be induced by injecting an occasional error in
> rxrpc_wait_for_channel() before the call to schedule().
> 
> Make two further changes also:
> 
>  (1) Add a trace for wait failure in rxrpc_connect_call().
> 
>  (2) Drop channel_lock before BUG'ing in the case of the assertion failure.
> 
> The failure causes a trace akin to the following:
 ...
> Fixes: 45025bceef17 ("rxrpc: Improve management and caching of client connection objects")
> Signed-off-by: David Howells <dhowells@...hat.com>
> Reviewed-by: Marc Dionne <marc.dionne@...istor.com>

Applied, thanks David.