freeaddrinfo() behavior change in 1.1.21: crash on NULL

freeaddrinfo() behavior change in 1.1.21: crash on NULL

[周欣潼]

[-- Attachment #1: Type: text/plain, Size: 716 bytes --]

Hi,


We were testing connman with musl 1.1.21and hit this issue. The behavior change was introduced in http://git.musl-libc.org/cgit/musl/commit/?id=d1395c43c019aec6b855cf3c656bf47c8a719e7f. Before the change, freeaddrinfo() accepts NULL since it just calls free(). Now it expects the input pointer to be valid.


I am writing to check whether this is expected. connman calls freeaddrinfo() regard less of the return code of getaddrinfo() and crashes when getaddrinfo() fails. The document of freeaddrinfo() does not explicitly state its behavior on NULL, but the wording seems to indicate the argument should be valid. If this interpretation is correct, we can proceed to patch connman.


Thanks,
Xintong

[-- Attachment #2: Type: text/html, Size: 880 bytes --]

Re: freeaddrinfo() behavior change in 1.1.21: crash on NULL

[Rich Felker]

On Tue, Mar 26, 2019 at 08:13:40PM +0800, 周欣潼 wrote:
> Hi,
> 
> 
> We were testing connman with musl 1.1.21and hit this issue. The
> behavior change was introduced in
> http://git.musl-libc.org/cgit/musl/commit/?id=d1395c43c019aec6b855cf3c656bf47c8a719e7f.
> Before the change, freeaddrinfo() accepts NULL since it just calls
> free(). Now it expects the input pointer to be valid.
> 
> 
> I am writing to check whether this is expected. connman calls
> freeaddrinfo() regard less of the return code of getaddrinfo() and
> crashes when getaddrinfo() fails. The document of freeaddrinfo()
> does not explicitly state its behavior on NULL, but the wording
> seems to indicate the argument should be valid. If this
> interpretation is correct, we can proceed to patch connman.

This issue has been raised before. My reading is that it's UB to pass
a null pointer to freeaddrinfo, but I think there may be room for
disagreement on this. The relevant text is:

    "The freeaddrinfo() function shall free one or more addrinfo
                                            ^^^^^^^^^^^
    structures returned by getaddrinfo(), along with any additional
    storage associated with those structures. If the ai_next field of
    the structure is not null, the entire list of structures shall be
    freed. The freeaddrinfo() function shall support the freeing of
    arbitrary sublists of an addrinfo list originally returned by
    ^^^^^^^^^^^^^^^^^^
    getaddrinfo()."

"Arbitrary sublists" suggests that the empty sublist might be such a
valid sublist, but "one or more" suggests that it's not.

I believe a few programs have already been fixed not to pass a null
pointer here, but maybe we should consider changing this, or ask the
Austin Group for an interpretation. We could also look at the RFC for
this interface (from which the POSIX interfaces were ad[ao]pted) and
see if it implies that null should be okay.

Rich