This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: nonstrings in Glibc


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]