netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net 0/2] rxrpc: Miscellaneous fixes
@ 2024-10-01 13:26 David Howells
  2024-10-01 13:26 ` [PATCH net 1/2] rxrpc: Fix a race between socket set up and I/O thread creation David Howells
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: David Howells @ 2024-10-01 13:26 UTC (permalink / raw)
  To: netdev
  Cc: David Howells, Marc Dionne, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, linux-afs, linux-kernel

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(-)


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH net 1/2] rxrpc: Fix a race between socket set up and I/O thread creation
  2024-10-01 13:26 [PATCH net 0/2] rxrpc: Miscellaneous fixes David Howells
@ 2024-10-01 13:26 ` David Howells
  2024-10-01 13:28   ` Eric Dumazet
  2024-10-01 13:26 ` [PATCH net 2/2] rxrpc: Fix uninitialised variable in rxrpc_send_data() David Howells
  2024-10-03 23:40 ` [PATCH net 0/2] rxrpc: Miscellaneous fixes patchwork-bot+netdevbpf
  2 siblings, 1 reply; 9+ messages in thread
From: David Howells @ 2024-10-01 13:26 UTC (permalink / raw)
  To: netdev
  Cc: David Howells, Marc Dionne, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, linux-afs, linux-kernel, yuxuanzhe,
	Simon Horman

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.

Fixes: a275da62e8c1 ("rxrpc: Create a per-local endpoint receive queue and I/O thread")
Signed-off-by: David Howells <dhowells@redhat.com>
cc: yuxuanzhe@outlook.com
cc: Marc Dionne <marc.dionne@auristor.com>
cc: "David S. Miller" <davem@davemloft.net>
cc: Eric Dumazet <edumazet@google.com>
cc: Jakub Kicinski <kuba@kernel.org>
cc: Paolo Abeni <pabeni@redhat.com>
cc: Simon Horman <horms@kernel.org>
cc: linux-afs@lists.infradead.org
cc: netdev@vger.kernel.org
---
 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(-)

diff --git a/net/rxrpc/ar-internal.h b/net/rxrpc/ar-internal.h
index 80d682f89b23..d0fd37bdcfe9 100644
--- a/net/rxrpc/ar-internal.h
+++ b/net/rxrpc/ar-internal.h
@@ -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)
diff --git a/net/rxrpc/io_thread.c b/net/rxrpc/io_thread.c
index 0300baa9afcd..07c74c77d802 100644
--- a/net/rxrpc/io_thread.c
+++ b/net/rxrpc/io_thread.c
@@ -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;
 }
diff --git a/net/rxrpc/local_object.c b/net/rxrpc/local_object.c
index 504453c688d7..f9623ace2201 100644
--- a/net/rxrpc/local_object.c
+++ b/net/rxrpc/local_object.c
@@ -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;
 


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH net 2/2] rxrpc: Fix uninitialised variable in rxrpc_send_data()
  2024-10-01 13:26 [PATCH net 0/2] rxrpc: Miscellaneous fixes David Howells
  2024-10-01 13:26 ` [PATCH net 1/2] rxrpc: Fix a race between socket set up and I/O thread creation David Howells
@ 2024-10-01 13:26 ` David Howells
  2024-10-03 23:40 ` [PATCH net 0/2] rxrpc: Miscellaneous fixes patchwork-bot+netdevbpf
  2 siblings, 0 replies; 9+ messages in thread
From: David Howells @ 2024-10-01 13:26 UTC (permalink / raw)
  To: netdev
  Cc: David Howells, Marc Dionne, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, linux-afs, linux-kernel,
	Dan Carpenter

Fix 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.

Fixes: b0f571ecd794 ("rxrpc: Fix locking in rxrpc's sendmsg")
Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Closes: https://lists.infradead.org/pipermail/linux-afs/2024-October/008896.html
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: "David S. Miller" <davem@davemloft.net>
cc: Eric Dumazet <edumazet@google.com>
cc: Jakub Kicinski <kuba@kernel.org>
cc: Paolo Abeni <pabeni@redhat.com>
cc: linux-afs@lists.infradead.org
cc: netdev@vger.kernel.org
---
 net/rxrpc/sendmsg.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/net/rxrpc/sendmsg.c b/net/rxrpc/sendmsg.c
index 894b8fa68e5e..23d18fe5de9f 100644
--- a/net/rxrpc/sendmsg.c
+++ b/net/rxrpc/sendmsg.c
@@ -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;


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH net 1/2] rxrpc: Fix a race between socket set up and I/O thread creation
  2024-10-01 13:26 ` [PATCH net 1/2] rxrpc: Fix a race between socket set up and I/O thread creation David Howells
@ 2024-10-01 13:28   ` Eric Dumazet
  0 siblings, 0 replies; 9+ messages in thread
From: Eric Dumazet @ 2024-10-01 13:28 UTC (permalink / raw)
  To: David Howells
  Cc: netdev, Marc Dionne, David S. Miller, Jakub Kicinski, Paolo Abeni,
	linux-afs, linux-kernel, yuxuanzhe, Simon Horman

On Tue, Oct 1, 2024 at 3:27 PM David Howells <dhowells@redhat.com> wrote:
>
> 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.
>
> Fixes: a275da62e8c1 ("rxrpc: Create a per-local endpoint receive queue and I/O thread")
> Signed-off-by: David Howells <dhowells@redhat.com>

Reviewed-by: Eric Dumazet <edumazet@google.com>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH net 0/2] rxrpc: Miscellaneous fixes
  2024-10-01 13:26 [PATCH net 0/2] rxrpc: Miscellaneous fixes David Howells
  2024-10-01 13:26 ` [PATCH net 1/2] rxrpc: Fix a race between socket set up and I/O thread creation David Howells
  2024-10-01 13:26 ` [PATCH net 2/2] rxrpc: Fix uninitialised variable in rxrpc_send_data() David Howells
@ 2024-10-03 23:40 ` patchwork-bot+netdevbpf
  2 siblings, 0 replies; 9+ messages in thread
From: patchwork-bot+netdevbpf @ 2024-10-03 23:40 UTC (permalink / raw)
  To: David Howells
  Cc: netdev, marc.dionne, davem, edumazet, kuba, pabeni, linux-afs,
	linux-kernel

Hello:

This series was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

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!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH net 0/2] rxrpc: Miscellaneous fixes
@ 2025-02-03 11:03 David Howells
  2025-02-04 14:40 ` patchwork-bot+netdevbpf
  0 siblings, 1 reply; 9+ messages in thread
From: David Howells @ 2025-02-03 11:03 UTC (permalink / raw)
  To: netdev
  Cc: David Howells, Marc Dionne, Jakub Kicinski, David S. Miller,
	Eric Dumazet, Paolo Abeni, linux-afs, linux-kernel

Here some miscellaneous fixes for AF_RXRPC:

 (1) Fix the state of a call to not treat the challenge-response cycle as
     part of an incoming call's state set.  The problem is that it makes
     handling received of the final packet in the receive phase difficult
     as that wants to change the call state - but security negotiations may
     not yet be complete.

 (2) Fix the queuing of connections seeking attention from offloaded ops
     such as challenge/response.  The problem was that the attention link
     always seemed to be busy because it was never initialised from NULL.
     This further masked two further bugs, also fixed in the patch.

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 call state set to not include the SERVER_SECURING state
  rxrpc: Fix the rxrpc_connection attend queue handling

 include/trace/events/rxrpc.h |  1 +
 net/rxrpc/ar-internal.h      |  2 +-
 net/rxrpc/call_object.c      |  6 ++----
 net/rxrpc/conn_event.c       | 21 +++++++++++----------
 net/rxrpc/conn_object.c      |  1 +
 net/rxrpc/input.c            |  4 ++--
 net/rxrpc/sendmsg.c          |  2 +-
 7 files changed, 19 insertions(+), 18 deletions(-)


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH net 0/2] rxrpc: Miscellaneous fixes
  2025-02-03 11:03 David Howells
@ 2025-02-04 14:40 ` patchwork-bot+netdevbpf
  0 siblings, 0 replies; 9+ messages in thread
From: patchwork-bot+netdevbpf @ 2025-02-04 14:40 UTC (permalink / raw)
  To: David Howells
  Cc: netdev, marc.dionne, kuba, davem, edumazet, pabeni, linux-afs,
	linux-kernel

Hello:

This series was applied to netdev/net.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Mon,  3 Feb 2025 11:03:02 +0000 you wrote:
> Here some miscellaneous fixes for AF_RXRPC:
> 
>  (1) Fix the state of a call to not treat the challenge-response cycle as
>      part of an incoming call's state set.  The problem is that it makes
>      handling received of the final packet in the receive phase difficult
>      as that wants to change the call state - but security negotiations may
>      not yet be complete.
> 
> [...]

Here is the summary with links:
  - [net,1/2] rxrpc: Fix call state set to not include the SERVER_SECURING state
    (no matching commit)
  - [net,2/2] rxrpc: Fix the rxrpc_connection attend queue handling
    https://git.kernel.org/netdev/net/c/4241a702e0d0

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH net 0/2] rxrpc: Miscellaneous fixes
@ 2025-07-07 10:24 David Howells
  2025-07-08 20:20 ` patchwork-bot+netdevbpf
  0 siblings, 1 reply; 9+ messages in thread
From: David Howells @ 2025-07-07 10:24 UTC (permalink / raw)
  To: netdev
  Cc: David Howells, Marc Dionne, Jakub Kicinski, David S. Miller,
	Eric Dumazet, Paolo Abeni, linux-afs, linux-kernel

Here are some miscellaneous fixes for rxrpc:

 (1) Fix a warning about over-large frame size in rxrpc_send_response().

 (2) Fix assertion failure due to preallocation collision.

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 over large frame size warning
  rxrpc: Fix bug due to prealloc collision

 net/rxrpc/ar-internal.h | 15 +++++++++------
 net/rxrpc/call_accept.c |  2 ++
 net/rxrpc/output.c      |  5 ++++-
 3 files changed, 15 insertions(+), 7 deletions(-)


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH net 0/2] rxrpc: Miscellaneous fixes
  2025-07-07 10:24 David Howells
@ 2025-07-08 20:20 ` patchwork-bot+netdevbpf
  0 siblings, 0 replies; 9+ messages in thread
From: patchwork-bot+netdevbpf @ 2025-07-08 20:20 UTC (permalink / raw)
  To: David Howells
  Cc: netdev, marc.dionne, kuba, davem, edumazet, pabeni, linux-afs,
	linux-kernel

Hello:

This series was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Mon,  7 Jul 2025 11:24:32 +0100 you wrote:
> Here are some miscellaneous fixes for rxrpc:
> 
>  (1) Fix a warning about over-large frame size in rxrpc_send_response().
> 
>  (2) Fix assertion failure due to preallocation collision.
> 
> David
> 
> [...]

Here is the summary with links:
  - [net,1/2] rxrpc: Fix over large frame size warning
    https://git.kernel.org/netdev/net/c/31ec70afaaad
  - [net,2/2] rxrpc: Fix bug due to prealloc collision
    (no matching commit)

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2025-07-08 20:19 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-01 13:26 [PATCH net 0/2] rxrpc: Miscellaneous fixes David Howells
2024-10-01 13:26 ` [PATCH net 1/2] rxrpc: Fix a race between socket set up and I/O thread creation David Howells
2024-10-01 13:28   ` Eric Dumazet
2024-10-01 13:26 ` [PATCH net 2/2] rxrpc: Fix uninitialised variable in rxrpc_send_data() David Howells
2024-10-03 23:40 ` [PATCH net 0/2] rxrpc: Miscellaneous fixes patchwork-bot+netdevbpf
  -- strict thread matches above, loose matches on Subject: below --
2025-02-03 11:03 David Howells
2025-02-04 14:40 ` patchwork-bot+netdevbpf
2025-07-07 10:24 David Howells
2025-07-08 20:20 ` patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).