This is the mail archive of the 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: [PATCH] avoid buffer overflow in sunrpc clnt_create (BZ #22542)

On 02/07/2018 02:31 PM, Dmitry V. Levin wrote:
On Wed, Feb 07, 2018 at 08:57:46AM -0700, Martin Sebor wrote:
On 02/06/2018 02:06 PM, Dmitry V. Levin wrote:
On Mon, Dec 04, 2017 at 12:04:12AM -0800, Paul Eggert wrote:
Dmitry V. Levin wrote:
Do people really expect that?  Assuming that people are aware
of linux kernel behaviour, why would they expect that?

These days, it's because strncpy format is obsolete and is not something
programmers are ordinarily aware of. When in doubt (which there seems to be
here), glibc should use null-terminated strings rather than strncpy format.

Is there any statistics what programmers are ordinarily aware of?

I have no doubts that some valid code[1] no longer compiles with
-Werror=stringop-truncation, and the only plausible fix is to mark
struct sockaddr_un.sun_path with __attribute_nonstring__.

I think we should revisit the patch submitted by Martin.

[1] strace HEAD's tests no longer build in Fedora Rawhide with the following
net-accept-connect.c: In function ‘main’:
net-accept-connect.c:57:2: error: ‘strncpy’ specified bound 108 equals destination size [-Werror=stringop-truncation]
  strncpy(addr.sun_path, av[1], sizeof(addr.sun_path));

I was going to follow up on the original thread but got stuck
trying to come up with a test case showing the kernel creating
a path with no terminating nul, and I've been too busy with
GCC work to get back to it.

I'm also worried that annotating the member nonstring will
lead to many more false positives for the canonical use case
of computing the path length/size using strlen even on input
(to the kernel/library) than true positives for the elusive
cases when there is no nul on output.  (Attribute nonstring
causes warnings when an array or pointer declared with is
passed to strlen or other functions that expect a nul-
terminated string.)

struct sockaddr_un.sun_path is not a nul-terminated string,
one has to use strnlen instead of strlen.

It may not be guaranteed to be nul-terminated, but it is nul
terminated most of the time (when the path is shorter than
the array), otherwise there wouldn't be so much working code
that depends on it and so much documentation with examples
that assume it is, including Glibc's own manual and the Linux
man page:

Once sun_path is declared nonstring all code that follows
these examples and in which GCC cannot prove the string is
nul-terminated will trigger the same warning.  The only
difference will be that the warning will point to sun_path
being passed to strlen or as the second argument to strcpy.

The question on my mind is: will the true positives/bugs
the warning finds outnumber the false positives by enough
of a margin that the warning will be viewed as useful.
To be able to even start to answer the question I'd need
to see an example where the sun_path isn't nul-terminated
and get a sense of how frequently that might happen in


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