In the NFSATTRBIT_OWNERGROUP code in nfsv4_loadattr(): NFSM_DISSECT(tl, u_int32_t *, NFSX_UNSIGNED); j = fxdr_unsigned(int, *tl); if (j < 0) { error = NFSERR_BADXDR; goto nfsmout; } attrsum += (NFSX_UNSIGNED + NFSM_RNDUP(j)); if (j > NFSV4_SMALLSTR){ cp = malloc(j + 1, M_NFSSTRING, M_WAITOK); First, a big j can cause this code to allocate a few GB of memory, enough to cause my modest test machine to run out. Second, the "if (j < 0)" along with the "j + 1" means that if the RPC message contains j = 0x7fffffff, malloc will be called with (size_t)(j+1) = (size_t)(0x80000000) which is 0xffffffff80000000 if size_t is unsigned and 64 bits. This seems to cause malloc() to never return on my machine.
Created attachment 233181 [details] add a sanity check for a large owner/Owner_group string There is no upper limit for the length of an Owner/Owner_group string specified in the RFCs. As such, this patch uses a large (10K) sanity limit. I will post on a FreeBSD mailing list to try and get a better upper bound for a user/group name. However, any reasonable sanity limit should fix this problem. I did Owner as well as Owner_group, since they were both affected the same way. If the reporter has a way to test this, maybe they can report back if the patch fixes the problem for them?
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=ef4edb70c909fc2b1de867601c2230597d07daa0 commit ef4edb70c909fc2b1de867601c2230597d07daa0 Author: Rick Macklem <rmacklem@FreeBSD.org> AuthorDate: 2022-05-04 20:58:22 +0000 Commit: Rick Macklem <rmacklem@FreeBSD.org> CommitDate: 2022-05-04 20:58:22 +0000 nfsd: Add a sanity check for Owner/OwnerGroup string length Robert Morris reported that, if a client sends an absurdly large Owner/OwnerGroup string, the kernel malloc() for the large size string can block forever. This patch adds a sanity limit for Owner/OwnerGroup string length. Since the RFCs do not specify any limit and FreeBSD can handle a group name greater than 1Kbyte, the limit is set at a generous 10Kbytes. Reported by: rtm@lcs.mit.edu PR: 260546 MFC after: 2 weeks sys/fs/nfs/nfs.h | 7 +++++++ sys/fs/nfs/nfs_commonsubs.c | 4 ++-- 2 files changed, 9 insertions(+), 2 deletions(-)
A patch similar to the attachment has been committed to main and will be MFC'd.
A commit in branch stable/13 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=f01978f464cea1cffeb1a66a18f92d98380dd7f2 commit f01978f464cea1cffeb1a66a18f92d98380dd7f2 Author: Rick Macklem <rmacklem@FreeBSD.org> AuthorDate: 2022-05-04 20:58:22 +0000 Commit: Rick Macklem <rmacklem@FreeBSD.org> CommitDate: 2022-05-20 00:36:22 +0000 nfsd: Add a sanity check for Owner/OwnerGroup string length Robert Morris reported that, if a client sends an absurdly large Owner/OwnerGroup string, the kernel malloc() for the large size string can block forever. This patch adds a sanity limit for Owner/OwnerGroup string length. Since the RFCs do not specify any limit and FreeBSD can handle a group name greater than 1Kbyte, the limit is set at a generous 10Kbytes. PR: 260546 (cherry picked from commit ef4edb70c909fc2b1de867601c2230597d07daa0) sys/fs/nfs/nfs.h | 7 +++++++ sys/fs/nfs/nfs_commonsubs.c | 4 ++-- 2 files changed, 9 insertions(+), 2 deletions(-)
A commit in branch stable/12 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=c0ea059da22f0f1f20ee43db536a74032f140429 commit c0ea059da22f0f1f20ee43db536a74032f140429 Author: Rick Macklem <rmacklem@FreeBSD.org> AuthorDate: 2022-05-04 20:58:22 +0000 Commit: Rick Macklem <rmacklem@FreeBSD.org> CommitDate: 2022-05-20 00:43:22 +0000 nfsd: Add a sanity check for Owner/OwnerGroup string length Robert Morris reported that, if a client sends an absurdly large Owner/OwnerGroup string, the kernel malloc() for the large size string can block forever. This patch adds a sanity limit for Owner/OwnerGroup string length. Since the RFCs do not specify any limit and FreeBSD can handle a group name greater than 1Kbyte, the limit is set at a generous 10Kbytes. PR: 260546 (cherry picked from commit ef4edb70c909fc2b1de867601c2230597d07daa0) sys/fs/nfs/nfs.h | 7 +++++++ sys/fs/nfs/nfs_commonsubs.c | 4 ++-- 2 files changed, 9 insertions(+), 2 deletions(-)
Patch has been committed and MFC'd.