On Sun, Nov 18, 2012 at 05:41:55PM +0000, Emmanuel Dreyfus wrote:
> Also implement O_SEARCH for openat(2)
What is this logic supposed to do?
+ if (!(dfp->f_flag & FSEARCH)) {
+ error = VOP_ACCESS(dfp->f_data, VEXEC, l->l_cred);
+ if (error)
+ goto cleanup;
+ }
It appears three times, and in all three cases it:
(1) is highly suspect because it bypasses a security check if
FSEARCH (aka O_SEARCH) is set, but no security check that I can
find is required to set FSEARCH;
(2) is not atomic and is therefore just as useless as calling
access(2) for security checking;
(3) violates the vnode locking protocol;
(4) is fortunately unexploitable on these points because it is
redundant and therefore useless.
Also, the semantics of these new O_* flags remains under discussion in
tech-kern, as they seem to be suspect (not unrelated to point 1) so
at least the O_SEARCH part of this commit seems rather premature.
(Did you post the O_SEARCH changes for review? If so, I must have
missed them.)
In any event please revert at least the O_SEARCH changes. Once the
discussion of the semantics is finished, please post a new patch that
implements the conclusion of the discussion and doesn't suffer from
the points noted above.
thanks.
--
David A. Holland
dholl...@netbsd.org
