Received-SPF: pass (google.com: domain of linux-nfs+bounces-22-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
MIME-Version: 1.0
From: "NeilBrown" <neilb@suse.de>
To: Ahelenia =?utf-8?q?Ziemia=C5=84ska?= <nabijaczleweli@nabijaczleweli.xyz>
Cc: linux-nfs@vger.kernel.org, Salvatore Bonaccorso <carnil@debian.org>
Subject: Re: [PATCH nfs-utils 1/2] fsidd: call anonymous sockets by their name
 only, don't fill with NULs to 108 bytes
In-reply-to: =?utf-8?q?=3C04f3fe71defa757d518468f04f08334a5d0dfbb9=2E1693754?=
 =?utf-8?q?442=2Egit=2Enabijaczleweli=40nabijaczleweli=2Exyz=3E?=
References: =?utf-8?q?=3C04f3fe71defa757d518468f04f08334a5d0dfbb9=2E16937544?=
 =?utf-8?q?42=2Egit=2Enabijaczleweli=40nabijaczleweli=2Exyz=3E?=
Date: Wed, 22 Nov 2023 07:49:23 +1100
Message-id: <170059976394.7109.12337023467746204342@noble.neil.brown.name>

On Mon, 04 Sep 2023, Ahelenia Ziemia=C5=84ska 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.
>=20
> 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.
>=20
> So just specify the correct address length when we're using the abstract do=
main:
> unix(7) recommends "offsetof(struct sockaddr_un, sun_path) + strlen(sun_pat=
h) + 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-ap=
i-network 18495452                            * 18494406
> u_seq LISTEN    0      5                                             @/run/=
fsid.sock 27168796                            * 0
> u_str ESTAB     0      0                 @ac308f35f50797a2/bus/systemd-logi=
nd/system 19406                               * 15153
> u_str ESTAB     0      0                @b6606e0dfacbae75/bus/systemd/bus-a=
pi-system 18494353                            * 18495334
> u_str ESTAB     0      0                    @5880653d215718a7/bus/systemd/b=
us-system 26930876                            * 26930003
> -- >8 --
>=20
> Fixes: e00ab3c0616fe6d83ab0710d9e7d989c299088f7 ("fsidd: provide
>  better default socket name.")
> Signed-off-by: Ahelenia Ziemia=C5=84ska <nabijaczleweli@nabijaczleweli.xyz>
> ---
>  support/reexport/fsidd.c    | 8 +++++---
>  support/reexport/reexport.c | 7 +++++--
>  2 files changed, 10 insertions(+), 5 deletions(-)
>=20
> diff --git a/support/reexport/fsidd.c b/support/reexport/fsidd.c
> index d4b245e8..4c377415 100644
> --- a/support/reexport/fsidd.c
> +++ b/support/reexport/fsidd.c
> @@ -171,10 +171,12 @@ int main(void)
>  	memset(&addr, 0, sizeof(struct sockaddr_un));
>  	addr.sun_family =3D AF_UNIX;
>  	strncpy(addr.sun_path, sock_file, sizeof(addr.sun_path) - 1);
> -	if (addr.sun_path[0] =3D=3D '@')
> +	socklen_t addr_len =3D sizeof(struct sockaddr_un);

Could you please move the declaration of addr_len up to the top of the
block - for consistency with the rest of the code.

Then resend to the list, and to Steved and me?

Thanks,
NeilBrown


> +	if (addr.sun_path[0] =3D=3D '@') {
>  		/* "abstract" socket namespace */
> +		addr_len =3D offsetof(struct sockaddr_un, sun_path) + strlen(addr.sun_pa=
th);
>  		addr.sun_path[0] =3D 0;
> -	else
> +	} else
>  		unlink(sock_file);
> =20
>  	srv =3D socket(AF_UNIX, SOCK_SEQPACKET | SOCK_NONBLOCK, 0);
> @@ -183,7 +185,7 @@ int main(void)
>  		return 1;
>  	}
> =20
> -	if (bind(srv, (const struct sockaddr *)&addr, sizeof(struct sockaddr_un))=
 =3D=3D -1) {
> +	if (bind(srv, (const struct sockaddr *)&addr, addr_len) =3D=3D -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 d9a700af..b7ee6f46 100644
> --- a/support/reexport/reexport.c
> +++ b/support/reexport/reexport.c
> @@ -40,9 +40,12 @@ static bool connect_fsid_service(void)
>  	memset(&addr, 0, sizeof(struct sockaddr_un));
>  	addr.sun_family =3D AF_UNIX;
>  	strncpy(addr.sun_path, sock_file, sizeof(addr.sun_path) - 1);
> -	if (addr.sun_path[0] =3D=3D '@')
> +	socklen_t addr_len =3D sizeof(struct sockaddr_un);
> +	if (addr.sun_path[0] =3D=3D '@') {
>  		/* "abstract" socket namespace */
> +		addr_len =3D offsetof(struct sockaddr_un, sun_path) + strlen(addr.sun_pa=
th);
>  		addr.sun_path[0] =3D 0;
> +	}
> =20
>  	s =3D socket(AF_UNIX, SOCK_SEQPACKET, 0);
>  	if (s =3D=3D -1) {
> @@ -50,7 +53,7 @@ static bool connect_fsid_service(void)
>  		return false;
>  	}
> =20
> -	ret =3D connect(s, (const struct sockaddr *)&addr, sizeof(struct sockaddr=
_un));
> +	ret =3D connect(s, (const struct sockaddr *)&addr, addr_len);
>  	if (ret =3D=3D -1) {
>  		xlog(L_WARNING, "Unable to connect %s: %m, is fsidd running?\n", sock_fi=
le);
>  		return false;
> --=20
> 2.40.1
>=20
>=20