Chuck Lever [Fri, 10 Nov 2023 16:28:33 +0000 (11:28 -0500)]
NFSD: Fix "start of NFS reply" pointer passed to nfsd_cache_update()
The "statp + 1" pointer that is passed to nfsd_cache_update() is
supposed to point to the start of the egress NFS Reply header. In
fact, it does point there for AUTH_SYS and RPCSEC_GSS_KRB5 requests.
But both krb5i and krb5p add fields between the RPC header's
accept_stat field and the start of the NFS Reply header. In those
cases, "statp + 1" points at the extra fields instead of the Reply.
The result is that nfsd_cache_update() caches what looks to the
client like garbage.
A connection break can occur for a number of reasons, but the most
common reason when using krb5i/p is a GSS sequence number window
underrun. When an underrun is detected, the server is obliged to
drop the RPC and the connection to force a retransmit with a fresh
GSS sequence number. The client presents the same XID, it hits in
the server's DRC, and the server returns the garbage cache entry.
The "statp + 1" argument has been used since the oldest changeset
in the kernel history repo, so it has been in nfsd_dispatch()
literally since before history began. The problem arose only when
the server-side GSS implementation was added twenty years ago.
Reviewed-by: Jeff Layton <jlayton@kernel.org> Tested-by: Jeff Layton <jlayton@kernel.org Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Chuck Lever [Fri, 10 Nov 2023 16:28:39 +0000 (11:28 -0500)]
NFSD: Update nfsd_cache_append() to use xdr_stream
When inserting a DRC-cached response into the reply buffer, ensure
that the reply buffer's xdr_stream is updated properly. Otherwise
the server will send a garbage response.
Cc: stable@vger.kernel.org # v6.3+ Reviewed-by: Jeff Layton <jlayton@kernel.org> Tested-by: Jeff Layton <jlayton@kernel.org> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Mahmoud Adam [Fri, 10 Nov 2023 18:21:04 +0000 (19:21 +0100)]
nfsd: fix file memleak on client_opens_release
seq_release should be called to free the allocated seq_file
Cc: stable@vger.kernel.org # v5.3+ Signed-off-by: Mahmoud Adam <mngyadam@amazon.com> Reviewed-by: Jeff Layton <jlayton@kernel.org> Fixes: 78599c42ae3c ("nfsd4: add file to display list of client's opens") Reviewed-by: NeilBrown <neilb@suse.de> Tested-by: Jeff Layton <jlayton@kernel.org> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Chuck Lever [Fri, 13 Oct 2023 12:22:24 +0000 (08:22 -0400)]
svcrdma: Fix tracepoint printk format
Other tracepoints use "cq.id=" rather than "cq_id=". Let's make it
more reliable to grep for the CQ restracker ID.
Reviewed-by: Benjamin Coddington <bcodding@redhat.com> Reviewed-by: Jeff Layton <jlayton@kernel.org> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Chuck Lever [Tue, 10 Oct 2023 17:23:41 +0000 (13:23 -0400)]
svcrdma: Drop connection after an RDMA Read error
When an RPC Call message cannot be pulled from the client, that
is a message loss, by definition. Close the connection to trigger
the client to resend.
Cc: <stable@vger.kernel.org> Reviewed-by: Tom Talpey <tom@talpey.com> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Sicong Huang [Thu, 12 Oct 2023 08:34:58 +0000 (16:34 +0800)]
NFSD: clean up alloc_init_deleg()
Modify the conditional statement for null pointer check in the function
'alloc_init_deleg' to make this function more robust and clear. Otherwise,
this function may have potential pointer dereference problem in the future,
when modifying or expanding the nfs4_delegation structure.
Signed-off-by: Sicong Huang <huangsicong@iie.ac.cn> Reviewed-by: Jeff Layton <jlayton@kernel.org> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Chuck Lever [Sun, 1 Oct 2023 17:25:16 +0000 (13:25 -0400)]
NFSD: Fix frame size warning in svc_export_parse()
fs/nfsd/export.c: In function 'svc_export_parse':
fs/nfsd/export.c:737:1: warning: the frame size of 1040 bytes is larger than 1024 bytes [-Wframe-larger-than=]
737 | }
On my systems, svc_export_parse() has a stack frame of over 800
bytes, not 1040, but nonetheless, it could do with some reduction.
When a struct svc_export is on the stack, it's a temporary structure
used as an argument, and not visible as an actual exported FS. No
need to reserve space for export_stats in such cases.
Reported-by: kernel test robot <lkp@intel.com> Closes: https://lore.kernel.org/oe-kbuild-all/202310012359.YEw5IrK6-lkp@intel.com/ Cc: Amir Goldstein <amir73il@gmail.com> Reviewed-by: Jeff Layton <jlayton@kernel.org> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Chuck Lever [Sun, 1 Oct 2023 17:25:09 +0000 (13:25 -0400)]
NFSD: Rewrite synopsis of nfsd_percpu_counters_init()
In function ‘export_stats_init’,
inlined from ‘svc_export_alloc’ at fs/nfsd/export.c:866:6:
fs/nfsd/export.c:337:16: warning: ‘nfsd_percpu_counters_init’ accessing 40 bytes in a region of size 0 [-Wstringop-overflow=]
337 | return nfsd_percpu_counters_init(&stats->counter, EXP_STATS_COUNTERS_NUM);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
fs/nfsd/export.c:337:16: note: referencing argument 1 of type ‘struct percpu_counter[0]’
fs/nfsd/stats.h: In function ‘svc_export_alloc’:
fs/nfsd/stats.h:40:5: note: in a call to function ‘nfsd_percpu_counters_init’
40 | int nfsd_percpu_counters_init(struct percpu_counter counters[], int num);
| ^~~~~~~~~~~~~~~~~~~~~~~~~
Cc: Amir Goldstein <amir73il@gmail.com> Reviewed-by: Jeff Layton <jlayton@kernel.org> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
KaiLong Wang [Thu, 28 Sep 2023 02:43:04 +0000 (10:43 +0800)]
nfsd: Clean up errors in nfs4state.c
Fix the following errors reported by checkpatch:
ERROR: spaces required around that '=' (ctx:VxW)
ERROR: space required after that ',' (ctx:VxO)
ERROR: space required before that '~' (ctx:OxV) Signed-off-by: KaiLong Wang <wangkailong@jari.cn> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
The error paths in nfsd_svc() are needlessly complex and can result in a
final call to svc_put() without nfsd_last_thread() being called. This
results in the listening sockets not being closed properly.
The per-netns setup provided by nfsd_startup_new() and removed by
nfsd_shutdown_net() is needed precisely when there are running threads.
So we don't need nfsd_up_before. We don't need to know if it *was* up.
We only need to know if any threads are left. If none are, then we must
call nfsd_shutdown_net(). But we don't need to do that explicitly as
nfsd_last_thread() does that for us.
So simply call nfsd_last_thread() before the last svc_put() if there are
no running threads. That will always do the right thing.
Also discard:
pr_info("nfsd: last server has exited, flushing export cache\n");
It may not be true if an attempt to start the first server failed, and
it isn't particularly helpful and it simply reports normal behaviour.
Signed-off-by: NeilBrown <neilb@suse.de> Reviewed-by: Jeff Layton <jlayton@kernel.org> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Chuck Lever [Mon, 9 Oct 2023 18:30:16 +0000 (14:30 -0400)]
NFSD: Clean up nfsd4_encode_copy_notify()
Replace open-coded encoding logic with the use of conventional XDR
utility functions.
Note that if we replace the cpn_sec and cpn_nsec fields with a
single struct timespec64 field, the encoder can use
nfsd4_encode_nfstime4(), as that is the data type specified by the
XDR spec.
NFS4ERR_INVAL seems inappropriate if the encoder doesn't support
encoding the response. Instead use NFS4ERR_SERVERFAULT, since this
condition is a software bug on the server.
Reviewed-by: Jeff Layton <jlayton@kernel.org> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Chuck Lever [Mon, 9 Oct 2023 18:29:56 +0000 (14:29 -0400)]
NFSD: Clean up nfsd4_encode_exchange_id()
Restructure nfsd4_encode_exchange_id() so that it will be more
straightforward to add support for SSV one day. Also, adopt the use
of the conventional XDR utility functions.
Reviewed-by: Jeff Layton <jlayton@kernel.org> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Chuck Lever [Wed, 4 Oct 2023 13:41:44 +0000 (09:41 -0400)]
NFSD: Rename nfsd4_encode_dirent()
Rename nfsd4_encode_dirent() to match the naming convention already
used in the NFSv2 and NFSv3 readdir paths. The new name reflects the
name of the spec-defined XDR data type for an NFSv4 directory entry.
Reviewed-by: Jeff Layton <jlayton@kernel.org> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Chuck Lever [Fri, 29 Sep 2023 13:59:05 +0000 (09:59 -0400)]
NFSD: Add nfsd4_encode_open_read_delegation4()
Refactor nfsd4_encode_open() so the open_read_delegation4 type is
encoded in a separate function. This makes it more straightforward
to later add support for returning an nfsace4 in OPEN responses that
offer a delegation.
Reviewed-by: Jeff Layton <jlayton@kernel.org> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Chuck Lever [Fri, 29 Sep 2023 13:58:59 +0000 (09:58 -0400)]
NFSD: Refactor nfsd4_encode_lock_denied()
Use the modern XDR utility functions.
The LOCK and LOCKT encoder functions need to return nfserr_denied
when a lock is denied, but nfsd4_encode_lock4denied() should return
a status code that is consistent with other XDR encoders.
Reviewed-by: Jeff Layton <jlayton@kernel.org> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Chuck Lever [Fri, 29 Sep 2023 13:58:53 +0000 (09:58 -0400)]
NFSD: Add nfsd4_encode_lock_owner4()
To improve readability and better align the LOCK encoders with the
XDR specification, add an explicit encoder named for the lock_owner4
type.
In particular, to avoid code duplication, use
nfsd4_encode_clientid4() to encode the clientid in the lock owner
rather than open-coding it.
It looks to me like nfs4_set_lock_denied() already clears the
clientid if it won't return an owner (cf: the nevermind: label). The
code in the XDR encoder appears to be redundant and can safely be
removed.
Reviewed-by: Jeff Layton <jlayton@kernel.org> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Chuck Lever [Thu, 12 Oct 2023 17:46:39 +0000 (13:46 -0400)]
NFSD: Remove a layering violation when encoding lock_denied
An XDR encoder is responsible for marshaling results, not releasing
memory that was allocated by the upper layer. We have .op_release
for that purpose.
Move the release of the ld_owner.data string to op_release functions
for LOCK and LOCKT.
Reviewed-by: Jeff Layton <jlayton@kernel.org> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Chuck Lever [Mon, 25 Sep 2023 13:28:23 +0000 (09:28 -0400)]
NFSD: Clean up nfsd4_encode_getdeviceinfo()
Adopt the conventional XDR utility functions. Also, restructure to
make the function align more closely with the spec -- there doesn't
seem to be a performance need for speciality code, so prioritize
readability.
Reviewed-by: Jeff Layton <jlayton@kernel.org> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Chuck Lever [Mon, 25 Sep 2023 13:27:58 +0000 (09:27 -0400)]
NFSD: Clean up nfsd4_encode_layoutget()
De-duplicate the open-coded stateid4 encoder. Adopt the use of the
conventional current XDR encoding helpers. Refactor the encoder to
align with the XDR specification.
Reviewed-by: Jeff Layton <jlayton@kernel.org> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Chuck Lever [Mon, 25 Sep 2023 13:27:45 +0000 (09:27 -0400)]
NFSD: Clean up nfsd4_encode_stateid()
Update the encoder function name to match the type name, as is the
convention with other such encoder utility functions, and with
nfsd4_decode_stateid4().
Make the @stateid argument a const so that callers of
nfsd4_encode_stateid4() in the future can be passed const pointers
to structures.
Since the compiler is allowed to add padding to structs, use the
wire (spec-defined) size when reserving buffer space.
Reviewed-by: Jeff Layton <jlayton@kernel.org> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Chuck Lever [Mon, 18 Sep 2023 14:02:06 +0000 (10:02 -0400)]
NFSD: Copy FATTR4 bit number definitions from RFCs
I'd like to convert nfsd4_encode_fattr() to rotate through the
attrmask using for_each_bit() instead of explicitly testing the
bitmask for each bit value. This means I need the bit numbers, as
defined in the specs, instead of our internal bitmask constants.
As a clean up, use the new spec-derived values to define the WORD#
bitmask constants.
Reviewed-by: Jeff Layton <jlayton@kernel.org> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Chuck Lever [Mon, 18 Sep 2023 13:57:51 +0000 (09:57 -0400)]
NFSD: Add nfsd4_encode_fattr4_change()
Refactor the encoder for FATTR4_CHANGE into a helper. In a
subsequent patch, this helper will be called from a bitmask loop.
The code is restructured a bit to use the modern xdr_stream flow,
and the encoded cinfo value is made const so that callers of the
encoders can be passed a const cinfo.
Reviewed-by: Jeff Layton <jlayton@kernel.org> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Chuck Lever [Mon, 18 Sep 2023 13:57:39 +0000 (09:57 -0400)]
NFSD: Add nfsd4_encode_fattr4_type()
Refactor the encoder for FATTR4_TYPE into a helper. In a subsequent
patch, this helper will be called from a bitmask loop.
In addition, restructure the code so that byte-swapping is done on
constant values rather than at run time. Run-time swapping can be
costly on some platforms, and "type" is a frequently-requested
attribute.
Reviewed-by: Jeff Layton <jlayton@kernel.org> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Chuck Lever [Mon, 18 Sep 2023 13:57:13 +0000 (09:57 -0400)]
NFSD: Add struct nfsd4_fattr_args
I'm about to split nfsd4_encode_fattr() into a number of smaller
functions. Instead of passing a large number of arguments to each of
the smaller functions, create a struct that can gather the common
argument variables into something with a convenient handle on it.
Reviewed-by: Jeff Layton <jlayton@kernel.org> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Chuck Lever [Mon, 18 Sep 2023 13:56:54 +0000 (09:56 -0400)]
NFSD: Add simple u32, u64, and bool encoders
The generic XDR encoders return a length or a negative errno. NFSv4
encoders want to know simply whether the encode ran out of stream
buffer space. The return values for server-side encoding are either
nfs_ok or nfserr_resource.
So far I've found it adds a lot of duplicate code to try to use the
generic XDR encoder utilities when encoding the simple data types in
the NFSv4 operation encoders.
Add a set of NFSv4-specific utilities that handle the basic XDR data
types. These are added in xdr4.h so they might eventually be used by
the callback server and pNFS driver encoders too.
Reviewed-by: Jeff Layton <jlayton@kernel.org> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Chuck Lever [Tue, 19 Sep 2023 15:35:15 +0000 (11:35 -0400)]
SUNRPC: Remove BUG_ON call sites
There is no need to take down the whole system for these assertions.
I'd rather not attempt a heroic save here, as some bug has occurred
that has left the transport data structures in an unknown state.
Just warn and then leak the left-over resources.
Acked-by: Christian Brauner <brauner@kernel.org> Reviewed-by: NeilBrown <neilb@suse.de> Reviewed-by: Jeff Layton <jlayton@kernel.org> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
projects / linux.git / log