* [PATCH nfs-utils v2 1/2] fsidd: call anonymous sockets by their name only, don't fill with NULs to 108 bytes
@ 2023-11-21 21:14 Ahelenia Ziemiańska
2023-11-21 21:15 ` [PATCH nfs-utils v2 2/2] testlk: format off_t as llong instead of ssize_t Ahelenia Ziemiańska
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Ahelenia Ziemiańska @ 2023-11-21 21:14 UTC (permalink / raw)
To: linux-nfs, NeilBrown, Steve Dickson
[-- Attachment #1: Type: text/plain, Size: 4726 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 <nabijaczleweli@nabijaczleweli.xyz>
---
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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH nfs-utils v2 2/2] testlk: format off_t as llong instead of ssize_t
2023-11-21 21:14 [PATCH nfs-utils v2 1/2] fsidd: call anonymous sockets by their name only, don't fill with NULs to 108 bytes Ahelenia Ziemiańska
@ 2023-11-21 21:15 ` Ahelenia Ziemiańska
2023-11-29 12:34 ` Steve Dickson
2023-11-29 12:34 ` [PATCH nfs-utils v2 1/2] fsidd: call anonymous sockets by their name only, don't fill with NULs to 108 bytes Steve Dickson
2023-12-06 21:33 ` Olga Kornievskaia
2 siblings, 1 reply; 8+ messages in thread
From: Ahelenia Ziemiańska @ 2023-11-21 21:15 UTC (permalink / raw)
To: linux-nfs, NeilBrown, Steve Dickson
[-- Attachment #1: Type: text/plain, Size: 3469 bytes --]
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 <nabijaczleweli@nabijaczleweli.xyz>
---
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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH nfs-utils v2 2/2] testlk: format off_t as llong instead of ssize_t
2023-11-21 21:15 ` [PATCH nfs-utils v2 2/2] testlk: format off_t as llong instead of ssize_t Ahelenia Ziemiańska
@ 2023-11-29 12:34 ` Steve Dickson
0 siblings, 0 replies; 8+ messages in thread
From: Steve Dickson @ 2023-11-29 12:34 UTC (permalink / raw)
To: Ahelenia Ziemiańska, linux-nfs, NeilBrown
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 <nabijaczleweli@nabijaczleweli.xyz>
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;
> }
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH nfs-utils v2 1/2] fsidd: call anonymous sockets by their name only, don't fill with NULs to 108 bytes
2023-11-21 21:14 [PATCH nfs-utils v2 1/2] fsidd: call anonymous sockets by their name only, don't fill with NULs to 108 bytes Ahelenia Ziemiańska
2023-11-21 21:15 ` [PATCH nfs-utils v2 2/2] testlk: format off_t as llong instead of ssize_t Ahelenia Ziemiańska
@ 2023-11-29 12:34 ` Steve Dickson
2023-12-06 21:33 ` Olga Kornievskaia
2 siblings, 0 replies; 8+ messages in thread
From: Steve Dickson @ 2023-11-29 12:34 UTC (permalink / raw)
To: Ahelenia Ziemiańska, linux-nfs, NeilBrown
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 <nabijaczleweli@nabijaczleweli.xyz>
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;
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH nfs-utils v2 1/2] fsidd: call anonymous sockets by their name only, don't fill with NULs to 108 bytes
2023-11-21 21:14 [PATCH nfs-utils v2 1/2] fsidd: call anonymous sockets by their name only, don't fill with NULs to 108 bytes Ahelenia Ziemiańska
2023-11-21 21:15 ` [PATCH nfs-utils v2 2/2] testlk: format off_t as llong instead of ssize_t Ahelenia Ziemiańska
2023-11-29 12:34 ` [PATCH nfs-utils v2 1/2] fsidd: call anonymous sockets by their name only, don't fill with NULs to 108 bytes Steve Dickson
@ 2023-12-06 21:33 ` Olga Kornievskaia
2023-12-08 0:08 ` NeilBrown
2 siblings, 1 reply; 8+ messages in thread
From: Olga Kornievskaia @ 2023-12-06 21:33 UTC (permalink / raw)
To: Ahelenia Ziemiańska; +Cc: linux-nfs, NeilBrown, Steve Dickson
On Tue, Nov 21, 2023 at 4:15 PM Ahelenia Ziemiańska
<nabijaczleweli@nabijaczleweli.xyz> 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@nabijaczleweli.xyz>
> ---
> 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'
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH nfs-utils v2 1/2] fsidd: call anonymous sockets by their name only, don't fill with NULs to 108 bytes
2023-12-06 21:33 ` Olga Kornievskaia
@ 2023-12-08 0:08 ` NeilBrown
2023-12-08 18:55 ` Olga Kornievskaia
0 siblings, 1 reply; 8+ messages in thread
From: NeilBrown @ 2023-12-08 0:08 UTC (permalink / raw)
To: Olga Kornievskaia; +Cc: Ahelenia Ziemiańska, linux-nfs, Steve Dickson
On Thu, 07 Dec 2023, Olga Kornievskaia wrote:
> On Tue, Nov 21, 2023 at 4:15 PM Ahelenia Ziemiańska
> <nabijaczleweli@nabijaczleweli.xyz> 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@nabijaczleweli.xyz>
> > ---
> > 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
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH nfs-utils v2 1/2] fsidd: call anonymous sockets by their name only, don't fill with NULs to 108 bytes
2023-12-08 0:08 ` NeilBrown
@ 2023-12-08 18:55 ` Olga Kornievskaia
0 siblings, 0 replies; 8+ messages in thread
From: Olga Kornievskaia @ 2023-12-08 18:55 UTC (permalink / raw)
To: NeilBrown; +Cc: Ahelenia Ziemiańska, linux-nfs, Steve Dickson
On Thu, Dec 7, 2023 at 7:08 PM NeilBrown <neilb@suse.de> wrote:
>
> On Thu, 07 Dec 2023, Olga Kornievskaia wrote:
> > On Tue, Nov 21, 2023 at 4:15 PM Ahelenia Ziemiańska
> > <nabijaczleweli@nabijaczleweli.xyz> 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@nabijaczleweli.xyz>
> > > ---
> > > 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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH nfs-utils v2 1/2] fsidd: call anonymous sockets by their name only, don't fill with NULs to 108 bytes
@ 2023-11-21 21:17 NeilBrown
0 siblings, 0 replies; 8+ messages in thread
From: NeilBrown @ 2023-11-21 21:17 UTC (permalink / raw)
To: Ahelenia Ziemiańska; +Cc: linux-nfs, Steve Dickson
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 <nabijaczleweli@nabijaczleweli.xyz>
> ---
> 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 <neilb@suse.de>
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
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-12-08 18:55 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-11-21 21:14 [PATCH nfs-utils v2 1/2] fsidd: call anonymous sockets by their name only, don't fill with NULs to 108 bytes Ahelenia Ziemiańska
2023-11-21 21:15 ` [PATCH nfs-utils v2 2/2] testlk: format off_t as llong instead of ssize_t Ahelenia Ziemiańska
2023-11-29 12:34 ` Steve Dickson
2023-11-29 12:34 ` [PATCH nfs-utils v2 1/2] fsidd: call anonymous sockets by their name only, don't fill with NULs to 108 bytes Steve Dickson
2023-12-06 21:33 ` Olga Kornievskaia
2023-12-08 0:08 ` NeilBrown
2023-12-08 18:55 ` Olga Kornievskaia
-- strict thread matches above, loose matches on Subject: below --
2023-11-21 21:17 NeilBrown
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox