* [PATCH net 1/6] rxrpc: fix uninitialized pointer dereference in debug code
2016-08-09 16:33 [PATCH net 0/6] rxrpc: Miscellaneous fixes David Howells
@ 2016-08-09 16:33 ` David Howells
2016-08-09 16:33 ` [PATCH net 2/6] rxrpc: Need to flag call as being released on connect failure David Howells
` (5 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: David Howells @ 2016-08-09 16:33 UTC (permalink / raw)
To: netdev; +Cc: dhowells, linux-afs, linux-kernel
From: Arnd Bergmann <arnd@arndb.de>
A newly added bugfix caused an uninitialized variable to be
used for printing debug output. This is harmless as long
as the debug setting is disabled, but otherwise leads to an
immediate crash.
gcc warns about this when -Wmaybe-uninitialized is enabled:
net/rxrpc/call_object.c: In function 'rxrpc_release_call':
net/rxrpc/call_object.c:496:163: error: 'sp' may be used uninitialized in this function [-Werror=maybe-uninitialized]
The initialization was removed but one of the users remains.
This adds back the initialization.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Fixes: 372ee16386bb ("rxrpc: Fix races between skb free, ACK generation and replying")
Signed-off-by: David Howells <dhowells@redhat.com>
---
net/rxrpc/call_object.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/rxrpc/call_object.c b/net/rxrpc/call_object.c
index c47f14fc5e88..e8c953c48cb8 100644
--- a/net/rxrpc/call_object.c
+++ b/net/rxrpc/call_object.c
@@ -493,6 +493,7 @@ void rxrpc_release_call(struct rxrpc_call *call)
(skb = skb_dequeue(&call->rx_oos_queue))) {
spin_unlock_bh(&call->lock);
+ sp = rxrpc_skb(skb);
_debug("- zap %s %%%u #%u",
rxrpc_pkts[sp->hdr.type],
sp->hdr.serial, sp->hdr.seq);
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH net 2/6] rxrpc: Need to flag call as being released on connect failure
2016-08-09 16:33 [PATCH net 0/6] rxrpc: Miscellaneous fixes David Howells
2016-08-09 16:33 ` [PATCH net 1/6] rxrpc: fix uninitialized pointer dereference in debug code David Howells
@ 2016-08-09 16:33 ` David Howells
2016-08-09 16:33 ` [PATCH net 3/6] rxrpc: Don't access connection from call if pointer is NULL David Howells
` (4 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: David Howells @ 2016-08-09 16:33 UTC (permalink / raw)
To: netdev; +Cc: dhowells, linux-afs, linux-kernel
If rxrpc_new_client_call() fails to make a connection, the call record that
it allocated needs to be marked as RXRPC_CALL_RELEASED before it is passed
to rxrpc_put_call() to indicate that it no longer has any attachment to the
AF_RXRPC socket.
Without this, an assertion failure may occur at:
net/rxrpc/call_object:635
Signed-off-by: David Howells <dhowells@redhat.com>
---
net/rxrpc/call_object.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/rxrpc/call_object.c b/net/rxrpc/call_object.c
index e8c953c48cb8..ae057e0740f3 100644
--- a/net/rxrpc/call_object.c
+++ b/net/rxrpc/call_object.c
@@ -275,6 +275,7 @@ error:
list_del_init(&call->link);
write_unlock_bh(&rxrpc_call_lock);
+ set_bit(RXRPC_CALL_RELEASED, &call->flags);
call->state = RXRPC_CALL_DEAD;
rxrpc_put_call(call);
_leave(" = %d", ret);
@@ -287,6 +288,7 @@ error:
*/
found_user_ID_now_present:
write_unlock(&rx->call_lock);
+ set_bit(RXRPC_CALL_RELEASED, &call->flags);
call->state = RXRPC_CALL_DEAD;
rxrpc_put_call(call);
_leave(" = -EEXIST [%p]", call);
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH net 3/6] rxrpc: Don't access connection from call if pointer is NULL
2016-08-09 16:33 [PATCH net 0/6] rxrpc: Miscellaneous fixes David Howells
2016-08-09 16:33 ` [PATCH net 1/6] rxrpc: fix uninitialized pointer dereference in debug code David Howells
2016-08-09 16:33 ` [PATCH net 2/6] rxrpc: Need to flag call as being released on connect failure David Howells
@ 2016-08-09 16:33 ` David Howells
2016-08-09 16:33 ` [PATCH net 4/6] rxrpc: Once packet posted in data_ready, don't retry posting David Howells
` (3 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: David Howells @ 2016-08-09 16:33 UTC (permalink / raw)
To: netdev; +Cc: dhowells, linux-afs, linux-kernel
The call state machine processor sets up the message parameters for a UDP
message that it might need to transmit in advance on the basis that there's
a very good chance it's going to have to transmit either an ACK or an
ABORT. This requires it to look in the connection struct to retrieve some
of the parameters.
However, if the call is complete, the call connection pointer may be NULL
to dissuade the processor from transmitting a message. However, there are
some situations where the processor is still going to be called - and it's
still going to set up message parameters whether it needs them or not.
This results in a NULL pointer dereference at:
net/rxrpc/call_event.c:837
To fix this, skip the message pre-initialisation if there's no connection
attached.
Signed-off-by: David Howells <dhowells@redhat.com>
---
net/rxrpc/call_event.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/net/rxrpc/call_event.c b/net/rxrpc/call_event.c
index f5e99163a09e..e60cf65c2232 100644
--- a/net/rxrpc/call_event.c
+++ b/net/rxrpc/call_event.c
@@ -837,6 +837,9 @@ void rxrpc_process_call(struct work_struct *work)
return;
}
+ if (!call->conn)
+ goto skip_msg_init;
+
/* there's a good chance we're going to have to send a message, so set
* one up in advance */
msg.msg_name = &call->conn->params.peer->srx.transport;
@@ -859,6 +862,7 @@ void rxrpc_process_call(struct work_struct *work)
memset(iov, 0, sizeof(iov));
iov[0].iov_base = &whdr;
iov[0].iov_len = sizeof(whdr);
+skip_msg_init:
/* deal with events of a final nature */
if (test_bit(RXRPC_CALL_EV_RCVD_ERROR, &call->events)) {
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH net 4/6] rxrpc: Once packet posted in data_ready, don't retry posting
2016-08-09 16:33 [PATCH net 0/6] rxrpc: Miscellaneous fixes David Howells
` (2 preceding siblings ...)
2016-08-09 16:33 ` [PATCH net 3/6] rxrpc: Don't access connection from call if pointer is NULL David Howells
@ 2016-08-09 16:33 ` David Howells
2016-08-09 16:33 ` [PATCH net 5/6] rxrpc: Fix a use-after-push in data_ready handler David Howells
` (2 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: David Howells @ 2016-08-09 16:33 UTC (permalink / raw)
To: netdev; +Cc: dhowells, linux-afs, linux-kernel
Once a packet has been posted to a connection in the data_ready handler, we
mustn't try reposting if we then find that the connection is dying as the
refcount has been given over to the dying connection and the packet might
no longer exist.
Losing the packet isn't a problem as the peer will retransmit.
Signed-off-by: David Howells <dhowells@redhat.com>
---
net/rxrpc/input.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/net/rxrpc/input.c b/net/rxrpc/input.c
index 9e0f58edcd01..04afdc09557b 100644
--- a/net/rxrpc/input.c
+++ b/net/rxrpc/input.c
@@ -567,13 +567,13 @@ done:
* post connection-level events to the connection
* - this includes challenges, responses and some aborts
*/
-static bool rxrpc_post_packet_to_conn(struct rxrpc_connection *conn,
+static void rxrpc_post_packet_to_conn(struct rxrpc_connection *conn,
struct sk_buff *skb)
{
_enter("%p,%p", conn, skb);
skb_queue_tail(&conn->rx_queue, skb);
- return rxrpc_queue_conn(conn);
+ rxrpc_queue_conn(conn);
}
/*
@@ -694,7 +694,6 @@ void rxrpc_data_ready(struct sock *sk)
rcu_read_lock();
-retry_find_conn:
conn = rxrpc_find_connection_rcu(local, skb);
if (!conn)
goto cant_route_call;
@@ -702,8 +701,7 @@ retry_find_conn:
if (sp->hdr.callNumber == 0) {
/* Connection-level packet */
_debug("CONN %p {%d}", conn, conn->debug_id);
- if (!rxrpc_post_packet_to_conn(conn, skb))
- goto retry_find_conn;
+ rxrpc_post_packet_to_conn(conn, skb);
} else {
/* Call-bound packets are routed by connection channel. */
unsigned int channel = sp->hdr.cid & RXRPC_CHANNELMASK;
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH net 5/6] rxrpc: Fix a use-after-push in data_ready handler
2016-08-09 16:33 [PATCH net 0/6] rxrpc: Miscellaneous fixes David Howells
` (3 preceding siblings ...)
2016-08-09 16:33 ` [PATCH net 4/6] rxrpc: Once packet posted in data_ready, don't retry posting David Howells
@ 2016-08-09 16:33 ` David Howells
2016-08-09 16:33 ` [PATCH net 6/6] rxrpc: Free packets discarded in data_ready David Howells
2016-08-09 19:57 ` [PATCH net 0/6] rxrpc: Miscellaneous fixes David Miller
6 siblings, 0 replies; 8+ messages in thread
From: David Howells @ 2016-08-09 16:33 UTC (permalink / raw)
To: netdev; +Cc: dhowells, linux-afs, linux-kernel
Fix a use of a packet after it has been enqueued onto the packet processing
queue in the data_ready handler. Once on a call's Rx queue, we mustn't
touch it any more as it may be dequeued and freed by the call processor
running on a work queue.
Save the values we need before enqueuing.
Without this, we can get an oops like the following:
BUG: unable to handle kernel NULL pointer dereference at 000000000000009c
IP: [<ffffffffa01854e8>] rxrpc_fast_process_packet+0x724/0xa11 [af_rxrpc]
PGD 0
Oops: 0000 [#1] SMP
Modules linked in: kafs(E) af_rxrpc(E) [last unloaded: af_rxrpc]
CPU: 2 PID: 0 Comm: swapper/2 Tainted: G E 4.7.0-fsdevel+ #1336
Hardware name: ASUS All Series/H97-PLUS, BIOS 2306 10/09/2014
task: ffff88040d6863c0 task.stack: ffff88040d68c000
RIP: 0010:[<ffffffffa01854e8>] [<ffffffffa01854e8>] rxrpc_fast_process_packet+0x724/0xa11 [af_rxrpc]
RSP: 0018:ffff88041fb03a78 EFLAGS: 00010246
RAX: ffffffffffffffff RBX: ffff8803ff195b00 RCX: 0000000000000001
RDX: ffffffffa01854d1 RSI: 0000000000000008 RDI: ffff8803ff195b00
RBP: ffff88041fb03ab0 R08: 0000000000000000 R09: 0000000000000001
R10: ffff88041fb038c8 R11: 0000000000000000 R12: ffff880406874800
R13: 0000000000000001 R14: 0000000000000000 R15: 0000000000000000
FS: 0000000000000000(0000) GS:ffff88041fb00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 000000000000009c CR3: 0000000001c14000 CR4: 00000000001406e0
Stack:
ffff8803ff195ea0 ffff880408348800 ffff880406874800 ffff8803ff195b00
ffff880408348800 ffff8803ff195ed8 0000000000000000 ffff88041fb03af0
ffffffffa0186072 0000000000000000 ffff8804054da000 0000000000000000
Call Trace:
<IRQ>
[<ffffffffa0186072>] rxrpc_data_ready+0x89d/0xbae [af_rxrpc]
[<ffffffff814c94d7>] __sock_queue_rcv_skb+0x24c/0x2b2
[<ffffffff8155c59a>] __udp_queue_rcv_skb+0x4b/0x1bd
[<ffffffff8155e048>] udp_queue_rcv_skb+0x281/0x4db
[<ffffffff8155ea8f>] __udp4_lib_rcv+0x7ed/0x963
[<ffffffff8155ef9a>] udp_rcv+0x15/0x17
[<ffffffff81531d86>] ip_local_deliver_finish+0x1c3/0x318
[<ffffffff81532544>] ip_local_deliver+0xbb/0xc4
[<ffffffff81531bc3>] ? inet_del_offload+0x40/0x40
[<ffffffff815322a9>] ip_rcv_finish+0x3ce/0x42c
[<ffffffff81532851>] ip_rcv+0x304/0x33d
[<ffffffff81531edb>] ? ip_local_deliver_finish+0x318/0x318
[<ffffffff814dff9d>] __netif_receive_skb_core+0x601/0x6e8
[<ffffffff814e072e>] __netif_receive_skb+0x13/0x54
[<ffffffff814e082a>] netif_receive_skb_internal+0xbb/0x17c
[<ffffffff814e1838>] napi_gro_receive+0xf9/0x1bd
[<ffffffff8144eb9f>] rtl8169_poll+0x32b/0x4a8
[<ffffffff814e1c7b>] net_rx_action+0xe8/0x357
[<ffffffff81051074>] __do_softirq+0x1aa/0x414
[<ffffffff810514ab>] irq_exit+0x3d/0xb0
[<ffffffff810184a2>] do_IRQ+0xe4/0xfc
[<ffffffff81612053>] common_interrupt+0x93/0x93
<EOI>
[<ffffffff814af837>] ? cpuidle_enter_state+0x1ad/0x2be
[<ffffffff814af832>] ? cpuidle_enter_state+0x1a8/0x2be
[<ffffffff814af96a>] cpuidle_enter+0x12/0x14
[<ffffffff8108956f>] call_cpuidle+0x39/0x3b
[<ffffffff81089855>] cpu_startup_entry+0x230/0x35d
[<ffffffff810312ea>] start_secondary+0xf4/0xf7
Signed-off-by: David Howells <dhowells@redhat.com>
---
net/rxrpc/input.c | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/net/rxrpc/input.c b/net/rxrpc/input.c
index 04afdc09557b..789719031462 100644
--- a/net/rxrpc/input.c
+++ b/net/rxrpc/input.c
@@ -124,11 +124,15 @@ static int rxrpc_fast_process_data(struct rxrpc_call *call,
struct rxrpc_skb_priv *sp;
bool terminal;
int ret, ackbit, ack;
+ u32 serial;
+ u8 flags;
_enter("{%u,%u},,{%u}", call->rx_data_post, call->rx_first_oos, seq);
sp = rxrpc_skb(skb);
ASSERTCMP(sp->call, ==, NULL);
+ flags = sp->hdr.flags;
+ serial = sp->hdr.serial;
spin_lock(&call->lock);
@@ -192,8 +196,8 @@ static int rxrpc_fast_process_data(struct rxrpc_call *call,
sp->call = call;
rxrpc_get_call(call);
atomic_inc(&call->skb_count);
- terminal = ((sp->hdr.flags & RXRPC_LAST_PACKET) &&
- !(sp->hdr.flags & RXRPC_CLIENT_INITIATED));
+ terminal = ((flags & RXRPC_LAST_PACKET) &&
+ !(flags & RXRPC_CLIENT_INITIATED));
ret = rxrpc_queue_rcv_skb(call, skb, false, terminal);
if (ret < 0) {
if (ret == -ENOMEM || ret == -ENOBUFS) {
@@ -205,12 +209,13 @@ static int rxrpc_fast_process_data(struct rxrpc_call *call,
}
skb = NULL;
+ sp = NULL;
_debug("post #%u", seq);
ASSERTCMP(call->rx_data_post, ==, seq);
call->rx_data_post++;
- if (sp->hdr.flags & RXRPC_LAST_PACKET)
+ if (flags & RXRPC_LAST_PACKET)
set_bit(RXRPC_CALL_RCVD_LAST, &call->flags);
/* if we've reached an out of sequence packet then we need to drain
@@ -226,7 +231,7 @@ static int rxrpc_fast_process_data(struct rxrpc_call *call,
spin_unlock(&call->lock);
atomic_inc(&call->ackr_not_idle);
- rxrpc_propose_ACK(call, RXRPC_ACK_DELAY, sp->hdr.serial, false);
+ rxrpc_propose_ACK(call, RXRPC_ACK_DELAY, serial, false);
_leave(" = 0 [posted]");
return 0;
@@ -239,7 +244,7 @@ out:
discard_and_ack:
_debug("discard and ACK packet %p", skb);
- __rxrpc_propose_ACK(call, ack, sp->hdr.serial, true);
+ __rxrpc_propose_ACK(call, ack, serial, true);
discard:
spin_unlock(&call->lock);
rxrpc_free_skb(skb);
@@ -247,7 +252,7 @@ discard:
return 0;
enqueue_and_ack:
- __rxrpc_propose_ACK(call, ack, sp->hdr.serial, true);
+ __rxrpc_propose_ACK(call, ack, serial, true);
enqueue_packet:
_net("defer skb %p", skb);
spin_unlock(&call->lock);
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH net 6/6] rxrpc: Free packets discarded in data_ready
2016-08-09 16:33 [PATCH net 0/6] rxrpc: Miscellaneous fixes David Howells
` (4 preceding siblings ...)
2016-08-09 16:33 ` [PATCH net 5/6] rxrpc: Fix a use-after-push in data_ready handler David Howells
@ 2016-08-09 16:33 ` David Howells
2016-08-09 19:57 ` [PATCH net 0/6] rxrpc: Miscellaneous fixes David Miller
6 siblings, 0 replies; 8+ messages in thread
From: David Howells @ 2016-08-09 16:33 UTC (permalink / raw)
To: netdev; +Cc: dhowells, linux-afs, linux-kernel
Under certain conditions, the data_ready handler will discard a packet.
These need to be freed.
Signed-off-by: David Howells <dhowells@redhat.com>
---
net/rxrpc/input.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/rxrpc/input.c b/net/rxrpc/input.c
index 789719031462..70bb77818dea 100644
--- a/net/rxrpc/input.c
+++ b/net/rxrpc/input.c
@@ -744,6 +744,8 @@ cant_route_call:
if (sp->hdr.type != RXRPC_PACKET_TYPE_ABORT) {
_debug("reject type %d",sp->hdr.type);
rxrpc_reject_packet(local, skb);
+ } else {
+ rxrpc_free_skb(skb);
}
_leave(" [no call]");
return;
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH net 0/6] rxrpc: Miscellaneous fixes
2016-08-09 16:33 [PATCH net 0/6] rxrpc: Miscellaneous fixes David Howells
` (5 preceding siblings ...)
2016-08-09 16:33 ` [PATCH net 6/6] rxrpc: Free packets discarded in data_ready David Howells
@ 2016-08-09 19:57 ` David Miller
6 siblings, 0 replies; 8+ messages in thread
From: David Miller @ 2016-08-09 19:57 UTC (permalink / raw)
To: dhowells; +Cc: netdev, linux-afs, linux-kernel
From: David Howells <dhowells@redhat.com>
Date: Tue, 09 Aug 2016 17:33:12 +0100
> Here are a bunch of miscellaneous fixes to AF_RXRPC:
>
> (*) Fix an uninitialised pointer.
>
> (*) Fix error handling when we fail to connect a call.
>
> (*) Fix a NULL pointer dereference.
>
> (*) Fix two occasions where a packet is accessed again after being queued
> for someone else to deal with.
>
> (*) Fix a missing skb free.
>
> ---
> The patches can be found here also:
>
> http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=rxrpc-fixes
>
> Tagged thusly:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git
> rxrpc-fixes-20160809
Pulled, thanks David.
^ permalink raw reply [flat|nested] 8+ messages in thread