2020-10-06 05:49:46

by Martin Schiller

Subject: [PATCH v2] net/x25: Fix null-ptr-deref in x25_connect

This fixes a regression for blocking connects introduced by commit
4becb7ee5b3d ("net/x25: Fix x25_neigh refcnt leak when x25 disconnect").

The x25->neighbour is already set to "NULL" by x25_disconnect() now,
while a blocking connect is waiting in
x25_wait_for_connection_establishment(). Therefore x25->neighbour must
not be accessed here again and x25->state is also already set to
X25_STATE_0 by x25_disconnect().

Fixes: 4becb7ee5b3d ("net/x25: Fix x25_neigh refcnt leak when x25 disconnect")
Signed-off-by: Martin Schiller <[email protected]>
---

Change from v1:
also handle interrupting signals correctly

---
net/x25/af_x25.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/x25/af_x25.c b/net/x25/af_x25.c
index 0bbb283f23c9..046d3fee66a9 100644
--- a/net/x25/af_x25.c
+++ b/net/x25/af_x25.c
@@ -825,7 +825,7 @@ static int x25_connect(struct socket *sock, struct sockaddr *uaddr,
sock->state = SS_CONNECTED;
rc = 0;
out_put_neigh:
- if (rc) {
+ if (rc && x25->neighbour) {
read_lock_bh(&x25_list_lock);
x25_neigh_put(x25->neighbour);
x25->neighbour = NULL;
--
2.20.1

2020-11-06 06:25:57

by Martin Schiller

Subject: Re: [PATCH v2] net/x25: Fix null-ptr-deref in x25_connect

On 2020-10-06 07:45, Martin Schiller wrote:
> This fixes a regression for blocking connects introduced by commit
> 4becb7ee5b3d ("net/x25: Fix x25_neigh refcnt leak when x25
> disconnect").
>
> The x25->neighbour is already set to "NULL" by x25_disconnect() now,
> while a blocking connect is waiting in
> x25_wait_for_connection_establishment(). Therefore x25->neighbour must
> not be accessed here again and x25->state is also already set to
> X25_STATE_0 by x25_disconnect().
>
> Fixes: 4becb7ee5b3d ("net/x25: Fix x25_neigh refcnt leak when x25
> disconnect")
> Signed-off-by: Martin Schiller <[email protected]>
> ---
>
> Change from v1:
> also handle interrupting signals correctly
>
> ---
> net/x25/af_x25.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/x25/af_x25.c b/net/x25/af_x25.c
> index 0bbb283f23c9..046d3fee66a9 100644
> --- a/net/x25/af_x25.c
> +++ b/net/x25/af_x25.c
> @@ -825,7 +825,7 @@ static int x25_connect(struct socket *sock, struct
> sockaddr *uaddr,
> sock->state = SS_CONNECTED;
> rc = 0;
> out_put_neigh:
> - if (rc) {
> + if (rc && x25->neighbour) {
> read_lock_bh(&x25_list_lock);
> x25_neigh_put(x25->neighbour);
> x25->neighbour = NULL;

@David
Is there anything left I need to do, to get this fix merged?

2020-11-06 16:09:16

by Jakub Kicinski

Subject: Re: [PATCH v2] net/x25: Fix null-ptr-deref in x25_connect

On Fri, 06 Nov 2020 07:23:05 +0100 Martin Schiller wrote:
> On 2020-10-06 07:45, Martin Schiller wrote:
> > This fixes a regression for blocking connects introduced by commit
> > 4becb7ee5b3d ("net/x25: Fix x25_neigh refcnt leak when x25
> > disconnect").
> >
> > The x25->neighbour is already set to "NULL" by x25_disconnect() now,
> > while a blocking connect is waiting in
> > x25_wait_for_connection_establishment(). Therefore x25->neighbour must
> > not be accessed here again and x25->state is also already set to
> > X25_STATE_0 by x25_disconnect().
> >
> > Fixes: 4becb7ee5b3d ("net/x25: Fix x25_neigh refcnt leak when x25
> > disconnect")
> > Signed-off-by: Martin Schiller <[email protected]>
>
> @David
> Is there anything left I need to do, to get this fix merged?

Hm, no idea what happened here (you could try to check the state in
patchwork but it's gonna take some digging to find a month old patch).

Please resend and we'll take it from there.