Re: PUFFS lookup/reclaim race

[David Holland <dholland-tech%netbsd.org@localhost>]

On Tue, Jul 17, 2012 at 07:38:01AM +0000, David Holland wrote:
 > > > Yeah: don't return copies of things that are basically pointers (in
 > > > this case, the cookie) without doing proper usage tracking on them. It
 > > > isn't valid to reclaim something that someone else is using; 
 > > 
 > > The problem is that the kernel reclaims a vnode that is not yet
 > > been used by lookup. 
 > > 
 > > When kernel sends a lookup reuest to userland, it causes a node
 > > to be referenced in userland, but it does not know what vnode
 > > is associated before it gets the lookup reply. If the vnode
 > > already existed, it can be reclaimed before the lookup reply,
 > > there is no way to hold a reference on it, as we do not yet
 > > know what vnode we have to work with.
 > 
 > No, but you can (and need to) hold a reference on the "cookie". 
 >
 > [and a bunch of meaningless bollocks, ignore it all]

From your original mail:

 > - vnode V for file F is associated with cookie C
 >
 > - enter puffs_vnop_lookup(), looking up file F
 > - puffs_vnop_lookup() sends PUFFS_LOOKUP to userland
 > - userland finds cookie C for F, sends reply to kernel [X]
 > - someone else in kernel do vput(V),  puffs_vnop_reclaim(V) is called
 > - puffs_vnop_reclaim(V) sends PUFFS_RECLAIM for C
 > - userland reclaim cookie C [Y]
 >
 > - puffs_vnop_lookup() gets its PUFFS_LOOKUP reply with cookie C
 > - puffs_vnop_lookup() calls puffs_cookie2vnode() and do not find a vnode
 > - puffs_vnop_lookup() calls puffs_getvnode() and get vnode V from freelist
 > - puffs_vnop_lookup() sets V's cookie to C and returns
 >
 > Next PUFFS operation on V will crash the filesystem, since C has been
 > reclaimed in userland and is now invalid.   

The bug here is in the points marked [X] and [Y]. At point [X] you
need to take out a new reference on the cookie C and pass it to the
kernel. This additional reference will prevent C from going away at
point [Y].

You also need to make sure that the kernel knows how many references
to C it's been given, so it can drop that many when it calls
PUFFS_RECLAIM.

In the case where PUFFS_RECLAIM happens before the PUFFS_LOOKUP reply
drops in another reference, PUFFS_RECLAIM will not drop all the extant
references to C and C will remain active. If PUFFS_RECLAIM happens
after the PUFFS_LOOKUP reply is received, it will drop all the
references... but in that case PUFFS_RECLAIM won't be called until the
kernel's done with the reference sent back by PUFFS_LOOKUP.

-- 
David A. Holland
dholland%netbsd.org@localhost