| 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: <b4435709-e112-4667-b458-411856a28389@gmail.com>
Date: Tue, 24 Sep 2024 07:38:21 +0200
From: Mirsad Todorovac <mtodorovac69@...il.com>
To: NeilBrown <neilb@...e.de>
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>
Subject: Re: [PATCH v1 1/1] SUNRPC: Make enough room in servername[] for
AF_UNIX addresses
Hi, Neil,
Apparently I was duplicating work.
However, using
char servername[UNIX_PATH_MAX];
has some advantages when compared to hard-coded integer?
Correct me if I'm wrong.
Best regards,
Mirsad Todorovac
On 9/23/24 23:24, NeilBrown wrote:
> 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