| Message ID | 20200608182539.29415-1-olaf@aepfle.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] libxl__prepare_sockaddr_un | expand |
Olaf Hering writes ("[PATCH v2] libxl__prepare_sockaddr_un"):
> libxl: remove usage of strncpy from libxl__prepare_sockaddr_un
>
> The runtime check for the length of path already prevents malfunction.
> But gcc does not detect this runtime check and reports incorrect
> usage of strncpy. Remove the usage of strncpy and work directly with
> the calculated path length.
>
> In file included from /usr/include/string.h:495,
> from libxl_internal.h:38,
> from libxl_utils.c:20:
> In function 'strncpy',
> inlined from 'libxl__prepare_sockaddr_un' at libxl_utils.c:1262:5:
> /usr/include/bits/string_fortified.h:106:10: error: '__builtin_strncpy' specified bound 108 equals destination size [-Werror=stringop-truncation]
> 106 | return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> cc1: all warnings being treated as errors
Thanks for working on this.
I found this code a bit confusing:
> - if (sizeof(un->sun_path) <= strlen(path)) {
> + size_t len = strlen(path);
> +
> + if (sizeof(un->sun_path) <= len) {
> 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));
> + LOG(DEBUG, "Path of len %zu must be less than %zu bytes", len, sizeof(un->sun_path));
> return ERROR_INVAL;
> }
> memset(un, 0, sizeof(struct sockaddr_un));
> un->sun_family = AF_UNIX;
> - strncpy(un->sun_path, path, sizeof(un->sun_path));
> + memcpy(un->sun_path, path, len);
This does not copy the trailing nul. The reader must read up to see
the call to memset. Why do you not use strcpy here ?
Nevertheless, as this is a minor point of style,
Reviewed-by: Ian Jackson <ian.jackson@eu.citrix.com>
Ian.
Am Tue, 9 Jun 2020 14:00:31 +0100
schrieb Ian Jackson <ian.jackson@citrix.com>:
> Why do you not use strcpy here ?
Either variant of 'cpy' will work in this context. I decided to use memcpy for no specific reason.
Olaf
diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c index f360f5e228..40885794c9 100644 --- a/tools/libxl/libxl_utils.c +++ b/tools/libxl/libxl_utils.c @@ -1252,14 +1252,16 @@ int libxl__prepare_sockaddr_un(libxl__gc *gc, struct sockaddr_un *un, const char *path, const char *what) { - if (sizeof(un->sun_path) <= strlen(path)) { + size_t len = strlen(path); + + if (sizeof(un->sun_path) <= len) { 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)); + LOG(DEBUG, "Path of len %zu must be less than %zu bytes", len, sizeof(un->sun_path)); return ERROR_INVAL; } memset(un, 0, sizeof(struct sockaddr_un)); un->sun_family = AF_UNIX; - strncpy(un->sun_path, path, sizeof(un->sun_path)); + memcpy(un->sun_path, path, len); return 0; }
libxl: remove usage of strncpy from libxl__prepare_sockaddr_un The runtime check for the length of path already prevents malfunction. But gcc does not detect this runtime check and reports incorrect usage of strncpy. Remove the usage of strncpy and work directly with the calculated path length. In file included from /usr/include/string.h:495, from libxl_internal.h:38, from libxl_utils.c:20: In function 'strncpy', inlined from 'libxl__prepare_sockaddr_un' at libxl_utils.c:1262:5: /usr/include/bits/string_fortified.h:106:10: error: '__builtin_strncpy' specified bound 108 equals destination size [-Werror=stringop-truncation] 106 | return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest)); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ cc1: all warnings being treated as errors Signed-off-by: Olaf Hering <olaf@aepfle.de> --- tools/libxl/libxl_utils.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)