Linux NFS development
 help / color / mirror / Atom feed
From: Kees Cook <kees@kernel.org>
To: Jeff Layton <jlayton@kernel.org>, SSH <originalssh@pm.me>,
	"linux-nfs@vger.kernel.org" <linux-nfs@vger.kernel.org>
Subject: Re: Question about potential buffer issue in nfs_request_mount() - seeking feedback
Date: Mon, 01 Sep 2025 12:18:09 -0700	[thread overview]
Message-ID: <C584CAD4-B954-42C5-89DF-F3033998BEE7@kernel.org> (raw)
In-Reply-To: <71fa0055f1ddd5a7f8606515579889e85390d8e9.camel@kernel.org>



On September 1, 2025 5:25:29 AM PDT, Jeff Layton <jlayton@kernel.org> wrote:
>On Mon, 2025-09-01 at 01:38 +0000, SSH wrote:
>> Hi NFS maintainers,
>> 
>> I was looking at a kernel warning from 6.1-rc1 to understand it better and tried to trace through the code to understand what was happening. I think I may have found something, although now the most up-to-date kernel HEAD is August, 2025 and most of all, I'm not a kernel developer so I wanted to ask for your feedback on whether my analysis makes sense.
>> 
>> ## Context
>> * This was on all NFS v3 TCP mounts
>> * The warning came from kernel's hardened memcpy detection
>> * The mount seemed to work despite the warning
>> 
>> ### Additional Context
>> I noticed this warning was originally reported around 6.1-rc1 timeframe (~2022), but checking the current kernel source, it would appear that the same code pattern exists.
>> I'm not sure if this was previously reported to the NFS maintainers specifically, or if there's a reason it wasn't addressed. Either way, I thought it was worth bringing up again in case it got missed or deprioritized.
>> 
>> Source: https://lkml.org/lkml/2022/10/16/461
>> 
>> ## The Original Warning
>> I saw this warning during NFS v3 TCP mount:
>> 
>> ```
>> [   19.617475] memcpy: detected field-spanning write (size 28) of single field "request.sap" at fs/nfs/super.c:857 (size 18446744073709551615)
>> [   19.617504] WARNING: CPU: 3 PID: 1300 at fs/nfs/super.c:857 nfs_request_mount.constprop.0.isra.0+0x1c0/0x1f0
>> ```
>> 
>> ## Likely Source of Failure
>> 
>> Looking at `nfs_request_mount()` in `fs/nfs/super.c`, I see this code:
>> 
>> ```c
>> // Around line 850
>> struct nfs_mount_request request = {
>>     .sap = &ctx->mount_server._address,
>>     // ... other fields
>> };
>> 
>> // Later, around line 881
>> if (ctx->mount_server.address.sa_family == AF_UNSPEC) {
>>     memcpy(request.sap, &ctx->nfs_server._address, ctx->nfs_server.addrlen);
>>     ctx->mount_server.addrlen = ctx->nfs_server.addrlen;
>> }
>> ```
>> 
>> My understanding is:
>> 1. `request.sap` points to `ctx->mount_server._address`
>> 2. We're copying from `ctx->nfs_server._address` (which could be 28 bytes for IPv6)
>> 3. Into whatever `mount_server._address` points to (which might be smaller?)
>> 
>> The weird size value (18446744073709551615) in the warning makes me think there might be memory corruption happening.
>> 
>> Does this seem like a real issue? If so, would adding a size check before the memcpy make sense, something like:
>> 
>> ```c
>> if (ctx->mount_server.address.sa_family == AF_UNSPEC) {
>>     if (ctx->nfs_server.addrlen <= sizeof(ctx->mount_server._address)) {
>>         memcpy(request.sap, &ctx->nfs_server._address, ctx->nfs_server.addrlen);
>>         ctx->mount_server.addrlen = ctx->nfs_server.addrlen;
>>     } else {
>>         // handle error case; maybe -EINVAL?
>>         return -EINVAL;
>>     }
>> }
>> ```
>> 
>> I could easily be misunderstanding something fundamental here, so please let me know if I'm off track. I just wanted to share this in case it's helpful.
>> 
>> Thanks for your time and for maintaining NFS!
>> 
>
>(cc'ing Kees, our resident hardening expert)
>
>FYI, that large size field is 0xffffffffffffffff (a 64-bit integer with
>all bits set to 1). The doc header over __fortify_memcpy_chk()
>definition is a little helpful, but the commit it refers to
>(6f7630b1b5bc) has a bit more info.
>
>It looks like that means that the size detection was broken for this
>memcpy check? That commit mentions that this may be due to a GCC bug.
>
>Kees, any thoughts?

Yup, the referenced commit in the comment is specifically fixing the above report (v6.1-rc1)

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/include/linux/fortify-string.h?id=6f7630b1b5bc672b54c1285ee6aba752b446672c


-- 
Kees Cook

      reply	other threads:[~2025-09-01 19:18 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-01  1:38 Question about potential buffer issue in nfs_request_mount() - seeking feedback SSH
2025-09-01 12:25 ` Jeff Layton
2025-09-01 19:18   ` Kees Cook [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=C584CAD4-B954-42C5-89DF-F3033998BEE7@kernel.org \
    --to=kees@kernel.org \
    --cc=jlayton@kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=originalssh@pm.me \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox