On Sat, Jun 05, 2021 at 11:03:05PM +0700, Robert Elz wrote:
> | It applies, compiled, and builds a release with no problems, running
> | tests now.
>
> Unfortunately, it doesn't work, kernel segv in vn_open().
If it had worked first go I would have been very nervous :-)
> I believe the cause is this code (in namei()):
>
> if (cnp->cn_nameiop != LOOKUP &&
> (searchdir == NULL ||
> searchdir->v_mount != foundobj->v_mount)) {
> if (searchdir) {
> /*... irrelevant for now */
> }
> vrele(foundobj);
> foundobj = NULL;
> ndp->ni_dvp = NULL;
> ndp->ni_vp = NULL;
> state->attempt_retry = 1;
Right, it would need to keep foundobj on that path. Missed tht, I
guess...
> I am trying a fix for this by making the initial test shown above be:
>
> if (cnp->cn_nameiop != LOOKUP &&
> (cnp->cn_flags & NONEXCLHACK) == 0 &&
> (searchdir == NULL ||
> searchdir->v_mount != foundobj->v_mount)) {
>
> which of course then makes the test of NONEXCLHACK inside "case CREATE:"
> meaningless, but harmless, so I just left that for now.
Yeah, but please revert it before committing.
> I added
>
> if (foundobj != NULL && cnp->cn_flags & NONEXCLHACK) {
> if (searchdir != NULL) {
> if (searchdir_locked) {
> VOP_UNLOCK(searchdir);
> searchdir_locked = false;
> }
> vrele(searchdir);
> }
> searchdir = NULL;
> }
>
> which might be overly complicated, but seems to fit with what is needed
> (or done anyway) in what comes later when searchdir != NULL.
> (searchdir is later placed into ndp->ni_dvp).
It's not complicated enough, I think it probably needs a test that
foundobj != searchdir. :-|
also slap a comment on that that it's only actually needed for the
case where foundobj is a mount point, or maybe explicitly include that
test, or otherwise we'll forget the context and a few years down the
line be confused about what's actually going on.
--
David A. Holland
dholl...@netbsd.org
