On Thu, 07 Dec 2023, Olga Kornievskaia wrote:
> On Tue, Nov 21, 2023 at 4:15 PM Ahelenia Ziemiańska
> <nabijaczleweli@xxxxxxxxxxxxxxxxxx> wrote:
> >
> > Since e00ab3c0616fe6d83ab0710d9e7d989c299088f7, ss -l looks like this:
> > u_seq LISTEN 0 5 @/run/fsid.sock@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@ 26989379 * 0
> > with fsidd pushing all the addresses to 108 bytes wide, which is deeply
> > egregious if you don't filter it out and recolumnate.
> >
> > This is because, naturally (unix(7)), "Null bytes in the name have
> > no special significance": abstract addresses are binary blobs, but
> > paths automatically terminate at the first NUL byte, since paths
> > can't contain those.
> >
> > So just specify the correct address length when we're using the abstract domain:
> > unix(7) recommends "offsetof(struct sockaddr_un, sun_path) + strlen(sun_path) + 1"
> > for paths, but we don't want to include the terminating NUL, so it's just
> > "offsetof(struct sockaddr_un, sun_path) + strlen(sun_path)".
> > This brings the width back to order:
> > -- >8 --
> > $ ss -la | grep @
> > u_str ESTAB 0 0 @45208536ec96909a/bus/systemd-timesyn/bus-api-timesync 18500238 * 18501249
> > u_str ESTAB 0 0 @fecc9657d2315eb7/bus/systemd-network/bus-api-network 18495452 * 18494406
> > u_seq LISTEN 0 5 @/run/fsid.sock 27168796 * 0
> > u_str ESTAB 0 0 @ac308f35f50797a2/bus/systemd-logind/system 19406 * 15153
> > u_str ESTAB 0 0 @b6606e0dfacbae75/bus/systemd/bus-api-system 18494353 * 18495334
> > u_str ESTAB 0 0 @5880653d215718a7/bus/systemd/bus-system 26930876 * 26930003
> > -- >8 --
> >
> > Fixes: e00ab3c0616fe6d83ab0710d9e7d989c299088f7 ("fsidd: provide
> > better default socket name.")
> > Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@xxxxxxxxxxxxxxxxxx>
> > ---
> > v1: <04f3fe71defa757d518468f04f08334a5d0dfbb9.1693754442.git.nabijaczleweli@xxxxxxxxxxxxxxxxxx>
> > v2 NFC, addr_len declared at top of function
> >
> > support/reexport/fsidd.c | 9 ++++++---
> > support/reexport/reexport.c | 8 ++++++--
> > 2 files changed, 12 insertions(+), 5 deletions(-)
> >
> > diff --git a/support/reexport/fsidd.c b/support/reexport/fsidd.c
> > index 3e62b3fc..8a70b78f 100644
> > --- a/support/reexport/fsidd.c
> > +++ b/support/reexport/fsidd.c
> > @@ -147,6 +147,7 @@ int main(void)
> > {
> > struct event *srv_ev;
> > struct sockaddr_un addr;
> > + socklen_t addr_len;
> > char *sock_file;
> > int srv;
> >
> > @@ -161,10 +162,12 @@ int main(void)
> > memset(&addr, 0, sizeof(struct sockaddr_un));
> > addr.sun_family = AF_UNIX;
> > strncpy(addr.sun_path, sock_file, sizeof(addr.sun_path) - 1);
> > - if (addr.sun_path[0] == '@')
> > + addr_len = sizeof(struct sockaddr_un);
> > + if (addr.sun_path[0] == '@') {
> > /* "abstract" socket namespace */
> > + addr_len = offsetof(struct sockaddr_un, sun_path) + strlen(addr.sun_path);
> > addr.sun_path[0] = 0;
> > - else
> > + } else
> > unlink(sock_file);
> >
> > srv = socket(AF_UNIX, SOCK_SEQPACKET | SOCK_NONBLOCK, 0);
> > @@ -173,7 +176,7 @@ int main(void)
> > return 1;
> > }
> >
> > - if (bind(srv, (const struct sockaddr *)&addr, sizeof(struct sockaddr_un)) == -1) {
> > + if (bind(srv, (const struct sockaddr *)&addr, addr_len) == -1) {
> > xlog(L_WARNING, "Unable to bind %s: %m\n", sock_file);
> > return 1;
> > }
> > diff --git a/support/reexport/reexport.c b/support/reexport/reexport.c
> > index 78516586..0fb49a46 100644
> > --- a/support/reexport/reexport.c
> > +++ b/support/reexport/reexport.c
> > @@ -21,6 +21,7 @@ static int fsidd_srv = -1;
> > static bool connect_fsid_service(void)
> > {
> > struct sockaddr_un addr;
> > + socklen_t addr_len;
> > char *sock_file;
> > int ret;
> > int s;
> > @@ -33,9 +34,12 @@ static bool connect_fsid_service(void)
> > memset(&addr, 0, sizeof(struct sockaddr_un));
> > addr.sun_family = AF_UNIX;
> > strncpy(addr.sun_path, sock_file, sizeof(addr.sun_path) - 1);
> > - if (addr.sun_path[0] == '@')
> > + addr_len = sizeof(struct sockaddr_un);
> > + if (addr.sun_path[0] == '@') {
> > /* "abstract" socket namespace */
> > + addr_len = offsetof(struct sockaddr_un, sun_path) + strlen(addr.sun_path);
> > addr.sun_path[0] = 0;
> > + }
> >
> > s = socket(AF_UNIX, SOCK_SEQPACKET, 0);
> > if (s == -1) {
> > @@ -43,7 +47,7 @@ static bool connect_fsid_service(void)
> > return false;
> > }
> >
> > - ret = connect(s, (const struct sockaddr *)&addr, sizeof(struct sockaddr_un));
> > + ret = connect(s, (const struct sockaddr *)&addr, addr_len);
> > if (ret == -1) {
> > xlog(L_WARNING, "Unable to connect %s: %m, is fsidd running?\n", sock_file);
> > return false;
> > --
> > 2.39.2
> >
>
> Hi folks,
>
> I'm hitting the following compile error (in RHEL9.2). I believe the
> code is missing an include for stddef.h. When I add it, things
> compile. I'll submit a patch.
>
> gcc -DHAVE_CONFIG_H -I. -I../../support/include -I/usr/include/tirpc
> -D_GNU_SOURCE -pipe -Wall -Wextra -Werror=strict-prototypes
> -Werror=missing-prototypes -Werror=missing-declarations
> -Werror=format=2 -Werror=undef -Werror=missing-include-dirs
> -Werror=strict-aliasing=2 -Werror=init-self
> -Werror=implicit-function-declaration -Werror=return-type
> -Werror=switch -Werror=overflow -Werror=parentheses
> -Werror=aggregate-return -Werror=unused-result -fno-strict-aliasing
> -Werror=format-overflow=2 -Werror=int-conversion
> -Werror=incompatible-pointer-types -Werror=misleading-indentation
> -Wno-cast-function-type -g -O2 -MT reexport.o -MD -MP -MF
> .deps/reexport.Tpo -c -o reexport.o reexport.c
> reexport.c: In function ‘connect_fsid_service’:
> reexport.c:40:28: error: implicit declaration of function ‘offsetof’
> [-Werror=implicit-function-declaration]
> 40 | addr_len = offsetof(struct sockaddr_un,
> sun_path) + strlen(addr.sun_path);
> | ^~~~~~~~
> reexport.c:18:1: note: ‘offsetof’ is defined in header ‘<stddef.h>’;
> did you forget to ‘#include <stddef.h>’?
> 17 | #include "xlog.h"
> +++ |+#include <stddef.h>
> 18 |
> reexport.c:40:37: error: expected expression before ‘struct’
> 40 | addr_len = offsetof(struct sockaddr_un,
> sun_path) + strlen(addr.sun_path);
> | ^~~~~~
> cc1: some warnings being treated as errors
> make[2]: *** [Makefile:529: reexport.o] Error 1
> make[2]: Leaving directory '/home/aglo/nfs-utils/support/reexport'
> make[1]: *** [Makefile:448: all-recursive] Error 1
> make[1]: Leaving directory '/home/aglo/nfs-utils/support'
>
Interesting - in openSUSE, dlfcn.h includes stddef.h, which is why the
compile works for me. Obviously we cannot rely on that.
reexport.c and fsidd.c will both need stddef.
Thanks!
NeilBrown
