net/rose: hold listener socket during call request handling
Subsystems (2)
A message that is not in the archive (sent off-list or not yet received): [email protected]
PATCH 1/1 net/rose: hold listener socket during call request handling
From: Nan Li <[email protected]>
The call request receive path keeps using the listener socket after the lookup lock has been dropped. Keep the listener alive across the remaining validation and child socket setup by taking a reference in the lookup path and releasing it once request handling is finished.
This makes listener lifetime handling explicit and avoids races with concurrent socket teardown.
--- net/rose/af_rose.c | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-)
net/rose/af_rose.c +19 -5
@@ -277,8 +277,10 @@ static struct sock *rose_find_listener(rose_address *addr, ax25_address *call)if (!rosecmp (&rose->source_addr, addr) && !ax25cmp (&rose->source_call, call) && - !rose->source_ndigis && s->sk_state ==TCP_LISTEN ) + !rose->source_ndigis && s->sk_state ==TCP_LISTEN ) { +sock_hold (s); goto found; + } }sk_for_each (s, &rose_list) {
@@ -286,8 +288,10 @@ static struct sock *rose_find_listener(rose_address *addr, ax25_address *call)if (!rosecmp (&rose->source_addr, addr) && !ax25cmp (&rose->source_call, &null_ax25_address) && - s->sk_state ==TCP_LISTEN ) + s->sk_state ==TCP_LISTEN ) { +sock_hold (s); goto found; + } } s =NULL ; found:
@@ -1056,10 +1060,13 @@ int rose_rx_call_request(struct sk_buff *skb, struct net_device *dev, struct ros/* * We can't accept the Call Request. */ -if (sk ==NULL ||sk_acceptq_is_full (sk) || +if (sk ==NULL ) + goto out_clear_request; + +if (sk_acceptq_is_full (sk) || (make =rose_make_new (sk)) ==NULL ) { -rose_transmit_clear_request (neigh, lci,ROSE_NETWORK_CONGESTION ,120 ); -return 0 ; +sock_put (sk); + goto out_clear_request; } skb->sk = make;
@@ -1110,7 +1117,14 @@ int rose_rx_call_request(struct sk_buff *skb, struct net_device *dev, struct rosif (!sock_flag (sk,SOCK_DEAD )) sk->sk_data_ready (sk); +sock_put (sk); +return 1 ; + +out_clear_request: +rose_transmit_clear_request (neigh, lci,ROSE_NETWORK_CONGESTION ,120 ); + +return 0 ; }static int rose_sendmsg (struct socket *sock,struct msghdr *msg,size_t len)
Signature · 1 line
2.43.0
Quoted 19 lines · Ren Wei
From: Nan Li <[email protected]>
The call request receive path keeps using the listener socket after the lookup lock has been dropped. Keep the listener alive across the remaining validation and child socket setup by taking a reference in the lookup path and releasing it once request handling is finished.
This makes listener lifetime handling explicit and avoids races with concurrent socket teardown.
Fixes 1da177e4c3f4 ("Linux-2.6.12-rc2")Reported-by Yifan Wu <[email protected]>Reported-by Juefei Pu <[email protected]>Reported-by Xin Liu <[email protected]>Co-developed-by Yuan Tan <[email protected]>Signed-off-by Yuan Tan <[email protected]>Signed-off-by Nan Li <[email protected]>Signed-off-by Ren Wei <[email protected]>
Quoted 3 lines
--- net/rose/af_rose.c | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-)
Sachiko has provided some feedback on this patch.
I do not believe they relate to shortcomings in this patch, and I do not believe they should block progress of this patch. You may want to look over them for areas to investigate as follow-up (maybe you already did :)
...
Quoted 25 lines · Simon Horman
Quoted 19 lines · Ren Wei
From: Nan Li <[email protected]>
The call request receive path keeps using the listener socket after the lookup lock has been dropped. Keep the listener alive across the remaining validation and child socket setup by taking a reference in the lookup path and releasing it once request handling is finished.
This makes listener lifetime handling explicit and avoids races with concurrent socket teardown.
Fixes 1da177e4c3f4 ("Linux-2.6.12-rc2")Reported-by Yifan Wu <[email protected]>Reported-by Juefei Pu <[email protected]>Reported-by Xin Liu <[email protected]>Co-developed-by Yuan Tan <[email protected]>Signed-off-by Yuan Tan <[email protected]>Signed-off-by Nan Li <[email protected]>Signed-off-by Ren Wei <[email protected]>Quoted 3 lines
--- net/rose/af_rose.c | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-)Reviewed-by Simon Horman <[email protected]>
Quoted 7 lines
Sachiko has provided some feedback on this patch.
I do not believe they relate to shortcomings in this patch, and I do not believe they should block progress of this patch. You may want to look over them for areas to investigate as follow-up (maybe you already did :)...
Thanks for your review! Yes this module still has other issues that haven't been fixed. We'll finish what we're currently working on and then take a look :)
Paolo Abeni
maintainerQuoted 10 lines · Ren Wei
From: Nan Li <[email protected]>
The call request receive path keeps using the listener socket after the lookup lock has been dropped. Keep the listener alive across the remaining validation and child socket setup by taking a reference in the lookup path and releasing it once request handling is finished.
This makes listener lifetime handling explicit and avoids races with concurrent socket teardown.
Quoted 9 lines
Fixes 1da177e4c3f4 ("Linux-2.6.12-rc2")Reported-by Yifan Wu <[email protected]>Reported-by Juefei Pu <[email protected]>Reported-by Xin Liu <[email protected]>Co-developed-by Yuan Tan <[email protected]>Signed-off-by Yuan Tan <[email protected]>Signed-off-by Nan Li <[email protected]>Signed-off-by Ren Wei <[email protected]>
we are moving rose out of tree:
https://lore.kernel.org/netdev/[email protected]/
please hold off until Thursday (after that our net PR will land into mainline), and eventually resend if the code still exists in Linus's tree at that point.
Thanks,
Paolo