NetBSD-Bugs archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: kern/42348 (lookup protocol use in do_sys_rename() is wrong)



Synopsis: lookup protocol use in do_sys_rename() is wrong

State-Changed-From-To: open->closed
State-Changed-By: dholland%NetBSD.org@localhost
State-Changed-When: Sun, 14 Aug 2011 19:29:14 +0000
State-Changed-Why:
Not a bug.

Each vnode operation consists of a lookup, one or more relookup calls,
and then either a call to VOP_ABORTOP or a real VOP_* vnode operation.
File system code can allocate at lookup time for later use, but must
then clean up the results in either VOP_ABORTOP or the real VOP call.
It is incorrect to call both VOP_ABORTOP *and* the real VOP call like
the patch here does; that results in anything allocated at lookup time
being freed twice.

Furthermore, because rename does two simultaneous lookups on what might
or might not be the same directory vnode, any such allocations have to
be done carefully so that one of the operations does not step on the
other. As far as I can tell the proposed extra VOP_ABORTOP is supposed
to help address that problem, but it does not because it doesn't take
into account the full complexity of the situation.

As I already said once, the correct way to maintain state from lookup
to a VOP call is to allocate or update it on every entry to VOP_LOOKUP;
otherwise any call to relookup will lead to memory corruption. In the
future this may change because relookup will eventually go away, but
the changes necessary for doing that will probably also invalidate this
entire issue so it can't be taken as a guide for solving the problem
now.

Even in that case there is no practical way to deal with rename correctly
without resorting to methods like I recently added in ufs_lookup; if the
two lookups in rename are in the same directory, the lookup material from
one will overwrite the other no matter how many silly tricks you play,
unless you pull it out of the vnode at the right moment. But if they are
not in the same directory, simple/naive hacks like the proposed patch
break wide open.

Also, as far as I know the underlying issue in this PR was fixed by
adding INRENAME to the namei flags.





Home | Main Index | Thread Index | Old Index