- 17 Nov, 2023 4 commits
-
-
Chuck Lever authored
nfsd_cache_csum() currently assumes that the server's RPC layer has been advancing rq_arg.head[0].iov_base as it decodes an incoming request, because that's the way it used to work. On entry, it expects that buf->head[0].iov_base points to the start of the NFS header, and excludes the already-decoded RPC header. These days however, head[0].iov_base now points to the start of the RPC header during all processing. It no longer points at the NFS Call header when execution arrives at nfsd_cache_csum(). In a retransmitted RPC the XID and the NFS header are supposed to be the same as the original message, but the contents of the retransmitted RPC header can be different. For example, for krb5, the GSS sequence number will be different between the two. Thus if the RPC header is always included in the DRC checksum computation, the checksum of the retransmitted message might not match the checksum of the original message, even though the NFS part of these messages is identical. The result is that, even if a matching XID is found in the DRC, the checksum mismatch causes the server to execute the retransmitted RPC transaction again. Reviewed-by:
Jeff Layton <jlayton@kernel.org> Tested-by:
Chuck Lever <chuck.lever@oracle.com>
-
Chuck Lever authored
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:
-
Chuck Lever authored
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:
-
Mahmoud Adam authored
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:
NeilBrown <neilb@suse.de> Tested-by:
-
- 16 Oct, 2023 36 commits
-
-
Chuck Lever authored
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:
-
Chuck Lever authored
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:
-
Sicong Huang authored
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:
-
Chuck Lever authored
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:
-
Chuck Lever authored
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: -
KaiLong Wang authored
Fix the following errors reported by checkpatch: ERROR: need consistent spacing around '+' (ctx:WxV) ERROR: spaces required around that '?' (ctx:VxW) Signed-off-by:
KaiLong Wang <wangkailong@jari.cn> Signed-off-by:
-
KaiLong Wang authored
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 authored
Fix the following errors reported by checkpatch: ERROR: space required after that ',' (ctx:VxV) Signed-off-by:
-
NeilBrown authored
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: -
Chuck Lever authored
Use modern XDR encoder utilities. Reviewed-by:
-
Chuck Lever authored
Use modern XDR encoder utilities. Reviewed-by:
-
Chuck Lever authored
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:
-
Chuck Lever authored
Restructure this function using conventional XDR utility functions and so it aligns better with the XDR in the specification. I've also moved nfsd4_encode_copy() closer to the data type encoders that only it uses. Reviewed-by:
-
Chuck Lever authored
Use conventional XDR utilities. Reviewed-by:
-
Chuck Lever authored
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:
-
Chuck Lever authored
Refactor nfsd4_encode_secinfo() so it is more clear what XDR data item is being encoded by which piece of code. Reviewed-by:
-
Chuck Lever authored
Convert nfsd4_encode_access() to use modern XDR utility functions. Reviewed-by:
-
Chuck Lever authored
Untangle nfsd4_encode_readdir() so it is more clear what XDR data item is being encoded by which piece of code. Reviewed-by:
-
Chuck Lever authored
Reshape nfsd4_encode_entry4() to be more like the legacy dirent encoders, which were recently rewritten to use xdr_stream. Reviewed-by:
-
Chuck Lever authored
De-duplicate the entry4 cookie encoder, similar to the arrangement for the NFSv2 and NFSv3 directory entry encoders. Reviewed-by:
-
Chuck Lever authored
No need for specialized code here, as this function is invoked only rarely. Convert it to encode to xdr_stream using conventional XDR helpers. Reviewed-by:
-
Chuck Lever authored
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:
-
Chuck Lever authored
De-duplicate open-coded encoding of the sessionid, and convert the rest of the function to use conventional XDR utility functions. Reviewed-by:
-
Chuck Lever authored
Convert nfsd4_encode_create_session() to use the conventional XDR encoding utilities. Reviewed-by:
-
Chuck Lever authored
De-duplicate the encoding of the fore channel and backchannel attributes. Reviewed-by:
-
Chuck Lever authored
There is more than one NFSv4 operation that needs to encode a sessionid4, so extract that data type into a separate helper. Reviewed-by:
-
Chuck Lever authored
Finish cleaning up nfsd4_encode_open(). Reviewed-by:
-
Chuck Lever authored
To better align our implementation with the XDR specification, refactor the part of nfsd4_encode_open() that encodes delegation metadata. As part of that refactor, remove an unnecessary BUG() call site and a comment that appears to be stale. Reviewed-by:
-
Chuck Lever authored
To better align our implementation with the XDR specification, refactor the part of nfsd4_encode_open() that encodes the open_none_delegation4 type. Reviewed-by:
-
Chuck Lever authored
Make it easier to adjust the XDR encoder to handle new features related to write delegations. Reviewed-by:
-
Chuck Lever authored
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:
-
Chuck Lever authored
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:
-
Chuck Lever authored
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:
-
Chuck Lever authored
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:
-
Chuck Lever authored
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:
-
Chuck Lever authored
This enables callers to be passed const pointer parameters. Reviewed-by:
-
