[PATCH] connman: fix segfault with musl >2.21

[PATCH] connman: fix segfault with musl >2.21

[Nicola Lunghi]

musl > 2.21 changed the implementation of the freeaddrinfo() function
not allowing anymore to pass null pointers to it.
This was causing a segmentation fault in connman.
---
 ...-gweb-fix-segfault-with-musl-v1.1.21.patch | 34 +++++++++++++++++++
 .../connman/connman_1.37.bb                   |  5 ++-
 2 files changed, 38 insertions(+), 1 deletion(-)
 create mode 100644 meta/recipes-connectivity/connman/connman/0003-gweb-fix-segfault-with-musl-v1.1.21.patch

diff --git a/meta/recipes-connectivity/connman/connman/0003-gweb-fix-segfault-with-musl-v1.1.21.patch b/meta/recipes-connectivity/connman/connman/0003-gweb-fix-segfault-with-musl-v1.1.21.patch
new file mode 100644
index 0000000000..43b43bc9f8
--- /dev/null
+++ b/meta/recipes-connectivity/connman/connman/0003-gweb-fix-segfault-with-musl-v1.1.21.patch
@@ -0,0 +1,34 @@
+From f0a8c69971b30ea7ca255bb885fdd1179fa5d298 Mon Sep 17 00:00:00 2001
+From: Nicola Lunghi <nick83ola@gmail.com>
+Date: Thu, 23 May 2019 07:55:25 +0100
+Subject: [PATCH] gweb: fix segfault with musl v1.1.21
+
+In musl > 1.1.21 freeaddrinfo() implementation changed and
+was causing a segmentation fault on recent Yocto using musl.
+
+See this commit:
+
+ https://git.musl-libc.org/cgit/musl/commit/src/network/freeaddrinfo.c?id=d1395c43c019aec6b855cf3c656bf47c8a719e7f
+
+Upstream-Status: Submitted
+---
+ gweb/gweb.c | 3 ++-
+ 1 file changed, 2 insertions(+), 1 deletion(-)
+
+diff --git a/gweb/gweb.c b/gweb/gweb.c
+index 393afe0a..12fcb1d8 100644
+--- a/gweb/gweb.c
++++ b/gweb/gweb.c
+@@ -1274,7 +1274,8 @@ static bool is_ip_address(const char *host)
+ 	addr = NULL;
+ 
+ 	result = getaddrinfo(host, NULL, &hints, &addr);
+-	freeaddrinfo(addr);
++	if(!result)
++		freeaddrinfo(addr);
+ 
+ 	return result == 0;
+ }
+-- 
+2.19.1
+
diff --git a/meta/recipes-connectivity/connman/connman_1.37.bb b/meta/recipes-connectivity/connman/connman_1.37.bb
index 2cf904cd85..f52b21cae3 100644
--- a/meta/recipes-connectivity/connman/connman_1.37.bb
+++ b/meta/recipes-connectivity/connman/connman_1.37.bb
@@ -7,7 +7,10 @@ SRC_URI  = "${KERNELORG_MIRROR}/linux/network/${BPN}/${BP}.tar.xz \
             file://no-version-scripts.patch \
 "
 
-SRC_URI_append_libc-musl = " file://0002-resolve-musl-does-not-implement-res_ninit.patch"
+SRC_URI_append_libc-musl = " \
+            file://0002-resolve-musl-does-not-implement-res_ninit.patch \
+            file://0003-gweb-fix-segfault-with-musl-v1.1.21.patch \
+"
 
 SRC_URI[md5sum] = "75012084f14fb63a84b116e66c6e94fb"
 SRC_URI[sha256sum] = "6ce29b3eb0bb16a7387bc609c39455fd13064bdcde5a4d185fab3a0c71946e16"
-- 
2.19.1

Re: [PATCH] connman: fix segfault with musl >2.21

[André Draszik]

On Thu, 2019-05-23 at 08:47 +0100, Nicola Lunghi wrote:
> musl > 2.21 changed the implementation of the freeaddrinfo() function
> not allowing anymore to pass null pointers to it.
> This was causing a segmentation fault in connman.
> ---
>  ...-gweb-fix-segfault-with-musl-v1.1.21.patch | 34 +++++++++++++++++++
>  .../connman/connman_1.37.bb                   |  5 ++-
>  2 files changed, 38 insertions(+), 1 deletion(-)
>  create mode 100644 meta/recipes-connectivity/connman/connman/0003-gweb-fix-segfault-with-musl-v1.1.21.patch
> 
> diff --git a/meta/recipes-connectivity/connman/connman/0003-gweb-fix-segfault-with-musl-v1.1.21.patch b/meta/recipes-
> connectivity/connman/connman/0003-gweb-fix-segfault-with-musl-v1.1.21.patch
> new file mode 100644
> index 0000000000..43b43bc9f8
> --- /dev/null
> +++ b/meta/recipes-connectivity/connman/connman/0003-gweb-fix-segfault-with-musl-v1.1.21.patch
> @@ -0,0 +1,34 @@
> +From f0a8c69971b30ea7ca255bb885fdd1179fa5d298 Mon Sep 17 00:00:00 2001
> +From: Nicola Lunghi <nick83ola@gmail.com>
> +Date: Thu, 23 May 2019 07:55:25 +0100
> +Subject: [PATCH] gweb: fix segfault with musl v1.1.21
> +
> +In musl > 1.1.21 freeaddrinfo() implementation changed and
> +was causing a segmentation fault on recent Yocto using musl.
> +
> +See this commit:
> +
> + https://git.musl-libc.org/cgit/musl/commit/src/network/freeaddrinfo.c?id=d1395c43c019aec6b855cf3c656bf47c8a719e7f
> +
> +Upstream-Status: Submitted
> +---
> + gweb/gweb.c | 3 ++-
> + 1 file changed, 2 insertions(+), 1 deletion(-)
> +
> +diff --git a/gweb/gweb.c b/gweb/gweb.c
> +index 393afe0a..12fcb1d8 100644
> +--- a/gweb/gweb.c
> ++++ b/gweb/gweb.c
> +@@ -1274,7 +1274,8 @@ static bool is_ip_address(const char *host)
> + 	addr = NULL;
> + 
> + 	result = getaddrinfo(host, NULL, &hints, &addr);
> +-	freeaddrinfo(addr);
> ++	if(!result)
> ++		freeaddrinfo(addr);
> + 
> + 	return result == 0;
> + }
> +-- 
> +2.19.1
> +
> diff --git a/meta/recipes-connectivity/connman/connman_1.37.bb b/meta/recipes-connectivity/connman/connman_1.37.bb
> index 2cf904cd85..f52b21cae3 100644
> --- a/meta/recipes-connectivity/connman/connman_1.37.bb
> +++ b/meta/recipes-connectivity/connman/connman_1.37.bb
> @@ -7,7 +7,10 @@ SRC_URI  = "${KERNELORG_MIRROR}/linux/network/${BPN}/${BP}.tar.xz \
>              file://no-version-scripts.patch \
>  "
>  
> -SRC_URI_append_libc-musl = " file://0002-resolve-musl-does-not-implement-res_ninit.patch"
> +SRC_URI_append_libc-musl = " \
> +            file://0002-resolve-musl-does-not-implement-res_ninit.patch \
> +            file://0003-gweb-fix-segfault-with-musl-v1.1.21.patch \
> +"

This fix is not musl specific.

Cheers,
Andre'

>  
>  SRC_URI[md5sum] = "75012084f14fb63a84b116e66c6e94fb"
>  SRC_URI[sha256sum] = "6ce29b3eb0bb16a7387bc609c39455fd13064bdcde5a4d185fab3a0c71946e16"
> -- 
> 2.19.1
> 

Re: [PATCH] connman: fix segfault with musl >2.21

[André Draszik]

On Thu, 2019-05-23 at 08:47 +0100, Nicola Lunghi wrote:
> musl > 2.21 changed the implementation of the freeaddrinfo() function
         ^^^^

I think you mean 1.1.21 here and in the commit subject.

A.


> not allowing anymore to pass null pointers to it.
> This was causing a segmentation fault in connman.
> ---
>  ...-gweb-fix-segfault-with-musl-v1.1.21.patch | 34 +++++++++++++++++++
>  .../connman/connman_1.37.bb                   |  5 ++-
>  2 files changed, 38 insertions(+), 1 deletion(-)
>  create mode 100644 meta/recipes-connectivity/connman/connman/0003-gweb-fix-segfault-with-musl-v1.1.21.patch
> 
> diff --git a/meta/recipes-connectivity/connman/connman/0003-gweb-fix-segfault-with-musl-v1.1.21.patch b/meta/recipes-
> connectivity/connman/connman/0003-gweb-fix-segfault-with-musl-v1.1.21.patch
> new file mode 100644
> index 0000000000..43b43bc9f8
> --- /dev/null
> +++ b/meta/recipes-connectivity/connman/connman/0003-gweb-fix-segfault-with-musl-v1.1.21.patch
> @@ -0,0 +1,34 @@
> +From f0a8c69971b30ea7ca255bb885fdd1179fa5d298 Mon Sep 17 00:00:00 2001
> +From: Nicola Lunghi <nick83ola@gmail.com>
> +Date: Thu, 23 May 2019 07:55:25 +0100
> +Subject: [PATCH] gweb: fix segfault with musl v1.1.21
> +
> +In musl > 1.1.21 freeaddrinfo() implementation changed and
> +was causing a segmentation fault on recent Yocto using musl.
> +
> +See this commit:
> +
> + https://git.musl-libc.org/cgit/musl/commit/src/network/freeaddrinfo.c?id=d1395c43c019aec6b855cf3c656bf47c8a719e7f
> +
> +Upstream-Status: Submitted
> +---
> + gweb/gweb.c | 3 ++-
> + 1 file changed, 2 insertions(+), 1 deletion(-)
> +
> +diff --git a/gweb/gweb.c b/gweb/gweb.c
> +index 393afe0a..12fcb1d8 100644
> +--- a/gweb/gweb.c
> ++++ b/gweb/gweb.c
> +@@ -1274,7 +1274,8 @@ static bool is_ip_address(const char *host)
> + 	addr = NULL;
> + 
> + 	result = getaddrinfo(host, NULL, &hints, &addr);
> +-	freeaddrinfo(addr);
> ++	if(!result)
> ++		freeaddrinfo(addr);
> + 
> + 	return result == 0;
> + }
> +-- 
> +2.19.1
> +
> diff --git a/meta/recipes-connectivity/connman/connman_1.37.bb b/meta/recipes-connectivity/connman/connman_1.37.bb
> index 2cf904cd85..f52b21cae3 100644
> --- a/meta/recipes-connectivity/connman/connman_1.37.bb
> +++ b/meta/recipes-connectivity/connman/connman_1.37.bb
> @@ -7,7 +7,10 @@ SRC_URI  = "${KERNELORG_MIRROR}/linux/network/${BPN}/${BP}.tar.xz \
>              file://no-version-scripts.patch \
>  "
>  
> -SRC_URI_append_libc-musl = " file://0002-resolve-musl-does-not-implement-res_ninit.patch"
> +SRC_URI_append_libc-musl = " \
> +            file://0002-resolve-musl-does-not-implement-res_ninit.patch \
> +            file://0003-gweb-fix-segfault-with-musl-v1.1.21.patch \
> +"
>  
>  SRC_URI[md5sum] = "75012084f14fb63a84b116e66c6e94fb"
>  SRC_URI[sha256sum] = "6ce29b3eb0bb16a7387bc609c39455fd13064bdcde5a4d185fab3a0c71946e16"
> -- 
> 2.19.1
> 

Re: [PATCH] connman: fix segfault with musl >2.21

[nick83ola]

Hi Andre,

in glibc freeaddrinfo is checking for null

https://github.molgen.mpg.de/git-mirror/glibc/blob/20003c49884422da7ffbc459cdeee768a6fee07b/sysdeps/posix/getaddrinfo.c#L2663-L2674

void
freeaddrinfo (struct addrinfo *ai)
{
    struct addrinfo *p;

    while (ai != NULL)
    {
        p = ai;
        ai = ai->ai_next;
        free (p->ai_canonname);
        free (p);
    }
}

that is why I put it into the musl specific patches.
But ok I will repost it putting it in the generic SRC_URI.

regarding the 2.21 -> yes is a typo.

Thanks
Nicola Lunghi


On Thu, 23 May 2019 at 10:18, André Draszik <git@andred.net> wrote:
>
> On Thu, 2019-05-23 at 08:47 +0100, Nicola Lunghi wrote:
> > musl > 2.21 changed the implementation of the freeaddrinfo() function
> > not allowing anymore to pass null pointers to it.
> > This was causing a segmentation fault in connman.
> > ---
> >  ...-gweb-fix-segfault-with-musl-v1.1.21.patch | 34 +++++++++++++++++++
> >  .../connman/connman_1.37.bb                   |  5 ++-
> >  2 files changed, 38 insertions(+), 1 deletion(-)
> >  create mode 100644 meta/recipes-connectivity/connman/connman/0003-gweb-fix-segfault-with-musl-v1.1.21.patch
> >
> > diff --git a/meta/recipes-connectivity/connman/connman/0003-gweb-fix-segfault-with-musl-v1.1.21.patch b/meta/recipes-
> > connectivity/connman/connman/0003-gweb-fix-segfault-with-musl-v1.1.21.patch
> > new file mode 100644
> > index 0000000000..43b43bc9f8
> > --- /dev/null
> > +++ b/meta/recipes-connectivity/connman/connman/0003-gweb-fix-segfault-with-musl-v1.1.21.patch
> > @@ -0,0 +1,34 @@
> > +From f0a8c69971b30ea7ca255bb885fdd1179fa5d298 Mon Sep 17 00:00:00 2001
> > +From: Nicola Lunghi <nick83ola@gmail.com>
> > +Date: Thu, 23 May 2019 07:55:25 +0100
> > +Subject: [PATCH] gweb: fix segfault with musl v1.1.21
> > +
> > +In musl > 1.1.21 freeaddrinfo() implementation changed and
> > +was causing a segmentation fault on recent Yocto using musl.
> > +
> > +See this commit:
> > +
> > + https://git.musl-libc.org/cgit/musl/commit/src/network/freeaddrinfo.c?id=d1395c43c019aec6b855cf3c656bf47c8a719e7f
> > +
> > +Upstream-Status: Submitted
> > +---
> > + gweb/gweb.c | 3 ++-
> > + 1 file changed, 2 insertions(+), 1 deletion(-)
> > +
> > +diff --git a/gweb/gweb.c b/gweb/gweb.c
> > +index 393afe0a..12fcb1d8 100644
> > +--- a/gweb/gweb.c
> > ++++ b/gweb/gweb.c
> > +@@ -1274,7 +1274,8 @@ static bool is_ip_address(const char *host)
> > +     addr = NULL;
> > +
> > +     result = getaddrinfo(host, NULL, &hints, &addr);
> > +-    freeaddrinfo(addr);
> > ++    if(!result)
> > ++            freeaddrinfo(addr);
> > +
> > +     return result == 0;
> > + }
> > +--
> > +2.19.1
> > +
> > diff --git a/meta/recipes-connectivity/connman/connman_1.37.bb b/meta/recipes-connectivity/connman/connman_1.37.bb
> > index 2cf904cd85..f52b21cae3 100644
> > --- a/meta/recipes-connectivity/connman/connman_1.37.bb
> > +++ b/meta/recipes-connectivity/connman/connman_1.37.bb
> > @@ -7,7 +7,10 @@ SRC_URI  = "${KERNELORG_MIRROR}/linux/network/${BPN}/${BP}.tar.xz \
> >              file://no-version-scripts.patch \
> >  "
> >
> > -SRC_URI_append_libc-musl = " file://0002-resolve-musl-does-not-implement-res_ninit.patch"
> > +SRC_URI_append_libc-musl = " \
> > +            file://0002-resolve-musl-does-not-implement-res_ninit.patch \
> > +            file://0003-gweb-fix-segfault-with-musl-v1.1.21.patch \
> > +"
>
> This fix is not musl specific.
>
> Cheers,
> Andre'
>
> >
> >  SRC_URI[md5sum] = "75012084f14fb63a84b116e66c6e94fb"
> >  SRC_URI[sha256sum] = "6ce29b3eb0bb16a7387bc609c39455fd13064bdcde5a4d185fab3a0c71946e16"
> > --
> > 2.19.1
> >
>
> --
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core

Re: [PATCH] connman: fix segfault with musl >2.21

[Burton, Ross]

On Thu, 23 May 2019 at 12:07, nick83ola <nick83ola@gmail.com> wrote:
> that is why I put it into the musl specific patches.
> But ok I will repost it putting it in the generic SRC_URI.

Rule of thumb is to only apply patches in an override if they break
other cases outside of the override.  In this case, it's just an extra
guard, so nothing else breaks.

Rationale being that when a patch is in an override it doesn't get
tested as much.

Ross

Re: [PATCH] connman: fix segfault with musl >2.21

[André Draszik]

On Thu, 2019-05-23 at 12:06 +0100, nick83ola wrote:
> Hi Andre,
> 
> in glibc freeaddrinfo is checking for null
> 
> https://github.molgen.mpg.de/git-mirror/glibc/blob/20003c49884422da7ffbc459cdeee768a6fee07b/sysdeps/posix/getaddrinfo.c#L2663-L2674
> 
> void
> freeaddrinfo (struct addrinfo *ai)
> {
>     struct addrinfo *p;
> 
>     while (ai != NULL)
>     {
>         p = ai;
>         ai = ai->ai_next;
>         free (p->ai_canonname);
>         free (p);
>     }
> }

This is an implementation detail that isn't mandated by the standard (and
hence could change).

Quoting https://pubs.opengroup.org/onlinepubs/9699919799/functions/freeaddrinfo.html
'The freeaddrinfo() function shall free one or more addrinfo structures
returned by getaddrinfo(), along with any additional storage associated
with those structures'

Technically, unless getaddrinfo() succeeds, one didn't get an
'addrinfo structure returned by getaddrinfo()', so one equally
can't pass it into freeaddrinfo() unless getaddrinfo() succeeded.

Also, in connman's gweb.c 'addr' is initialised to NULL. Again,
NULL isn't anything 'returned by getaddrinfo()', so even in glibc
it only works by pure luck.

In other words, applying the patch in both glibc & musl environments
is the right thing to do.

See below.

> 
> that is why I put it into the musl specific patches.
> But ok I will repost it putting it in the generic SRC_URI.
> 
> regarding the 2.21 -> yes is a typo.
> 
> Thanks
> Nicola Lunghi
> 
> 
> On Thu, 23 May 2019 at 10:18, André Draszik <git@andred.net> wrote:
> > On Thu, 2019-05-23 at 08:47 +0100, Nicola Lunghi wrote:
> > > musl > 2.21 changed the implementation of the freeaddrinfo() function
> > > not allowing anymore to pass null pointers to it.
> > > This was causing a segmentation fault in connman.
> > > ---
> > >  ...-gweb-fix-segfault-with-musl-v1.1.21.patch | 34 +++++++++++++++++++
> > >  .../connman/connman_1.37.bb                   |  5 ++-
> > >  2 files changed, 38 insertions(+), 1 deletion(-)
> > >  create mode 100644 meta/recipes-connectivity/connman/connman/0003-gweb-fix-segfault-with-musl-v1.1.21.patch
> > > 
> > > diff --git a/meta/recipes-connectivity/connman/connman/0003-gweb-fix-segfault-with-musl-v1.1.21.patch
> > > b/meta/recipes-
> > > connectivity/connman/connman/0003-gweb-fix-segfault-with-musl-v1.1.21.patch
> > > new file mode 100644
> > > index 0000000000..43b43bc9f8
> > > --- /dev/null
> > > +++ b/meta/recipes-connectivity/connman/connman/0003-gweb-fix-segfault-with-musl-v1.1.21.patch
> > > @@ -0,0 +1,34 @@
> > > +From f0a8c69971b30ea7ca255bb885fdd1179fa5d298 Mon Sep 17 00:00:00 2001
> > > +From: Nicola Lunghi <nick83ola@gmail.com>
> > > +Date: Thu, 23 May 2019 07:55:25 +0100
> > > +Subject: [PATCH] gweb: fix segfault with musl v1.1.21
> > > +
> > > +In musl > 1.1.21 freeaddrinfo() implementation changed and
> > > +was causing a segmentation fault on recent Yocto using musl.
> > > +
> > > +See this commit:
> > > +
> > > + 
> > > https://git.musl-libc.org/cgit/musl/commit/src/network/freeaddrinfo.c?id=d1395c43c019aec6b855cf3c656bf47c8a719e7f
> > > +
> > > +Upstream-Status: Submitted
> > > +---
> > > + gweb/gweb.c | 3 ++-
> > > + 1 file changed, 2 insertions(+), 1 deletion(-)
> > > +
> > > +diff --git a/gweb/gweb.c b/gweb/gweb.c
> > > +index 393afe0a..12fcb1d8 100644
> > > +--- a/gweb/gweb.c
> > > ++++ b/gweb/gweb.c
> > > +@@ -1274,7 +1274,8 @@ static bool is_ip_address(const char *host)
> > > +     addr = NULL;

Ideally, this patch should also remove the assignment of addr = NULL;
as per above. But that's a discussion for the connman list.

Cheers,
Andre'


> > > +
> > > +     result = getaddrinfo(host, NULL, &hints, &addr);
> > > +-    freeaddrinfo(addr);
> > > ++    if(!result)
> > > ++            freeaddrinfo(addr);
> > > +
> > > +     return result == 0;
> > > + }
> > > +--
> > > +2.19.1
> > > +
> > > diff --git a/meta/recipes-connectivity/connman/connman_1.37.bb b/meta/recipes-connectivity/connman/connman_1.37.bb
> > > index 2cf904cd85..f52b21cae3 100644
> > > --- a/meta/recipes-connectivity/connman/connman_1.37.bb
> > > +++ b/meta/recipes-connectivity/connman/connman_1.37.bb
> > > @@ -7,7 +7,10 @@ SRC_URI  = "${KERNELORG_MIRROR}/linux/network/${BPN}/${BP}.tar.xz \
> > >              file://no-version-scripts.patch \
> > >  "
> > > 
> > > -SRC_URI_append_libc-musl = " file://0002-resolve-musl-does-not-implement-res_ninit.patch"
> > > +SRC_URI_append_libc-musl = " \
> > > +            file://0002-resolve-musl-does-not-implement-res_ninit.patch \
> > > +            file://0003-gweb-fix-segfault-with-musl-v1.1.21.patch \
> > > +"
> > 
> > This fix is not musl specific.
> > 
> > Cheers,
> > Andre'
> > 
> > >  SRC_URI[md5sum] = "75012084f14fb63a84b116e66c6e94fb"
> > >  SRC_URI[sha256sum] = "6ce29b3eb0bb16a7387bc609c39455fd13064bdcde5a4d185fab3a0c71946e16"
> > > --
> > > 2.19.1
> > > 
> > 
> > --
> > _______________________________________________
> > Openembedded-core mailing list
> > Openembedded-core@lists.openembedded.org
> > http://lists.openembedded.org/mailman/listinfo/openembedded-core

Re: [PATCH] connman: fix segfault with musl >2.21

[Adrian Bunk]

On Thu, May 23, 2019 at 02:56:11PM +0100, André Draszik wrote:
>...
> Also, in connman's gweb.c 'addr' is initialised to NULL. Again,
> NULL isn't anything 'returned by getaddrinfo()', so even in glibc
> it only works by pure luck.
>...

Not pure luck, free(NULL) is valid but passing random garbage to 
freeaddrinfo() would always fail.[1]

The critical point is that freeaddrinfo(NULL) working with a C library
is only an implementation detail of this specific version of the library,
not something a C library has to support according to POSIX.

> Cheers,
> Andre'

cu
Adrian

[1] unless the garbage happens to be NULL

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed