| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-id: <172712665050.17050.14126694149839508223@noble.neil.brown.name>
Date: Tue, 24 Sep 2024 07:24:10 +1000
From: "NeilBrown" <neilb@...e.de>
To: "Mirsad Todorovac" <mtodorovac69@...il.com>
Cc: linux-nfs@...r.kernel.org, netdev@...r.kernel.org,
linux-kernel@...r.kernel.org, "Chuck Lever" <chuck.lever@...cle.com>,
"Jeff Layton" <jlayton@...nel.org>, "Olga Kornievskaia" <okorniev@...hat.com>,
"Dai Ngo" <Dai.Ngo@...cle.com>, "Tom Talpey" <tom@...pey.com>,
"Trond Myklebust" <trondmy@...nel.org>, "Anna Schumaker" <anna@...nel.org>,
"David S. Miller" <davem@...emloft.net>, "Eric Dumazet" <edumazet@...gle.com>,
"Jakub Kicinski" <kuba@...nel.org>, "Paolo Abeni" <pabeni@...hat.com>,
"Mirsad Todorovac" <mtodorovac69@...il.com>
Subject: Re: [PATCH v1 1/1] SUNRPC: Make enough room in servername[] for
AF_UNIX addresses
On Tue, 24 Sep 2024, Mirsad Todorovac wrote:
> GCC 13.2.0 reported with W=1 build option the following warning:
See
https://lore.kernel.org/all/20240814093853.48657-1-kunwu.chan@linux.dev/
I don't think anyone really cares about this one.
NeilBrown
>
> net/sunrpc/clnt.c: In function ‘rpc_create’:
> net/sunrpc/clnt.c:582:75: warning: ‘%s’ directive output may be truncated writing up to 107 bytes into \
> a region of size 48 [-Wformat-truncation=]
> 582 | snprintf(servername, sizeof(servername), "%s",
> | ^~
> net/sunrpc/clnt.c:582:33: note: ‘snprintf’ output between 1 and 108 bytes into a destination of size 48
> 582 | snprintf(servername, sizeof(servername), "%s",
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 583 | sun->sun_path);
> | ~~~~~~~~~~~~~~
>
> 548 };
> → 549 char servername[48];
> 550 struct rpc_clnt *clnt;
> 551 int i;
> 552
> 553 if (args->bc_xprt) {
> 554 WARN_ON_ONCE(!(args->protocol & XPRT_TRANSPORT_BC));
> 555 xprt = args->bc_xprt->xpt_bc_xprt;
> 556 if (xprt) {
> 557 xprt_get(xprt);
> 558 return rpc_create_xprt(args, xprt);
> 559 }
> 560 }
> 561
> 562 if (args->flags & RPC_CLNT_CREATE_INFINITE_SLOTS)
> 563 xprtargs.flags |= XPRT_CREATE_INFINITE_SLOTS;
> 564 if (args->flags & RPC_CLNT_CREATE_NO_IDLE_TIMEOUT)
> 565 xprtargs.flags |= XPRT_CREATE_NO_IDLE_TIMEOUT;
> 566 /*
> 567 * If the caller chooses not to specify a hostname, whip
> 568 * up a string representation of the passed-in address.
> 569 */
> 570 if (xprtargs.servername == NULL) {
> 571 struct sockaddr_un *sun =
> 572 (struct sockaddr_un *)args->address;
> 573 struct sockaddr_in *sin =
> 574 (struct sockaddr_in *)args->address;
> 575 struct sockaddr_in6 *sin6 =
> 576 (struct sockaddr_in6 *)args->address;
> 577
> 578 servername[0] = '\0';
> 579 switch (args->address->sa_family) {
> → 580 case AF_LOCAL:
> → 581 if (sun->sun_path[0])
> → 582 snprintf(servername, sizeof(servername), "%s",
> → 583 sun->sun_path);
> → 584 else
> → 585 snprintf(servername, sizeof(servername), "@%s",
> → 586 sun->sun_path+1);
> → 587 break;
> 588 case AF_INET:
> 589 snprintf(servername, sizeof(servername), "%pI4",
> 590 &sin->sin_addr.s_addr);
> 591 break;
> 592 case AF_INET6:
> 593 snprintf(servername, sizeof(servername), "%pI6",
> 594 &sin6->sin6_addr);
> 595 break;
> 596 default:
> 597 /* caller wants default server name, but
> 598 * address family isn't recognized. */
> 599 return ERR_PTR(-EINVAL);
> 600 }
> 601 xprtargs.servername = servername;
> 602 }
> 603
> 604 xprt = xprt_create_transport(&xprtargs);
> 605 if (IS_ERR(xprt))
> 606 return (struct rpc_clnt *)xprt;
>
> After the address family AF_LOCAL was added in the commit 176e21ee2ec89, the old hard-coded
> size for servername of char servername[48] no longer fits. The maximum AF_UNIX address size
> has now grown to UNIX_PATH_MAX defined as 108 in "include/uapi/linux/un.h" .
>
> The lines 580-587 were added later, addressing the leading zero byte '\0', but did not fix
> the hard-coded servername limit.
>
> The AF_UNIX address was truncated to 47 bytes + terminating null byte. This patch will fix the
> servername in AF_UNIX family to the maximum permitted by the system:
>
> 548 };
> → 549 char servername[UNIX_PATH_MAX];
> 550 struct rpc_clnt *clnt;
>
> Fixes: 4388ce05fa38b ("SUNRPC: support abstract unix socket addresses")
> Fixes: 510deb0d7035d ("SUNRPC: rpc_create() default hostname should support AF_INET6 addresses")
> Fixes: 176e21ee2ec89 ("SUNRPC: Support for RPC over AF_LOCAL transports")
> Cc: Neil Brown <neilb@...e.de>
> Cc: Chuck Lever <chuck.lever@...cle.com>
> Cc: Trond Myklebust <trondmy@...nel.org>
> Cc: Anna Schumaker <anna@...nel.org>
> Cc: Jeff Layton <jlayton@...nel.org>
> Cc: Olga Kornievskaia <okorniev@...hat.com>
> Cc: Dai Ngo <Dai.Ngo@...cle.com>
> Cc: Tom Talpey <tom@...pey.com>
> Cc: "David S. Miller" <davem@...emloft.net>
> Cc: Eric Dumazet <edumazet@...gle.com>
> Cc: Jakub Kicinski <kuba@...nel.org>
> Cc: Paolo Abeni <pabeni@...hat.com>
> Cc: linux-nfs@...r.kernel.org
> Cc: netdev@...r.kernel.org
> Cc: linux-kernel@...r.kernel.org
> Signed-off-by: Mirsad Todorovac <mtodorovac69@...il.com>
> ---
> v1:
> initial version.
>
> net/sunrpc/clnt.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
> index 09f29a95f2bc..67099719893e 100644
> --- a/net/sunrpc/clnt.c
> +++ b/net/sunrpc/clnt.c
> @@ -546,7 +546,7 @@ struct rpc_clnt *rpc_create(struct rpc_create_args *args)
> .connect_timeout = args->connect_timeout,
> .reconnect_timeout = args->reconnect_timeout,
> };
> - char servername[48];
> + char servername[UNIX_PATH_MAX];
> struct rpc_clnt *clnt;
> int i;
>
> --
> 2.43.0
>
>
Powered by blists - more mailing lists