rxrpc: Miscellaneous fixes
Subsystems (3)
David Howells
maintainercover rxrpc: Miscellaneous fixes
Here some miscellaneous fixes for AF_RXRPC:
(1) Fix a race in the I/O thread vs UDP socket setup.
(2) Fix an uninitialised variable.
David
---
The patches can be found here also:
http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=rxrpc-fixes
David Howells (2):
rxrpc: Fix a race between socket set up and I/O thread creation
rxrpc: Fix uninitialised variable in rxrpc_send_data()
net/rxrpc/ar-internal.h | 2 +- net/rxrpc/io_thread.c | 10 ++++++++-- net/rxrpc/local_object.c | 2 +- net/rxrpc/sendmsg.c | 10 +++++----- 4 files changed, 15 insertions(+), 9 deletions(-)
David Howells
maintainerPATCH 1/2 rxrpc: Fix a race between socket set up and I/O thread creation
In rxrpc_open_socket(), it sets up the socket and then sets up the I/O thread that will handle it. This is a problem, however, as there's a gap between the two phases in which a packet may come into rxrpc_encap_rcv() from the UDP packet but we oops when trying to wake the not-yet created I/O thread.
As a quick fix, just make rxrpc_encap_rcv() discard the packet if there's no I/O thread yet.
A better, but more intrusive fix would perhaps be to rearrange things such that the socket creation is done by the I/O thread.
--- net/rxrpc/ar-internal.h | 2 +- net/rxrpc/io_thread.c | 10 ++++++++-- net/rxrpc/local_object.c | 2 +- 3 files changed, 10 insertions(+), 4 deletions(-)
net/rxrpc/ar-internal.h +1 -1
@@ -1056,7 +1056,7 @@ bool rxrpc_direct_abort(struct sk_buff *skb, enum rxrpc_abort_reason why,int rxrpc_io_thread (void *data);static inline void rxrpc_wake_up_io_thread (struct rxrpc_local *local) { -wake_up_process (local->io_thread); +wake_up_process (READ_ONCE (local->io_thread)); }static inline bool rxrpc_protocol_error (struct sk_buff *skb,enum rxrpc_abort_reason why)
net/rxrpc/io_thread.c +8 -2
@@ -27,11 +27,17 @@ int rxrpc_encap_rcv(struct sock *udp_sk, struct sk_buff *skb) {struct sk_buff_head *rx_queue;struct rxrpc_local *local =rcu_dereference_sk_user_data (udp_sk); +struct task_struct *io_thread;if (unlikely (!local)) {kfree_skb (skb);return 0 ; } + io_thread =READ_ONCE (local->io_thread); +if (!io_thread) { +kfree_skb (skb); +return 0 ; + }if (skb->tstamp ==0 ) skb->tstamp =ktime_get_real ();
@@ -47,7 +53,7 @@ int rxrpc_encap_rcv(struct sock *udp_sk, struct sk_buff *skb)#endif skb_queue_tail (rx_queue, skb); -rxrpc_wake_up_io_thread (local); +wake_up_process (io_thread);return 0 ; }
@@ -565,7 +571,7 @@ int rxrpc_io_thread(void *data)__set_current_state (TASK_RUNNING );rxrpc_see_local (local, rxrpc_local_stop);rxrpc_destroy_local (local); - local->io_thread =NULL ; +WRITE_ONCE (local->io_thread,NULL );rxrpc_see_local (local, rxrpc_local_stopped);return 0 ; }
net/rxrpc/local_object.c +1 -1
@@ -232,7 +232,7 @@ static int rxrpc_open_socket(struct rxrpc_local *local, struct net *net) }wait_for_completion (&local->io_thread_ready); - local->io_thread = io_thread; +WRITE_ONCE (local->io_thread, io_thread);_leave (" = 0" );return 0 ;
Re: [PATCH net 1/2] rxrpc: Fix a race between socket set up and I/O thread creation
Quoted 7 lines · David Howells
In
rxrpc_open_socket(), it sets up the socket and then sets up the I/O thread that will handle it. This is a problem, however, as there's a gap between the two phases in which a packet may come intorxrpc_encap_rcv()from the UDP packet but we oops when trying to wake the not-yet created I/O thread.
Quoted 8 lines
As a quick fix, just make
rxrpc_encap_rcv()discard the packet if there's no I/O thread yet.A better, but more intrusive fix would perhaps be to rearrange things such that the socket creation is done by the I/O thread.
Fixes a275da62e8c1 ("rxrpc: Create a per-local endpoint receive queue and I/O thread")Signed-off-by David Howells <[email protected]>
David Howells
maintainerPATCH 2/2 rxrpc: Fix uninitialised variable in rxrpc_send_data()
In reply to David HowellsFix the uninitialised txb variable in rxrpc_send_data() by moving the code that loads it above all the jumps to maybe_error, txb being stored back into call->tx_pending right before the normal return.
--- net/rxrpc/sendmsg.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
net/rxrpc/sendmsg.c +5 -5
@@ -303,6 +303,11 @@ static int rxrpc_send_data(struct rxrpc_sock *rx,sk_clear_bit (SOCKWQ_ASYNC_NOSPACE , sk); reload: + txb = call->tx_pending; + call->tx_pending =NULL ; +if (txb) +rxrpc_see_txbuf (txb, rxrpc_txbuf_see_send_more); + ret = -EPIPE ;if (sk->sk_shutdown &SEND_SHUTDOWN ) goto maybe_error;
@@ -329,11 +334,6 @@ static int rxrpc_send_data(struct rxrpc_sock *rx, goto maybe_error; } - txb = call->tx_pending; - call->tx_pending =NULL ; -if (txb) -rxrpc_see_txbuf (txb, rxrpc_txbuf_see_send_more); - do { if (!txb) { size_t remain;
This series was applied to netdev/net.git (main) by Jakub Kicinski <[email protected]>:
Hello:
This series was applied to netdev/net.git (main)
by Jakub Kicinski <[email protected]>:
Quoted 9 lines · On Tue, 1 Oct 2024 14:26:57 +0100 you wrote:
Here some miscellaneous fixes for
AF_RXRPC:(1) Fix a race in the I/O thread vs UDP socket setup.
(2) Fix an uninitialised variable.
David
[...]
Here is the summary with links:
- [net,1/2] rxrpc: Fix a race between socket set up and I/O thread creation
https://git.kernel.org/netdev/net/c/bc212465326e - [net,2/2] rxrpc: Fix uninitialised variable in rxrpc_send_data() https://git.kernel.org/netdev/net/c/7a310f8d7dfe
You are awesome, thank you!
Signature · 2 lines
Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html