Re: [OE-core] [PATCH] connman: fix segfault with musl >2.21
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 -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] [PATCH] connman: fix segfault with musl >2.21
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 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 00..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
> > > +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, , );
> > > +-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
> >
Re: [OE-core] [PATCH] connman: fix segfault with musl >2.21
On Thu, 23 May 2019 at 12:07, nick83ola 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 -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] [PATCH] connman: fix segfault with musl >2.21
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 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 00..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
> > +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, , );
> > +-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
--
___
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] [PATCH] connman: fix segfault with musl >2.21
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 00..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
> +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, , );
> +-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
>
--
___
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] [PATCH] connman: fix segfault with musl >2.21
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 00..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
> +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, , );
> +-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
