From: Jeff Layton <jlayton@kernel.org>
To: Mark Brown <broonie@kernel.org>
Cc: Trond Myklebust <trondmy@kernel.org>,
Anna Schumaker <anna@kernel.org>,
Jonathan Corbet <corbet@lwn.net>,
Shuah Khan <skhan@linuxfoundation.org>,
linux-nfs@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-doc@vger.kernel.org
Subject: Re: [PATCH 1/4] nfs: store the full NFS fileid in inode->i_ino
Date: Mon, 22 Jun 2026 18:38:59 -0400 [thread overview]
Message-ID: <655d0d2a5f8203c52c78d37462328449e49b7feb.camel@kernel.org> (raw)
In-Reply-To: <0750912a-f8dc-4714-ae11-4592d2e8eca7@sirena.org.uk>
On Mon, 2026-06-22 at 22:05 +0100, Mark Brown wrote:
> On Tue, May 12, 2026 at 12:12:42PM -0400, Jeff Layton wrote:
> > Now that inode->i_ino is a 64-bit value, store the full NFS fileid in
> > it directly instead of an XOR-folded hash. This makes NFS_FILEID() and
> > set_nfs_fileid() operate on inode->i_ino rather than the separate
> > nfsi->fileid field.
>
> This patch is in -next now and is triggering a failure for in the LTP
> ioctl10.c test for me on arm:
>
> tst_buffers.c:57: TINFO: Test is using guarded buffers
> tst_test.c:2047: TINFO: LTP version: 20260130
> tst_test.c:2050: TINFO: Tested kernel: 7.1.0-next-20260622 #1 SMP @1782128788 armv7l
>
> ...
>
> ioctl10.c:111: TFAIL: q->inode (11493907226) != entry.vm_inode (4294967295)
>
Note that the vm_inode value is arm32's ULONG_MAX.
> arm64 seems unaffected, I didn't really investigate but I'll note that
> unsigned long is 32 bit on arm.
>
> Full log:
>
> https://lava.sirena.org.uk/scheduler/job/2904745#L3852
>
> bisect log with more test job links:
>
The testcase does this:
static void parse_maps_file(const char *filename, const char *keyword, struct map_entry *entry)
{
FILE *fp = SAFE_FOPEN(filename, "r");
char line[1024];
while (fgets(line, sizeof(line), fp) != NULL) {
if (fnmatch(keyword, line, 0) == 0) {
if (sscanf(line, "%lx-%lx %s %lx %x:%x %lu %s",
&entry->vm_start, &entry->vm_end, entry->vm_flags_str,
&entry->vm_pgoff, &entry->vm_major, &entry->vm_minor,
&entry->vm_inode, entry->vm_name) < 7)
tst_brk(TFAIL, "parse maps file /proc/self/maps failed");
entry->vm_flags = parse_vm_flags(entry->vm_flags_str);
SAFE_FCLOSE(fp);
return;
}
}
SAFE_FCLOSE(fp);
tst_brk(TFAIL, "parse maps file /proc/self/maps failed");
}
Note that it's trying to stuff the inode number field into an unsigned
long. Before this patch, the maps file would have printed the old
(hashed) inode number on 32-bit. Now, it prints the full 64-bit inode
number.
I asked The Big Pickle and it says:
"In glibc (userspace): The C standard says this is undefined behavior.
In practice, glibc's scanf internally uses strtoul/strtoull, which on
overflow store ULONG_MAX/ULLONG_MAX and set errno = ERANGE. However,
scanf itself does not propagate ERANGE to the caller — it still returns
1 (success). So you'd silently get ULONG_MAX stored."
We could argue that this is a bug in the testcase. It assumes that the
maps file will never print a value larger than ULONG_MAX in that field,
and I don't see why it would make that assumption in this day and age.
Are there actual programs in the field that scrape the maps file that
might be affected by this change?
--
Jeff Layton <jlayton@kernel.org>
next prev parent reply other threads:[~2026-06-22 22:39 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-05-12 16:12 [PATCH 0/4] nfs: remove the fileid field from struct nfs_inode Jeff Layton
2026-05-12 16:12 ` [PATCH 1/4] nfs: store the full NFS fileid in inode->i_ino Jeff Layton
2026-06-22 21:05 ` Mark Brown
2026-06-22 22:38 ` Jeff Layton [this message]
2026-06-23 11:04 ` Jeff Layton
2026-06-23 13:25 ` Mark Brown
2026-05-12 16:12 ` [PATCH 2/4] nfs: remove nfs_compat_user_ino64() and deprecate enable_ino64 Jeff Layton
2026-05-12 16:12 ` [PATCH 3/4] nfs: replace NFS_FILEID() and nfsi->fileid with inode->i_ino Jeff Layton
2026-05-12 16:12 ` [PATCH 4/4] nfs: remove fileid field from struct nfs_inode Jeff Layton
2026-06-21 1:06 ` [PATCH 0/4] nfs: remove the " Jeff Layton
2026-06-22 14:45 ` Anna Schumaker
2026-06-22 11:23 ` Benjamin Coddington
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=655d0d2a5f8203c52c78d37462328449e49b7feb.camel@kernel.org \
--to=jlayton@kernel.org \
--cc=anna@kernel.org \
--cc=broonie@kernel.org \
--cc=corbet@lwn.net \
--cc=linux-doc@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-nfs@vger.kernel.org \
--cc=skhan@linuxfoundation.org \
--cc=trondmy@kernel.org \
/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