Re: [PATCH 3/5] rxrpc: fix last_call processing

[David Howells]

Arnd Bergmann  wrote:

> I'll follow up with the fixes, both of which are rather
> straightforward.

Are they both in?

[PATCH 2/2] rxrpc: fix undefined behavior in rxrpc_mark_call_released

David

Re: [PATCH 3/5] rxrpc: fix last_call processing

[Arnd Bergmann]

On Sunday, August 28, 2016 9:42:17 AM CEST David Howells wrote:
> This is fixed by:
> 
> commit 2266ffdef5737fdfa96005204fc5606dbd559956
> subject: rxrpc: Fix conn-based retransmit
> 
> which is in net-next.

I've merged net-next into the last linux-next release now for
testing (no linux-next this week) and can confirm that your
fix is correct. However, I got a new (valid) warning after
your f5c17aaeb2ae ("rxrpc: Calls should only have one terminal
state"), and another (false-positive) one for another patch
in net-next.

I'll follow up with the fixes, both of which are rather
straightforward.

Arnd

Re: [PATCH 3/5] rxrpc: fix last_call processing

[David Howells]

This is fixed by:

commit 2266ffdef5737fdfa96005204fc5606dbd559956
subject: rxrpc: Fix conn-based retransmit

which is in net-next.

David

Re: [PATCH 3/5] rxrpc: fix last_call processing

[David Howells]

Arnd Bergmann  wrote:

> A change to the retransmission handling in rxrpc caused a use-before-init
> bug in rxrpc_data_ready(), as indicated by "gcc -Wmaybe-uninitialized":
> 
> net/rxrpc/input.c: In function 'rxrpc_data_ready':
> net/rxrpc/input.c:735:34: error: 'call' may be used uninitialized in this 
> function [-Werror=maybe-uninitialized]
> 
> This moves the initialization of the local variable before the first
> user, which presumably is what was intended here.
> 
> Signed-off-by: Arnd Bergmann 
> Fixes: 18bfeba50dfd ("rxrpc: Perform terminal call ACK/ABORT retransmission 
> from conn processor")
> ---
> Cc: David Howells 
> Cc: "David S. Miller" 
> Cc: netdev@vger.kernel.org
> 
>  net/rxrpc/input.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/net/rxrpc/input.c b/net/rxrpc/input.c
> index 66cdeb56f44f..3c22e43a58fd 100644
> --- a/net/rxrpc/input.c
> +++ b/net/rxrpc/input.c
> @@ -728,6 +728,10 @@ void rxrpc_data_ready(struct sock *sk)
>   if (sp->hdr.callNumber < chan->last_call)
>   goto discard_unlock;
>  
> + call = rcu_dereference(chan->call);
> + if (!call || atomic_read(>usage) == 0)
> + goto cant_route_call;
> +
>   if (sp->hdr.callNumber == chan->last_call) {
>   /* For the previous service call, if completed
>* successfully, we discard all further packets.
> @@ -744,10 +748,6 @@ void rxrpc_data_ready(struct sock *sk)
>   goto out_unlock;
>   }
>  
> - call = rcu_dereference(chan->call);
> - if (!call || atomic_read(>usage) == 0)
> - goto cant_route_call;
> -
>   rxrpc_post_packet_to_call(call, skb);
>   goto out_unlock;
>   }

You can't rearrange these like this.  I have a different fix.

David