| 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: <172618031521.17050.16135461752183993428@noble.neil.brown.name>
Date: Fri, 13 Sep 2024 08:31:55 +1000
From: "NeilBrown" <neilb@...e.de>
To: Pali Rohár <pali@...nel.org>
Cc: "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>, linux-nfs@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH] nfsd: Fill NFSv4.1 server implementation fields in
OP_EXCHANGE_ID response
On Fri, 13 Sep 2024, Pali Rohár wrote:
> NFSv4.1 OP_EXCHANGE_ID response from server may contain server
> implementation details (domain, name and build time) in optional
> nfs_impl_id4 field. Currently nfsd does not fill this field.
>
> NFSv4.1 OP_EXCHANGE_ID call request from client may contain client
> implementation details and Linux NFSv4.1 client is already filling these
> information based on runtime module param "nfs.send_implementation_id" and
> build time Kconfig option "NFS_V4_1_IMPLEMENTATION_ID_DOMAIN". Module param
> send_implementation_id specify whether to fill implementation fields and
> Kconfig option "NFS_V4_1_IMPLEMENTATION_ID_DOMAIN" specify the domain
> string.
>
> Do same in nfsd, introduce new runtime param "nfsd.send_implementation_id"
> and build time Kconfig option "NFSD_V4_1_IMPLEMENTATION_ID_DOMAIN" and
> based on them fill NFSv4.1 server implementation details in OP_EXCHANGE_ID
> response. Logic in nfsd is exactly same as in nfs.
>
> This aligns Linux NFSv4.1 server logic with Linux NFSv4.1 client logic.
>
> NFSv4.1 client and server implementation fields are useful for statistic
> purposes or for identifying type of clients and servers.
>
> Signed-off-by: Pali Rohár <pali@...nel.org>
> ---
> fs/nfsd/Kconfig | 12 +++++++++++
> fs/nfsd/nfs4xdr.c | 55 +++++++++++++++++++++++++++++++++++++++++++++--
> 2 files changed, 65 insertions(+), 2 deletions(-)
>
> diff --git a/fs/nfsd/Kconfig b/fs/nfsd/Kconfig
> index ec2ab6429e00..70067c29316e 100644
> --- a/fs/nfsd/Kconfig
> +++ b/fs/nfsd/Kconfig
> @@ -136,6 +136,18 @@ config NFSD_FLEXFILELAYOUT
>
> If unsure, say N.
>
> +config NFSD_V4_1_IMPLEMENTATION_ID_DOMAIN
> + string "NFSv4.1 Implementation ID Domain"
> + depends on NFSD_V4
> + default "kernel.org"
> + help
> + This option defines the domain portion of the implementation ID that
> + may be sent in the NFS exchange_id operation. The value must be in
> + the format of a DNS domain name and should be set to the DNS domain
> + name of the distribution.
> + If the NFS server is unchanged from the upstream kernel, this
> + option should be set to the default "kernel.org".
> +
> config NFSD_V4_2_INTER_SSC
> bool "NFSv4.2 inter server to server COPY"
> depends on NFSD_V4 && NFS_V4_2
> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> index b45ea5757652..5e89f999d4c7 100644
> --- a/fs/nfsd/nfs4xdr.c
> +++ b/fs/nfsd/nfs4xdr.c
> @@ -62,6 +62,9 @@
> #include <linux/security.h>
> #endif
>
> +static bool send_implementation_id = true;
> +module_param(send_implementation_id, bool, 0644);
> +MODULE_PARM_DESC(send_implementation_id, "Send implementation ID with NFSv4.1 exchange_id");
>
> #define NFSDDBG_FACILITY NFSDDBG_XDR
>
> @@ -4833,6 +4836,53 @@ nfsd4_encode_server_owner4(struct xdr_stream *xdr, struct svc_rqst *rqstp)
> return nfsd4_encode_opaque(xdr, nn->nfsd_name, strlen(nn->nfsd_name));
> }
>
> +#define IMPL_NAME_LIMIT (sizeof(utsname()->sysname) + sizeof(utsname()->release) + \
> + sizeof(utsname()->version) + sizeof(utsname()->machine) + 8)
This "+8" seems strange. In the xdr_reserve_space() call below you are
very thorough about explaining the magic numbers - which is great. Here
that is this unexplained 8.
I don't think you need +8 at all. sizeof(string) will give room to
print the string plus a trailing space or nul, and that is all you need.
Otherwise the patch looks OK.
Thanks,
NeilBrown
> +
> +static __be32
> +nfsd4_encode_server_impl_id(struct xdr_stream *xdr)
> +{
> + char impl_name[IMPL_NAME_LIMIT];
> + int impl_name_len;
> + __be32 *p;
> +
> + impl_name_len = 0;
> + if (send_implementation_id &&
> + sizeof(CONFIG_NFSD_V4_1_IMPLEMENTATION_ID_DOMAIN) > 1 &&
> + sizeof(CONFIG_NFSD_V4_1_IMPLEMENTATION_ID_DOMAIN) <= NFS4_OPAQUE_LIMIT)
> + impl_name_len = snprintf(impl_name, sizeof(impl_name), "%s %s %s %s",
> + utsname()->sysname, utsname()->release,
> + utsname()->version, utsname()->machine);
> +
> + if (impl_name_len <= 0) {
> + if (xdr_stream_encode_u32(xdr, 0) != XDR_UNIT)
> + return nfserr_resource;
> + return nfs_ok;
> + }
> +
> + if (xdr_stream_encode_u32(xdr, 1) != XDR_UNIT)
> + return nfserr_resource;
> +
> + p = xdr_reserve_space(xdr,
> + 4 /* nii_domain.len */ +
> + (XDR_QUADLEN(sizeof(CONFIG_NFSD_V4_1_IMPLEMENTATION_ID_DOMAIN) - 1) * 4) +
> + 4 /* nii_name.len */ +
> + (XDR_QUADLEN(impl_name_len) * 4) +
> + 8 /* nii_time.tv_sec */ +
> + 4 /* nii_time.tv_nsec */);
> + if (!p)
> + return nfserr_resource;
> +
> + p = xdr_encode_opaque(p, CONFIG_NFSD_V4_1_IMPLEMENTATION_ID_DOMAIN,
> + sizeof(CONFIG_NFSD_V4_1_IMPLEMENTATION_ID_DOMAIN) - 1);
> + p = xdr_encode_opaque(p, impl_name, impl_name_len);
> + /* just send zeros for nii_date - the date is in nii_name */
> + p = xdr_encode_hyper(p, 0); /* tv_sec */
> + *p++ = cpu_to_be32(0); /* tv_nsec */
> +
> + return nfs_ok;
> +}
> +
> static __be32
> nfsd4_encode_exchange_id(struct nfsd4_compoundres *resp, __be32 nfserr,
> union nfsd4_op_u *u)
> @@ -4867,8 +4917,9 @@ nfsd4_encode_exchange_id(struct nfsd4_compoundres *resp, __be32 nfserr,
> if (nfserr != nfs_ok)
> return nfserr;
> /* eir_server_impl_id<1> */
> - if (xdr_stream_encode_u32(xdr, 0) != XDR_UNIT)
> - return nfserr_resource;
> + nfserr = nfsd4_encode_server_impl_id(xdr);
> + if (nfserr != nfs_ok)
> + return nfserr;
>
> return nfs_ok;
> }
> --
> 2.20.1
>
>
Powered by blists - more mailing lists