Re: procfs difference between NetBSD and Linux

[Robert Elz <kre%munnari.OZ.AU@localhost>]

    Date:        Sat, 05 Jun 2021 20:13:53 +0700
    From:        Robert Elz <kre%munnari.OZ.AU@localhost>
    Message-ID:  <16349.1622898833%jinx.noi.kre.to@localhost>

Replying to my own message again (draw your own conclusions)...

  | It applies, compiled, and builds a release with no problems, running
  | tests now.

Unfortunately, it doesn't work, kernel segv in vn_open().

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;

which is followed by the code that changed:

		        switch (cnp->cn_nameiop) {
                            case CREATE:
                                if (cnp->cn_flags & NONEXCLHACK) {

(etc).

The problem (of course) is those foundobj = NULL; and ndp->ni_vp = NULL;
lines, neither of which we want to happen in this case.

Then when we return to vn_open() (without an error) ndp->ni_vp == NULL
and kaboom.

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.   This change
makes a NONEXCLHACK CREATE op function identically to a LOOKUP op, which
I believe is what we want in this case.

Then because we're now no longer doing the

                        ndp->ni_dvp = NULL;

and the code in vn_open() relies on that, 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).

Building now and then will test this version soon (I had already run
the AFS tests, which don't test this particular scenario, apparently,
as they all worked about as well as they typically do for me, certainly
no kernel crash from them).

kre

ps: The rhialto@ suggested test:
	echo >/usr
made it really easy to test this).   Thanks.

My test setup has no (extra) mount points, or not until I get around to
mounting a procfs to test the code that failed anyway, so I can't use /usr
- but / works just as well -- / is a mount point.   Using an 8.1 kernel
(the relevant code hasn't changed in a decade - until today - so anything
vaguelly recent should give the same results):

$ echo >/
sh: cannot create /: file exists
$ echo >/bin
sh: cannot create /bin: is a directory

shows it is just as good to use for the test as any other mount point.
(the "echo" isn't needed, just ">/" works as a test, but that's immaterial).