HomeFreeBSD

PR#259071 provides a test program that fails for the NFS client.

Description

PR#259071 provides a test program that fails for the NFS client.
Testing with it, there appears to be a race between Lookup
and VOPs like Setattr-of-size, where Lookup ends up loading
stale attributes (including what might be the wrong file size)
into the NFS vnode's attribute cache.

The race occurs when the modifying VOP (which holds a lock
on the vnode), blocks the acquisition of the vnode in Lookup,
after the RPC (with now potentially stale attributes).

Here's what seems to happen:
Child Parent

does stat(), which does
VOP_LOOKUP(), doing the Lookup
RPC with the directory vnode
locked, acquiring file attributes
valid at this point in time

blocks waiting for locked file does ftruncate(), which
vnode does VOP_SETATTR() of Size,

changing the file's size
while holding an exclusive
lock on the file's vnode
releases the vnode lock

acquires file vnode and fills in
now stale attributes including
the old wrong Size

does a read() which returns
wrong data size

This patch fixes the problem by saving a timestamp in the NFS vnode
in the VOPs that modify the file (Setattr-of-size, Allocate).
Then lookup/readdirplus compares that timestamp with the time just
before starting the RPC after it has acquired the file's vnode.
If the modifying RPC occurred during the Lookup, the attributes
in the RPC reply are discarded, since they might be stale.

With this patch the test program works as expected.

Note that the test program does not fail on a July stable/12,
although this race is in the NFS client code. I suspect a
fairly recent change to the name caching code exposed this
bug.

PR: 259071
Reviewed by: asomers
MFC after: 2 weeks
Differential Revision: https://reviews.freebsd.org/D32635

Details

Provenance
rmacklemAuthored on Oct 30 2021, 11:35 PM
Reviewer
asomers
Differential Revision
D32635: fix race between Lookup and file modifying RPCs in the NFS client
Parents
rGf0d9a6a781f3: linux: make PTRACE_SETREGS use a correct struct
Branches
Unknown
Tags
Unknown