Re: [ovs-dev] [PATCH] netdev-linux: Fix a null pointer dereference in netdev_linux_notify_sock()

[Flavio Leitner]

On Thu, Sep 09, 2021 at 02:08:50PM +0200, David Marchand wrote:
> On Wed, Sep 8, 2021 at 1:53 PM Yunjian Wang  wrote:
> >
> > If nl_sock_join_mcgroup() returns an error, the 'sock' is freed
> > and set to NULL. So we should add NULL check of 'sock' before calling
> > nl_sock_listen_all_nsid().
> >
> > Fixes: cf114a7fce80 ("netlink linux: enable listening to all nsids")
> > Cc: Flavio Leitner 
> > Signed-off-by: Yunjian Wang 
> > ---
> >  lib/netdev-linux.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
> > index 60dd13891..7fec5f5a6 100644
> > --- a/lib/netdev-linux.c
> > +++ b/lib/netdev-linux.c
> > @@ -636,7 +636,9 @@ netdev_linux_notify_sock(void)
> >  }
> >  }
> >  }
> > -nl_sock_listen_all_nsid(sock, true);
> > +if (sock) {
> > +nl_sock_listen_all_nsid(sock, true);
> > +}
> >  ovsthread_once_done();
> >  }
> >
> 
> Would it make sense to move this call before the loop on groups?
> Something like:

It does to me. The nl_sock_listen_all_nsid() only sets a flag in
the socket, so it should not matter whether it is done before or
after joining the mcgroups.

fbl


> 
> @@ -627,6 +627,7 @@ netdev_linux_notify_sock(void)
>  if (!error) {
>  size_t i;
> 
> +nl_sock_listen_all_nsid(sock, true);
>  for (i = 0; i < ARRAY_SIZE(mcgroups); i++) {
>  error = nl_sock_join_mcgroup(sock, mcgroups[i]);
>  if (error) {
> @@ -636,7 +637,6 @@ netdev_linux_notify_sock(void)
>  }
>  }
>  }
> -nl_sock_listen_all_nsid(sock, true);
>  ovsthread_once_done();
>  }
> 
> 
> -- 
> David Marchand
> 

-- 
fbl

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] netdev-linux: Fix a null pointer dereference in netdev_linux_notify_sock()

[David Marchand]

On Wed, Sep 8, 2021 at 1:53 PM Yunjian Wang  wrote:
>
> If nl_sock_join_mcgroup() returns an error, the 'sock' is freed
> and set to NULL. So we should add NULL check of 'sock' before calling
> nl_sock_listen_all_nsid().
>
> Fixes: cf114a7fce80 ("netlink linux: enable listening to all nsids")
> Cc: Flavio Leitner 
> Signed-off-by: Yunjian Wang 
> ---
>  lib/netdev-linux.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
> index 60dd13891..7fec5f5a6 100644
> --- a/lib/netdev-linux.c
> +++ b/lib/netdev-linux.c
> @@ -636,7 +636,9 @@ netdev_linux_notify_sock(void)
>  }
>  }
>  }
> -nl_sock_listen_all_nsid(sock, true);
> +if (sock) {
> +nl_sock_listen_all_nsid(sock, true);
> +}
>  ovsthread_once_done();
>  }
>

Would it make sense to move this call before the loop on groups?
Something like:

@@ -627,6 +627,7 @@ netdev_linux_notify_sock(void)
 if (!error) {
 size_t i;

+nl_sock_listen_all_nsid(sock, true);
 for (i = 0; i < ARRAY_SIZE(mcgroups); i++) {
 error = nl_sock_join_mcgroup(sock, mcgroups[i]);
 if (error) {
@@ -636,7 +637,6 @@ netdev_linux_notify_sock(void)
 }
 }
 }
-nl_sock_listen_all_nsid(sock, true);
 ovsthread_once_done();
 }


-- 
David Marchand

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] netdev-linux: Fix a null pointer dereference in netdev_linux_notify_sock()

[Yunjian Wang]

If nl_sock_join_mcgroup() returns an error, the 'sock' is freed
and set to NULL. So we should add NULL check of 'sock' before calling
nl_sock_listen_all_nsid().

Fixes: cf114a7fce80 ("netlink linux: enable listening to all nsids")
Cc: Flavio Leitner 
Signed-off-by: Yunjian Wang 
---
 lib/netdev-linux.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index 60dd13891..7fec5f5a6 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -636,7 +636,9 @@ netdev_linux_notify_sock(void)
 }
 }
 }
-nl_sock_listen_all_nsid(sock, true);
+if (sock) {
+nl_sock_listen_all_nsid(sock, true);
+}
 ovsthread_once_done();
 }
 
-- 
2.18.1

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev