Re: netbsd-7 kernel and older vnconfig(8)

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

    Date:        Mon, 9 Nov 2015 09:58:55 +0000
    From:        Emmanuel Dreyfus <manu%netbsd.org@localhost>
    Message-ID:  <20151109095855.GJ26689%homeworld.netbsd.org@localhost>

  | We have a backward compatibility problem with vnconfig(8).
[...]
  | I am not sure how it could be fixed.

Actually fairly easy I think.   vnconfig -l just opens vnd0
and does VNDIOCGET passing the desired unit number (which
increases from 0 until ENXIO is returned).

All that's needed is to make the kernel return ENXIO when doing
a VNDIOCGET with a unit number higher than the highest currently
configured vnd.   That's simple to do.

This won't prevent the auto creation of vnds when needed (which isn't
done using VNDIOCGET) nor does it prevent vndconfig -l vnd99
which actually opens vnd99 and then does a VNDIOCGET on that
(passing in -1 as the unit number).

Of course, neither this, nor anything, will help with someone running
existing netbsd-6 (or earlier) userland on a 7.0 release kernel.
We can get it fixed for 7.0.1 (and later) though.

But while looking at this I also see in dev/vnd.c ...

/* ARGSUSED */
static int
vndioctl(dev_t dev, u_long cmd, void *data, int flag, struct lwp *l)
{
/* ... */
	vnd = device_lookup_private(&vnd_cd, unit);
	if (vnd == NULL &&
#ifdef COMPAT_30
	    cmd != VNDIOCGET30 &&
#endif
#ifdef COMPAT_50
	    cmd != VNDIOCGET50 &&
#endif
	    cmd != VNDIOCGET)
		return ENXIO;

/*
 * so here, when cmd == VNDIOCGET (or one of the compat versions)
 * it is possible that vnd == NULL.   Or at least, that's what it suggests.
 */
	vio = (struct vnd_ioctl *)data;

	/* Must be open for writes for these commands... */
	switch (cmd) {
/* none of these is VNDIOCGET so this is irrelevant */
	}

	/* Must be initialized for these... */
	switch (cmd) {
/* none of these is VNDIOCGET so this is also irrelevant */
	}

	/*
	 * HUH???   vnd maybe NULL here!   Really???
	 */
	error = disk_ioctl(&vnd->sc_dkdev, dev, cmd, data, flag, l);
	if (error != EPASSTHROUGH)
		return error;


Anyone care to guess just how this works, or what's going on???

My suspicion is that vnd actually cannot be NULL, and the test that
allows it for VNDIOCGET is a waste of space.    That is, to get to
vndioctl() the device must have already been opened.  If necessary, that
created the necessary data structs, and NULL will never happen.
I think I'd delete the stuff that allows VNDIOCGET (and compat versions)
iv vnd happens to be NULL, and see if anything ever notices.  I'd expect
not.

To fix the driver to return ENXIO in the correct cases, I think all that
is needed is ...

	case VNDIOCGET: {
		struct vnd_user *vnu;
		struct vattr va;
		vnu = (struct vnd_user *)data;
		KASSERT(l);

		/*
		 * This fixes compat with older vnconfig(8) binaries
		 */
		if (vnu->vnu_unit >= vnd_cd.cd_ndevs)
			return ENXIO;

		switch (error = vnd_cget(l, unit, &vnu->vnu_unit, &va)) {


That one extra test...   It also needs to go in the VNDIOCGET30
and VNDIOCGET50 cases as well, to handle even older vnconfig commands.

kre