- 31 Mar, 2023 1 commit
-
-
Chuck Lever authored
OPDESC() simply indexes into nfsd4_ops[] by the op's operation number, without range checking that value. It assumes callers are careful to avoid calling it with an out-of-bounds opnum value. nfsd4_decode_compound() is not so careful, and can invoke OPDESC() with opnum set to OP_ILLEGAL, which is 10044 -- well beyond the end of nfsd4_ops[]. Reported-by:
Jeff Layton <jlayton@kernel.org> Fixes: f4f9ef4a ("nfsd4: opdesc will be useful outside nfs4proc.c") Signed-off-by:
Chuck Lever <chuck.lever@oracle.com>
-
- 22 Mar, 2023 1 commit
-
-
Chuck Lever authored
Anna says: > KASAN reports [...] a slab-out-of-bounds in gss_krb5_checksum(), > and it can cause my client to panic when running cthon basic > tests with krb5p. > Running faddr2line gives me: > > gss_krb5_checksum+0x4b6/0x630: > ahash_request_free at > /home/anna/Programs/linux-nfs.git/./include/crypto/hash.h:619 > (inlined by) gss_krb5_checksum at > /home/anna/Programs/linux-nfs.git/net/sunrpc/auth_gss/gss_krb5_crypto.c:358 My diagnosis is that the memcpy() at the end of gss_krb5_checksum() reads past the end of the buffer containing the checksum data because the callers have ignored gss_krb5_checksum()'s API contract: * Caller provides the truncation length of the output token (h) in * cksumout.len. Instead they provide the fixed length of the hmac buffer. This length happens to be larger than the value returned by crypto_ahash_digestsize(). Change these errant callers to work like krb5_etm_{en,de}crypt(). As a defensive measure, bound the length of the byte copy at the end of gss_krb5_checksum(). Kunit sez: Testing complete. Ran 68 tests: passed: 68 Elapsed time: 81.680s total, 5.875s configuring, 75.610s building, 0.103s running Reported-by:Anna Schumaker <schumaker.anna@gmail.com> Fixes: 8270dbfc ("SUNRPC: Obscure Kerberos integrity keys") Signed-off-by:
-
- 17 Mar, 2023 1 commit
-
-
Jeff Layton authored
The splice read calls nfsd_splice_actor to put the pages containing file data into the svc_rqst->rq_pages array. It's possible however to get a splice result that only has a partial page at the end, if (e.g.) the filesystem hands back a short read that doesn't cover the whole page. nfsd_splice_actor will plop the partial page into its rq_pages array and return. Then later, when nfsd_splice_actor is called again, the remainder of the page may end up being filled out. At this point, nfsd_splice_actor will put the page into the array _again_ corrupting the reply. If this is done enough times, rq_next_page will overrun the array and corrupt the trailing fields -- the rq_respages and rq_next_page pointers themselves. If we've already added the page to the array in the last pass, don't add it to the array a second time when dealing with a splice continuation. This was originally handled properly in nfsd_splice_actor, but commit 91e23b1c ("NFSD: Clean up nfsd_splice_actor()") removed the check for it. Fixes: 91e23b1c ("NFSD: Clean up nfsd_splice_actor()") Cc: Al Viro <viro@zeniv.linux.org.uk> Reported-by:
Dario Lesca <d.lesca@solinos.it> Tested-by:
David Critch <dcritch@redhat.com> Link: https://bugzilla.redhat.com/show_bug.cgi?id=2150630Signed-off-by:
-
- 10 Mar, 2023 1 commit
-
-
Chuck Lever authored
Geert reports that: > On v6.2, "make ARCH=m68k defconfig" gives you > CONFIG_RPCSEC_GSS_KRB5=m > On v6.3, it became builtin, due to dropping the dependencies on > the individual crypto modules. > > $ grep -E "CRYPTO_(MD5|DES|CBC|CTS|ECB|HMAC|SHA1|AES)" .config > CONFIG_CRYPTO_AES=y > CONFIG_CRYPTO_AES_TI=m > CONFIG_CRYPTO_DES=m > CONFIG_CRYPTO_CBC=m > CONFIG_CRYPTO_CTS=m > CONFIG_CRYPTO_ECB=m > CONFIG_CRYPTO_HMAC=m > CONFIG_CRYPTO_MD5=m > CONFIG_CRYPTO_SHA1=m This behavior is triggered by the "default y" in the definition of RPCSEC_GSS. The "default y" was added in 2010 by commit df486a25 ("NFS: Fix the selection of security flavours in Kconfig"). However, svc_gss_principal was removed in 2012 by commit 03a4e1f6 ("nfsd4: move principal name into svc_cred"), so the 2010 fix is no longer necessary. We can safely change the NFS_V4 and NFSD_V4 dependencies back to RPCSEC_GSS_KRB5 to get the nicer v6.2 behavior back. Selecting KRB5 symbolically represents the true requirement here: that all spec-compliant NFSv4 implementations must have Kerberos available to use. Reported-by:
Geert Uytterhoeven <geert@linux-m68k.org> Fixes: dfe9a123 ("SUNRPC: Enable rpcsec_gss_krb5.ko to be built without CRYPTO_DES") Signed-off-by:
-
- 08 Mar, 2023 1 commit
-
-
Benjamin Coddington authored
Fix a race where kthread_stop() may prevent the threadfn from ever getting called. If that happens the svc_rqst will not be cleaned up. Fixes: ed6473dd ("NFSv4: Fix callback server shutdown") Signed-off-by:
Benjamin Coddington <bcodding@redhat.com> Reviewed-by:
-
- 07 Mar, 2023 1 commit
-
-
Chuck Lever authored
Flole observes this WARNING on occasion: [1210423.486503] WARNING: CPU: 8 PID: 1524732 at fs/ext4/ext4_jbd2.c:75 ext4_journal_check_start+0x68/0xb0 Reported-by: <flole@flole.de> Suggested-by:
Jan Kara <jack@suse.cz> Link: https://bugzilla.kernel.org/show_bug.cgi?id=217123 Fixes: 73da852e ("nfsd: use vfs_iter_read/write") Reviewed-by:
-
- 27 Feb, 2023 2 commits
-
-
Chuck Lever authored
Unable to handle kernel paging request at virtual address 73657420 when execute [73657420] *pgd=00000000 Internal error: Oops: 80000005 [#1] ARM CPU: 0 PID: 1 Comm: swapper Tainted: G N 6.2.0-rc7-00133-g373f26a81164-dirty #9 Hardware name: Generic DT based system PC is at 0x73657420 LR is at kunit_run_tests+0x3e0/0x5f4 On x86 with GCC 12, the missing array terminators did not seem to matter. Other platforms appear to be more picky. Reported-by:
-
Chuck Lever authored
Allow the new GSS Kerberos encryption type test suites to run outside of the kunit infrastructure. Replace the assertion that fires when lookup_enctype() so that the case is skipped instead of failing outright. Reported-by:
-
- 20 Feb, 2023 32 commits
-
-
Chuck Lever authored
The pointer dentry is assigned a value that is never read, the assignment is redundant and can be removed. Cleans up clang-scan warning: fs/nfsd/nfsctl.c:1231:2: warning: Value stored to 'dentry' is never read [deadcode.DeadStores] dentry = ERR_PTR(ret); No need to initialize "int ret = -ENOMEM;" either. These are vestiges of nfsd_mkdir(), from whence I copied nfsd_symlink(). Reported-by:Colin Ian King <colin.i.king@gmail.com> Reported-by:
Dan Carpenter <error27@gmail.com> Signed-off-by:
-
Chuck Lever authored
Currently, we're only memcpy'ing the first __be32. Ensure we copy into both words. Fixes: 91d2e9b5 ("NFSD: Clean up the nfsd_net::nfssvc_boot field") Reported-by:
-
Jeff Layton authored
Most of the time, NFSv4 clients issue a COMMIT before the final CLOSE of an open stateid, so with NFSv4, the fsync in the nfsd_file_free path is usually a no-op and doesn't block. We have a customer running knfsd over very slow storage (XFS over Ceph RBD). They were using the "async" export option because performance was more important than data integrity for this application. That export option turns NFSv4 COMMIT calls into no-ops. Due to the fsync in this codepath however, their final CLOSE calls would still stall (since a CLOSE effectively became a COMMIT). I think this fsync is not strictly necessary. We only use that result to reset the write verifier. Instead of fsync'ing all of the data when we free an nfsd_file, we can just check for writeback errors when one is acquired and when it is freed. If the client never comes back, then it'll never see the error anyway and there is no point in resetting it. If an error occurs after the nfsd_file is removed from the cache but before the inode is evicted, then it will reset the write verifier on the next nfsd_file_acquire, (since there will be an unseen error). The only exception here is if something else opens and fsyncs the file during that window. Given that local applications work with this limitation today, I don't see that as an issue. Link: https://bugzilla.redhat.com/show_bug.cgi?id=2166658 Fixes: ac3a2585 ("nfsd: rework refcounting in filecache") Reported-and-tested-by:
Pierguido Lambri <plambri@redhat.com> Signed-off-by:
-
Chuck Lever authored
I'm guessing that the warning fired because there's some code path that is called on module unload where the gss_krb5_enctypes file was never set up. name 'gss_krb5_enctypes' WARNING: CPU: 0 PID: 6187 at fs/proc/generic.c:712 remove_proc_entry+0x38d/0x460 fs/proc/generic.c:712 destroy_krb5_enctypes_proc_entry net/sunrpc/auth_gss/svcauth_gss.c:1543 [inline] gss_svc_shutdown_net+0x7d/0x2b0 net/sunrpc/auth_gss/svcauth_gss.c:2120 ops_exit_list+0xb0/0x170 net/core/net_namespace.c:169 setup_net+0x9bd/0xe60 net/core/net_namespace.c:356 copy_net_ns+0x320/0x6b0 net/core/net_namespace.c:483 create_new_namespaces+0x3f6/0xb20 kernel/nsproxy.c:110 copy_namespaces+0x410/0x500 kernel/nsproxy.c:179 copy_process+0x311d/0x76b0 kernel/fork.c:2272 kernel_clone+0xeb/0x9a0 kernel/fork.c:2684 __do_sys_clone+0xba/0x100 kernel/fork.c:2825 do_syscall_x64 arch/x86/entry/common.c:50 [inline] do_syscall_64+0x39/0xb0 arch/x86/entry/common.c:80 entry_SYSCALL_64_after_hwframe+0x63/0xcd Reported-by: syzbot+04a8437497bcfb4afa95@syzkaller.appspotmail.com Signed-off-by: -
Jeff Layton authored
The nested if statements here make no sense, as you can never reach "else" branch in the nested statement. Fix the error handling for when there is a courtesy client that holds a conflicting deny mode. Fixes: 3d694271 ("NFSD: add support for share reservation conflict to courteous server") Reported-by:
張智諺 <cc85nod@gmail.com> Signed-off-by:
Dai Ngo <dai.ngo@oracle.com> Signed-off-by:
-
Dai Ngo authored
When nfsd4_copy fails to allocate memory for async_copy->cp_src, or nfs4_init_copy_state fails, it calls cleanup_async_copy to do the cleanup for the async_copy which causes page fault since async_copy is not yet initialized. This patche rearranges the order of initializing the fields in async_copy and adds checks in cleanup_async_copy to skip un-initialized fields. Fixes: ce0887ac ("NFSD add nfs4 inter ssc to nfsd4_copy") Fixes: 87689df6 ("NFSD: Shrink size of struct nfsd4_copy") Signed-off-by:
-
Benjamin Coddington authored
Its possible for __break_lease to find the layout's lease before we've added the layout to the owner's ls_layouts list. In that case, setting ls_recalled = true without actually recalling the layout will cause the server to never send a recall callback. Move the check for ls_layouts before setting ls_recalled. Fixes: c5c707f9 ("nfsd: implement pNFS layout recalls") Signed-off-by:
-
Jeff Layton authored
We had a bug report that xfstest generic/355 was failing on NFSv4.0. This test sets various combinations of setuid/setgid modes and tests whether DIO writes will cause them to be stripped. What I found was that the server did properly strip those bits, but the client didn't notice because it held a delegation that was not recalled. The recall didn't occur because the client itself was the one generating the activity and we avoid recalls in that case. Clearing setuid bits is an "implicit" activity. The client didn't specifically request that we do that, so we need the server to issue a CB_RECALL, or avoid the situation entirely by not issuing a delegation. The easiest fix here is to simply not give out a delegation if the file is being opened for write, and the mode has the setuid and/or setgid bit set. Note that there is a potential race between the mode and lease being set, so we test for this condition both before and after setting the lease. This patch fixes generic/355, generic/683 and generic/684 for me. (Note that 355 fails only on v4.0, and 683 and 684 require NFSv4.2 to run and fail). Reported-by:
Boyang Xue <bxue@redhat.com> Signed-off-by:
-
Chuck Lever authored
There's no need for the cost of this extra virtual function call during every RPC transaction: the RQ_SECURE bit can be set properly in ->xpo_recvfrom() instead. Reviewed-by:
-
Chuck Lever authored
Make this macro more conventional: - Use BIT() instead of open-coding " 1UL << " - Don't display the "XPT_" in every flag name Reviewed-by:
-
Jeff Layton authored
This file is no longer built at all. Signed-off-by:
-
Dai Ngo authored
The reference count of nfsd4_ssc_umount_item is not decremented on error conditions. This prevents the laundromat from unmounting the vfsmount of the source file. This patch decrements the reference count of nfsd4_ssc_umount_item on error. Fixes: f4e44b39 ("NFSD: delay unmount source's export after inter-server copy completed.") Signed-off-by:
-
Jeff Layton authored
There are two different flavors of the nfsd4_copy struct. One is embedded in the compound and is used directly in synchronous copies. The other is dynamically allocated, refcounted and tracked in the client struture. For the embedded one, the cleanup just involves releasing any nfsd_files held on its behalf. For the async one, the cleanup is a bit more involved, and we need to dequeue it from lists, unhash it, etc. There is at least one potential refcount leak in this code now. If the kthread_create call fails, then both the src and dst nfsd_files in the original nfsd4_copy object are leaked. The cleanup in this codepath is also sort of weird. In the async copy case, we'll have up to four nfsd_file references (src and dst for both flavors of copy structure). They are both put at the end of nfsd4_do_async_copy, even though the ones held on behalf of the embedded one outlive that structure. Change it so that we always clean up the nfsd_file refs held by the embedded copy structure before nfsd4_copy returns. Rework cleanup_async_copy to handle both inter and intra copies. Eliminate nfsd4_cleanup_intra_ssc since it now becomes a no-op. Signed-off-by:
-
Jeff Layton authored
At first, I thought this might be a source of nfsd_file overputs, but the current callers seem to avoid an extra put when nfsd4_verify_copy returns an error. Still, it's "bad form" to leave the pointers filled out when we don't have a reference to them anymore, and that might lead to bugs later. Zero them out as a defensive coding measure. Signed-off-by:
-
Chuck Lever authored
Signed-off-by: -
Jeff Layton authored
This is wrapper is pointless, and just obscures what's going on. Signed-off-by:
-
Jeff Layton authored
Signed-off-by:
-
Jeff Layton authored
We really don't need an accessor function here. Signed-off-by:
-
Jeff Layton authored
We're not doing any blocking operations for OP_OFFLOAD_STATUS, so taking and putting a reference is a waste of effort. Take the client lock, search for the copy and fetch the wr_bytes_written field and return. Also, make find_async_copy a static function. Signed-off-by:
Olga Kornievskaia <kolga@netapp.com> Signed-off-by:
-
Chuck Lever authored
With the KUnit infrastructure recently added, we are free to define other unit tests particular to our implementation. As an example, I've added a self-test that encrypts then decrypts a string, and checks the result. Tested-by:
Scott Mayhew <smayhew@redhat.com> Reviewed-by:
Simo Sorce <simo@redhat.com> Signed-off-by:
-
Chuck Lever authored
RFC 8009 provides sample encryption results. Add KUnit tests to ensure our implementation derives the expected results for the provided sample input. I hate how large this test is, but using non-standard key usage values means rfc8009_encrypt_case() can't simply reuse ->import_ctx to allocate and key its ciphers; and the test provides its own confounders, which means krb5_etm_encrypt() can't be used directly. Tested-by:
-
Chuck Lever authored
RFC 8009 provides sample checksum results. Add KUnit tests to ensure our implementation derives the expected results for the provided sample input. Tested-by:
-
Chuck Lever authored
RFC 8009 provides sample key derivation results, so Kunit tests are added to ensure our implementation derives the expected keys for the provided sample input. Tested-by:
-
Chuck Lever authored
Add tests for the new-to-RPCSEC Camellia cipher. Tested-by:
-
Chuck Lever authored
Test the new-to-RPCSEC CMAC digest algorithm. Tested-by:
-
Chuck Lever authored
The Camellia enctypes use a new KDF, so add some tests to ensure it is working properly. Tested-by:
-
Chuck Lever authored
Add Kunit tests for ENCTYPE_AES128_CTS_HMAC_SHA1_96. The test vectors come from RFC 3962 Appendix B. Tested-by:
-
Chuck Lever authored
RFC 3961 Appendix A provides tests for the KDF specified in that document as well as other parts of Kerberos. The other three usage scenarios in Section 10 are not implemented by the Linux kernel's RPCSEC GSS Kerberos 5 mechanism, so tests are not added for those. Tested-by:
-
Chuck Lever authored
I plan to add KUnit tests that will need enctype profile information. Export the enctype profile lookup function. Tested-by:
-
Chuck Lever authored
The Kerberos RFCs provide test vectors to verify the operation of an implementation. Introduce a KUnit test framework to exercise the Linux kernel's implementation of Kerberos. Start with test cases for the RFC 3961-defined n-fold function. The sample vectors for that are found in RFC 3961 Section 10. Run the GSS Kerberos 5 mechanism's unit tests with this command: $ ./tools/testing/kunit/kunit.py run \ --kunitconfig ./net/sunrpc/.kunitconfig Tested-by:
-
Chuck Lever authored
The goal is to leave only protocol-defined items in gss_krb5.h so that it can be easily replaced by a generic header. Implementation specific items are moved to the new internal header. Tested-by:
-
Chuck Lever authored
Add the RFC 6803 encryption types to the string of integers that is reported to gssd during upcalls. This enables gssd to utilize keys with these encryption types when support for them is built into the kernel. Tested-by:
-
