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

[christos%zoulas.com@localhost (Christos Zoulas)]

On Nov 9,  9:38pm, kre%munnari.OZ.AU@localhost (Robert Elz) wrote:
-- Subject: Re: netbsd-7 kernel and older vnconfig(8)

| 	/*
| 	 * 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???

Yes, and this works because &vnd->sc_dkdev returns the offset from NULL
of sc_dkdev. Since the disk ioctl never touches it (dereferences it),
because it does not understand the VND ioctls, it is fine. Yes, it could
be improved. For example the following program prints 0x4:

#include <stdio.h>

struct foo {
	int x;
	int y;
};

static int *
gety(struct foo *f) {
	return &f->y;
}

int
main(void) {
	printf("%p\n", gety(NULL));
	return 0;
}

| 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.

I think that the whole thing needs a good cleanup... It is really messy now.

| 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.

Yes, that should work.

christos