Message ID | 20160826152546.604384-4-arnd@arndb.de |
---|---|
State | Not Applicable, archived |
Delegated to: | David Miller |
Headers | show |
Arnd Bergmann <arnd@arndb.de> 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 <arnd@arndb.de> > Fixes: 18bfeba50dfd ("rxrpc: Perform terminal call ACK/ABORT retransmission from conn processor") > --- > Cc: David Howells <dhowells@redhat.com> > Cc: "David S. Miller" <davem@davemloft.net> > 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(&call->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(&call->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
This is fixed by: commit 2266ffdef5737fdfa96005204fc5606dbd559956 subject: rxrpc: Fix conn-based retransmit which is in net-next. David
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
Arnd Bergmann <arnd@arndb.de> 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
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(&call->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(&call->usage) == 0) - goto cant_route_call; - rxrpc_post_packet_to_call(call, skb); goto out_unlock; }
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 <arnd@arndb.de> Fixes: 18bfeba50dfd ("rxrpc: Perform terminal call ACK/ABORT retransmission from conn processor") --- Cc: David Howells <dhowells@redhat.com> Cc: "David S. Miller" <davem@davemloft.net> Cc: netdev@vger.kernel.org net/rxrpc/input.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)