2023-11-21 21:15:15

by Ahelenia Ziemiańska

[permalink] [raw]
Subject: [PATCH nfs-utils v2 1/2] fsidd: call anonymous sockets by their name only, don't fill with NULs to 108 bytes

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 <[email protected]>
---
v1: <04f3fe71defa757d518468f04f08334a5d0dfbb9.1693754442.git.nabijaczleweli@nabijaczleweli.xyz>
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

Attachments:
(No filename) (4.62 kB)
 signature.asc (849.00 B)
 Download all attachments

2023-11-21 21:15:20

by Ahelenia Ziemiańska

[permalink] [raw]
Subject: [PATCH nfs-utils v2 2/2] testlk: format off_t as llong instead of ssize_t

This, naturally, produces a warning on x32 (and other ILP32 platforms
with 64-bit off_t, presumably, but you need to ask for it explicitly
there usually):
gcc -DHAVE_CONFIG_H -I. -I../../support/include -D_GNU_SOURCE -Wdate-time -D_FORTIFY_SOURCE=2 -D_GNU_SOURCE -g -O2 -ffile-prefix-map=/tmp/nfs-utils-2.6.3=. -specs=/usr/share/dpkg/pie-compile.specs -fstack-protector-strong -Wformat -Werror=format-security -g -O2 -ffile-prefix-map=/tmp/nfs-utils-2.6.3=. -specs=/usr/share/dpkg/pie-compile.specs -fstack-protector-strong -Wformat -Werror=format-security -c -o testlk-testlk.o `test -f 'testlk.c' || echo './'`testlk.c
testlk.c: In function ‘main’:
testlk.c:84:66: warning: format ‘%zd’ expects argument of type ‘signed size_t’, but argument 4 has type ‘__off_t’ {aka ‘long long int’} [-Wformat=]
84 | printf("%s: conflicting lock by %d on (%zd;%zd)\n",
| ~~^
| |
| int
| %lld
85 | fname, fl.l_pid, fl.l_start, fl.l_len);
| ~~~~~~~~~~
| |
| __off_t {aka long long int}
testlk.c:84:70: warning: format ‘%zd’ expects argument of type ‘signed size_t’, but argument 5 has type ‘__off_t’ {aka ‘long long int’} [-Wformat=]
84 | printf("%s: conflicting lock by %d on (%zd;%zd)\n",
| ~~^
| |
| int
| %lld
85 | fname, fl.l_pid, fl.l_start, fl.l_len);
| ~~~~~~~~
| |
| __off_t {aka long long int}

Upcast to long long, doesn't really matter.

It does, of course, raise the question of whether other bits of
nfs-utils do something equally broken that just isn't caught by the
format validator.

Signed-off-by: Ahelenia Ziemiańska <[email protected]>
---
Same as v1: <44adec629186e220ee5d8fd936980ac4a33dc510.1693754442.git.nabijaczleweli@nabijaczleweli.xyz>

tools/locktest/testlk.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/locktest/testlk.c b/tools/locktest/testlk.c
index ea51f788..c9bd6bac 100644
--- a/tools/locktest/testlk.c
+++ b/tools/locktest/testlk.c
@@ -81,8 +81,8 @@ main(int argc, char **argv)
if (fl.l_type == F_UNLCK) {
printf("%s: no conflicting lock\n", fname);
} else {
- printf("%s: conflicting lock by %d on (%zd;%zd)\n",
- fname, fl.l_pid, fl.l_start, fl.l_len);
+ printf("%s: conflicting lock by %d on (%lld;%lld)\n",
+ fname, fl.l_pid, (long long)fl.l_start, (long long)fl.l_len);
}
return 0;
}
--
2.39.2

Attachments:
(No filename) (3.39 kB)
 signature.asc (849.00 B)
 Download all attachments

2023-11-21 21:18:26

by NeilBrown

[permalink] [raw]
Subject: Re: [PATCH nfs-utils v2 1/2] fsidd: call anonymous sockets by their name only, don't fill with NULs to 108 bytes

On Wed, 22 Nov 2023, Ahelenia Ziemiańska 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 <[email protected]>
> ---
> v1: <04f3fe71defa757d518468f04f08334a5d0dfbb9.1693754442.git.nabijaczleweli@nabijaczleweli.xyz>
> 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(-)


Reviewed-by: NeilBrown <[email protected]>

Thanks,
NeilBrown


>
> 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
>
>


2023-11-21 21:19:40

by NeilBrown

[permalink] [raw]
Subject: Re: [PATCH nfs-utils v2 2/2] testlk: format off_t as llong instead of ssize_t

On Wed, 22 Nov 2023, Ahelenia Ziemiańska wrote:
> This, naturally, produces a warning on x32 (and other ILP32 platforms
> with 64-bit off_t, presumably, but you need to ask for it explicitly
> there usually):
> gcc -DHAVE_CONFIG_H -I. -I../../support/include -D_GNU_SOURCE -Wdate-time -D_FORTIFY_SOURCE=2 -D_GNU_SOURCE -g -O2 -ffile-prefix-map=/tmp/nfs-utils-2.6.3=. -specs=/usr/share/dpkg/pie-compile.specs -fstack-protector-strong -Wformat -Werror=format-security -g -O2 -ffile-prefix-map=/tmp/nfs-utils-2.6.3=. -specs=/usr/share/dpkg/pie-compile.specs -fstack-protector-strong -Wformat -Werror=format-security -c -o testlk-testlk.o `test -f 'testlk.c' || echo './'`testlk.c
> testlk.c: In function ‘main’:
> testlk.c:84:66: warning: format ‘%zd’ expects argument of type ‘signed size_t’, but argument 4 has type ‘__off_t’ {aka ‘long long int’} [-Wformat=]
> 84 | printf("%s: conflicting lock by %d on (%zd;%zd)\n",
> | ~~^
> | |
> | int
> | %lld
> 85 | fname, fl.l_pid, fl.l_start, fl.l_len);
> | ~~~~~~~~~~
> | |
> | __off_t {aka long long int}
> testlk.c:84:70: warning: format ‘%zd’ expects argument of type ‘signed size_t’, but argument 5 has type ‘__off_t’ {aka ‘long long int’} [-Wformat=]
> 84 | printf("%s: conflicting lock by %d on (%zd;%zd)\n",
> | ~~^
> | |
> | int
> | %lld
> 85 | fname, fl.l_pid, fl.l_start, fl.l_len);
> | ~~~~~~~~
> | |
> | __off_t {aka long long int}
>
> Upcast to long long, doesn't really matter.
>
> It does, of course, raise the question of whether other bits of
> nfs-utils do something equally broken that just isn't caught by the
> format validator.
>
> Signed-off-by: Ahelenia Ziemiańska <[email protected]>
> ---
> Same as v1: <44adec629186e220ee5d8fd936980ac4a33dc510.1693754442.git.nabijaczleweli@nabijaczleweli.xyz>
>
> tools/locktest/testlk.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/locktest/testlk.c b/tools/locktest/testlk.c
> index ea51f788..c9bd6bac 100644
> --- a/tools/locktest/testlk.c
> +++ b/tools/locktest/testlk.c
> @@ -81,8 +81,8 @@ main(int argc, char **argv)
> if (fl.l_type == F_UNLCK) {
> printf("%s: no conflicting lock\n", fname);
> } else {
> - printf("%s: conflicting lock by %d on (%zd;%zd)\n",
> - fname, fl.l_pid, fl.l_start, fl.l_len);
> + printf("%s: conflicting lock by %d on (%lld;%lld)\n",
> + fname, fl.l_pid, (long long)fl.l_start, (long long)fl.l_len);
> }
> return 0;
> }
> --
> 2.39.2
>

Reviewed-by: NeilBrown <[email protected]>

Thanks,
NeilBrown


2023-11-29 12:34:43

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH nfs-utils v2 1/2] fsidd: call anonymous sockets by their name only, don't fill with NULs to 108 bytes



On 11/21/23 4:14 PM, Ahelenia Ziemiańska 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 <[email protected]>
Committed... (tag: nfs-utils-2-7-1-rc1)

steved
> ---
> v1: <04f3fe71defa757d518468f04f08334a5d0dfbb9.1693754442.git.nabijaczleweli@nabijaczleweli.xyz>
> 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;


2023-11-29 12:35:07

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH nfs-utils v2 2/2] testlk: format off_t as llong instead of ssize_t



On 11/21/23 4:15 PM, Ahelenia Ziemiańska wrote:
> This, naturally, produces a warning on x32 (and other ILP32 platforms
> with 64-bit off_t, presumably, but you need to ask for it explicitly
> there usually):
> gcc -DHAVE_CONFIG_H -I. -I../../support/include -D_GNU_SOURCE -Wdate-time -D_FORTIFY_SOURCE=2 -D_GNU_SOURCE -g -O2 -ffile-prefix-map=/tmp/nfs-utils-2.6.3=. -specs=/usr/share/dpkg/pie-compile.specs -fstack-protector-strong -Wformat -Werror=format-security -g -O2 -ffile-prefix-map=/tmp/nfs-utils-2.6.3=. -specs=/usr/share/dpkg/pie-compile.specs -fstack-protector-strong -Wformat -Werror=format-security -c -o testlk-testlk.o `test -f 'testlk.c' || echo './'`testlk.c
> testlk.c: In function ‘main’:
> testlk.c:84:66: warning: format ‘%zd’ expects argument of type ‘signed size_t’, but argument 4 has type ‘__off_t’ {aka ‘long long int’} [-Wformat=]
> 84 | printf("%s: conflicting lock by %d on (%zd;%zd)\n",
> | ~~^
> | |
> | int
> | %lld
> 85 | fname, fl.l_pid, fl.l_start, fl.l_len);
> | ~~~~~~~~~~
> | |
> | __off_t {aka long long int}
> testlk.c:84:70: warning: format ‘%zd’ expects argument of type ‘signed size_t’, but argument 5 has type ‘__off_t’ {aka ‘long long int’} [-Wformat=]
> 84 | printf("%s: conflicting lock by %d on (%zd;%zd)\n",
> | ~~^
> | |
> | int
> | %lld
> 85 | fname, fl.l_pid, fl.l_start, fl.l_len);
> | ~~~~~~~~
> | |
> | __off_t {aka long long int}
>
> Upcast to long long, doesn't really matter.
>
> It does, of course, raise the question of whether other bits of
> nfs-utils do something equally broken that just isn't caught by the
> format validator.
>
> Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Committed... (tag: nfs-utils-2-7-1-rc1)

steved
> ---
> Same as v1: <44adec629186e220ee5d8fd936980ac4a33dc510.1693754442.git.nabijaczleweli@nabijaczleweli.xyz>
>
> tools/locktest/testlk.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/locktest/testlk.c b/tools/locktest/testlk.c
> index ea51f788..c9bd6bac 100644
> --- a/tools/locktest/testlk.c
> +++ b/tools/locktest/testlk.c
> @@ -81,8 +81,8 @@ main(int argc, char **argv)
> if (fl.l_type == F_UNLCK) {
> printf("%s: no conflicting lock\n", fname);
> } else {
> - printf("%s: conflicting lock by %d on (%zd;%zd)\n",
> - fname, fl.l_pid, fl.l_start, fl.l_len);
> + printf("%s: conflicting lock by %d on (%lld;%lld)\n",
> + fname, fl.l_pid, (long long)fl.l_start, (long long)fl.l_len);
> }
> return 0;
> }


2023-12-06 21:34:11

by Olga Kornievskaia

[permalink] [raw]
Subject: Re: [PATCH nfs-utils v2 1/2] fsidd: call anonymous sockets by their name only, don't fill with NULs to 108 bytes

On Tue, Nov 21, 2023 at 4:15 PM Ahelenia Ziemiańska
<[email protected]> 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 <[email protected]>
> ---
> v1: <04f3fe71defa757d518468f04f08334a5d0dfbb9.1693754442.git.nabijaczleweli@nabijaczleweli.xyz>
> 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'

2023-12-08 00:08:53

by NeilBrown

[permalink] [raw]
Subject: Re: [PATCH nfs-utils v2 1/2] fsidd: call anonymous sockets by their name only, don't fill with NULs to 108 bytes

On Thu, 07 Dec 2023, Olga Kornievskaia wrote:
> On Tue, Nov 21, 2023 at 4:15 PM Ahelenia Ziemiańska
> <[email protected]> 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 <[email protected]>
> > ---
> > v1: <04f3fe71defa757d518468f04f08334a5d0dfbb9.1693754442.git.nabijaczleweli@nabijaczleweli.xyz>
> > 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

2023-12-08 18:55:25

by Olga Kornievskaia

[permalink] [raw]
Subject: Re: [PATCH nfs-utils v2 1/2] fsidd: call anonymous sockets by their name only, don't fill with NULs to 108 bytes

On Thu, Dec 7, 2023 at 7:08 PM NeilBrown <[email protected]> wrote:
>
> On Thu, 07 Dec 2023, Olga Kornievskaia wrote:
> > On Tue, Nov 21, 2023 at 4:15 PM Ahelenia Ziemiańska
> > <[email protected]> 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 <[email protected]>
> > > ---
> > > v1: <04f3fe71defa757d518468f04f08334a5d0dfbb9.1693754442.git.nabijaczleweli@nabijaczleweli.xyz>
> > > 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.

Hm. ok once I added to reexport.c things recompiled for me. so my
patch only covered that.

I can write a v2 of the patch I submitted to include both.

>
> Thanks!
>
> NeilBrown