Re: [PATCH] avoid buffer overflow in sunrpc clnt_create (BZ #22542)

["Dmitry V. Levin" <ldv at altlinux dot org>]

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
> > diagnostics:
> > 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.


-- 
ldv

Attachment: signature.asc
Description: PGP signature