* [PATCH v2] net/rose: Fix to not accept on connected socket
@ 2023-01-23 19:40 Hyunwoo Kim
2023-01-25 2:08 ` Kuniyuki Iwashima
0 siblings, 1 reply; 5+ messages in thread
From: Hyunwoo Kim @ 2023-01-23 19:40 UTC (permalink / raw)
To: ralf, davem, edumazet, kuba, pabeni; +Cc: v4bel, imv4bel, linux-hams, netdev
If listen() and accept() are called on a rose socket
that connect() is successful, accept() succeeds immediately.
This is because rose_connect() queues the skb to
sk->sk_receive_queue, and rose_accept() dequeues it.
This creates a child socket with the sk of the parent
rose socket, which can cause confusion.
Fix rose_listen() to return -EINVAL if the socket has
already been successfully connected, and add lock_sock
to prevent this issue.
Signed-off-by: Hyunwoo Kim <v4bel@theori.io>
---
net/rose/af_rose.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/net/rose/af_rose.c b/net/rose/af_rose.c
index 36fefc3957d7..ca2b17f32670 100644
--- a/net/rose/af_rose.c
+++ b/net/rose/af_rose.c
@@ -488,6 +488,12 @@ static int rose_listen(struct socket *sock, int backlog)
{
struct sock *sk = sock->sk;
+ lock_sock(sk);
+ if (sock->state != SS_UNCONNECTED) {
+ release_sock(sk);
+ return -EINVAL;
+ }
+
if (sk->sk_state != TCP_LISTEN) {
struct rose_sock *rose = rose_sk(sk);
@@ -497,8 +503,10 @@ static int rose_listen(struct socket *sock, int backlog)
memset(rose->dest_digis, 0, AX25_ADDR_LEN * ROSE_MAX_DIGIS);
sk->sk_max_ack_backlog = backlog;
sk->sk_state = TCP_LISTEN;
+ release_sock(sk);
return 0;
}
+ release_sock(sk);
return -EOPNOTSUPP;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] net/rose: Fix to not accept on connected socket
2023-01-23 19:40 [PATCH v2] net/rose: Fix to not accept on connected socket Hyunwoo Kim
@ 2023-01-25 2:08 ` Kuniyuki Iwashima
0 siblings, 0 replies; 5+ messages in thread
From: Kuniyuki Iwashima @ 2023-01-25 2:08 UTC (permalink / raw)
To: v4bel
Cc: davem, edumazet, imv4bel, kuba, linux-hams, netdev, pabeni, ralf, kuniyu
From: Hyunwoo Kim <v4bel@theori.io>
Date: Mon, 23 Jan 2023 11:40:20 -0800
> If listen() and accept() are called on a rose socket
> that connect() is successful, accept() succeeds immediately.
> This is because rose_connect() queues the skb to
> sk->sk_receive_queue, and rose_accept() dequeues it.
>
> This creates a child socket with the sk of the parent
> rose socket, which can cause confusion.
>
> Fix rose_listen() to return -EINVAL if the socket has
> already been successfully connected, and add lock_sock
> to prevent this issue.
>
> Signed-off-by: Hyunwoo Kim <v4bel@theori.io>
Reviewed-by: Kuniyuki Iwashima <kuniyu@amazon.com>
> ---
> net/rose/af_rose.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/net/rose/af_rose.c b/net/rose/af_rose.c
> index 36fefc3957d7..ca2b17f32670 100644
> --- a/net/rose/af_rose.c
> +++ b/net/rose/af_rose.c
> @@ -488,6 +488,12 @@ static int rose_listen(struct socket *sock, int backlog)
> {
> struct sock *sk = sock->sk;
>
> + lock_sock(sk);
> + if (sock->state != SS_UNCONNECTED) {
> + release_sock(sk);
> + return -EINVAL;
> + }
> +
> if (sk->sk_state != TCP_LISTEN) {
> struct rose_sock *rose = rose_sk(sk);
>
> @@ -497,8 +503,10 @@ static int rose_listen(struct socket *sock, int backlog)
> memset(rose->dest_digis, 0, AX25_ADDR_LEN * ROSE_MAX_DIGIS);
> sk->sk_max_ack_backlog = backlog;
> sk->sk_state = TCP_LISTEN;
> + release_sock(sk);
> return 0;
> }
> + release_sock(sk);
>
> return -EOPNOTSUPP;
> }
> --
> 2.25.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] net/rose: Fix to not accept on connected socket
@ 2023-01-25 2:08 ` Kuniyuki Iwashima
0 siblings, 0 replies; 5+ messages in thread
From: Kuniyuki Iwashima @ 2023-01-25 2:08 UTC (permalink / raw)
To: v4bel
Cc: davem, edumazet, imv4bel, kuba, linux-hams, netdev, pabeni, ralf, kuniyu
From: Hyunwoo Kim <v4bel@theori.io>
Date: Mon, 23 Jan 2023 11:40:20 -0800
> If listen() and accept() are called on a rose socket
> that connect() is successful, accept() succeeds immediately.
> This is because rose_connect() queues the skb to
> sk->sk_receive_queue, and rose_accept() dequeues it.
>
> This creates a child socket with the sk of the parent
> rose socket, which can cause confusion.
>
> Fix rose_listen() to return -EINVAL if the socket has
> already been successfully connected, and add lock_sock
> to prevent this issue.
>
> Signed-off-by: Hyunwoo Kim <v4bel@theori.io>
Reviewed-by: Kuniyuki Iwashima <kuniyu@amazon.com>
> ---
> net/rose/af_rose.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/net/rose/af_rose.c b/net/rose/af_rose.c
> index 36fefc3957d7..ca2b17f32670 100644
> --- a/net/rose/af_rose.c
> +++ b/net/rose/af_rose.c
> @@ -488,6 +488,12 @@ static int rose_listen(struct socket *sock, int backlog)
> {
> struct sock *sk = sock->sk;
>
> + lock_sock(sk);
> + if (sock->state != SS_UNCONNECTED) {
> + release_sock(sk);
> + return -EINVAL;
> + }
> +
> if (sk->sk_state != TCP_LISTEN) {
> struct rose_sock *rose = rose_sk(sk);
>
> @@ -497,8 +503,10 @@ static int rose_listen(struct socket *sock, int backlog)
> memset(rose->dest_digis, 0, AX25_ADDR_LEN * ROSE_MAX_DIGIS);
> sk->sk_max_ack_backlog = backlog;
> sk->sk_state = TCP_LISTEN;
> + release_sock(sk);
> return 0;
> }
> + release_sock(sk);
>
> return -EOPNOTSUPP;
> }
> --
> 2.25.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] net/rose: Fix to not accept on connected socket
2023-01-25 2:08 ` Kuniyuki Iwashima
@ 2023-01-25 2:28 ` Kuniyuki Iwashima
-1 siblings, 0 replies; 5+ messages in thread
From: Kuniyuki Iwashima @ 2023-01-25 2:28 UTC (permalink / raw)
To: v4bel
Cc: kuniyu, davem, edumazet, imv4bel, kuba, linux-hams, netdev, pabeni, ralf
From: Kuniyuki Iwashima <kuniyu@amazon.com>
Date: Tue, 24 Jan 2023 18:08:09 -0800
> From: Hyunwoo Kim <v4bel@theori.io>
> Date: Mon, 23 Jan 2023 11:40:20 -0800
> > If listen() and accept() are called on a rose socket
> > that connect() is successful, accept() succeeds immediately.
> > This is because rose_connect() queues the skb to
> > sk->sk_receive_queue, and rose_accept() dequeues it.
Same comment for the netrom patch here.
https://lore.kernel.org/netdev/20230125014347.65971-1-kuniyu@amazon.com/
The skb which the problematic accept() dequeues is created by
sendmsg(), not connect(), right ?
> >
> > This creates a child socket with the sk of the parent
> > rose socket, which can cause confusion.
> >
> > Fix rose_listen() to return -EINVAL if the socket has
> > already been successfully connected, and add lock_sock
> > to prevent this issue.
> >
> > Signed-off-by: Hyunwoo Kim <v4bel@theori.io>
>
> Reviewed-by: Kuniyuki Iwashima <kuniyu@amazon.com>
>
>
> > ---
> > net/rose/af_rose.c | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/net/rose/af_rose.c b/net/rose/af_rose.c
> > index 36fefc3957d7..ca2b17f32670 100644
> > --- a/net/rose/af_rose.c
> > +++ b/net/rose/af_rose.c
> > @@ -488,6 +488,12 @@ static int rose_listen(struct socket *sock, int backlog)
> > {
> > struct sock *sk = sock->sk;
> >
> > + lock_sock(sk);
> > + if (sock->state != SS_UNCONNECTED) {
> > + release_sock(sk);
> > + return -EINVAL;
> > + }
> > +
> > if (sk->sk_state != TCP_LISTEN) {
> > struct rose_sock *rose = rose_sk(sk);
> >
> > @@ -497,8 +503,10 @@ static int rose_listen(struct socket *sock, int backlog)
> > memset(rose->dest_digis, 0, AX25_ADDR_LEN * ROSE_MAX_DIGIS);
> > sk->sk_max_ack_backlog = backlog;
> > sk->sk_state = TCP_LISTEN;
> > + release_sock(sk);
> > return 0;
> > }
> > + release_sock(sk);
> >
> > return -EOPNOTSUPP;
> > }
> > --
> > 2.25.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] net/rose: Fix to not accept on connected socket
@ 2023-01-25 2:28 ` Kuniyuki Iwashima
0 siblings, 0 replies; 5+ messages in thread
From: Kuniyuki Iwashima @ 2023-01-25 2:28 UTC (permalink / raw)
To: v4bel
Cc: kuniyu, davem, edumazet, imv4bel, kuba, linux-hams, netdev, pabeni, ralf
From: Kuniyuki Iwashima <kuniyu@amazon.com>
Date: Tue, 24 Jan 2023 18:08:09 -0800
> From: Hyunwoo Kim <v4bel@theori.io>
> Date: Mon, 23 Jan 2023 11:40:20 -0800
> > If listen() and accept() are called on a rose socket
> > that connect() is successful, accept() succeeds immediately.
> > This is because rose_connect() queues the skb to
> > sk->sk_receive_queue, and rose_accept() dequeues it.
Same comment for the netrom patch here.
https://lore.kernel.org/netdev/20230125014347.65971-1-kuniyu@amazon.com/
The skb which the problematic accept() dequeues is created by
sendmsg(), not connect(), right ?
> >
> > This creates a child socket with the sk of the parent
> > rose socket, which can cause confusion.
> >
> > Fix rose_listen() to return -EINVAL if the socket has
> > already been successfully connected, and add lock_sock
> > to prevent this issue.
> >
> > Signed-off-by: Hyunwoo Kim <v4bel@theori.io>
>
> Reviewed-by: Kuniyuki Iwashima <kuniyu@amazon.com>
>
>
> > ---
> > net/rose/af_rose.c | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/net/rose/af_rose.c b/net/rose/af_rose.c
> > index 36fefc3957d7..ca2b17f32670 100644
> > --- a/net/rose/af_rose.c
> > +++ b/net/rose/af_rose.c
> > @@ -488,6 +488,12 @@ static int rose_listen(struct socket *sock, int backlog)
> > {
> > struct sock *sk = sock->sk;
> >
> > + lock_sock(sk);
> > + if (sock->state != SS_UNCONNECTED) {
> > + release_sock(sk);
> > + return -EINVAL;
> > + }
> > +
> > if (sk->sk_state != TCP_LISTEN) {
> > struct rose_sock *rose = rose_sk(sk);
> >
> > @@ -497,8 +503,10 @@ static int rose_listen(struct socket *sock, int backlog)
> > memset(rose->dest_digis, 0, AX25_ADDR_LEN * ROSE_MAX_DIGIS);
> > sk->sk_max_ack_backlog = backlog;
> > sk->sk_state = TCP_LISTEN;
> > + release_sock(sk);
> > return 0;
> > }
> > + release_sock(sk);
> >
> > return -EOPNOTSUPP;
> > }
> > --
> > 2.25.1
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-01-25 2:29 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-23 19:40 [PATCH v2] net/rose: Fix to not accept on connected socket Hyunwoo Kim
2023-01-25 2:08 ` Kuniyuki Iwashima
2023-01-25 2:08 ` Kuniyuki Iwashima
2023-01-25 2:28 ` Kuniyuki Iwashima
2023-01-25 2:28 ` Kuniyuki Iwashima
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.