From 994b9d4e90ef0dcfa3697d8e6d5ca51372f946bd Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 4 Dec 2018 18:20:12 +0100 Subject: [PATCH 1/3] socket-util: shorten code a bit --- src/basic/socket-util.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/basic/socket-util.c b/src/basic/socket-util.c index 9bae1552fc..104d6df495 100644 --- a/src/basic/socket-util.c +++ b/src/basic/socket-util.c @@ -651,14 +651,10 @@ int sockaddr_pretty(const struct sockaddr *_sa, socklen_t salen, bool translate_ case AF_UNIX: if (salen <= offsetof(struct sockaddr_un, sun_path) || - (sa->un.sun_path[0] == 0 && salen == offsetof(struct sockaddr_un, sun_path) + 1)) { + (sa->un.sun_path[0] == 0 && salen == offsetof(struct sockaddr_un, sun_path) + 1)) /* The name must have at least one character (and the leading NUL does not count) */ - p = strdup(""); - if (!p) - return -ENOMEM; - - } else { + else { size_t path_len = salen - offsetof(struct sockaddr_un, sun_path); if (sa->un.sun_path[0] == 0) { @@ -681,9 +677,9 @@ int sockaddr_pretty(const struct sockaddr *_sa, socklen_t salen, bool translate_ p = cescape_length(sa->un.sun_path, path_len); } - if (!p) - return -ENOMEM; } + if (!p) + return -ENOMEM; break; From 836f9cfebd63aa4285c005a3ff1bd59b3315eaac Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 4 Dec 2018 18:20:48 +0100 Subject: [PATCH 2/3] socket-util: break lines in sockaddr_pretty() signature It's soo long, let's make this more readable. --- src/basic/socket-util.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/basic/socket-util.c b/src/basic/socket-util.c index 104d6df495..9e70cc8c36 100644 --- a/src/basic/socket-util.c +++ b/src/basic/socket-util.c @@ -579,7 +579,13 @@ int sockaddr_port(const struct sockaddr *_sa, unsigned *ret_port) { } } -int sockaddr_pretty(const struct sockaddr *_sa, socklen_t salen, bool translate_ipv6, bool include_port, char **ret) { +int sockaddr_pretty( + const struct sockaddr *_sa, + socklen_t salen, + bool translate_ipv6, + bool include_port, + char **ret) { + union sockaddr_union *sa = (union sockaddr_union*) _sa; char *p; int r; From 085b39e9df957b4a0ae7276d22011ac5dd20bc06 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 4 Dec 2018 18:21:25 +0100 Subject: [PATCH 3/3] socket-util: Let's trick out ubsan when it validate struct sockaddr_un.sun_path[] Linux is stupid and sometimes returns a "struct sockaddr_un" that is longer than its fields, as it NUL terminates .sun_path[] even if it has full length. ubsan detects this, rightfully. Since this is a Linux misdesign let's trick out ubsan a bit. Fixes: #11024 --- src/basic/socket-util.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/basic/socket-util.c b/src/basic/socket-util.c index 9e70cc8c36..91bf801cdf 100644 --- a/src/basic/socket-util.c +++ b/src/basic/socket-util.c @@ -661,9 +661,14 @@ int sockaddr_pretty( /* The name must have at least one character (and the leading NUL does not count) */ p = strdup(""); else { + /* Note that we calculate the path pointer here through the .un_buffer[] field, in order to + * outtrick bounds checking tools such as ubsan, which are too smart for their own good: on + * Linux the kernel may return sun_path[] data one byte longer than the declared size of the + * field. */ + char *path = (char*) sa->un_buffer + offsetof(struct sockaddr_un, sun_path); size_t path_len = salen - offsetof(struct sockaddr_un, sun_path); - if (sa->un.sun_path[0] == 0) { + if (path[0] == 0) { /* Abstract socket. When parsing address information from, we * explicitly reject overly long paths and paths with embedded NULs. * But we might get such a socket from the outside. Let's return @@ -671,17 +676,17 @@ int sockaddr_pretty( _cleanup_free_ char *e = NULL; - e = cescape_length(sa->un.sun_path + 1, path_len - 1); + e = cescape_length(path + 1, path_len - 1); if (!e) return -ENOMEM; p = strjoin("@", e); } else { - if (sa->un.sun_path[path_len - 1] == '\0') + if (path[path_len - 1] == '\0') /* We expect a terminating NUL and don't print it */ path_len --; - p = cescape_length(sa->un.sun_path, path_len); + p = cescape_length(path, path_len); } } if (!p)