Re: nonstrings in Glibc

[Martin Sebor <msebor@gmail.com>]

On 11/21/2017 05:14 PM, Dmitry V. Levin wrote:
> On Tue, Nov 21, 2017 at 04:41:27PM -0700, Martin Sebor wrote:
>> On 11/20/2017 11:20 AM, Carlos O'Donell wrote:
>>> On 11/20/2017 08:54 AM, Martin Sebor wrote:
>>>> I'm done testing my update to the -Wstringop-truncation GCC patch
>>>> to find misuses of non-string arrays.  With the very limited use
>>>> of attribute nonstring it only found one potential bug (22447).
>>>> I've been looking at other uses of strncpy in Glibc to see if there
>>>> are other arrays that would benefit from the attribute.  I'm not
>>>> sufficiently familiar with Glibc data structures so it's a very
>>>> slow going.  Could someone help suggests data structures with
>>>> array members that might be candidates?
>>>
>>> struct sockaddr's sun_path?
>>>
>>> http://thread.gmane.org/gmane.comp.standards.posix.austin.general/5735
>>>
>>> Is that what you need help finding?
>>
>> Yes, that's what I'm looking for, thanks!
>>
>>  From the referenced thread it sounds like POSIX doesn't require
>> sun_path to be nul-terminated and BSD UNIX doesn't terminate it.
>> But I'm not sure what happens on Linux.  According to Michael
>> Kerrisk's response it sounds like it is nul-terminated, but
>> then according to the longer discussion on linux.kernel.api
>> it sounds like it isn't.  Which is it?
>
> When struct sockaddr_un is passed to linux kernel, the kernel doesn't
> treat sun_path as nul-terminated, see net/unix/af_unix.c:unix_mkname
> for implementation details.

Thank you.  This is good to know (and expected).  The Glibc
attribute is meant to help with the user side of things so
it's your note below that's especially relevant.  (There
are a few warnings in the Linux kernel suggesting it too
might benefit from the new attribute, if only to suppress
them.)

> However, when linux kernel returns struct sockaddr_un, sun_path is
> nul-terminated if there is enough room provided by userspace, e.g.
> it may need sizeof(struct sockaddr_un) + 1 bytes to write that NUL
> beyond struct sockaddr_un.sun_path.

So if I understand correctly, sun_path will contain a nul-
terminated string if there is enough room for the terminating
nul but not otherwise.  If that's so, that would suggest that
adding attribute nonstring to sum_path might be appropriate.
At the same time, based on existing usage it seems that it
would be prone to false positives.  Annotating sun_path with
it turns up only one warning in a Glibc build:

clnt_unix.c:137:13: warning: ‘strlen’ argument 1 declared attribute 
‘nonstring’ [-Wstringop-overflow=]
        len = strlen (raddr->sun_path) + sizeof (raddr->sun_family) + 1;

AFAICS, the function is only called from clnt_create() where
it's passed a nul-terminated string, so unless it can be called
from user code with a non-nul-terminated array this would be
an example of such a false positive.

Carlos, do you think the attribute would be helpful despite
the false positives?

Martin