Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

rxrpc: Fix NULL pointer deref due to call->conn being cleared on disconnect

When a call is disconnected, the connection pointer from the call is
cleared to make sure it isn't used again and to prevent further attempted
transmission for the call. Unfortunately, there might be a daemon trying
to use it at the same time to transmit a packet.

Fix this by keeping call->conn set, but setting a flag on the call to
indicate disconnection instead.

Remove also the bits in the transmission functions where the conn pointer is
checked and a ref taken under spinlock as this is now redundant.

Fixes: 8d94aa381dab ("rxrpc: Calls shouldn't hold socket refs")
Signed-off-by: David Howells <dhowells@redhat.com>

+15 -24
+1
net/rxrpc/ar-internal.h
··· 490 490 RXRPC_CALL_RX_HEARD, /* The peer responded at least once to this call */ 491 491 RXRPC_CALL_RX_UNDERRUN, /* Got data underrun */ 492 492 RXRPC_CALL_IS_INTR, /* The call is interruptible */ 493 + RXRPC_CALL_DISCONNECTED, /* The call has been disconnected */ 493 494 }; 494 495 495 496 /*
+2 -2
net/rxrpc/call_object.c
··· 493 493 494 494 _debug("RELEASE CALL %p (%d CONN %p)", call, call->debug_id, conn); 495 495 496 - if (conn) 496 + if (conn && !test_bit(RXRPC_CALL_DISCONNECTED, &call->flags)) 497 497 rxrpc_disconnect_call(call); 498 498 if (call->security) 499 499 call->security->free_call_crypto(call); ··· 569 569 struct rxrpc_call *call = container_of(rcu, struct rxrpc_call, rcu); 570 570 struct rxrpc_net *rxnet = call->rxnet; 571 571 572 + rxrpc_put_connection(call->conn); 572 573 rxrpc_put_peer(call->peer); 573 574 kfree(call->rxtx_buffer); 574 575 kfree(call->rxtx_annotations); ··· 591 590 592 591 ASSERTCMP(call->state, ==, RXRPC_CALL_COMPLETE); 593 592 ASSERT(test_bit(RXRPC_CALL_RELEASED, &call->flags)); 594 - ASSERTCMP(call->conn, ==, NULL); 595 593 596 594 rxrpc_cleanup_ring(call); 597 595 rxrpc_free_skb(call->tx_pending, rxrpc_skb_cleaned);
+1 -2
net/rxrpc/conn_client.c
··· 785 785 u32 cid; 786 786 787 787 spin_lock(&conn->channel_lock); 788 + set_bit(RXRPC_CALL_DISCONNECTED, &call->flags); 788 789 789 790 cid = call->cid; 790 791 if (cid) { ··· 793 792 chan = &conn->channels[channel]; 794 793 } 795 794 trace_rxrpc_client(conn, channel, rxrpc_client_chan_disconnect); 796 - call->conn = NULL; 797 795 798 796 /* Calls that have never actually been assigned a channel can simply be 799 797 * discarded. If the conn didn't get used either, it will follow ··· 908 908 spin_unlock(&rxnet->client_conn_cache_lock); 909 909 out_2: 910 910 spin_unlock(&conn->channel_lock); 911 - rxrpc_put_connection(conn); 912 911 _leave(""); 913 912 return; 914 913
+2 -2
net/rxrpc/conn_object.c
··· 171 171 172 172 _enter("%d,%x", conn->debug_id, call->cid); 173 173 174 + set_bit(RXRPC_CALL_DISCONNECTED, &call->flags); 175 + 174 176 if (rcu_access_pointer(chan->call) == call) { 175 177 /* Save the result of the call so that we can repeat it if necessary 176 178 * through the channel, whilst disposing of the actual call record. ··· 225 223 __rxrpc_disconnect_call(conn, call); 226 224 spin_unlock(&conn->channel_lock); 227 225 228 - call->conn = NULL; 229 226 conn->idle_timestamp = jiffies; 230 - rxrpc_put_connection(conn); 231 227 } 232 228 233 229 /*
+9 -18
net/rxrpc/output.c
··· 129 129 int rxrpc_send_ack_packet(struct rxrpc_call *call, bool ping, 130 130 rxrpc_serial_t *_serial) 131 131 { 132 - struct rxrpc_connection *conn = NULL; 132 + struct rxrpc_connection *conn; 133 133 struct rxrpc_ack_buffer *pkt; 134 134 struct msghdr msg; 135 135 struct kvec iov[2]; ··· 139 139 int ret; 140 140 u8 reason; 141 141 142 - spin_lock_bh(&call->lock); 143 - if (call->conn) 144 - conn = rxrpc_get_connection_maybe(call->conn); 145 - spin_unlock_bh(&call->lock); 146 - if (!conn) 142 + if (test_bit(RXRPC_CALL_DISCONNECTED, &call->flags)) 147 143 return -ECONNRESET; 148 144 149 145 pkt = kzalloc(sizeof(*pkt), GFP_KERNEL); 150 - if (!pkt) { 151 - rxrpc_put_connection(conn); 146 + if (!pkt) 152 147 return -ENOMEM; 153 - } 148 + 149 + conn = call->conn; 154 150 155 151 msg.msg_name = &call->peer->srx.transport; 156 152 msg.msg_namelen = call->peer->srx.transport_len; ··· 240 244 } 241 245 242 246 out: 243 - rxrpc_put_connection(conn); 244 247 kfree(pkt); 245 248 return ret; 246 249 } ··· 249 254 */ 250 255 int rxrpc_send_abort_packet(struct rxrpc_call *call) 251 256 { 252 - struct rxrpc_connection *conn = NULL; 257 + struct rxrpc_connection *conn; 253 258 struct rxrpc_abort_buffer pkt; 254 259 struct msghdr msg; 255 260 struct kvec iov[1]; ··· 266 271 test_bit(RXRPC_CALL_TX_LAST, &call->flags)) 267 272 return 0; 268 273 269 - spin_lock_bh(&call->lock); 270 - if (call->conn) 271 - conn = rxrpc_get_connection_maybe(call->conn); 272 - spin_unlock_bh(&call->lock); 273 - if (!conn) 274 + if (test_bit(RXRPC_CALL_DISCONNECTED, &call->flags)) 274 275 return -ECONNRESET; 276 + 277 + conn = call->conn; 275 278 276 279 msg.msg_name = &call->peer->srx.transport; 277 280 msg.msg_namelen = call->peer->srx.transport_len; ··· 305 312 trace_rxrpc_tx_packet(call->debug_id, &pkt.whdr, 306 313 rxrpc_tx_point_call_abort); 307 314 rxrpc_tx_backoff(call, ret); 308 - 309 - rxrpc_put_connection(conn); 310 315 return ret; 311 316 } 312 317