Subject: Re: broken LKM, modload and so on. (was Re: LKM module for aperture)
To: MAEKAWA Masahide <bishop@rr.iij4u.or.jp>
From: Jaromir Dolecek <jdolecek@netbsd.org>
List: current-users
Date: 09/09/2002 09:46:22
Please do NOT expose struct lkm_* to userland.
Jaromir
MAEKAWA Masahide wrote:
>
> Mihai Chelaru <kefren@netbastards.org> wrote:
> >Same issue for vmware-module package. This patch corrects the problem:
>
> Your patch and my prev patch are imcomplete.
> I prepared the new patches.
>
> Details:
>
> In the old framework, we cannot load block/character device switch at once.
> In certain situation, the kernel try to convert from block major number to
> character major number and vice versa, but the kernel doesn't know the
> relationship between loaded block/character device switch. So maybe
> some operations will fail or cause the kernel panic.
>
> Why is no bug reports?
> Because there is no 3rd party LKM module with both device switch.
>
> iwm_fd for mac68k in our tree has both device switch.
> This module is available, but nobody encounters errors or bugs.
> The reason is that this module is in out tree and knows which numbers
> should be assigned. So this module doesn't take the generic way of LKM and
> hijack the associated entries of the device switch table by memcpy(9).
>
> After my merge,
> - LKM can load block/character device switch at once.
> Kernel can convert them in anytime.
>
> - No need to hard-code the device major numbers.
> If a LKM module for the device in our tree is loaded,
> LKM framework will assign the specific number which is listed up
> in majors. If not, LKM framework will assign the number dynamically
> and reserve the number for the module as long as the machine runs.
>
> These changes are necessary to make some device drivers loadable. e.g. sd.
> (sd is not loadable yet, but...in the future...)
> But I forgot one very important thing.
> I said that "LKM can load block/character device switch at once."
> Of course, LKM framework return both assigned numbers to userland,
> but I changed nothing to modload(8), so modload(8) fails to load the module.
>
> To fix this, I have prepared the some patches.
>
> base.diff - the patches for modload(8)/modunload(8)/modstat(8).
> sys.diff - the patches for sys
> aperture.diff - the patches for pkgsrc/sysutils/aperture
> vmware-module.diff - the patches for pkgsrc/emulators/vmware-module
>
> 1. apply sys.diff
> 2. update the kernel
> 3. install kernel headers (or cp sys/sys/lkm.h /usr/include/sys)
> 4. apply base.diff
> 5. update sbin/modload, sbin/modunload and usr.bin/modstat
> 6. apply aperture.diff or vmware-module.diff and update them if necessary.
>
> Please try these patches and send the result to me.
> If good, I will commit them.
>
> Sorry my faults and thanks for bug reports.
>
> --- MAEKAWA Masahide
> --- Key fingerprint = BC5E D8CB 816C 2CB5 8560 FDE3 6CB8 BF5D 8D50 F2EE
> Index: sys/kern/kern_lkm.c
> ===================================================================
> RCS file: /cvsroot/cvs/NetBSD/syssrc/sys/kern/kern_lkm.c,v
> retrieving revision 1.58
> diff -u -u -r1.58 kern_lkm.c
> --- sys/kern/kern_lkm.c 2002/09/06 13:23:47 1.58
> +++ sys/kern/kern_lkm.c 2002/09/09 03:07:07
> @@ -550,12 +550,12 @@
> statp->type = curp->private.lkm_any->lkm_type;
> statp->area = curp->area;
> statp->size = curp->size / PAGESIZE;
> - statp->private = (unsigned long)curp->private.lkm_any;
> statp->ver = curp->private.lkm_any->lkm_ver;
> copystr(curp->private.lkm_any->lkm_name,
> statp->name,
> MAXLKMNAME - 2,
> (size_t *)0);
> + memmove(statp->private, curp->private.lkm_any, curp->privlen);
>
> break;
>
> @@ -678,7 +678,8 @@
>
> break;
>
> - case LKM_E_STAT: /* no special handling... */
> + case LKM_E_STAT:
> + lkmtp->privlen = sizeof(*args);
> break;
> }
>
> @@ -716,7 +717,8 @@
> return (error);
> break;
>
> - case LKM_E_STAT: /* no special handling... */
> + case LKM_E_STAT:
> + lkmtp->privlen = sizeof(*args);
> break;
> }
>
> @@ -752,7 +754,8 @@
> args->lkm_cdevmaj = -1;
> break;
>
> - case LKM_E_STAT: /* no special handling... */
> + case LKM_E_STAT:
> + lkmtp->privlen = sizeof(*args);
> break;
> }
>
> @@ -783,7 +786,8 @@
> case LKM_E_UNLOAD:
> break;
>
> - case LKM_E_STAT: /* no special handling... */
> + case LKM_E_STAT:
> + lkmtp->privlen = sizeof(*args);
> break;
> }
>
> @@ -818,7 +822,8 @@
> error = exec_remove(args->lkm_execsw);
> break;
>
> - case LKM_E_STAT: /* no special handling... */
> + case LKM_E_STAT:
> + lkmtp->privlen = sizeof(*args);
> break;
> }
>
> @@ -850,7 +855,8 @@
> error = emul_unregister(args->lkm_compat->e_name);
> break;
>
> - case LKM_E_STAT: /* no special handling... */
> + case LKM_E_STAT:
> + lkmtp->privlen = sizeof(*args);
> break;
> }
>
> Index: sys/kern/subr_devsw.c
> ===================================================================
> RCS file: /cvsroot/cvs/NetBSD/syssrc/sys/kern/subr_devsw.c,v
> retrieving revision 1.2
> diff -u -u -r1.2 subr_devsw.c
> --- sys/kern/subr_devsw.c 2002/09/06 13:23:49 1.2
> +++ sys/kern/subr_devsw.c 2002/09/09 01:32:43
> @@ -172,9 +172,9 @@
> }
> if (i != max_devsw_convs)
> continue;
> - *devmajor = bmajor;
> break;
> }
> + *devmajor = bmajor;
> }
> if (*devmajor >= MAXDEVSW) {
> #ifdef DEVSW_DEBUG
> @@ -226,9 +226,9 @@
> }
> if (i != max_devsw_convs)
> continue;
> - *devmajor = cmajor;
> break;
> }
> + *devmajor = cmajor;
> }
> if (*devmajor >= MAXDEVSW) {
> #ifdef DEVSW_DEBUG
> Index: sys/sys/lkm.h
> ===================================================================
> RCS file: /cvsroot/cvs/NetBSD/syssrc/sys/sys/lkm.h,v
> retrieving revision 1.21
> diff -u -u -r1.21 lkm.h
> --- sys/sys/lkm.h 2002/09/06 13:24:09 1.21
> +++ sys/sys/lkm.h 2002/09/09 02:05:01
> @@ -57,11 +57,10 @@
> #define LKM_OLDVERSION 1 /* version of module loader */
> #define LKM_VERSION 1 /* version of module loader */
> #define MAXLKMNAME 32
> +#define MAXLKMPRIVSIZE 256
>
> /****************************************************************************/
>
> -#ifdef _KERNEL
> -
> /*
> * Loadable system call
> */
> @@ -156,6 +155,7 @@
> u_long lkm_offset;
> };
>
> +#ifdef _KERNEL
>
> /*
> * Generic reference ala XEvent to allow single entry point in the xxxinit()
> @@ -189,6 +189,7 @@
>
> int (*entry) __P((struct lkm_table *, int, int));/* entry function */
> union lkm_generic private; /* module private data */
> + size_t privlen; /* size of private data */
>
> /* ddb support */
> u_long syms; /* start of symbol table */
> @@ -388,7 +389,7 @@
> MODTYPE type; /* OUT: type of module */
> u_long area; /* OUT: kernel load addr */
> u_long size; /* OUT: module size (pages) */
> - u_long private; /* OUT: module private data */
> + u_char private[MAXLKMPRIVSIZE];/* OUT: module private data */
> int ver; /* OUT: lkm compile version */
> };
>
> Index: sbin/modload/a.out.c
> ===================================================================
> RCS file: /cvsroot/cvs/NetBSD/basesrc/sbin/modload/a.out.c,v
> retrieving revision 1.1
> diff -u -u -r1.1 a.out.c
> --- sbin/modload/a.out.c 1999/06/13 12:54:40 1.1
> +++ sbin/modload/a.out.c 2002/09/09 01:04:11
> @@ -36,6 +36,7 @@
> __RCSID("$NetBSD: a.out.c,v 1.1 1999/06/13 12:54:40 mrg Exp $");
>
> #include <sys/param.h>
> +#include <sys/systm.h>
> #include <sys/ioctl.h>
> #include <sys/lkm.h>
>
> Index: sbin/modload/elf.c
> ===================================================================
> RCS file: /cvsroot/cvs/NetBSD/basesrc/sbin/modload/elf.c,v
> retrieving revision 1.10
> diff -u -u -r1.10 elf.c
> --- sbin/modload/elf.c 2002/07/24 14:14:10 1.10
> +++ sbin/modload/elf.c 2002/09/09 01:04:11
> @@ -32,6 +32,7 @@
> __RCSID("$NetBSD: elf.c,v 1.10 2002/07/24 14:14:10 joda Exp $");
>
> #include <sys/param.h>
> +#include <sys/systm.h>
>
> #if defined(__alpha__) || defined(__arch64__) || defined(__x86_64__)
> #define ELFSIZE 64
> Index: sbin/modload/modload.c
> ===================================================================
> RCS file: /cvsroot/cvs/NetBSD/basesrc/sbin/modload/modload.c,v
> retrieving revision 1.30
> diff -u -u -r1.30 modload.c
> --- sbin/modload/modload.c 2001/11/08 15:33:15 1.30
> +++ sbin/modload/modload.c 2002/09/09 03:36:43
> @@ -38,6 +38,7 @@
> #endif /* not lint */
>
> #include <sys/param.h>
> +#include <sys/systm.h>
> #include <sys/ioctl.h>
> #include <sys/conf.h>
> #include <sys/mount.h>
> @@ -454,22 +455,25 @@
> */
> if (post) {
> struct lmc_stat sbuf;
> - char id[16], type[16], offset[16];
> + char id[16], type[16], offset[2][16];
>
> sbuf.id = resrv.slot;
> if (ioctl(devfd, LMSTAT, &sbuf) == -1)
> err(15, "error fetching module stats for post-install");
> (void)snprintf(id, sizeof(id), "%d", sbuf.id);
> (void)snprintf(type, sizeof(type), "0x%x", sbuf.type);
> - (void)snprintf(offset, sizeof(offset), "%ld",
> - (long)sbuf.offset);
> - /*
> - * XXX
> - * The modload docs say that drivers can install bdevsw &
> - * cdevsw, but the interface only supports one at a time.
> - */
> - execl(post, post, id, type, offset, 0);
> - err(16, "can't exec `%s'", post);
> + if (sbuf.type == LM_DEV) {
> + struct lkm_dev *dev = (struct lkm_dev *)(sbuf.private);
> + (void)snprintf(offset[1], sizeof(offset[1]), "%ld",
> + (long)dev->lkm_cdevmaj);
> + (void)snprintf(offset[2], sizeof(offset[2]), "%ld",
> + (long)dev->lkm_bdevmaj);
> + execl(post, post, id, type, offset[1], offset[2], 0);
> + } else {
> + (void)snprintf(offset[1], sizeof(offset[1]), "%ld",
> + (long)sbuf.offset);
> + execl(post, post, id, type, offset[1], 0);
> + }
> }
>
> exit (0);
> Index: sbin/modunload/modunload.c
> ===================================================================
> RCS file: /cvsroot/cvs/NetBSD/basesrc/sbin/modunload/modunload.c,v
> retrieving revision 1.11
> diff -u -u -r1.11 modunload.c
> --- sbin/modunload/modunload.c 1999/07/22 02:04:13 1.11
> +++ sbin/modunload/modunload.c 2002/09/09 01:04:50
> @@ -38,6 +38,7 @@
> #endif /* not lint */
>
> #include <sys/param.h>
> +#include <sys/systm.h>
> #include <sys/ioctl.h>
> #include <sys/conf.h>
> #include <sys/mount.h>
> Index: usr.bin/modstat/modstat.c
> ===================================================================
> RCS file: /cvsroot/cvs/NetBSD/basesrc/usr.bin/modstat/modstat.c,v
> retrieving revision 1.16
> diff -u -u -r1.16 modstat.c
> --- usr.bin/modstat/modstat.c 2000/12/10 11:52:09 1.16
> +++ usr.bin/modstat/modstat.c 2002/09/09 01:03:08
> @@ -38,6 +38,7 @@
> #endif
>
> #include <sys/param.h>
> +#include <sys/systm.h>
> #include <sys/conf.h>
> #include <sys/file.h>
> #include <sys/ioctl.h>
> diff -x CVS -urN aperture.old/Makefile aperture/Makefile
> --- aperture.old/Makefile Sat Feb 9 07:58:54 2002
> +++ aperture/Makefile Mon Sep 9 15:48:22 2002
> @@ -2,7 +2,7 @@
>
> DISTNAME= apNetBSD
> PKGNAME= aperture-2.0
> -PKGREVISION= 1
> +PKGREVISION= 2
> CATEGORIES= sysutils x11
> EXTRACT_SUFX= .shar
>
> diff -x CVS -urN aperture.old/distinfo aperture/distinfo
> --- aperture.old/distinfo Wed May 9 01:33:23 2001
> +++ aperture/distinfo Mon Sep 9 15:47:41 2002
> @@ -6,3 +6,5 @@
> SHA1 (patch-ad) = 22aa063237f08f38712017f310d535bc3e3ad0c8
> SHA1 (patch-ae) = b6ae1d888db3a0cbefd7088a7acf7ca881fdc767
> SHA1 (patch-af) = 475c127658ba1050154a7dcabe39482c088e2d02
> +SHA1 (patch-ag) = 299cdbd9d2906a77fe4746996a7cf57b663dd07b
> +SHA1 (patch-ah) = b711bfaf6c76c8f9cf57ff314cbab771766ddf42
> diff -x CVS -urN aperture.old/patches/patch-ag aperture/patches/patch-ag
> --- aperture.old/patches/patch-ag Thu Jan 1 09:00:00 1970
> +++ aperture/patches/patch-ag Mon Sep 9 15:46:58 2002
> @@ -0,0 +1,16 @@
> +$NetBSD$
> +
> +--- module/xf86_mod.c.orig Mon Sep 9 12:29:28 2002
> ++++ module/xf86_mod.c Mon Sep 9 12:30:05 2002
> +@@ -31,7 +31,11 @@
> + 0,
> + seltrue, xf86mmap, 0};
> +
> ++#if __NetBSD_Version__ >= 106080000
> ++MOD_DEV("xf86", "xf86ap", NULL, -1, &newdev, -1)
> ++#else
> + MOD_DEV("xf86", LM_DT_CHAR, -1, &newdev)
> ++#endif
> +
> + char *xf86_major_version = "2";
> + char *xf86_minor_version = "0";
> diff -x CVS -urN aperture.old/patches/patch-ah aperture/patches/patch-ah
> --- aperture.old/patches/patch-ah Thu Jan 1 09:00:00 1970
> +++ aperture/patches/patch-ah Mon Sep 9 15:46:58 2002
> @@ -0,0 +1,15 @@
> +$NetBSD$
> +
> +--- module/xf86_mod_install.orig Mon Sep 9 12:30:38 2002
> ++++ module/xf86_mod_install Mon Sep 9 12:30:47 2002
> +@@ -4,8 +4,8 @@
> + #
> + # Copyright (C) 1994,1999 The XFree86 Project Inc.
> + #
> +-if [ $# -ne 3 ]; then
> +- echo "$0: should be called by modload(8) with 3 arguments"
> ++if [ $# -ne 4 ]; then
> ++ echo "$0: should be called by modload(8) with 4 arguments"
> + exit 1
> + fi
> +
> diff -x CVS -urN vmware-module.old/distinfo vmware-module/distinfo
> --- vmware-module.old/distinfo Sat Aug 11 08:17:22 2001
> +++ vmware-module/distinfo Mon Sep 9 14:43:58 2002
> @@ -5,5 +5,7 @@
> SHA1 (patch-aa) = dc008ea571bb2fd15be77a3bd39bfdecf9f1f243
> SHA1 (patch-ab) = 2ff6e47f7e57c56ac026749e579bf4ebe0d8ac6f
> SHA1 (patch-ac) = 528dab24b0daebb97fcfd911d9abe0c60804351a
> -SHA1 (patch-ad) = b04cb3a5422fbe5f97c9f5998272b9c7e2c64e47
> -SHA1 (patch-ae) = 9f4025f006228505d14d241e2a89be291105e7f4
> +SHA1 (patch-ad) = 2622ed9136a78cfe1d0ab2c27340d49bc26fa84a
> +SHA1 (patch-ae) = 4705b4b34bc19ed405cb3f05e8212eeda3683b5b
> +SHA1 (patch-ag) = f2471e0b29748d65c50af9415e2a420903e1efdc
> +SHA1 (patch-ah) = 67a8e4dc9d43bbde562752a3e660bf459747922a
> diff -x CVS -urN vmware-module.old/patches/patch-ad vmware-module/patches/patch-ad
> --- vmware-module.old/patches/patch-ad Sat Aug 11 08:17:23 2001
> +++ vmware-module/patches/patch-ad Mon Sep 9 14:30:50 2002
> @@ -12,9 +12,21 @@
> +#else
> +# define HUBMAXPKT (ETHER_MAX_FRAME(ETHERTYPE_VLAN, 1))
> +#endif
> ---- source/vmnet/if_hubmod.c Tue Apr 3 22:51:19 2001
> -+++ source/vmnet/if_hubmod.c Sat Jun 23 23:56:58 2001
> -@@ -141,10 +141,19 @@
> +--- source/vmnet/if_hubmod.c.orig Wed Apr 4 05:51:19 2001
> ++++ source/vmnet/if_hubmod.c Mon Sep 9 14:28:13 2002
> +@@ -128,7 +128,11 @@
> + static struct hubdev_softc *hub_scs[MAXHUBDEVS];
> +
> +
> ++#if __NetBSD_Version__ >= 106080000
> ++MOD_DEV("vmnet", "vmnet", NULL, -1, &hub_dev, -1)
> ++#else
> + MOD_DEV("vmnet", LM_DT_CHAR, -1, &hub_dev)
> ++#endif
> +
> + int
> + if_hub_lkmentry(struct lkm_table *lkmtp, int cmd, int ver)
> +@@ -141,10 +145,19 @@
> {
> int error = 0, i, j;
> struct hubdev_softc *sc;
> @@ -34,7 +46,7 @@
> printf("if_hub: HUBMAXPKT > MCLBYTES\n");
> return EIO;
> }
> -@@ -496,6 +505,11 @@
> +@@ -496,6 +509,11 @@
> fp->f_type = DTYPE_VNODE;
> fp->f_ops = &vnops;
> fp->f_data = (caddr_t)vp;
> @@ -46,7 +58,7 @@
> FILE_UNUSE(fp, p);
>
> p->p_dupfd = fd;
> -@@ -903,6 +917,9 @@
> +@@ -903,6 +921,9 @@
> struct hubport_softc *portsc;
> struct mbuf *m;
> int error;
> @@ -56,7 +68,7 @@
>
> hubsc = hub_scs[HUBUNIT(dev)];
> if (hubsc == NULL)
> -@@ -917,7 +934,12 @@
> +@@ -917,7 +938,12 @@
> if (uio->uio_resid == 0)
> return (0);
>
> diff -x CVS -urN vmware-module.old/patches/patch-ae vmware-module/patches/patch-ae
> --- vmware-module.old/patches/patch-ae Sat Jul 7 23:40:07 2001
> +++ vmware-module/patches/patch-ae Mon Sep 9 14:31:07 2002
> @@ -1,8 +1,20 @@
> $NetBSD: patch-ae,v 1.1.1.1 2001/07/07 14:40:07 veego Exp $
>
> ---- source/vmmon/netbsd/drv.c Tue Apr 3 00:41:32 2001
> -+++ source/vmmon/netbsd/drv.c Sat Jun 23 22:12:21 2001
> -@@ -280,6 +280,11 @@
> +--- source/vmmon/netbsd/drv.c.orig Tue Apr 3 07:41:32 2001
> ++++ source/vmmon/netbsd/drv.c Mon Sep 9 14:26:47 2002
> +@@ -121,7 +121,11 @@
> + static int vmmon_debug = 0;
> +
> +
> ++#if __NetBSD_Version__ >= 106080000
> ++MOD_DEV("vmmon", "vmmon", NULL, -1, &vmmon_dev, -1)
> ++#else
> + MOD_DEV("vmmon", LM_DT_CHAR, -1, &vmmon_dev)
> ++#endif
> +
> + int
> + vmmon_lkmentry(struct lkm_table *lkmtp, int cmd, int ver)
> +@@ -280,6 +284,11 @@
> fp->f_type = DTYPE_VNODE;
> fp->f_ops = &vnops;
> fp->f_data = (caddr_t)vp;
> diff -x CVS -urN vmware-module.old/patches/patch-ag vmware-module/patches/patch-ag
> --- vmware-module.old/patches/patch-ag Thu Jan 1 09:00:00 1970
> +++ vmware-module/patches/patch-ag Mon Sep 9 14:31:40 2002
> @@ -0,0 +1,16 @@
> +$NetBSD$
> +
> +--- source/linuxrtc/rtc.c.orig Mon Sep 9 14:23:48 2002
> ++++ source/linuxrtc/rtc.c Mon Sep 9 14:28:22 2002
> +@@ -87,7 +87,11 @@
> + };
> +
> +
> ++#if __NetBSD_Version__ >= 106080000
> ++MOD_DEV("linuxrtc", "linuxrtc", NULL, -1, &rtc_dev, -1)
> ++#else
> + MOD_DEV("linuxrtc", LM_DT_CHAR, -1, &rtc_dev)
> ++#endif
> +
> + int
> + rtc_lkmentry(struct lkm_table *lkmtp, int cmd, int ver)
> diff -x CVS -urN vmware-module.old/patches/patch-ah vmware-module/patches/patch-ah
> --- vmware-module.old/patches/patch-ah Thu Jan 1 09:00:00 1970
> +++ vmware-module/patches/patch-ah Mon Sep 9 14:31:58 2002
> @@ -0,0 +1,41 @@
> +$NetBSD$
> +
> +--- share/lkm/if_hub_post.sh.orig Mon Sep 9 14:18:18 2002
> ++++ share/lkm/if_hub_post.sh Mon Sep 9 14:18:35 2002
> +@@ -7,8 +7,8 @@
> + #
> + # modload netbsd_post.sh if_hub.o
> + #
> +-if [ $# -ne 3 ]; then
> +- echo "$0 should only be called from modload(8) with 3 args"
> ++if [ $# -ne 4 ]; then
> ++ echo "$0 should only be called from modload(8) with 4 args"
> + exit 1
> + fi
> +
> +--- share/lkm/linuxrtc_post.sh.orig Mon Sep 9 14:18:42 2002
> ++++ share/lkm/linuxrtc_post.sh Mon Sep 9 14:18:54 2002
> +@@ -7,8 +7,8 @@
> + #
> + # modload -e rtc_lkmentry -p rtc_post.sh linuxrtc.o
> + #
> +-if [ $# -ne 3 ]; then
> +- echo "$0 should only be called from modload(8) with 3 args"
> ++if [ $# -ne 4 ]; then
> ++ echo "$0 should only be called from modload(8) with 4 args"
> + exit 1
> + fi
> +
> +--- share/lkm/vmmon_post.sh.orig Mon Sep 9 14:18:59 2002
> ++++ share/lkm/vmmon_post.sh Mon Sep 9 14:19:09 2002
> +@@ -7,8 +7,8 @@
> + #
> + # modload -e vmmon_lkmentry -p vmmon_post.sh vmmon.o
> + #
> +-if [ $# -ne 3 ]; then
> +- echo "$0 should only be called from modload(8) with 3 args"
> ++if [ $# -ne 4 ]; then
> ++ echo "$0 should only be called from modload(8) with 4 args"
> + exit 1
> + fi
> +
--
Jaromir Dolecek <jdolecek@NetBSD.org> http://www.NetBSD.org/
-=- We should be mindful of the potential goal, but as the tantric -=-
-=- Buddhist masters say, ``You may notice during meditation that you -=-
-=- sometimes levitate or glow. Do not let this distract you.'' -=-