Re: [PATCH 2/2] libxl: fix -Werror=stringop-truncation in libxl__prepare_sockaddr_un

[Ian Jackson <ian.jackson@xxxxxxxxxx>]

Marek Marczykowski-G??recki writes ("Re: [PATCH 2/2] libxl: fix 
-Werror=stringop-truncation in libxl__prepare_sockaddr_un"):
> On Tue, Aug 18, 2020 at 08:43:56PM -0700, Elliott Mitchell wrote:
> > On Wed, Aug 19, 2020 at 04:00:36AM +0200, Marek Marczykowski-G??recki wrote:
> > > diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
> > > index f360f5e228..b039143b8a 100644
> > > --- a/tools/libxl/libxl_utils.c
> > > +++ b/tools/libxl/libxl_utils.c
> > 
> > 
> > >      }
> > >      memset(un, 0, sizeof(struct sockaddr_un));
> > >      un->sun_family = AF_UNIX;
> > > -    strncpy(un->sun_path, path, sizeof(un->sun_path));
> > > +    strncpy(un->sun_path, path, sizeof(un->sun_path) - 1);
> > >      return 0;
> > >  }
> > 
> > While the earlier lines are okay, this line introduces an error.  
> 
> Why exactly? strncpy() copies up to n characters, quoting its manual
> page:
> 
>     If there is no null byte among the first n bytes of src, the string
>     placed in dest will not be null-terminated
> 
> But since the whole struct is zeroed out initially, this should still
> result in a null terminated string, as the last byte of that buffer will
> not be touched by the strncpy.

Everyone here so far, including the compiler, seems to be assuming
that sun_path must be nul-terminated.  But that is not strictly
correct.  So the old code is not buggy and the compiler is wrong.

Some systems insist on sun_path being nul-terminated, but I don't
think that includes any we care about.  AFAICT from the manpage
FreeBSD doesn't and uses a variable socklen for AF_UNIX.

OTOH I don't think there is much benefit in the additional byte so I
don't mind if we take some version of these changes.

I think Marek is right that his patch does leave sun_path
nul-terminated, so, for that original patch:

Reviewed-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>

Thanks,
Ian.