Re: [PATCH 1/4] nfs: store the full NFS fileid in inode->i_ino

[Jeff Layton]

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@xxxxxxxxxx>