[PATCH] sctp: Initialize daddr on peeled off socket

[PATCH] sctp: Initialize daddr on peeled off socket

[Petr Malat]

Function sctp_do_peeloff() wrongly initializes daddr of the original
socket instead of the peeled off one, which makes getpeername() return
zeroes instead of the primary address. Initialize the new socket
instead.

Signed-off-by: Petr Malat <oss@malat.biz>
---
 net/sctp/socket.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 3e1a9600be5e..7b0427658056 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -5636,7 +5636,7 @@ int sctp_do_peeloff(struct sock *sk, sctp_assoc_t id, struct socket **sockp)
 	 * Set the daddr and initialize id to something more random and also
 	 * copy over any ip options.
 	 */
-	sp->pf->to_sk_daddr(&asoc->peer.primary_addr, sk);
+	sp->pf->to_sk_daddr(&asoc->peer.primary_addr, sock->sk);
 	sp->pf->copy_ip_options(sk, sock->sk);
 
 	/* Populate the fields of the newsk from the oldsk and migrate the
-- 
2.30.2

Re: [PATCH] sctp: Initialize daddr on peeled off socket

[Jakub Kicinski]

On Mon,  7 Mar 2022 20:59:29 +0100 Petr Malat wrote:
> Function sctp_do_peeloff() wrongly initializes daddr of the original
> socket instead of the peeled off one, which makes getpeername() return
> zeroes instead of the primary address. Initialize the new socket
> instead.

Could you add a Fixes tag?

[PATCH v2] sctp: Initialize daddr on peeled off socket

[Petr Malat]

Function sctp_do_peeloff() wrongly initializes daddr of the original
socket instead of the peeled off socket, which makes getpeername()
return zeroes instead of the primary address. Initialize the new socket
instead.

Fixes: d570ee490fb1 ("[SCTP]: Correctly set daddr for IPv6 sockets during peeloff")
Signed-off-by: Petr Malat <oss@malat.biz>
---
 net/sctp/socket.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 3e1a9600be5e..7b0427658056 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -5636,7 +5636,7 @@ int sctp_do_peeloff(struct sock *sk, sctp_assoc_t id, struct socket **sockp)
 	 * Set the daddr and initialize id to something more random and also
 	 * copy over any ip options.
 	 */
-	sp->pf->to_sk_daddr(&asoc->peer.primary_addr, sk);
+	sp->pf->to_sk_daddr(&asoc->peer.primary_addr, sock->sk);
 	sp->pf->copy_ip_options(sk, sock->sk);
 
 	/* Populate the fields of the newsk from the oldsk and migrate the
-- 
2.30.2

Re: [PATCH v2] sctp: Initialize daddr on peeled off socket

[Marcelo Ricardo Leitner]

On Mon, Mar 07, 2022 at 11:02:21PM +0100, Petr Malat wrote:
> Function sctp_do_peeloff() wrongly initializes daddr of the original
> socket instead of the peeled off socket, which makes getpeername()
> return zeroes instead of the primary address. Initialize the new socket
> instead.
> 
> Fixes: d570ee490fb1 ("[SCTP]: Correctly set daddr for IPv6 sockets during peeloff")
> Signed-off-by: Petr Malat <oss@malat.biz>

Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>

You need to re-post it to netdev@, btw, but please feel free to carry
the tag above on it.

> ---
>  net/sctp/socket.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index 3e1a9600be5e..7b0427658056 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -5636,7 +5636,7 @@ int sctp_do_peeloff(struct sock *sk, sctp_assoc_t id, struct socket **sockp)
>  	 * Set the daddr and initialize id to something more random and also
>  	 * copy over any ip options.
>  	 */
> -	sp->pf->to_sk_daddr(&asoc->peer.primary_addr, sk);
> +	sp->pf->to_sk_daddr(&asoc->peer.primary_addr, sock->sk);
>  	sp->pf->copy_ip_options(sk, sock->sk);
>  
>  	/* Populate the fields of the newsk from the oldsk and migrate the
> -- 
> 2.30.2
> 

[PATCH] sctp: Initialize daddr on peeled off socket

[Petr Malat]

Function sctp_do_peeloff() wrongly initializes daddr of the original
socket instead of the peeled off socket, which makes getpeername()
return zeroes instead of the primary address. Initialize the new socket
instead.

Fixes: d570ee490fb1 ("[SCTP]: Correctly set daddr for IPv6 sockets during peeloff")
Signed-off-by: Petr Malat <oss@malat.biz>
Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
---
 net/sctp/socket.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 3e1a9600be5e..7b0427658056 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -5636,7 +5636,7 @@ int sctp_do_peeloff(struct sock *sk, sctp_assoc_t id, struct socket **sockp)
 	 * Set the daddr and initialize id to something more random and also
 	 * copy over any ip options.
 	 */
-	sp->pf->to_sk_daddr(&asoc->peer.primary_addr, sk);
+	sp->pf->to_sk_daddr(&asoc->peer.primary_addr, sock->sk);
 	sp->pf->copy_ip_options(sk, sock->sk);
 
 	/* Populate the fields of the newsk from the oldsk and migrate the
-- 
2.30.2

Re: [PATCH] sctp: Initialize daddr on peeled off socket

[patchwork-bot+netdevbpf]

Hello:

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

On Sat,  9 Apr 2022 08:36:11 +0200 you wrote:
> Function sctp_do_peeloff() wrongly initializes daddr of the original
> socket instead of the peeled off socket, which makes getpeername()
> return zeroes instead of the primary address. Initialize the new socket
> instead.
> 
> Fixes: d570ee490fb1 ("[SCTP]: Correctly set daddr for IPv6 sockets during peeloff")
> Signed-off-by: Petr Malat <oss@malat.biz>
> Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> 
> [...]

Here is the summary with links:
  - sctp: Initialize daddr on peeled off socket
    https://git.kernel.org/netdev/net/c/8467dda0c265

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

Re: [PATCH] sctp: Initialize daddr on peeled off socket

[Petr Malat]

On Mon, Mar 07, 2022 at 01:33:21PM -0800, Jakub Kicinski wrote:
> On Mon,  7 Mar 2022 20:59:29 +0100 Petr Malat wrote:
> > Function sctp_do_peeloff() wrongly initializes daddr of the original
> > socket instead of the peeled off one, which makes getpeername() return
> > zeroes instead of the primary address. Initialize the new socket
> > instead.
> 
> Could you add a Fixes tag?

Hi Jakub,
have you got some time to review the updated version with "Fixes" tag
added?

The issue has been in the kernel for a while, because my app is using
peer addresses for storing sockets in a hash table and the hash table can
handle collisions, thus I haven't noticed it's broken until I dumped
the hash table while working on another problem.
BR,
  Petr

Re: [PATCH] sctp: Initialize daddr on peeled off socket

[Jakub Kicinski]

On Fri, 8 Apr 2022 14:35:22 +0200 Petr Malat wrote:
> On Mon, Mar 07, 2022 at 01:33:21PM -0800, Jakub Kicinski wrote:
> > On Mon,  7 Mar 2022 20:59:29 +0100 Petr Malat wrote:  
> > > Function sctp_do_peeloff() wrongly initializes daddr of the original
> > > socket instead of the peeled off one, which makes getpeername() return
> > > zeroes instead of the primary address. Initialize the new socket
> > > instead.  
> > 
> > Could you add a Fixes tag?  
> 
> Hi Jakub,
> have you got some time to review the updated version with "Fixes" tag
> added?

Thanks for adding the tag. Long story short if you got no replies from
SCTP folks on the posting to linux-sctp@ repost with netdev@ added.
That way it'll get into general networking patchwork / patch processing
queue.

> The issue has been in the kernel for a while, because my app is using
> peer addresses for storing sockets in a hash table and the hash table can
> handle collisions, thus I haven't noticed it's broken until I dumped
> the hash table while working on another problem.

Re: [PATCH] sctp: Initialize daddr on peeled off socket

[Marcelo Ricardo Leitner]

On Fri, Apr 08, 2022 at 09:33:42AM -0700, Jakub Kicinski wrote:
> On Fri, 8 Apr 2022 14:35:22 +0200 Petr Malat wrote:
> > On Mon, Mar 07, 2022 at 01:33:21PM -0800, Jakub Kicinski wrote:
> > > On Mon,  7 Mar 2022 20:59:29 +0100 Petr Malat wrote:  
> > > > Function sctp_do_peeloff() wrongly initializes daddr of the original
> > > > socket instead of the peeled off one, which makes getpeername() return
> > > > zeroes instead of the primary address. Initialize the new socket
> > > > instead.  
> > > 
> > > Could you add a Fixes tag?  
> > 
> > Hi Jakub,
> > have you got some time to review the updated version with "Fixes" tag
> > added?
> 
> Thanks for adding the tag. Long story short if you got no replies from
> SCTP folks on the posting to linux-sctp@ repost with netdev@ added.
> That way it'll get into general networking patchwork / patch processing
> queue.

Yup. Also, the patch itself always need to be posted to netdev@
anyway, because we can't take patches directly from linux-sctp@. Well,
unless you mean to not officially submit the patch at a given time for
some reason, that is.

> 
> > The issue has been in the kernel for a while, because my app is using
> > peer addresses for storing sockets in a hash table and the hash table can
> > handle collisions, thus I haven't noticed it's broken until I dumped
> > the hash table while working on another problem.