On Wed, Aug 22, 2018 at 12:39:33PM +0100, Ian Jackson wrote:
> Wei Liu writes ("[PATCH] libxl: make sure string is null-terminated in
> libxl__prepare_sockaddr_un"):
> > Coverity-ID: 1438472
> > Signed-off-by: Wei Liu <wei.l...@citrix.com>
>
> But...
>
> > diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
> > index 5854717b11..e06f765699 100644
> > --- a/tools/libxl/libxl_utils.c
> > +++ b/tools/libxl/libxl_utils.c
> > @@ -1238,6 +1238,8 @@ int libxl__prepare_sockaddr_un(libxl__gc *gc,
> > struct sockaddr_un *un, const char *path,
> > const char *what)
> > {
> > + size_t sz = sizeof(un->sun_path);
> > +
> > if (sizeof(un->sun_path) <= strlen(path)) {
> > LOG(ERROR, "UNIX socket path '%s' is too long for %s", path, what);
> > LOG(DEBUG, "Path must be less than %zu bytes",
> > sizeof(un->sun_path));
> return ERROR_INVAL;
> > @@ -1245,7 +1247,8 @@ int libxl__prepare_sockaddr_un(libxl__gc *gc,
> > }
> > 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, sz);
> > + un->sun_path[sz - 1] = '\0';
>
> If we have reached this point, sizeof(un->sun_path) > strlen(path).
> So in fact, strcpy would do. strncpy will always add a nul.
>
> We just memset the whole struct to 0.
>
> If this new code has any effect at all, it will corrupt the string by
> truncating it.
Corrupt? Per your analysis above, isn't the last byte always going to be
nul? This change is to placate coverity more than anything else.
Isn't setting the last byte to 0 a common pattern to ensure a
string is null-terminated?
Wei.
>
> Ian.
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
