netdev - Re: [PATCH net] rxrpc: Kernel calls get stuck in recvmsg

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: <20170226.213104.1497172987206712559.davem@davemloft.net>
Date:   Sun, 26 Feb 2017 21:31:04 -0500 (EST)
From:   David Miller <davem@...emloft.net>
To:     dhowells@...hat.com
Cc:     netdev@...r.kernel.org, linux-afs@...ts.infradead.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH net] rxrpc: Kernel calls get stuck in recvmsg

From: David Howells <dhowells@...hat.com>
Date: Fri, 24 Feb 2017 21:57:13 +0000

> Calls made through the in-kernel interface can end up getting stuck because
> of a missed variable update in a loop in rxrpc_recvmsg_data().  The problem
> is like this:
> 
>  (1) A new packet comes in and doesn't cause a notification to be given to
>      the client as there's still another packet in the ring - the
>      assumption being that if the client will keep drawing off data until
>      the ring is empty.
> 
>  (2) The client is in rxrpc_recvmsg_data(), inside the big while loop that
>      iterates through the packets.  This copies the window pointers into
>      variables rather than using the information in the call struct
>      because:
> 
>      (a) MSG_PEEK might be in effect;
> 
>      (b) we need a barrier after reading call->rx_top to pair with the
>      	 barrier in the softirq routine that loads the buffer.
> 
>  (3) The reading of call->rx_top is done outside of the loop, and top is
>      never updated whilst we're in the loop.  This means that even through
>      there's a new packet available, we don't see it and may return -EFAULT
>      to the caller - who will happily return to the scheduler and await the
>      next notification.
> 
>  (4) No further notifications are forthcoming until there's an abort as the
>      ring isn't empty.
> 
> The fix is to move the read of call->rx_top inside the loop - but it needs
> to be done before the condition is checked.
> 
> Reported-by: Marc Dionne <marc.dionne@...istor.com>
> Signed-off-by: David Howells <dhowells@...hat.com>
> Tested-by: Marc Dionne <marc.dionne@...istor.com>

Applied, thanks.