* [PATCH net] rxrpc: Kernel calls get stuck in recvmsg
@ 2017-02-24 21:57 David Howells
2017-02-27 2:31 ` David Miller
0 siblings, 1 reply; 2+ messages in thread
From: David Howells @ 2017-02-24 21:57 UTC (permalink / raw)
To: netdev; +Cc: dhowells, linux-afs, linux-kernel
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@auristor.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Tested-by: Marc Dionne <marc.dionne@auristor.com>
---
net/rxrpc/recvmsg.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/net/rxrpc/recvmsg.c b/net/rxrpc/recvmsg.c
index c29362d50a92..f3a688e10843 100644
--- a/net/rxrpc/recvmsg.c
+++ b/net/rxrpc/recvmsg.c
@@ -320,8 +320,10 @@ static int rxrpc_recvmsg_data(struct socket *sock, struct rxrpc_call *call,
/* Barriers against rxrpc_input_data(). */
hard_ack = call->rx_hard_ack;
- top = smp_load_acquire(&call->rx_top);
- for (seq = hard_ack + 1; before_eq(seq, top); seq++) {
+ seq = hard_ack + 1;
+ while (top = smp_load_acquire(&call->rx_top),
+ before_eq(seq, top)
+ ) {
ix = seq & RXRPC_RXTX_BUFF_MASK;
skb = call->rxtx_buffer[ix];
if (!skb) {
@@ -394,6 +396,8 @@ static int rxrpc_recvmsg_data(struct socket *sock, struct rxrpc_call *call,
ret = 1;
goto out;
}
+
+ seq++;
}
out:
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH net] rxrpc: Kernel calls get stuck in recvmsg
2017-02-24 21:57 [PATCH net] rxrpc: Kernel calls get stuck in recvmsg David Howells
@ 2017-02-27 2:31 ` David Miller
0 siblings, 0 replies; 2+ messages in thread
From: David Miller @ 2017-02-27 2:31 UTC (permalink / raw)
To: dhowells; +Cc: netdev, linux-afs, linux-kernel
From: David Howells <dhowells@redhat.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@auristor.com>
> Signed-off-by: David Howells <dhowells@redhat.com>
> Tested-by: Marc Dionne <marc.dionne@auristor.com>
Applied, thanks.
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2017-02-27 3:12 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-24 21:57 [PATCH net] rxrpc: Kernel calls get stuck in recvmsg David Howells
2017-02-27 2:31 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).