All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net 1/2] sctp: remove redundant assignment when call sctp_get_port_local
  2019-09-10  7:13 ` Mao Wenan
  (?)
@ 2019-09-10  6:56   ` Mao Wenan
  -1 siblings, 0 replies; 43+ messages in thread
From: Mao Wenan @ 2019-09-10  6:56 UTC (permalink / raw)
  To: vyasevich, nhorman, marcelo.leitner, davem
  Cc: linux-sctp, netdev, linux-kernel, kernel-janitors, Mao Wenan

There are more parentheses in if clause when call sctp_get_port_local
in sctp_do_bind, and redundant assignment to 'ret'. This patch is to
do cleanup.

Signed-off-by: Mao Wenan <maowenan@huawei.com>
---
 net/sctp/socket.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 9d1f83b10c0a..766b68b55ebe 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -399,9 +399,8 @@ static int sctp_do_bind(struct sock *sk, union sctp_addr *addr, int len)
 	 * detection.
 	 */
 	addr->v4.sin_port = htons(snum);
-	if ((ret = sctp_get_port_local(sk, addr))) {
+	if (sctp_get_port_local(sk, addr))
 		return -EADDRINUSE;
-	}
 
 	/* Refresh ephemeral port.  */
 	if (!bp->port)
-- 
2.20.1

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

* [PATCH net 0/2] fix memory leak for sctp_do_bind
@ 2019-09-10  7:13 ` Mao Wenan
  0 siblings, 0 replies; 43+ messages in thread
From: Mao Wenan @ 2019-09-10  6:56 UTC (permalink / raw)
  To: vyasevich, nhorman, marcelo.leitner, davem
  Cc: linux-sctp, netdev, linux-kernel, kernel-janitors, Mao Wenan

First patch is to do cleanup, remove redundant assignment,
second patch is to fix memory leak for sctp_do_bind if failed
to bind address.

Mao Wenan (2):
  sctp: remove redundant assignment when call sctp_get_port_local
  sctp: destroy bucket if failed to bind addr

 net/sctp/socket.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

-- 
2.20.1

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

* [PATCH net 2/2] sctp: destroy bucket if failed to bind addr
  2019-09-10  7:13 ` Mao Wenan
  (?)
@ 2019-09-10  6:56   ` Mao Wenan
  -1 siblings, 0 replies; 43+ messages in thread
From: Mao Wenan @ 2019-09-10  6:56 UTC (permalink / raw)
  To: vyasevich, nhorman, marcelo.leitner, davem
  Cc: linux-sctp, netdev, linux-kernel, kernel-janitors, Mao Wenan, Hulk Robot

There is one memory leak bug report:
BUG: memory leak
unreferenced object 0xffff8881dc4c5ec0 (size 40):
  comm "syz-executor.0", pid 5673, jiffies 4298198457 (age 27.578s)
  hex dump (first 32 bytes):
    02 00 00 00 81 88 ff ff 00 00 00 00 00 00 00 00  ................
    f8 63 3d c1 81 88 ff ff 00 00 00 00 00 00 00 00  .c=.............
  backtrace:
    [<0000000072006339>] sctp_get_port_local+0x2a1/0xa00 [sctp]
    [<00000000c7b379ec>] sctp_do_bind+0x176/0x2c0 [sctp]
    [<000000005be274a2>] sctp_bind+0x5a/0x80 [sctp]
    [<00000000b66b4044>] inet6_bind+0x59/0xd0 [ipv6]
    [<00000000c68c7f42>] __sys_bind+0x120/0x1f0 net/socket.c:1647
    [<000000004513635b>] __do_sys_bind net/socket.c:1658 [inline]
    [<000000004513635b>] __se_sys_bind net/socket.c:1656 [inline]
    [<000000004513635b>] __x64_sys_bind+0x3e/0x50 net/socket.c:1656
    [<0000000061f2501e>] do_syscall_64+0x72/0x2e0 arch/x86/entry/common.c:296
    [<0000000003d1e05e>] entry_SYSCALL_64_after_hwframe+0x49/0xbe

This is because in sctp_do_bind, if sctp_get_port_local is to
create hash bucket successfully, and sctp_add_bind_addr failed
to bind address, e.g return -ENOMEM, so memory leak found, it
needs to destroy allocated bucket.

Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: Mao Wenan <maowenan@huawei.com>
---
 net/sctp/socket.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 766b68b55ebe..ab37fc1f7bb6 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -412,11 +412,13 @@ static int sctp_do_bind(struct sock *sk, union sctp_addr *addr, int len)
 	ret = sctp_add_bind_addr(bp, addr, af->sockaddr_len,
 				 SCTP_ADDR_SRC, GFP_ATOMIC);
 
-	/* Copy back into socket for getsockname() use. */
-	if (!ret) {
-		inet_sk(sk)->inet_sport = htons(inet_sk(sk)->inet_num);
-		sp->pf->to_sk_saddr(addr, sk);
+	if (ret) {
+		sctp_put_port(sk);
+		return ret;
 	}
+	/* Copy back into socket for getsockname() use. */
+	inet_sk(sk)->inet_sport = htons(inet_sk(sk)->inet_num);
+	sp->pf->to_sk_saddr(addr, sk);
 
 	return ret;
 }
-- 
2.20.1

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

* [PATCH net 2/2] sctp: destroy bucket if failed to bind addr
@ 2019-09-10  6:56   ` Mao Wenan
  0 siblings, 0 replies; 43+ messages in thread
From: Mao Wenan @ 2019-09-10  6:56 UTC (permalink / raw)
  To: vyasevich, nhorman, marcelo.leitner, davem
  Cc: linux-sctp, netdev, linux-kernel, kernel-janitors, Mao Wenan, Hulk Robot

There is one memory leak bug report:
BUG: memory leak
unreferenced object 0xffff8881dc4c5ec0 (size 40):
  comm "syz-executor.0", pid 5673, jiffies 4298198457 (age 27.578s)
  hex dump (first 32 bytes):
    02 00 00 00 81 88 ff ff 00 00 00 00 00 00 00 00  ................
    f8 63 3d c1 81 88 ff ff 00 00 00 00 00 00 00 00  .c=.............
  backtrace:
    [<0000000072006339>] sctp_get_port_local+0x2a1/0xa00 [sctp]
    [<00000000c7b379ec>] sctp_do_bind+0x176/0x2c0 [sctp]
    [<000000005be274a2>] sctp_bind+0x5a/0x80 [sctp]
    [<00000000b66b4044>] inet6_bind+0x59/0xd0 [ipv6]
    [<00000000c68c7f42>] __sys_bind+0x120/0x1f0 net/socket.c:1647
    [<000000004513635b>] __do_sys_bind net/socket.c:1658 [inline]
    [<000000004513635b>] __se_sys_bind net/socket.c:1656 [inline]
    [<000000004513635b>] __x64_sys_bind+0x3e/0x50 net/socket.c:1656
    [<0000000061f2501e>] do_syscall_64+0x72/0x2e0 arch/x86/entry/common.c:296
    [<0000000003d1e05e>] entry_SYSCALL_64_after_hwframe+0x49/0xbe

This is because in sctp_do_bind, if sctp_get_port_local is to
create hash bucket successfully, and sctp_add_bind_addr failed
to bind address, e.g return -ENOMEM, so memory leak found, it
needs to destroy allocated bucket.

Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: Mao Wenan <maowenan@huawei.com>
---
 net/sctp/socket.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 766b68b55ebe..ab37fc1f7bb6 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -412,11 +412,13 @@ static int sctp_do_bind(struct sock *sk, union sctp_addr *addr, int len)
 	ret = sctp_add_bind_addr(bp, addr, af->sockaddr_len,
 				 SCTP_ADDR_SRC, GFP_ATOMIC);
 
-	/* Copy back into socket for getsockname() use. */
-	if (!ret) {
-		inet_sk(sk)->inet_sport = htons(inet_sk(sk)->inet_num);
-		sp->pf->to_sk_saddr(addr, sk);
+	if (ret) {
+		sctp_put_port(sk);
+		return ret;
 	}
+	/* Copy back into socket for getsockname() use. */
+	inet_sk(sk)->inet_sport = htons(inet_sk(sk)->inet_num);
+	sp->pf->to_sk_saddr(addr, sk);
 
 	return ret;
 }
-- 
2.20.1

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

* [PATCH net 1/2] sctp: remove redundant assignment when call sctp_get_port_local
@ 2019-09-10  6:56   ` Mao Wenan
  0 siblings, 0 replies; 43+ messages in thread
From: Mao Wenan @ 2019-09-10  6:56 UTC (permalink / raw)
  To: vyasevich, nhorman, marcelo.leitner, davem
  Cc: linux-sctp, netdev, linux-kernel, kernel-janitors, Mao Wenan

There are more parentheses in if clause when call sctp_get_port_local
in sctp_do_bind, and redundant assignment to 'ret'. This patch is to
do cleanup.

Signed-off-by: Mao Wenan <maowenan@huawei.com>
---
 net/sctp/socket.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 9d1f83b10c0a..766b68b55ebe 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -399,9 +399,8 @@ static int sctp_do_bind(struct sock *sk, union sctp_addr *addr, int len)
 	 * detection.
 	 */
 	addr->v4.sin_port = htons(snum);
-	if ((ret = sctp_get_port_local(sk, addr))) {
+	if (sctp_get_port_local(sk, addr))
 		return -EADDRINUSE;
-	}
 
 	/* Refresh ephemeral port.  */
 	if (!bp->port)
-- 
2.20.1

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

* [PATCH net 0/2] fix memory leak for sctp_do_bind
@ 2019-09-10  7:13 ` Mao Wenan
  0 siblings, 0 replies; 43+ messages in thread
From: Mao Wenan @ 2019-09-10  7:13 UTC (permalink / raw)
  To: vyasevich, nhorman, marcelo.leitner, davem
  Cc: linux-sctp, netdev, linux-kernel, kernel-janitors, Mao Wenan

First patch is to do cleanup, remove redundant assignment,
second patch is to fix memory leak for sctp_do_bind if failed
to bind address.

Mao Wenan (2):
  sctp: remove redundant assignment when call sctp_get_port_local
  sctp: destroy bucket if failed to bind addr

 net/sctp/socket.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

-- 
2.20.1


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

* [PATCH net 1/2] sctp: remove redundant assignment when call sctp_get_port_local
@ 2019-09-10  6:56   ` Mao Wenan
  0 siblings, 0 replies; 43+ messages in thread
From: Mao Wenan @ 2019-09-10  7:13 UTC (permalink / raw)
  To: vyasevich, nhorman, marcelo.leitner, davem
  Cc: linux-sctp, netdev, linux-kernel, kernel-janitors, Mao Wenan

There are more parentheses in if clause when call sctp_get_port_local
in sctp_do_bind, and redundant assignment to 'ret'. This patch is to
do cleanup.

Signed-off-by: Mao Wenan <maowenan@huawei.com>
---
 net/sctp/socket.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 9d1f83b10c0a..766b68b55ebe 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -399,9 +399,8 @@ static int sctp_do_bind(struct sock *sk, union sctp_addr *addr, int len)
 	 * detection.
 	 */
 	addr->v4.sin_port = htons(snum);
-	if ((ret = sctp_get_port_local(sk, addr))) {
+	if (sctp_get_port_local(sk, addr))
 		return -EADDRINUSE;
-	}
 
 	/* Refresh ephemeral port.  */
 	if (!bp->port)
-- 
2.20.1


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

* [PATCH net 2/2] sctp: destroy bucket if failed to bind addr
@ 2019-09-10  6:56   ` Mao Wenan
  0 siblings, 0 replies; 43+ messages in thread
From: Mao Wenan @ 2019-09-10  7:13 UTC (permalink / raw)
  To: vyasevich, nhorman, marcelo.leitner, davem
  Cc: linux-sctp, netdev, linux-kernel, kernel-janitors, Mao Wenan, Hulk Robot

There is one memory leak bug report:
BUG: memory leak
unreferenced object 0xffff8881dc4c5ec0 (size 40):
  comm "syz-executor.0", pid 5673, jiffies 4298198457 (age 27.578s)
  hex dump (first 32 bytes):
    02 00 00 00 81 88 ff ff 00 00 00 00 00 00 00 00  ................
    f8 63 3d c1 81 88 ff ff 00 00 00 00 00 00 00 00  .c=.............
  backtrace:
    [<0000000072006339>] sctp_get_port_local+0x2a1/0xa00 [sctp]
    [<00000000c7b379ec>] sctp_do_bind+0x176/0x2c0 [sctp]
    [<000000005be274a2>] sctp_bind+0x5a/0x80 [sctp]
    [<00000000b66b4044>] inet6_bind+0x59/0xd0 [ipv6]
    [<00000000c68c7f42>] __sys_bind+0x120/0x1f0 net/socket.c:1647
    [<000000004513635b>] __do_sys_bind net/socket.c:1658 [inline]
    [<000000004513635b>] __se_sys_bind net/socket.c:1656 [inline]
    [<000000004513635b>] __x64_sys_bind+0x3e/0x50 net/socket.c:1656
    [<0000000061f2501e>] do_syscall_64+0x72/0x2e0 arch/x86/entry/common.c:296
    [<0000000003d1e05e>] entry_SYSCALL_64_after_hwframe+0x49/0xbe

This is because in sctp_do_bind, if sctp_get_port_local is to
create hash bucket successfully, and sctp_add_bind_addr failed
to bind address, e.g return -ENOMEM, so memory leak found, it
needs to destroy allocated bucket.

Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: Mao Wenan <maowenan@huawei.com>
---
 net/sctp/socket.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 766b68b55ebe..ab37fc1f7bb6 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -412,11 +412,13 @@ static int sctp_do_bind(struct sock *sk, union sctp_addr *addr, int len)
 	ret = sctp_add_bind_addr(bp, addr, af->sockaddr_len,
 				 SCTP_ADDR_SRC, GFP_ATOMIC);
 
-	/* Copy back into socket for getsockname() use. */
-	if (!ret) {
-		inet_sk(sk)->inet_sport = htons(inet_sk(sk)->inet_num);
-		sp->pf->to_sk_saddr(addr, sk);
+	if (ret) {
+		sctp_put_port(sk);
+		return ret;
 	}
+	/* Copy back into socket for getsockname() use. */
+	inet_sk(sk)->inet_sport = htons(inet_sk(sk)->inet_num);
+	sp->pf->to_sk_saddr(addr, sk);
 
 	return ret;
 }
-- 
2.20.1


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

* Re: [PATCH net 0/2] fix memory leak for sctp_do_bind
  2019-09-10  7:13 ` Mao Wenan
@ 2019-09-10  7:16   ` Neil Horman
  -1 siblings, 0 replies; 43+ messages in thread
From: Neil Horman @ 2019-09-10  7:16 UTC (permalink / raw)
  To: Mao Wenan
  Cc: vyasevich, marcelo.leitner, davem, linux-sctp, netdev,
	linux-kernel, kernel-janitors

On Tue, Sep 10, 2019 at 03:13:41PM +0800, Mao Wenan wrote:
> First patch is to do cleanup, remove redundant assignment,
> second patch is to fix memory leak for sctp_do_bind if failed
> to bind address.
> 
> Mao Wenan (2):
>   sctp: remove redundant assignment when call sctp_get_port_local
>   sctp: destroy bucket if failed to bind addr
> 
>  net/sctp/socket.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> -- 
> 2.20.1
> 
> 
Series
Acked-by: Neil Horman <nhorman@tuxdriver.com>


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

* Re: [PATCH net 0/2] fix memory leak for sctp_do_bind
@ 2019-09-10  7:16   ` Neil Horman
  0 siblings, 0 replies; 43+ messages in thread
From: Neil Horman @ 2019-09-10  7:16 UTC (permalink / raw)
  To: Mao Wenan
  Cc: vyasevich, marcelo.leitner, davem, linux-sctp, netdev,
	linux-kernel, kernel-janitors

On Tue, Sep 10, 2019 at 03:13:41PM +0800, Mao Wenan wrote:
> First patch is to do cleanup, remove redundant assignment,
> second patch is to fix memory leak for sctp_do_bind if failed
> to bind address.
> 
> Mao Wenan (2):
>   sctp: remove redundant assignment when call sctp_get_port_local
>   sctp: destroy bucket if failed to bind addr
> 
>  net/sctp/socket.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> -- 
> 2.20.1
> 
> 
Series
Acked-by: Neil Horman <nhorman@tuxdriver.com>

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

* Re: [PATCH net 1/2] sctp: remove redundant assignment when call sctp_get_port_local
  2019-09-10  6:56   ` Mao Wenan
@ 2019-09-10 18:57     ` Dan Carpenter
  -1 siblings, 0 replies; 43+ messages in thread
From: Dan Carpenter @ 2019-09-10 18:57 UTC (permalink / raw)
  To: Mao Wenan
  Cc: vyasevich, nhorman, marcelo.leitner, davem, linux-sctp, netdev,
	linux-kernel, kernel-janitors

On Tue, Sep 10, 2019 at 03:13:42PM +0800, Mao Wenan wrote:
> There are more parentheses in if clause when call sctp_get_port_local
> in sctp_do_bind, and redundant assignment to 'ret'. This patch is to
> do cleanup.
> 
> Signed-off-by: Mao Wenan <maowenan@huawei.com>
> ---
>  net/sctp/socket.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index 9d1f83b10c0a..766b68b55ebe 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -399,9 +399,8 @@ static int sctp_do_bind(struct sock *sk, union sctp_addr *addr, int len)
>  	 * detection.
>  	 */
>  	addr->v4.sin_port = htons(snum);
> -	if ((ret = sctp_get_port_local(sk, addr))) {
> +	if (sctp_get_port_local(sk, addr))
>  		return -EADDRINUSE;

sctp_get_port_local() returns a long which is either 0,1 or a pointer
casted to long.  It's not documented what it means and neither of the
callers use the return since commit 62208f12451f ("net: sctp: simplify
sctp_get_port").

Probably it should just return a bool?

regards,
dan carpenter


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

* Re: [PATCH net 1/2] sctp: remove redundant assignment when call sctp_get_port_local
@ 2019-09-10 18:57     ` Dan Carpenter
  0 siblings, 0 replies; 43+ messages in thread
From: Dan Carpenter @ 2019-09-10 18:57 UTC (permalink / raw)
  To: Mao Wenan
  Cc: vyasevich, nhorman, marcelo.leitner, davem, linux-sctp, netdev,
	linux-kernel, kernel-janitors

On Tue, Sep 10, 2019 at 03:13:42PM +0800, Mao Wenan wrote:
> There are more parentheses in if clause when call sctp_get_port_local
> in sctp_do_bind, and redundant assignment to 'ret'. This patch is to
> do cleanup.
> 
> Signed-off-by: Mao Wenan <maowenan@huawei.com>
> ---
>  net/sctp/socket.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index 9d1f83b10c0a..766b68b55ebe 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -399,9 +399,8 @@ static int sctp_do_bind(struct sock *sk, union sctp_addr *addr, int len)
>  	 * detection.
>  	 */
>  	addr->v4.sin_port = htons(snum);
> -	if ((ret = sctp_get_port_local(sk, addr))) {
> +	if (sctp_get_port_local(sk, addr))
>  		return -EADDRINUSE;

sctp_get_port_local() returns a long which is either 0,1 or a pointer
casted to long.  It's not documented what it means and neither of the
callers use the return since commit 62208f12451f ("net: sctp: simplify
sctp_get_port").

Probably it should just return a bool?

regards,
dan carpenter

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

* Re: [PATCH net 1/2] sctp: remove redundant assignment when call sctp_get_port_local
  2019-09-10 18:57     ` Dan Carpenter
@ 2019-09-10 19:22       ` Dan Carpenter
  -1 siblings, 0 replies; 43+ messages in thread
From: Dan Carpenter @ 2019-09-10 19:22 UTC (permalink / raw)
  To: Mao Wenan
  Cc: vyasevich, nhorman, marcelo.leitner, davem, linux-sctp, netdev,
	linux-kernel, kernel-janitors

On Tue, Sep 10, 2019 at 09:57:10PM +0300, Dan Carpenter wrote:
> On Tue, Sep 10, 2019 at 03:13:42PM +0800, Mao Wenan wrote:
> > There are more parentheses in if clause when call sctp_get_port_local
> > in sctp_do_bind, and redundant assignment to 'ret'. This patch is to
> > do cleanup.
> > 
> > Signed-off-by: Mao Wenan <maowenan@huawei.com>
> > ---
> >  net/sctp/socket.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> > 
> > diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> > index 9d1f83b10c0a..766b68b55ebe 100644
> > --- a/net/sctp/socket.c
> > +++ b/net/sctp/socket.c
> > @@ -399,9 +399,8 @@ static int sctp_do_bind(struct sock *sk, union sctp_addr *addr, int len)
> >  	 * detection.
> >  	 */
> >  	addr->v4.sin_port = htons(snum);
> > -	if ((ret = sctp_get_port_local(sk, addr))) {
> > +	if (sctp_get_port_local(sk, addr))
> >  		return -EADDRINUSE;
> 
> sctp_get_port_local() returns a long which is either 0,1 or a pointer
> casted to long.  It's not documented what it means and neither of the
> callers use the return since commit 62208f12451f ("net: sctp: simplify
> sctp_get_port").

Actually it was commit 4e54064e0a13 ("sctp: Allow only 1 listening
socket with SO_REUSEADDR") from 11 years ago.  That patch fixed a bug,
because before the code assumed that a pointer casted to an int was the
same as a pointer casted to a long.

regards,
dan carpenter


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

* Re: [PATCH net 1/2] sctp: remove redundant assignment when call sctp_get_port_local
@ 2019-09-10 19:22       ` Dan Carpenter
  0 siblings, 0 replies; 43+ messages in thread
From: Dan Carpenter @ 2019-09-10 19:22 UTC (permalink / raw)
  To: Mao Wenan
  Cc: vyasevich, nhorman, marcelo.leitner, davem, linux-sctp, netdev,
	linux-kernel, kernel-janitors

On Tue, Sep 10, 2019 at 09:57:10PM +0300, Dan Carpenter wrote:
> On Tue, Sep 10, 2019 at 03:13:42PM +0800, Mao Wenan wrote:
> > There are more parentheses in if clause when call sctp_get_port_local
> > in sctp_do_bind, and redundant assignment to 'ret'. This patch is to
> > do cleanup.
> > 
> > Signed-off-by: Mao Wenan <maowenan@huawei.com>
> > ---
> >  net/sctp/socket.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> > 
> > diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> > index 9d1f83b10c0a..766b68b55ebe 100644
> > --- a/net/sctp/socket.c
> > +++ b/net/sctp/socket.c
> > @@ -399,9 +399,8 @@ static int sctp_do_bind(struct sock *sk, union sctp_addr *addr, int len)
> >  	 * detection.
> >  	 */
> >  	addr->v4.sin_port = htons(snum);
> > -	if ((ret = sctp_get_port_local(sk, addr))) {
> > +	if (sctp_get_port_local(sk, addr))
> >  		return -EADDRINUSE;
> 
> sctp_get_port_local() returns a long which is either 0,1 or a pointer
> casted to long.  It's not documented what it means and neither of the
> callers use the return since commit 62208f12451f ("net: sctp: simplify
> sctp_get_port").

Actually it was commit 4e54064e0a13 ("sctp: Allow only 1 listening
socket with SO_REUSEADDR") from 11 years ago.  That patch fixed a bug,
because before the code assumed that a pointer casted to an int was the
same as a pointer casted to a long.

regards,
dan carpenter

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

* Re: [PATCH net 1/2] sctp: remove redundant assignment when call sctp_get_port_local
  2019-09-10 19:22       ` Dan Carpenter
@ 2019-09-11  1:30         ` maowenan
  -1 siblings, 0 replies; 43+ messages in thread
From: maowenan @ 2019-09-11  1:30 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: vyasevich, nhorman, marcelo.leitner, davem, linux-sctp, netdev,
	linux-kernel, kernel-janitors



On 2019/9/11 3:22, Dan Carpenter wrote:
> On Tue, Sep 10, 2019 at 09:57:10PM +0300, Dan Carpenter wrote:
>> On Tue, Sep 10, 2019 at 03:13:42PM +0800, Mao Wenan wrote:
>>> There are more parentheses in if clause when call sctp_get_port_local
>>> in sctp_do_bind, and redundant assignment to 'ret'. This patch is to
>>> do cleanup.
>>>
>>> Signed-off-by: Mao Wenan <maowenan@huawei.com>
>>> ---
>>>  net/sctp/socket.c | 3 +--
>>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>>
>>> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
>>> index 9d1f83b10c0a..766b68b55ebe 100644
>>> --- a/net/sctp/socket.c
>>> +++ b/net/sctp/socket.c
>>> @@ -399,9 +399,8 @@ static int sctp_do_bind(struct sock *sk, union sctp_addr *addr, int len)
>>>  	 * detection.
>>>  	 */
>>>  	addr->v4.sin_port = htons(snum);
>>> -	if ((ret = sctp_get_port_local(sk, addr))) {
>>> +	if (sctp_get_port_local(sk, addr))
>>>  		return -EADDRINUSE;
>>
>> sctp_get_port_local() returns a long which is either 0,1 or a pointer
>> casted to long.  It's not documented what it means and neither of the
>> callers use the return since commit 62208f12451f ("net: sctp: simplify
>> sctp_get_port").
> 
> Actually it was commit 4e54064e0a13 ("sctp: Allow only 1 listening
> socket with SO_REUSEADDR") from 11 years ago.  That patch fixed a bug,
> because before the code assumed that a pointer casted to an int was the
> same as a pointer casted to a long.

commit 4e54064e0a13 treated non-zero return value as unexpected, so the current
cleanup is ok?

> 
> regards,
> dan carpenter
> 
> 
> .
> 


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

* Re: [PATCH net 1/2] sctp: remove redundant assignment when call sctp_get_port_local
@ 2019-09-11  1:30         ` maowenan
  0 siblings, 0 replies; 43+ messages in thread
From: maowenan @ 2019-09-11  1:30 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: vyasevich, nhorman, marcelo.leitner, davem, linux-sctp, netdev,
	linux-kernel, kernel-janitors



On 2019/9/11 3:22, Dan Carpenter wrote:
> On Tue, Sep 10, 2019 at 09:57:10PM +0300, Dan Carpenter wrote:
>> On Tue, Sep 10, 2019 at 03:13:42PM +0800, Mao Wenan wrote:
>>> There are more parentheses in if clause when call sctp_get_port_local
>>> in sctp_do_bind, and redundant assignment to 'ret'. This patch is to
>>> do cleanup.
>>>
>>> Signed-off-by: Mao Wenan <maowenan@huawei.com>
>>> ---
>>>  net/sctp/socket.c | 3 +--
>>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>>
>>> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
>>> index 9d1f83b10c0a..766b68b55ebe 100644
>>> --- a/net/sctp/socket.c
>>> +++ b/net/sctp/socket.c
>>> @@ -399,9 +399,8 @@ static int sctp_do_bind(struct sock *sk, union sctp_addr *addr, int len)
>>>  	 * detection.
>>>  	 */
>>>  	addr->v4.sin_port = htons(snum);
>>> -	if ((ret = sctp_get_port_local(sk, addr))) {
>>> +	if (sctp_get_port_local(sk, addr))
>>>  		return -EADDRINUSE;
>>
>> sctp_get_port_local() returns a long which is either 0,1 or a pointer
>> casted to long.  It's not documented what it means and neither of the
>> callers use the return since commit 62208f12451f ("net: sctp: simplify
>> sctp_get_port").
> 
> Actually it was commit 4e54064e0a13 ("sctp: Allow only 1 listening
> socket with SO_REUSEADDR") from 11 years ago.  That patch fixed a bug,
> because before the code assumed that a pointer casted to an int was the
> same as a pointer casted to a long.

commit 4e54064e0a13 treated non-zero return value as unexpected, so the current
cleanup is ok?

> 
> regards,
> dan carpenter
> 
> 
> .
> 

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

* Re: [PATCH net 1/2] sctp: remove redundant assignment when call sctp_get_port_local
  2019-09-11  1:30         ` maowenan
@ 2019-09-11  8:30           ` Dan Carpenter
  -1 siblings, 0 replies; 43+ messages in thread
From: Dan Carpenter @ 2019-09-11  8:30 UTC (permalink / raw)
  To: maowenan
  Cc: vyasevich, nhorman, marcelo.leitner, davem, linux-sctp, netdev,
	linux-kernel, kernel-janitors

On Wed, Sep 11, 2019 at 09:30:47AM +0800, maowenan wrote:
> 
> 
> On 2019/9/11 3:22, Dan Carpenter wrote:
> > On Tue, Sep 10, 2019 at 09:57:10PM +0300, Dan Carpenter wrote:
> >> On Tue, Sep 10, 2019 at 03:13:42PM +0800, Mao Wenan wrote:
> >>> There are more parentheses in if clause when call sctp_get_port_local
> >>> in sctp_do_bind, and redundant assignment to 'ret'. This patch is to
> >>> do cleanup.
> >>>
> >>> Signed-off-by: Mao Wenan <maowenan@huawei.com>
> >>> ---
> >>>  net/sctp/socket.c | 3 +--
> >>>  1 file changed, 1 insertion(+), 2 deletions(-)
> >>>
> >>> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> >>> index 9d1f83b10c0a..766b68b55ebe 100644
> >>> --- a/net/sctp/socket.c
> >>> +++ b/net/sctp/socket.c
> >>> @@ -399,9 +399,8 @@ static int sctp_do_bind(struct sock *sk, union sctp_addr *addr, int len)
> >>>  	 * detection.
> >>>  	 */
> >>>  	addr->v4.sin_port = htons(snum);
> >>> -	if ((ret = sctp_get_port_local(sk, addr))) {
> >>> +	if (sctp_get_port_local(sk, addr))
> >>>  		return -EADDRINUSE;
> >>
> >> sctp_get_port_local() returns a long which is either 0,1 or a pointer
> >> casted to long.  It's not documented what it means and neither of the
> >> callers use the return since commit 62208f12451f ("net: sctp: simplify
> >> sctp_get_port").
> > 
> > Actually it was commit 4e54064e0a13 ("sctp: Allow only 1 listening
> > socket with SO_REUSEADDR") from 11 years ago.  That patch fixed a bug,
> > because before the code assumed that a pointer casted to an int was the
> > same as a pointer casted to a long.
> 
> commit 4e54064e0a13 treated non-zero return value as unexpected, so the current
> cleanup is ok?

Yeah.  It's fine, I was just confused why we weren't preserving the
error code and then I saw that we didn't return errors at all and got
confused.

regards,
dan carpenter


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

* Re: [PATCH net 1/2] sctp: remove redundant assignment when call sctp_get_port_local
@ 2019-09-11  8:30           ` Dan Carpenter
  0 siblings, 0 replies; 43+ messages in thread
From: Dan Carpenter @ 2019-09-11  8:30 UTC (permalink / raw)
  To: maowenan
  Cc: vyasevich, nhorman, marcelo.leitner, davem, linux-sctp, netdev,
	linux-kernel, kernel-janitors

On Wed, Sep 11, 2019 at 09:30:47AM +0800, maowenan wrote:
> 
> 
> On 2019/9/11 3:22, Dan Carpenter wrote:
> > On Tue, Sep 10, 2019 at 09:57:10PM +0300, Dan Carpenter wrote:
> >> On Tue, Sep 10, 2019 at 03:13:42PM +0800, Mao Wenan wrote:
> >>> There are more parentheses in if clause when call sctp_get_port_local
> >>> in sctp_do_bind, and redundant assignment to 'ret'. This patch is to
> >>> do cleanup.
> >>>
> >>> Signed-off-by: Mao Wenan <maowenan@huawei.com>
> >>> ---
> >>>  net/sctp/socket.c | 3 +--
> >>>  1 file changed, 1 insertion(+), 2 deletions(-)
> >>>
> >>> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> >>> index 9d1f83b10c0a..766b68b55ebe 100644
> >>> --- a/net/sctp/socket.c
> >>> +++ b/net/sctp/socket.c
> >>> @@ -399,9 +399,8 @@ static int sctp_do_bind(struct sock *sk, union sctp_addr *addr, int len)
> >>>  	 * detection.
> >>>  	 */
> >>>  	addr->v4.sin_port = htons(snum);
> >>> -	if ((ret = sctp_get_port_local(sk, addr))) {
> >>> +	if (sctp_get_port_local(sk, addr))
> >>>  		return -EADDRINUSE;
> >>
> >> sctp_get_port_local() returns a long which is either 0,1 or a pointer
> >> casted to long.  It's not documented what it means and neither of the
> >> callers use the return since commit 62208f12451f ("net: sctp: simplify
> >> sctp_get_port").
> > 
> > Actually it was commit 4e54064e0a13 ("sctp: Allow only 1 listening
> > socket with SO_REUSEADDR") from 11 years ago.  That patch fixed a bug,
> > because before the code assumed that a pointer casted to an int was the
> > same as a pointer casted to a long.
> 
> commit 4e54064e0a13 treated non-zero return value as unexpected, so the current
> cleanup is ok?

Yeah.  It's fine, I was just confused why we weren't preserving the
error code and then I saw that we didn't return errors at all and got
confused.

regards,
dan carpenter

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

* Re: [PATCH net 1/2] sctp: remove redundant assignment when call sctp_get_port_local
  2019-09-11  8:30           ` Dan Carpenter
@ 2019-09-11 14:30             ` Marcelo Ricardo Leitner
  -1 siblings, 0 replies; 43+ messages in thread
From: Marcelo Ricardo Leitner @ 2019-09-11 14:30 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: maowenan, vyasevich, nhorman, davem, linux-sctp, netdev,
	linux-kernel, kernel-janitors

On Wed, Sep 11, 2019 at 11:30:38AM +0300, Dan Carpenter wrote:
> On Wed, Sep 11, 2019 at 09:30:47AM +0800, maowenan wrote:
> > 
> > 
> > On 2019/9/11 3:22, Dan Carpenter wrote:
> > > On Tue, Sep 10, 2019 at 09:57:10PM +0300, Dan Carpenter wrote:
> > >> On Tue, Sep 10, 2019 at 03:13:42PM +0800, Mao Wenan wrote:
> > >>> There are more parentheses in if clause when call sctp_get_port_local
> > >>> in sctp_do_bind, and redundant assignment to 'ret'. This patch is to
> > >>> do cleanup.
> > >>>
> > >>> Signed-off-by: Mao Wenan <maowenan@huawei.com>
> > >>> ---
> > >>>  net/sctp/socket.c | 3 +--
> > >>>  1 file changed, 1 insertion(+), 2 deletions(-)
> > >>>
> > >>> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> > >>> index 9d1f83b10c0a..766b68b55ebe 100644
> > >>> --- a/net/sctp/socket.c
> > >>> +++ b/net/sctp/socket.c
> > >>> @@ -399,9 +399,8 @@ static int sctp_do_bind(struct sock *sk, union sctp_addr *addr, int len)
> > >>>  	 * detection.
> > >>>  	 */
> > >>>  	addr->v4.sin_port = htons(snum);
> > >>> -	if ((ret = sctp_get_port_local(sk, addr))) {
> > >>> +	if (sctp_get_port_local(sk, addr))
> > >>>  		return -EADDRINUSE;
> > >>
> > >> sctp_get_port_local() returns a long which is either 0,1 or a pointer
> > >> casted to long.  It's not documented what it means and neither of the
> > >> callers use the return since commit 62208f12451f ("net: sctp: simplify
> > >> sctp_get_port").
> > > 
> > > Actually it was commit 4e54064e0a13 ("sctp: Allow only 1 listening
> > > socket with SO_REUSEADDR") from 11 years ago.  That patch fixed a bug,
> > > because before the code assumed that a pointer casted to an int was the
> > > same as a pointer casted to a long.
> > 
> > commit 4e54064e0a13 treated non-zero return value as unexpected, so the current
> > cleanup is ok?
> 
> Yeah.  It's fine, I was just confused why we weren't preserving the
> error code and then I saw that we didn't return errors at all and got
> confused.

But please lets seize the moment and do the change Dean suggested.
This was the last place saving this return value somewhere. It makes
sense to cleanup sctp_get_port_local() now and remove that masked
pointer return.

Then you may also cleanup:
socket.c:       return !!sctp_get_port_local(sk, &addr);
as it will be a direct map.

  Marcelo

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

* Re: [PATCH net 1/2] sctp: remove redundant assignment when call sctp_get_port_local
@ 2019-09-11 14:30             ` Marcelo Ricardo Leitner
  0 siblings, 0 replies; 43+ messages in thread
From: Marcelo Ricardo Leitner @ 2019-09-11 14:30 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: maowenan, vyasevich, nhorman, davem, linux-sctp, netdev,
	linux-kernel, kernel-janitors

On Wed, Sep 11, 2019 at 11:30:38AM +0300, Dan Carpenter wrote:
> On Wed, Sep 11, 2019 at 09:30:47AM +0800, maowenan wrote:
> > 
> > 
> > On 2019/9/11 3:22, Dan Carpenter wrote:
> > > On Tue, Sep 10, 2019 at 09:57:10PM +0300, Dan Carpenter wrote:
> > >> On Tue, Sep 10, 2019 at 03:13:42PM +0800, Mao Wenan wrote:
> > >>> There are more parentheses in if clause when call sctp_get_port_local
> > >>> in sctp_do_bind, and redundant assignment to 'ret'. This patch is to
> > >>> do cleanup.
> > >>>
> > >>> Signed-off-by: Mao Wenan <maowenan@huawei.com>
> > >>> ---
> > >>>  net/sctp/socket.c | 3 +--
> > >>>  1 file changed, 1 insertion(+), 2 deletions(-)
> > >>>
> > >>> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> > >>> index 9d1f83b10c0a..766b68b55ebe 100644
> > >>> --- a/net/sctp/socket.c
> > >>> +++ b/net/sctp/socket.c
> > >>> @@ -399,9 +399,8 @@ static int sctp_do_bind(struct sock *sk, union sctp_addr *addr, int len)
> > >>>  	 * detection.
> > >>>  	 */
> > >>>  	addr->v4.sin_port = htons(snum);
> > >>> -	if ((ret = sctp_get_port_local(sk, addr))) {
> > >>> +	if (sctp_get_port_local(sk, addr))
> > >>>  		return -EADDRINUSE;
> > >>
> > >> sctp_get_port_local() returns a long which is either 0,1 or a pointer
> > >> casted to long.  It's not documented what it means and neither of the
> > >> callers use the return since commit 62208f12451f ("net: sctp: simplify
> > >> sctp_get_port").
> > > 
> > > Actually it was commit 4e54064e0a13 ("sctp: Allow only 1 listening
> > > socket with SO_REUSEADDR") from 11 years ago.  That patch fixed a bug,
> > > because before the code assumed that a pointer casted to an int was the
> > > same as a pointer casted to a long.
> > 
> > commit 4e54064e0a13 treated non-zero return value as unexpected, so the current
> > cleanup is ok?
> 
> Yeah.  It's fine, I was just confused why we weren't preserving the
> error code and then I saw that we didn't return errors at all and got
> confused.

But please lets seize the moment and do the change Dean suggested.
This was the last place saving this return value somewhere. It makes
sense to cleanup sctp_get_port_local() now and remove that masked
pointer return.

Then you may also cleanup:
socket.c:       return !!sctp_get_port_local(sk, &addr);
as it will be a direct map.

  Marcelo

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

* Re: [PATCH net 1/2] sctp: remove redundant assignment when call sctp_get_port_local
  2019-09-11 14:30             ` Marcelo Ricardo Leitner
@ 2019-09-11 14:39               ` Marcelo Ricardo Leitner
  -1 siblings, 0 replies; 43+ messages in thread
From: Marcelo Ricardo Leitner @ 2019-09-11 14:39 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: maowenan, vyasevich, nhorman, davem, linux-sctp, netdev,
	linux-kernel, kernel-janitors

On Wed, Sep 11, 2019 at 11:30:08AM -0300, Marcelo Ricardo Leitner wrote:
> On Wed, Sep 11, 2019 at 11:30:38AM +0300, Dan Carpenter wrote:
> > On Wed, Sep 11, 2019 at 09:30:47AM +0800, maowenan wrote:
> > > 
> > > 
> > > On 2019/9/11 3:22, Dan Carpenter wrote:
> > > > On Tue, Sep 10, 2019 at 09:57:10PM +0300, Dan Carpenter wrote:
> > > >> On Tue, Sep 10, 2019 at 03:13:42PM +0800, Mao Wenan wrote:
> > > >>> There are more parentheses in if clause when call sctp_get_port_local
> > > >>> in sctp_do_bind, and redundant assignment to 'ret'. This patch is to
> > > >>> do cleanup.
> > > >>>
> > > >>> Signed-off-by: Mao Wenan <maowenan@huawei.com>
> > > >>> ---
> > > >>>  net/sctp/socket.c | 3 +--
> > > >>>  1 file changed, 1 insertion(+), 2 deletions(-)
> > > >>>
> > > >>> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> > > >>> index 9d1f83b10c0a..766b68b55ebe 100644
> > > >>> --- a/net/sctp/socket.c
> > > >>> +++ b/net/sctp/socket.c
> > > >>> @@ -399,9 +399,8 @@ static int sctp_do_bind(struct sock *sk, union sctp_addr *addr, int len)
> > > >>>  	 * detection.
> > > >>>  	 */
> > > >>>  	addr->v4.sin_port = htons(snum);
> > > >>> -	if ((ret = sctp_get_port_local(sk, addr))) {
> > > >>> +	if (sctp_get_port_local(sk, addr))
> > > >>>  		return -EADDRINUSE;
> > > >>
> > > >> sctp_get_port_local() returns a long which is either 0,1 or a pointer
> > > >> casted to long.  It's not documented what it means and neither of the
> > > >> callers use the return since commit 62208f12451f ("net: sctp: simplify
> > > >> sctp_get_port").
> > > > 
> > > > Actually it was commit 4e54064e0a13 ("sctp: Allow only 1 listening
> > > > socket with SO_REUSEADDR") from 11 years ago.  That patch fixed a bug,
> > > > because before the code assumed that a pointer casted to an int was the
> > > > same as a pointer casted to a long.
> > > 
> > > commit 4e54064e0a13 treated non-zero return value as unexpected, so the current
> > > cleanup is ok?
> > 
> > Yeah.  It's fine, I was just confused why we weren't preserving the
> > error code and then I saw that we didn't return errors at all and got
> > confused.
> 
> But please lets seize the moment and do the change Dean suggested.

*Dan*, sorry.

> This was the last place saving this return value somewhere. It makes
> sense to cleanup sctp_get_port_local() now and remove that masked
> pointer return.
> 
> Then you may also cleanup:
> socket.c:       return !!sctp_get_port_local(sk, &addr);
> as it will be a direct map.
> 
>   Marcelo
> 

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

* Re: [PATCH net 1/2] sctp: remove redundant assignment when call sctp_get_port_local
@ 2019-09-11 14:39               ` Marcelo Ricardo Leitner
  0 siblings, 0 replies; 43+ messages in thread
From: Marcelo Ricardo Leitner @ 2019-09-11 14:39 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: maowenan, vyasevich, nhorman, davem, linux-sctp, netdev,
	linux-kernel, kernel-janitors

On Wed, Sep 11, 2019 at 11:30:08AM -0300, Marcelo Ricardo Leitner wrote:
> On Wed, Sep 11, 2019 at 11:30:38AM +0300, Dan Carpenter wrote:
> > On Wed, Sep 11, 2019 at 09:30:47AM +0800, maowenan wrote:
> > > 
> > > 
> > > On 2019/9/11 3:22, Dan Carpenter wrote:
> > > > On Tue, Sep 10, 2019 at 09:57:10PM +0300, Dan Carpenter wrote:
> > > >> On Tue, Sep 10, 2019 at 03:13:42PM +0800, Mao Wenan wrote:
> > > >>> There are more parentheses in if clause when call sctp_get_port_local
> > > >>> in sctp_do_bind, and redundant assignment to 'ret'. This patch is to
> > > >>> do cleanup.
> > > >>>
> > > >>> Signed-off-by: Mao Wenan <maowenan@huawei.com>
> > > >>> ---
> > > >>>  net/sctp/socket.c | 3 +--
> > > >>>  1 file changed, 1 insertion(+), 2 deletions(-)
> > > >>>
> > > >>> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> > > >>> index 9d1f83b10c0a..766b68b55ebe 100644
> > > >>> --- a/net/sctp/socket.c
> > > >>> +++ b/net/sctp/socket.c
> > > >>> @@ -399,9 +399,8 @@ static int sctp_do_bind(struct sock *sk, union sctp_addr *addr, int len)
> > > >>>  	 * detection.
> > > >>>  	 */
> > > >>>  	addr->v4.sin_port = htons(snum);
> > > >>> -	if ((ret = sctp_get_port_local(sk, addr))) {
> > > >>> +	if (sctp_get_port_local(sk, addr))
> > > >>>  		return -EADDRINUSE;
> > > >>
> > > >> sctp_get_port_local() returns a long which is either 0,1 or a pointer
> > > >> casted to long.  It's not documented what it means and neither of the
> > > >> callers use the return since commit 62208f12451f ("net: sctp: simplify
> > > >> sctp_get_port").
> > > > 
> > > > Actually it was commit 4e54064e0a13 ("sctp: Allow only 1 listening
> > > > socket with SO_REUSEADDR") from 11 years ago.  That patch fixed a bug,
> > > > because before the code assumed that a pointer casted to an int was the
> > > > same as a pointer casted to a long.
> > > 
> > > commit 4e54064e0a13 treated non-zero return value as unexpected, so the current
> > > cleanup is ok?
> > 
> > Yeah.  It's fine, I was just confused why we weren't preserving the
> > error code and then I saw that we didn't return errors at all and got
> > confused.
> 
> But please lets seize the moment and do the change Dean suggested.

*Dan*, sorry.

> This was the last place saving this return value somewhere. It makes
> sense to cleanup sctp_get_port_local() now and remove that masked
> pointer return.
> 
> Then you may also cleanup:
> socket.c:       return !!sctp_get_port_local(sk, &addr);
> as it will be a direct map.
> 
>   Marcelo
> 

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

* Re: [PATCH net 1/2] sctp: remove redundant assignment when call sctp_get_port_local
  2019-09-11 14:39               ` Marcelo Ricardo Leitner
@ 2019-09-12  2:05                 ` maowenan
  -1 siblings, 0 replies; 43+ messages in thread
From: maowenan @ 2019-09-12  2:05 UTC (permalink / raw)
  To: Marcelo Ricardo Leitner, Dan Carpenter
  Cc: vyasevich, nhorman, davem, linux-sctp, netdev, linux-kernel,
	kernel-janitors



On 2019/9/11 22:39, Marcelo Ricardo Leitner wrote:
> On Wed, Sep 11, 2019 at 11:30:08AM -0300, Marcelo Ricardo Leitner wrote:
>> On Wed, Sep 11, 2019 at 11:30:38AM +0300, Dan Carpenter wrote:
>>> On Wed, Sep 11, 2019 at 09:30:47AM +0800, maowenan wrote:
>>>>
>>>>
>>>> On 2019/9/11 3:22, Dan Carpenter wrote:
>>>>> On Tue, Sep 10, 2019 at 09:57:10PM +0300, Dan Carpenter wrote:
>>>>>> On Tue, Sep 10, 2019 at 03:13:42PM +0800, Mao Wenan wrote:
>>>>>>> There are more parentheses in if clause when call sctp_get_port_local
>>>>>>> in sctp_do_bind, and redundant assignment to 'ret'. This patch is to
>>>>>>> do cleanup.
>>>>>>>
>>>>>>> Signed-off-by: Mao Wenan <maowenan@huawei.com>
>>>>>>> ---
>>>>>>>  net/sctp/socket.c | 3 +--
>>>>>>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>>>>>>
>>>>>>> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
>>>>>>> index 9d1f83b10c0a..766b68b55ebe 100644
>>>>>>> --- a/net/sctp/socket.c
>>>>>>> +++ b/net/sctp/socket.c
>>>>>>> @@ -399,9 +399,8 @@ static int sctp_do_bind(struct sock *sk, union sctp_addr *addr, int len)
>>>>>>>  	 * detection.
>>>>>>>  	 */
>>>>>>>  	addr->v4.sin_port = htons(snum);
>>>>>>> -	if ((ret = sctp_get_port_local(sk, addr))) {
>>>>>>> +	if (sctp_get_port_local(sk, addr))
>>>>>>>  		return -EADDRINUSE;
>>>>>>
>>>>>> sctp_get_port_local() returns a long which is either 0,1 or a pointer
>>>>>> casted to long.  It's not documented what it means and neither of the
>>>>>> callers use the return since commit 62208f12451f ("net: sctp: simplify
>>>>>> sctp_get_port").
>>>>>
>>>>> Actually it was commit 4e54064e0a13 ("sctp: Allow only 1 listening
>>>>> socket with SO_REUSEADDR") from 11 years ago.  That patch fixed a bug,
>>>>> because before the code assumed that a pointer casted to an int was the
>>>>> same as a pointer casted to a long.
>>>>
>>>> commit 4e54064e0a13 treated non-zero return value as unexpected, so the current
>>>> cleanup is ok?
>>>
>>> Yeah.  It's fine, I was just confused why we weren't preserving the
>>> error code and then I saw that we didn't return errors at all and got
>>> confused.
>>
>> But please lets seize the moment and do the change Dean suggested.
> 
> *Dan*, sorry.
> 
>> This was the last place saving this return value somewhere. It makes
>> sense to cleanup sctp_get_port_local() now and remove that masked
>> pointer return.
>>
>> Then you may also cleanup:
>> socket.c:       return !!sctp_get_port_local(sk, &addr);
>> as it will be a direct map.

Thanks Marcelo, shall I post a new individual patch for cleanup as your suggest?
>>
>>   Marcelo
>>
> 
> .
> 


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

* Re: [PATCH net 1/2] sctp: remove redundant assignment when call sctp_get_port_local
@ 2019-09-12  2:05                 ` maowenan
  0 siblings, 0 replies; 43+ messages in thread
From: maowenan @ 2019-09-12  2:05 UTC (permalink / raw)
  To: Marcelo Ricardo Leitner, Dan Carpenter
  Cc: vyasevich, nhorman, davem, linux-sctp, netdev, linux-kernel,
	kernel-janitors



On 2019/9/11 22:39, Marcelo Ricardo Leitner wrote:
> On Wed, Sep 11, 2019 at 11:30:08AM -0300, Marcelo Ricardo Leitner wrote:
>> On Wed, Sep 11, 2019 at 11:30:38AM +0300, Dan Carpenter wrote:
>>> On Wed, Sep 11, 2019 at 09:30:47AM +0800, maowenan wrote:
>>>>
>>>>
>>>> On 2019/9/11 3:22, Dan Carpenter wrote:
>>>>> On Tue, Sep 10, 2019 at 09:57:10PM +0300, Dan Carpenter wrote:
>>>>>> On Tue, Sep 10, 2019 at 03:13:42PM +0800, Mao Wenan wrote:
>>>>>>> There are more parentheses in if clause when call sctp_get_port_local
>>>>>>> in sctp_do_bind, and redundant assignment to 'ret'. This patch is to
>>>>>>> do cleanup.
>>>>>>>
>>>>>>> Signed-off-by: Mao Wenan <maowenan@huawei.com>
>>>>>>> ---
>>>>>>>  net/sctp/socket.c | 3 +--
>>>>>>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>>>>>>
>>>>>>> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
>>>>>>> index 9d1f83b10c0a..766b68b55ebe 100644
>>>>>>> --- a/net/sctp/socket.c
>>>>>>> +++ b/net/sctp/socket.c
>>>>>>> @@ -399,9 +399,8 @@ static int sctp_do_bind(struct sock *sk, union sctp_addr *addr, int len)
>>>>>>>  	 * detection.
>>>>>>>  	 */
>>>>>>>  	addr->v4.sin_port = htons(snum);
>>>>>>> -	if ((ret = sctp_get_port_local(sk, addr))) {
>>>>>>> +	if (sctp_get_port_local(sk, addr))
>>>>>>>  		return -EADDRINUSE;
>>>>>>
>>>>>> sctp_get_port_local() returns a long which is either 0,1 or a pointer
>>>>>> casted to long.  It's not documented what it means and neither of the
>>>>>> callers use the return since commit 62208f12451f ("net: sctp: simplify
>>>>>> sctp_get_port").
>>>>>
>>>>> Actually it was commit 4e54064e0a13 ("sctp: Allow only 1 listening
>>>>> socket with SO_REUSEADDR") from 11 years ago.  That patch fixed a bug,
>>>>> because before the code assumed that a pointer casted to an int was the
>>>>> same as a pointer casted to a long.
>>>>
>>>> commit 4e54064e0a13 treated non-zero return value as unexpected, so the current
>>>> cleanup is ok?
>>>
>>> Yeah.  It's fine, I was just confused why we weren't preserving the
>>> error code and then I saw that we didn't return errors at all and got
>>> confused.
>>
>> But please lets seize the moment and do the change Dean suggested.
> 
> *Dan*, sorry.
> 
>> This was the last place saving this return value somewhere. It makes
>> sense to cleanup sctp_get_port_local() now and remove that masked
>> pointer return.
>>
>> Then you may also cleanup:
>> socket.c:       return !!sctp_get_port_local(sk, &addr);
>> as it will be a direct map.

Thanks Marcelo, shall I post a new individual patch for cleanup as your suggest?
>>
>>   Marcelo
>>
> 
> .
> 

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

* [PATCH v2 net 1/3] sctp: change return type of sctp_get_port_local
  2019-09-12  4:02                   ` Mao Wenan
  (?)
@ 2019-09-12  3:45                     ` Mao Wenan
  -1 siblings, 0 replies; 43+ messages in thread
From: Mao Wenan @ 2019-09-12  3:45 UTC (permalink / raw)
  To: vyasevich, nhorman, marcelo.leitner, davem
  Cc: linux-sctp, netdev, linux-kernel, kernel-janitors, Mao Wenan

Currently sctp_get_port_local() returns a long
which is either 0,1 or a pointer casted to long.
It's neither of the callers use the return value since
commit 62208f12451f ("net: sctp: simplify sctp_get_port").
Now two callers are sctp_get_port and sctp_do_bind,
they actually assumend a casted to an int was the same as
a pointer casted to a long, and they don't save the return
value just check whether it is zero or non-zero, so
it would better change return type from long to int for
sctp_get_port_local.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Mao Wenan <maowenan@huawei.com>
---
 net/sctp/socket.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 9d1f83b10c0a..5e1934c48709 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -309,7 +309,7 @@ static int sctp_bind(struct sock *sk, struct sockaddr *addr, int addr_len)
 	return retval;
 }
 
-static long sctp_get_port_local(struct sock *, union sctp_addr *);
+static int sctp_get_port_local(struct sock *, union sctp_addr *);
 
 /* Verify this is a valid sockaddr. */
 static struct sctp_af *sctp_sockaddr_af(struct sctp_sock *opt,
@@ -7998,7 +7998,7 @@ static void sctp_unhash(struct sock *sk)
 static struct sctp_bind_bucket *sctp_bucket_create(
 	struct sctp_bind_hashbucket *head, struct net *, unsigned short snum);
 
-static long sctp_get_port_local(struct sock *sk, union sctp_addr *addr)
+static int sctp_get_port_local(struct sock *sk, union sctp_addr *addr)
 {
 	struct sctp_sock *sp = sctp_sk(sk);
 	bool reuse = (sk->sk_reuse || sp->reuse);
@@ -8108,7 +8108,7 @@ static long sctp_get_port_local(struct sock *sk, union sctp_addr *addr)
 
 			if (sctp_bind_addr_conflict(&ep2->base.bind_addr,
 						    addr, sp2, sp)) {
-				ret = (long)sk2;
+				ret = 1;
 				goto fail_unlock;
 			}
 		}
@@ -8180,7 +8180,7 @@ static int sctp_get_port(struct sock *sk, unsigned short snum)
 	addr.v4.sin_port = htons(snum);
 
 	/* Note: sk->sk_num gets filled in if ephemeral port request. */
-	return !!sctp_get_port_local(sk, &addr);
+	return sctp_get_port_local(sk, &addr);
 }
 
 /*
-- 
2.20.1

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

* [PATCH v2 net 2/3] sctp: remove redundant assignment when call sctp_get_port_local
  2019-09-12  4:02                   ` Mao Wenan
  (?)
@ 2019-09-12  3:45                     ` Mao Wenan
  -1 siblings, 0 replies; 43+ messages in thread
From: Mao Wenan @ 2019-09-12  3:45 UTC (permalink / raw)
  To: vyasevich, nhorman, marcelo.leitner, davem
  Cc: linux-sctp, netdev, linux-kernel, kernel-janitors, Mao Wenan

There are more parentheses in if clause when call sctp_get_port_local
in sctp_do_bind, and redundant assignment to 'ret'. This patch is to
do cleanup.

Signed-off-by: Mao Wenan <maowenan@huawei.com>
Acked-by: Neil Horman <nhorman@tuxdriver.com>
---
 net/sctp/socket.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 5e1934c48709..2f810078c91d 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -399,9 +399,8 @@ static int sctp_do_bind(struct sock *sk, union sctp_addr *addr, int len)
 	 * detection.
 	 */
 	addr->v4.sin_port = htons(snum);
-	if ((ret = sctp_get_port_local(sk, addr))) {
+	if (sctp_get_port_local(sk, addr))
 		return -EADDRINUSE;
-	}
 
 	/* Refresh ephemeral port.  */
 	if (!bp->port)
-- 
2.20.1

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

* [PATCH v2 net 3/3] sctp: destroy bucket if failed to bind addr
  2019-09-12  4:02                   ` Mao Wenan
  (?)
@ 2019-09-12  3:45                     ` Mao Wenan
  -1 siblings, 0 replies; 43+ messages in thread
From: Mao Wenan @ 2019-09-12  3:45 UTC (permalink / raw)
  To: vyasevich, nhorman, marcelo.leitner, davem
  Cc: linux-sctp, netdev, linux-kernel, kernel-janitors, Mao Wenan, Hulk Robot

There is one memory leak bug report:
BUG: memory leak
unreferenced object 0xffff8881dc4c5ec0 (size 40):
  comm "syz-executor.0", pid 5673, jiffies 4298198457 (age 27.578s)
  hex dump (first 32 bytes):
    02 00 00 00 81 88 ff ff 00 00 00 00 00 00 00 00  ................
    f8 63 3d c1 81 88 ff ff 00 00 00 00 00 00 00 00  .c=.............
  backtrace:
    [<0000000072006339>] sctp_get_port_local+0x2a1/0xa00 [sctp]
    [<00000000c7b379ec>] sctp_do_bind+0x176/0x2c0 [sctp]
    [<000000005be274a2>] sctp_bind+0x5a/0x80 [sctp]
    [<00000000b66b4044>] inet6_bind+0x59/0xd0 [ipv6]
    [<00000000c68c7f42>] __sys_bind+0x120/0x1f0 net/socket.c:1647
    [<000000004513635b>] __do_sys_bind net/socket.c:1658 [inline]
    [<000000004513635b>] __se_sys_bind net/socket.c:1656 [inline]
    [<000000004513635b>] __x64_sys_bind+0x3e/0x50 net/socket.c:1656
    [<0000000061f2501e>] do_syscall_64+0x72/0x2e0 arch/x86/entry/common.c:296
    [<0000000003d1e05e>] entry_SYSCALL_64_after_hwframe+0x49/0xbe

This is because in sctp_do_bind, if sctp_get_port_local is to
create hash bucket successfully, and sctp_add_bind_addr failed
to bind address, e.g return -ENOMEM, so memory leak found, it
needs to destroy allocated bucket.

Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: Mao Wenan <maowenan@huawei.com>
Acked-by: Neil Horman <nhorman@tuxdriver.com>
---
 net/sctp/socket.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 2f810078c91d..69ec3b796197 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -412,11 +412,13 @@ static int sctp_do_bind(struct sock *sk, union sctp_addr *addr, int len)
 	ret = sctp_add_bind_addr(bp, addr, af->sockaddr_len,
 				 SCTP_ADDR_SRC, GFP_ATOMIC);
 
-	/* Copy back into socket for getsockname() use. */
-	if (!ret) {
-		inet_sk(sk)->inet_sport = htons(inet_sk(sk)->inet_num);
-		sp->pf->to_sk_saddr(addr, sk);
+	if (ret) {
+		sctp_put_port(sk);
+		return ret;
 	}
+	/* Copy back into socket for getsockname() use. */
+	inet_sk(sk)->inet_sport = htons(inet_sk(sk)->inet_num);
+	sp->pf->to_sk_saddr(addr, sk);
 
 	return ret;
 }
-- 
2.20.1

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

* [PATCH v2 net 2/3] sctp: remove redundant assignment when call sctp_get_port_local
@ 2019-09-12  3:45                     ` Mao Wenan
  0 siblings, 0 replies; 43+ messages in thread
From: Mao Wenan @ 2019-09-12  3:45 UTC (permalink / raw)
  To: vyasevich, nhorman, marcelo.leitner, davem
  Cc: linux-sctp, netdev, linux-kernel, kernel-janitors, Mao Wenan

There are more parentheses in if clause when call sctp_get_port_local
in sctp_do_bind, and redundant assignment to 'ret'. This patch is to
do cleanup.

Signed-off-by: Mao Wenan <maowenan@huawei.com>
Acked-by: Neil Horman <nhorman@tuxdriver.com>
---
 net/sctp/socket.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 5e1934c48709..2f810078c91d 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -399,9 +399,8 @@ static int sctp_do_bind(struct sock *sk, union sctp_addr *addr, int len)
 	 * detection.
 	 */
 	addr->v4.sin_port = htons(snum);
-	if ((ret = sctp_get_port_local(sk, addr))) {
+	if (sctp_get_port_local(sk, addr))
 		return -EADDRINUSE;
-	}
 
 	/* Refresh ephemeral port.  */
 	if (!bp->port)
-- 
2.20.1

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

* [PATCH v2 net 0/3] fix memory leak for sctp_do_bind
  2019-09-12  2:05                 ` maowenan
@ 2019-09-12  4:02                   ` Mao Wenan
  -1 siblings, 0 replies; 43+ messages in thread
From: Mao Wenan @ 2019-09-12  3:45 UTC (permalink / raw)
  To: vyasevich, nhorman, marcelo.leitner, davem
  Cc: linux-sctp, netdev, linux-kernel, kernel-janitors, Mao Wenan

First two patches are to do cleanup, remove redundant assignment,
and change return type of sctp_get_port_local.
Third patch is to fix memory leak for sctp_do_bind if failed
to bind address.

---
 v2: add one patch to change return type of sctp_get_port_local.
---
Mao Wenan (3):
  sctp: change return type of sctp_get_port_local
  sctp: remove redundant assignment when call sctp_get_port_local
  sctp: destroy bucket if failed to bind addr

 net/sctp/socket.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

-- 
2.20.1

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

* [PATCH v2 net 3/3] sctp: destroy bucket if failed to bind addr
@ 2019-09-12  3:45                     ` Mao Wenan
  0 siblings, 0 replies; 43+ messages in thread
From: Mao Wenan @ 2019-09-12  3:45 UTC (permalink / raw)
  To: vyasevich, nhorman, marcelo.leitner, davem
  Cc: linux-sctp, netdev, linux-kernel, kernel-janitors, Mao Wenan, Hulk Robot

There is one memory leak bug report:
BUG: memory leak
unreferenced object 0xffff8881dc4c5ec0 (size 40):
  comm "syz-executor.0", pid 5673, jiffies 4298198457 (age 27.578s)
  hex dump (first 32 bytes):
    02 00 00 00 81 88 ff ff 00 00 00 00 00 00 00 00  ................
    f8 63 3d c1 81 88 ff ff 00 00 00 00 00 00 00 00  .c=.............
  backtrace:
    [<0000000072006339>] sctp_get_port_local+0x2a1/0xa00 [sctp]
    [<00000000c7b379ec>] sctp_do_bind+0x176/0x2c0 [sctp]
    [<000000005be274a2>] sctp_bind+0x5a/0x80 [sctp]
    [<00000000b66b4044>] inet6_bind+0x59/0xd0 [ipv6]
    [<00000000c68c7f42>] __sys_bind+0x120/0x1f0 net/socket.c:1647
    [<000000004513635b>] __do_sys_bind net/socket.c:1658 [inline]
    [<000000004513635b>] __se_sys_bind net/socket.c:1656 [inline]
    [<000000004513635b>] __x64_sys_bind+0x3e/0x50 net/socket.c:1656
    [<0000000061f2501e>] do_syscall_64+0x72/0x2e0 arch/x86/entry/common.c:296
    [<0000000003d1e05e>] entry_SYSCALL_64_after_hwframe+0x49/0xbe

This is because in sctp_do_bind, if sctp_get_port_local is to
create hash bucket successfully, and sctp_add_bind_addr failed
to bind address, e.g return -ENOMEM, so memory leak found, it
needs to destroy allocated bucket.

Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: Mao Wenan <maowenan@huawei.com>
Acked-by: Neil Horman <nhorman@tuxdriver.com>
---
 net/sctp/socket.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 2f810078c91d..69ec3b796197 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -412,11 +412,13 @@ static int sctp_do_bind(struct sock *sk, union sctp_addr *addr, int len)
 	ret = sctp_add_bind_addr(bp, addr, af->sockaddr_len,
 				 SCTP_ADDR_SRC, GFP_ATOMIC);
 
-	/* Copy back into socket for getsockname() use. */
-	if (!ret) {
-		inet_sk(sk)->inet_sport = htons(inet_sk(sk)->inet_num);
-		sp->pf->to_sk_saddr(addr, sk);
+	if (ret) {
+		sctp_put_port(sk);
+		return ret;
 	}
+	/* Copy back into socket for getsockname() use. */
+	inet_sk(sk)->inet_sport = htons(inet_sk(sk)->inet_num);
+	sp->pf->to_sk_saddr(addr, sk);
 
 	return ret;
 }
-- 
2.20.1

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

* [PATCH v2 net 1/3] sctp: change return type of sctp_get_port_local
@ 2019-09-12  3:45                     ` Mao Wenan
  0 siblings, 0 replies; 43+ messages in thread
From: Mao Wenan @ 2019-09-12  3:45 UTC (permalink / raw)
  To: vyasevich, nhorman, marcelo.leitner, davem
  Cc: linux-sctp, netdev, linux-kernel, kernel-janitors, Mao Wenan

Currently sctp_get_port_local() returns a long
which is either 0,1 or a pointer casted to long.
It's neither of the callers use the return value since
commit 62208f12451f ("net: sctp: simplify sctp_get_port").
Now two callers are sctp_get_port and sctp_do_bind,
they actually assumend a casted to an int was the same as
a pointer casted to a long, and they don't save the return
value just check whether it is zero or non-zero, so
it would better change return type from long to int for
sctp_get_port_local.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Mao Wenan <maowenan@huawei.com>
---
 net/sctp/socket.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 9d1f83b10c0a..5e1934c48709 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -309,7 +309,7 @@ static int sctp_bind(struct sock *sk, struct sockaddr *addr, int addr_len)
 	return retval;
 }
 
-static long sctp_get_port_local(struct sock *, union sctp_addr *);
+static int sctp_get_port_local(struct sock *, union sctp_addr *);
 
 /* Verify this is a valid sockaddr. */
 static struct sctp_af *sctp_sockaddr_af(struct sctp_sock *opt,
@@ -7998,7 +7998,7 @@ static void sctp_unhash(struct sock *sk)
 static struct sctp_bind_bucket *sctp_bucket_create(
 	struct sctp_bind_hashbucket *head, struct net *, unsigned short snum);
 
-static long sctp_get_port_local(struct sock *sk, union sctp_addr *addr)
+static int sctp_get_port_local(struct sock *sk, union sctp_addr *addr)
 {
 	struct sctp_sock *sp = sctp_sk(sk);
 	bool reuse = (sk->sk_reuse || sp->reuse);
@@ -8108,7 +8108,7 @@ static long sctp_get_port_local(struct sock *sk, union sctp_addr *addr)
 
 			if (sctp_bind_addr_conflict(&ep2->base.bind_addr,
 						    addr, sp2, sp)) {
-				ret = (long)sk2;
+				ret = 1;
 				goto fail_unlock;
 			}
 		}
@@ -8180,7 +8180,7 @@ static int sctp_get_port(struct sock *sk, unsigned short snum)
 	addr.v4.sin_port = htons(snum);
 
 	/* Note: sk->sk_num gets filled in if ephemeral port request. */
-	return !!sctp_get_port_local(sk, &addr);
+	return sctp_get_port_local(sk, &addr);
 }
 
 /*
-- 
2.20.1

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

* [PATCH v2 net 0/3] fix memory leak for sctp_do_bind
@ 2019-09-12  4:02                   ` Mao Wenan
  0 siblings, 0 replies; 43+ messages in thread
From: Mao Wenan @ 2019-09-12  4:02 UTC (permalink / raw)
  To: vyasevich, nhorman, marcelo.leitner, davem
  Cc: linux-sctp, netdev, linux-kernel, kernel-janitors, Mao Wenan

First two patches are to do cleanup, remove redundant assignment,
and change return type of sctp_get_port_local.
Third patch is to fix memory leak for sctp_do_bind if failed
to bind address.

---
 v2: add one patch to change return type of sctp_get_port_local.
---
Mao Wenan (3):
  sctp: change return type of sctp_get_port_local
  sctp: remove redundant assignment when call sctp_get_port_local
  sctp: destroy bucket if failed to bind addr

 net/sctp/socket.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

-- 
2.20.1


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

* [PATCH v2 net 1/3] sctp: change return type of sctp_get_port_local
@ 2019-09-12  3:45                     ` Mao Wenan
  0 siblings, 0 replies; 43+ messages in thread
From: Mao Wenan @ 2019-09-12  4:02 UTC (permalink / raw)
  To: vyasevich, nhorman, marcelo.leitner, davem
  Cc: linux-sctp, netdev, linux-kernel, kernel-janitors, Mao Wenan

Currently sctp_get_port_local() returns a long
which is either 0,1 or a pointer casted to long.
It's neither of the callers use the return value since
commit 62208f12451f ("net: sctp: simplify sctp_get_port").
Now two callers are sctp_get_port and sctp_do_bind,
they actually assumend a casted to an int was the same as
a pointer casted to a long, and they don't save the return
value just check whether it is zero or non-zero, so
it would better change return type from long to int for
sctp_get_port_local.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Mao Wenan <maowenan@huawei.com>
---
 net/sctp/socket.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 9d1f83b10c0a..5e1934c48709 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -309,7 +309,7 @@ static int sctp_bind(struct sock *sk, struct sockaddr *addr, int addr_len)
 	return retval;
 }
 
-static long sctp_get_port_local(struct sock *, union sctp_addr *);
+static int sctp_get_port_local(struct sock *, union sctp_addr *);
 
 /* Verify this is a valid sockaddr. */
 static struct sctp_af *sctp_sockaddr_af(struct sctp_sock *opt,
@@ -7998,7 +7998,7 @@ static void sctp_unhash(struct sock *sk)
 static struct sctp_bind_bucket *sctp_bucket_create(
 	struct sctp_bind_hashbucket *head, struct net *, unsigned short snum);
 
-static long sctp_get_port_local(struct sock *sk, union sctp_addr *addr)
+static int sctp_get_port_local(struct sock *sk, union sctp_addr *addr)
 {
 	struct sctp_sock *sp = sctp_sk(sk);
 	bool reuse = (sk->sk_reuse || sp->reuse);
@@ -8108,7 +8108,7 @@ static long sctp_get_port_local(struct sock *sk, union sctp_addr *addr)
 
 			if (sctp_bind_addr_conflict(&ep2->base.bind_addr,
 						    addr, sp2, sp)) {
-				ret = (long)sk2;
+				ret = 1;
 				goto fail_unlock;
 			}
 		}
@@ -8180,7 +8180,7 @@ static int sctp_get_port(struct sock *sk, unsigned short snum)
 	addr.v4.sin_port = htons(snum);
 
 	/* Note: sk->sk_num gets filled in if ephemeral port request. */
-	return !!sctp_get_port_local(sk, &addr);
+	return sctp_get_port_local(sk, &addr);
 }
 
 /*
-- 
2.20.1


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

* [PATCH v2 net 2/3] sctp: remove redundant assignment when call sctp_get_port_local
@ 2019-09-12  3:45                     ` Mao Wenan
  0 siblings, 0 replies; 43+ messages in thread
From: Mao Wenan @ 2019-09-12  4:02 UTC (permalink / raw)
  To: vyasevich, nhorman, marcelo.leitner, davem
  Cc: linux-sctp, netdev, linux-kernel, kernel-janitors, Mao Wenan

There are more parentheses in if clause when call sctp_get_port_local
in sctp_do_bind, and redundant assignment to 'ret'. This patch is to
do cleanup.

Signed-off-by: Mao Wenan <maowenan@huawei.com>
Acked-by: Neil Horman <nhorman@tuxdriver.com>
---
 net/sctp/socket.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 5e1934c48709..2f810078c91d 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -399,9 +399,8 @@ static int sctp_do_bind(struct sock *sk, union sctp_addr *addr, int len)
 	 * detection.
 	 */
 	addr->v4.sin_port = htons(snum);
-	if ((ret = sctp_get_port_local(sk, addr))) {
+	if (sctp_get_port_local(sk, addr))
 		return -EADDRINUSE;
-	}
 
 	/* Refresh ephemeral port.  */
 	if (!bp->port)
-- 
2.20.1


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

* [PATCH v2 net 3/3] sctp: destroy bucket if failed to bind addr
@ 2019-09-12  3:45                     ` Mao Wenan
  0 siblings, 0 replies; 43+ messages in thread
From: Mao Wenan @ 2019-09-12  4:02 UTC (permalink / raw)
  To: vyasevich, nhorman, marcelo.leitner, davem
  Cc: linux-sctp, netdev, linux-kernel, kernel-janitors, Mao Wenan, Hulk Robot

There is one memory leak bug report:
BUG: memory leak
unreferenced object 0xffff8881dc4c5ec0 (size 40):
  comm "syz-executor.0", pid 5673, jiffies 4298198457 (age 27.578s)
  hex dump (first 32 bytes):
    02 00 00 00 81 88 ff ff 00 00 00 00 00 00 00 00  ................
    f8 63 3d c1 81 88 ff ff 00 00 00 00 00 00 00 00  .c=.............
  backtrace:
    [<0000000072006339>] sctp_get_port_local+0x2a1/0xa00 [sctp]
    [<00000000c7b379ec>] sctp_do_bind+0x176/0x2c0 [sctp]
    [<000000005be274a2>] sctp_bind+0x5a/0x80 [sctp]
    [<00000000b66b4044>] inet6_bind+0x59/0xd0 [ipv6]
    [<00000000c68c7f42>] __sys_bind+0x120/0x1f0 net/socket.c:1647
    [<000000004513635b>] __do_sys_bind net/socket.c:1658 [inline]
    [<000000004513635b>] __se_sys_bind net/socket.c:1656 [inline]
    [<000000004513635b>] __x64_sys_bind+0x3e/0x50 net/socket.c:1656
    [<0000000061f2501e>] do_syscall_64+0x72/0x2e0 arch/x86/entry/common.c:296
    [<0000000003d1e05e>] entry_SYSCALL_64_after_hwframe+0x49/0xbe

This is because in sctp_do_bind, if sctp_get_port_local is to
create hash bucket successfully, and sctp_add_bind_addr failed
to bind address, e.g return -ENOMEM, so memory leak found, it
needs to destroy allocated bucket.

Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: Mao Wenan <maowenan@huawei.com>
Acked-by: Neil Horman <nhorman@tuxdriver.com>
---
 net/sctp/socket.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 2f810078c91d..69ec3b796197 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -412,11 +412,13 @@ static int sctp_do_bind(struct sock *sk, union sctp_addr *addr, int len)
 	ret = sctp_add_bind_addr(bp, addr, af->sockaddr_len,
 				 SCTP_ADDR_SRC, GFP_ATOMIC);
 
-	/* Copy back into socket for getsockname() use. */
-	if (!ret) {
-		inet_sk(sk)->inet_sport = htons(inet_sk(sk)->inet_num);
-		sp->pf->to_sk_saddr(addr, sk);
+	if (ret) {
+		sctp_put_port(sk);
+		return ret;
 	}
+	/* Copy back into socket for getsockname() use. */
+	inet_sk(sk)->inet_sport = htons(inet_sk(sk)->inet_num);
+	sp->pf->to_sk_saddr(addr, sk);
 
 	return ret;
 }
-- 
2.20.1


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

* Re: [PATCH v2 net 1/3] sctp: change return type of sctp_get_port_local
  2019-09-12  3:45                     ` Mao Wenan
@ 2019-09-12 14:51                       ` Marcelo Ricardo Leitner
  -1 siblings, 0 replies; 43+ messages in thread
From: Marcelo Ricardo Leitner @ 2019-09-12 14:51 UTC (permalink / raw)
  To: Mao Wenan
  Cc: vyasevich, nhorman, davem, linux-sctp, netdev, linux-kernel,
	kernel-janitors

On Thu, Sep 12, 2019 at 12:02:17PM +0800, Mao Wenan wrote:
> Currently sctp_get_port_local() returns a long
> which is either 0,1 or a pointer casted to long.
> It's neither of the callers use the return value since
> commit 62208f12451f ("net: sctp: simplify sctp_get_port").
> Now two callers are sctp_get_port and sctp_do_bind,
> they actually assumend a casted to an int was the same as
> a pointer casted to a long, and they don't save the return
> value just check whether it is zero or non-zero, so
> it would better change return type from long to int for
> sctp_get_port_local.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")

This Fixes tag is not needed. It's just a cleanup.
Maybe Dave can remove it for us.

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

Thanks.

> Signed-off-by: Mao Wenan <maowenan@huawei.com>
> ---
>  net/sctp/socket.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index 9d1f83b10c0a..5e1934c48709 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -309,7 +309,7 @@ static int sctp_bind(struct sock *sk, struct sockaddr *addr, int addr_len)
>  	return retval;
>  }
>  
> -static long sctp_get_port_local(struct sock *, union sctp_addr *);
> +static int sctp_get_port_local(struct sock *, union sctp_addr *);
>  
>  /* Verify this is a valid sockaddr. */
>  static struct sctp_af *sctp_sockaddr_af(struct sctp_sock *opt,
> @@ -7998,7 +7998,7 @@ static void sctp_unhash(struct sock *sk)
>  static struct sctp_bind_bucket *sctp_bucket_create(
>  	struct sctp_bind_hashbucket *head, struct net *, unsigned short snum);
>  
> -static long sctp_get_port_local(struct sock *sk, union sctp_addr *addr)
> +static int sctp_get_port_local(struct sock *sk, union sctp_addr *addr)
>  {
>  	struct sctp_sock *sp = sctp_sk(sk);
>  	bool reuse = (sk->sk_reuse || sp->reuse);
> @@ -8108,7 +8108,7 @@ static long sctp_get_port_local(struct sock *sk, union sctp_addr *addr)
>  
>  			if (sctp_bind_addr_conflict(&ep2->base.bind_addr,
>  						    addr, sp2, sp)) {
> -				ret = (long)sk2;
> +				ret = 1;
>  				goto fail_unlock;
>  			}
>  		}
> @@ -8180,7 +8180,7 @@ static int sctp_get_port(struct sock *sk, unsigned short snum)
>  	addr.v4.sin_port = htons(snum);
>  
>  	/* Note: sk->sk_num gets filled in if ephemeral port request. */
> -	return !!sctp_get_port_local(sk, &addr);
> +	return sctp_get_port_local(sk, &addr);
>  }
>  
>  /*
> -- 
> 2.20.1
> 

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

* Re: [PATCH v2 net 1/3] sctp: change return type of sctp_get_port_local
@ 2019-09-12 14:51                       ` Marcelo Ricardo Leitner
  0 siblings, 0 replies; 43+ messages in thread
From: Marcelo Ricardo Leitner @ 2019-09-12 14:51 UTC (permalink / raw)
  To: Mao Wenan
  Cc: vyasevich, nhorman, davem, linux-sctp, netdev, linux-kernel,
	kernel-janitors

On Thu, Sep 12, 2019 at 12:02:17PM +0800, Mao Wenan wrote:
> Currently sctp_get_port_local() returns a long
> which is either 0,1 or a pointer casted to long.
> It's neither of the callers use the return value since
> commit 62208f12451f ("net: sctp: simplify sctp_get_port").
> Now two callers are sctp_get_port and sctp_do_bind,
> they actually assumend a casted to an int was the same as
> a pointer casted to a long, and they don't save the return
> value just check whether it is zero or non-zero, so
> it would better change return type from long to int for
> sctp_get_port_local.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")

This Fixes tag is not needed. It's just a cleanup.
Maybe Dave can remove it for us.

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

Thanks.

> Signed-off-by: Mao Wenan <maowenan@huawei.com>
> ---
>  net/sctp/socket.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index 9d1f83b10c0a..5e1934c48709 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -309,7 +309,7 @@ static int sctp_bind(struct sock *sk, struct sockaddr *addr, int addr_len)
>  	return retval;
>  }
>  
> -static long sctp_get_port_local(struct sock *, union sctp_addr *);
> +static int sctp_get_port_local(struct sock *, union sctp_addr *);
>  
>  /* Verify this is a valid sockaddr. */
>  static struct sctp_af *sctp_sockaddr_af(struct sctp_sock *opt,
> @@ -7998,7 +7998,7 @@ static void sctp_unhash(struct sock *sk)
>  static struct sctp_bind_bucket *sctp_bucket_create(
>  	struct sctp_bind_hashbucket *head, struct net *, unsigned short snum);
>  
> -static long sctp_get_port_local(struct sock *sk, union sctp_addr *addr)
> +static int sctp_get_port_local(struct sock *sk, union sctp_addr *addr)
>  {
>  	struct sctp_sock *sp = sctp_sk(sk);
>  	bool reuse = (sk->sk_reuse || sp->reuse);
> @@ -8108,7 +8108,7 @@ static long sctp_get_port_local(struct sock *sk, union sctp_addr *addr)
>  
>  			if (sctp_bind_addr_conflict(&ep2->base.bind_addr,
>  						    addr, sp2, sp)) {
> -				ret = (long)sk2;
> +				ret = 1;
>  				goto fail_unlock;
>  			}
>  		}
> @@ -8180,7 +8180,7 @@ static int sctp_get_port(struct sock *sk, unsigned short snum)
>  	addr.v4.sin_port = htons(snum);
>  
>  	/* Note: sk->sk_num gets filled in if ephemeral port request. */
> -	return !!sctp_get_port_local(sk, &addr);
> +	return sctp_get_port_local(sk, &addr);
>  }
>  
>  /*
> -- 
> 2.20.1
> 

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

* Re: [PATCH v2 net 2/3] sctp: remove redundant assignment when call sctp_get_port_local
  2019-09-12  3:45                     ` Mao Wenan
@ 2019-09-12 14:52                       ` Marcelo Ricardo Leitner
  -1 siblings, 0 replies; 43+ messages in thread
From: Marcelo Ricardo Leitner @ 2019-09-12 14:52 UTC (permalink / raw)
  To: Mao Wenan
  Cc: vyasevich, nhorman, davem, linux-sctp, netdev, linux-kernel,
	kernel-janitors

On Thu, Sep 12, 2019 at 12:02:18PM +0800, Mao Wenan wrote:
> There are more parentheses in if clause when call sctp_get_port_local
> in sctp_do_bind, and redundant assignment to 'ret'. This patch is to
> do cleanup.
> 
> Signed-off-by: Mao Wenan <maowenan@huawei.com>
> Acked-by: Neil Horman <nhorman@tuxdriver.com>

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

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

* Re: [PATCH v2 net 2/3] sctp: remove redundant assignment when call sctp_get_port_local
@ 2019-09-12 14:52                       ` Marcelo Ricardo Leitner
  0 siblings, 0 replies; 43+ messages in thread
From: Marcelo Ricardo Leitner @ 2019-09-12 14:52 UTC (permalink / raw)
  To: Mao Wenan
  Cc: vyasevich, nhorman, davem, linux-sctp, netdev, linux-kernel,
	kernel-janitors

On Thu, Sep 12, 2019 at 12:02:18PM +0800, Mao Wenan wrote:
> There are more parentheses in if clause when call sctp_get_port_local
> in sctp_do_bind, and redundant assignment to 'ret'. This patch is to
> do cleanup.
> 
> Signed-off-by: Mao Wenan <maowenan@huawei.com>
> Acked-by: Neil Horman <nhorman@tuxdriver.com>

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

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

* Re: [PATCH v2 net 3/3] sctp: destroy bucket if failed to bind addr
  2019-09-12  3:45                     ` Mao Wenan
@ 2019-09-12 14:52                       ` Marcelo Ricardo Leitner
  -1 siblings, 0 replies; 43+ messages in thread
From: Marcelo Ricardo Leitner @ 2019-09-12 14:52 UTC (permalink / raw)
  To: Mao Wenan
  Cc: vyasevich, nhorman, davem, linux-sctp, netdev, linux-kernel,
	kernel-janitors, Hulk Robot

On Thu, Sep 12, 2019 at 12:02:19PM +0800, Mao Wenan wrote:
> There is one memory leak bug report:
> BUG: memory leak
> unreferenced object 0xffff8881dc4c5ec0 (size 40):
>   comm "syz-executor.0", pid 5673, jiffies 4298198457 (age 27.578s)
>   hex dump (first 32 bytes):
>     02 00 00 00 81 88 ff ff 00 00 00 00 00 00 00 00  ................
>     f8 63 3d c1 81 88 ff ff 00 00 00 00 00 00 00 00  .c=.............
>   backtrace:
>     [<0000000072006339>] sctp_get_port_local+0x2a1/0xa00 [sctp]
>     [<00000000c7b379ec>] sctp_do_bind+0x176/0x2c0 [sctp]
>     [<000000005be274a2>] sctp_bind+0x5a/0x80 [sctp]
>     [<00000000b66b4044>] inet6_bind+0x59/0xd0 [ipv6]
>     [<00000000c68c7f42>] __sys_bind+0x120/0x1f0 net/socket.c:1647
>     [<000000004513635b>] __do_sys_bind net/socket.c:1658 [inline]
>     [<000000004513635b>] __se_sys_bind net/socket.c:1656 [inline]
>     [<000000004513635b>] __x64_sys_bind+0x3e/0x50 net/socket.c:1656
>     [<0000000061f2501e>] do_syscall_64+0x72/0x2e0 arch/x86/entry/common.c:296
>     [<0000000003d1e05e>] entry_SYSCALL_64_after_hwframe+0x49/0xbe
> 
> This is because in sctp_do_bind, if sctp_get_port_local is to
> create hash bucket successfully, and sctp_add_bind_addr failed
> to bind address, e.g return -ENOMEM, so memory leak found, it
> needs to destroy allocated bucket.
> 
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Signed-off-by: Mao Wenan <maowenan@huawei.com>
> Acked-by: Neil Horman <nhorman@tuxdriver.com>

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

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

* Re: [PATCH v2 net 3/3] sctp: destroy bucket if failed to bind addr
@ 2019-09-12 14:52                       ` Marcelo Ricardo Leitner
  0 siblings, 0 replies; 43+ messages in thread
From: Marcelo Ricardo Leitner @ 2019-09-12 14:52 UTC (permalink / raw)
  To: Mao Wenan
  Cc: vyasevich, nhorman, davem, linux-sctp, netdev, linux-kernel,
	kernel-janitors, Hulk Robot

On Thu, Sep 12, 2019 at 12:02:19PM +0800, Mao Wenan wrote:
> There is one memory leak bug report:
> BUG: memory leak
> unreferenced object 0xffff8881dc4c5ec0 (size 40):
>   comm "syz-executor.0", pid 5673, jiffies 4298198457 (age 27.578s)
>   hex dump (first 32 bytes):
>     02 00 00 00 81 88 ff ff 00 00 00 00 00 00 00 00  ................
>     f8 63 3d c1 81 88 ff ff 00 00 00 00 00 00 00 00  .c=.............
>   backtrace:
>     [<0000000072006339>] sctp_get_port_local+0x2a1/0xa00 [sctp]
>     [<00000000c7b379ec>] sctp_do_bind+0x176/0x2c0 [sctp]
>     [<000000005be274a2>] sctp_bind+0x5a/0x80 [sctp]
>     [<00000000b66b4044>] inet6_bind+0x59/0xd0 [ipv6]
>     [<00000000c68c7f42>] __sys_bind+0x120/0x1f0 net/socket.c:1647
>     [<000000004513635b>] __do_sys_bind net/socket.c:1658 [inline]
>     [<000000004513635b>] __se_sys_bind net/socket.c:1656 [inline]
>     [<000000004513635b>] __x64_sys_bind+0x3e/0x50 net/socket.c:1656
>     [<0000000061f2501e>] do_syscall_64+0x72/0x2e0 arch/x86/entry/common.c:296
>     [<0000000003d1e05e>] entry_SYSCALL_64_after_hwframe+0x49/0xbe
> 
> This is because in sctp_do_bind, if sctp_get_port_local is to
> create hash bucket successfully, and sctp_add_bind_addr failed
> to bind address, e.g return -ENOMEM, so memory leak found, it
> needs to destroy allocated bucket.
> 
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Signed-off-by: Mao Wenan <maowenan@huawei.com>
> Acked-by: Neil Horman <nhorman@tuxdriver.com>

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

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

* Re: [PATCH v2 net 0/3] fix memory leak for sctp_do_bind
  2019-09-12  4:02                   ` Mao Wenan
@ 2019-09-13 20:06                     ` David Miller
  -1 siblings, 0 replies; 43+ messages in thread
From: David Miller @ 2019-09-13 20:06 UTC (permalink / raw)
  To: maowenan
  Cc: vyasevich, nhorman, marcelo.leitner, linux-sctp, netdev,
	linux-kernel, kernel-janitors

From: Mao Wenan <maowenan@huawei.com>
Date: Thu, 12 Sep 2019 12:02:16 +0800

> First two patches are to do cleanup, remove redundant assignment,
> and change return type of sctp_get_port_local.
> Third patch is to fix memory leak for sctp_do_bind if failed
> to bind address.
> 
> ---
>  v2: add one patch to change return type of sctp_get_port_local.

Series applied with Fixes: tag removed from patch #1.

Thanks.

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

* Re: [PATCH v2 net 0/3] fix memory leak for sctp_do_bind
@ 2019-09-13 20:06                     ` David Miller
  0 siblings, 0 replies; 43+ messages in thread
From: David Miller @ 2019-09-13 20:06 UTC (permalink / raw)
  To: maowenan
  Cc: vyasevich, nhorman, marcelo.leitner, linux-sctp, netdev,
	linux-kernel, kernel-janitors

From: Mao Wenan <maowenan@huawei.com>
Date: Thu, 12 Sep 2019 12:02:16 +0800

> First two patches are to do cleanup, remove redundant assignment,
> and change return type of sctp_get_port_local.
> Third patch is to fix memory leak for sctp_do_bind if failed
> to bind address.
> 
> ---
>  v2: add one patch to change return type of sctp_get_port_local.

Series applied with Fixes: tag removed from patch #1.

Thanks.

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

end of thread, other threads:[~2019-09-13 20:07 UTC | newest]

Thread overview: 43+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-10  6:56 [PATCH net 0/2] fix memory leak for sctp_do_bind Mao Wenan
2019-09-10  7:13 ` Mao Wenan
2019-09-10  6:56 ` [PATCH net 1/2] sctp: remove redundant assignment when call sctp_get_port_local Mao Wenan
2019-09-10  7:13   ` Mao Wenan
2019-09-10  6:56   ` Mao Wenan
2019-09-10 18:57   ` Dan Carpenter
2019-09-10 18:57     ` Dan Carpenter
2019-09-10 19:22     ` Dan Carpenter
2019-09-10 19:22       ` Dan Carpenter
2019-09-11  1:30       ` maowenan
2019-09-11  1:30         ` maowenan
2019-09-11  8:30         ` Dan Carpenter
2019-09-11  8:30           ` Dan Carpenter
2019-09-11 14:30           ` Marcelo Ricardo Leitner
2019-09-11 14:30             ` Marcelo Ricardo Leitner
2019-09-11 14:39             ` Marcelo Ricardo Leitner
2019-09-11 14:39               ` Marcelo Ricardo Leitner
2019-09-12  2:05               ` maowenan
2019-09-12  2:05                 ` maowenan
2019-09-12  3:45                 ` [PATCH v2 net 0/3] fix memory leak for sctp_do_bind Mao Wenan
2019-09-12  4:02                   ` Mao Wenan
2019-09-12  3:45                   ` [PATCH v2 net 1/3] sctp: change return type of sctp_get_port_local Mao Wenan
2019-09-12  4:02                     ` Mao Wenan
2019-09-12  3:45                     ` Mao Wenan
2019-09-12 14:51                     ` Marcelo Ricardo Leitner
2019-09-12 14:51                       ` Marcelo Ricardo Leitner
2019-09-12  3:45                   ` [PATCH v2 net 2/3] sctp: remove redundant assignment when call sctp_get_port_local Mao Wenan
2019-09-12  4:02                     ` Mao Wenan
2019-09-12  3:45                     ` Mao Wenan
2019-09-12 14:52                     ` Marcelo Ricardo Leitner
2019-09-12 14:52                       ` Marcelo Ricardo Leitner
2019-09-12  3:45                   ` [PATCH v2 net 3/3] sctp: destroy bucket if failed to bind addr Mao Wenan
2019-09-12  4:02                     ` Mao Wenan
2019-09-12  3:45                     ` Mao Wenan
2019-09-12 14:52                     ` Marcelo Ricardo Leitner
2019-09-12 14:52                       ` Marcelo Ricardo Leitner
2019-09-13 20:06                   ` [PATCH v2 net 0/3] fix memory leak for sctp_do_bind David Miller
2019-09-13 20:06                     ` David Miller
2019-09-10  6:56 ` [PATCH net 2/2] sctp: destroy bucket if failed to bind addr Mao Wenan
2019-09-10  7:13   ` Mao Wenan
2019-09-10  6:56   ` Mao Wenan
2019-09-10  7:16 ` [PATCH net 0/2] fix memory leak for sctp_do_bind Neil Horman
2019-09-10  7:16   ` Neil Horman

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.