Bug 18270

Some reports on current@freebsd.org
indicated that after "kldunload"ing the vn device, further use of
/dev/vn* caused panics.

I saw a few problems, and attach a patch that addresses them
and appears to fix the crashes on my -stable box.

1: There is no cdesw_remove() when the module is unloaded.

2: Any dev_t's that make it to the driver get associated with a vn
softc. After the driver is unloaded and reloaded, these pointers
are still set in the dev_t's, but refer to the old, free()d softcs.

3: There are no calls to destroy_dev to match the calls to make_dev.


The included patch is complicated slightly because more than one dev_t
can be associated with a vn_softc, and for each softc, only one call to make_dev
is made. The patch keeps track of the "aliases" for the vn_softc by
chaining such aliases through the si_drv2 field in the dev_t.


I reproduced the problem on 4.0-STABLE, and after the patch, several
"kldunload vn.ko/vnconfig" operations leave my machine in one piece.
The problem probably exists in -current, and the patch applies cleanly
(modulo line offset changes) to the -current vn.c also

I'm not the most experienced FreeBSD Kernel Hacker, so a solid review
and comments would be appreciated. I really think there is some ugliness
in the way the vn device sets up the softc's and devices. It should
probably do a make_dev for each different dev_t that refers to a unit
when the unit is first opened, and probably also a make_dev() for a
configurable number of vn's at load time, to get them to appear under
devfs. If someone can confirm that this is an accurate analysis, I'll
spend some time fixing it up, if no one else has time.

Fix: I've attached the patch as compressed uuencoded, as well as plaintext,
to get around cut-n-paste whitespace corruption.




UUencoded:begin 644 patch.Z
M'YV0*@*"L./&Q1@7;]B027"E#!D00>#(`2'#!H@8-G3(F*$#Q@R*,$(J:$%R
M8,$Q"8S(20-1(D4<%V/HJ)&11@R0(@/JW,E3A0*=%V?D8!&#!@T0.T>6C"&4
M*`V8)$DJ``$B01HW=!(D`#%GS)<V8?"$&4,GC9TR.Q*\4`$"+!X0(T"\,0-B
M;-FS(.24B5.GS!PZ<ZJJ>#&UZE\Y=<B"$%/'S-:N7^B$$9-V+0@Z<L*XF6.F
MS$2^9?I633"X<((Z7]B\<7-&*U>O;^"473VG,MO8LS>/)DUX154R9>Q$=@T9
MN!TV:?[:!H'\KURZQM.,\0OB#AKI:/*6Z3R1SAL%OK6*IWH931FN8=J<)PBB
MCILT=%R`H&*>>9B_X'>[IKKZ_.KRYS5'!PC)`2B7&V7D)]Y^((RA5QAT.+18
M'FV%L4897QBG56E4)3`%$TE,0<4713A!A119H$#0%W/,1<<8*;R66G)T[%!8
M'S92]5-///H$5`PW#"5#2$CI9)I[\('0`PADK($D'2@8ET*.5;&WI'$M^#!'
M&AG*84<,5%I%%PHA$!3C'J9Y"**(7QCQA!1%!#$$$BJZP0(()JPHX)V0"7AF
MFF*"4&>6D#VIY))/_MFA>%AJR2497MZT)$%ABB?&@VM4FD`?:7+:H:=5I3%F
MF6XH.IJ5%;*AVA@H;*E'&7,A1="=37Q!1!%6"%&%$;1^<44025#QQ!)3IBFJ
MH*2F`*A>=-0AAQN".E$%$TP4NZ@8K\KQ1IU\IO%JK"J862E!A'IE**+OU9AF
MHUMV^:62)MD8PFA@79AA<"CDZ<878Q@WQQUWPG!GFN2!4$421'PAQ1-/4''G
M$0A_\00414@1A+!2!&P##0*#(`)!)9`APIV)5OIAB",FX<04%8^(1)Q$Y*LG
MC7?.*J.?88*:`+/.0EMGL5%=A,,,+,@@PTU1M7!DNO`V^624P5E;);17!D?H
MHY&&>2R99H*`9GAJL35%&><=>\=Y:(2!5QAL/$@&A6&8Q)6+8X!@QAO=71<8
M&<FMT5ZZ\G$XVLELN@FGG'3:K.^,?_'IE9]>`[HUN5J:RW0/Z,+W)]A:6:9$
M'<Z%0<9#WAG(=AKW,4=CX+WIQR[6=L@`+^7%!2>@C9PG0+M7Q@G8='":)O"Z
MNY+&"V@"EY9AH:8Z;VJ:SELG&SG8:RFX%5M.O`&"1,&E\0;H[<U!7:QTZ"WW
MDSHT2!MFB0T(7V`MFO%B76Z083U2'H^AV6K2L2T"D]V;C@ON5QJP&2<R#=K?
M>_3'AGO985Q42]6J6N4M6-$E7'8"0:UNE:M=]>I7P1J6U+02/3,MJPS->E:T
MIE6M2F'+,]NRF:LL**M20;!<7SC7W^!3J>%!ZEV3<H.\1J,_-_"/@0Z$5[TP
M9!R9[:M?P?E7P`:VJ((=+&$+:]C#(C:QBEWL31KCV)T^YH:0C6R'="@6V(IX
M1+8Y\&K$FYT0K<?&!;K1AUZ2W9)@@+M3N0&'O:,1O.K8OP8:IX\D'!,AD6@<
M$(2@:G;8W&XL$X31F0XYJ3M;7?0B%SB4X3VLN8SVRG>>TZ7.=P7<#1YC)T=`
MVHY&B-3*[ASH.TC&LGF$2]G*6O:%EP4A9HO;DTD<Q[@TYLPT/%/ASVS4(Q[M
MB"TWL`'1;A#-(@5$*2T`P0UP0`,6X"`&LDM:FBQC!K:QH3S:JL,9LB.X!.A/
M?!I\0L*F@(0J4($(3[B"$W20IKM-!`4[V($D&14<!!KG3@C"@[H6I;L(YK)-
M29""B)Q8S!$F$EDF9"CR,*6IA\*I"4^P0A%Z"3.*"K-/-+)HH`95N2^8@0UA
M.$-@3``"*SC!"%]06;"*0`1E:90@8V"#\N2PS/Q8+VQ'Q=\0A*J9.L"A+JHR
MT`'I<(+`E`]"VN&.*,M3(((DE2U/NI-F'@*<P[PA#TDD90*-:,=S1F<Z7VW0
M@R+TD#O`)SM+=.!76R<>?PJJD4%TY7%@"<`'%A9>"4WC]/23V-_9`8X_E$$L
MM;+*XJU2CR#@HU%SM[7&8BZSRLJ=\/R"F;,Z$&J1G&SS$F`&O92!6QJT%:YT
G902+<FJ-_KK#%_32AC><A:)0M,._+)J\Y9D&..6L`QOHP,]K<50!--qGmZAqz3GeVUPvUFVxjEPznoyBWbcnvxoxJJhvk1245YFuiz
Content-Type: text/plain; name="file.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment; filename="file.diff"

*** vn.c.old    Wed Apr 26 16:23:03 2000
--- vn.c        Fri Apr 28 11:56:41 2000
***************
*** 139,144 ****
--- 139,148 ----
        int              sc_maxactive;  /* max # of active requests     */
        struct buf       sc_tab;        /* transfer queue               */
        u_long           sc_options;    /* options                      */
+       dev_t            sc_devlist;    /* list of devices which refer to
+                                          the same vn unit. The last
+                                          one on the list is the one
+                                          created by make_dev          */
        SLIST_ENTRY(vn_softc) sc_list;
  };
  
***************
*** 179,200 ****
        unit = dkunit(dev);
        vn = dev->si_drv1;
        if (!vn) {
                SLIST_FOREACH(vn, &vn_list, sc_list) {
                        if (vn->sc_unit == unit) {
                                dev->si_drv1 = vn;
                                break;
                        }
                }
        }
        if (!vn) {
                vn = malloc(sizeof *vn, M_DEVBUF, M_WAITOK);
                if (!vn)
                        return (NULL);
                bzero(vn, sizeof *vn);
                vn->sc_unit = unit;
                dev->si_drv1 = vn;
!               make_dev(&vn_cdevsw, 0, 
                    UID_ROOT, GID_OPERATOR, 0640, "vn%d", unit);
                SLIST_INSERT_HEAD(&vn_list, vn, sc_list);
        }
        return (vn);
--- 183,221 ----
        unit = dkunit(dev);
        vn = dev->si_drv1;
        if (!vn) {
+               /* See if we have already a vn softc for this disk unit. */
                SLIST_FOREACH(vn, &vn_list, sc_list) {
                        if (vn->sc_unit == unit) {
+                               /* Just add to the alias list. */
+                               dev->si_drv2 = vn->sc_devlist;
+                               vn->sc_devlist = dev;
                                dev->si_drv1 = vn;
                                break;
                        }
                }
        }
        if (!vn) {
+               /*
+                * No previous uses of this vn unit: construct its softc and
+                * "canonical" device.
+                */
+               dev_t canonical_dev;
                vn = malloc(sizeof *vn, M_DEVBUF, M_WAITOK);
                if (!vn)
                        return (NULL);
                bzero(vn, sizeof *vn);
                vn->sc_unit = unit;
                dev->si_drv1 = vn;
!               canonical_dev = make_dev(&vn_cdevsw, 0, 
                    UID_ROOT, GID_OPERATOR, 0640, "vn%d", unit);
+               canonical_dev->si_drv1 = vn;
+               canonical_dev->si_drv2 = 0;
+               vn->sc_devlist = canonical_dev;
+               if (canonical_dev != dev) {
+                       /* Add the alias we are opening to the alias list */
+                       dev->si_drv2 = vn->sc_devlist;
+                       vn->sc_devlist = dev;
+               }
                SLIST_INSERT_HEAD(&vn_list, vn, sc_list);
        }
        return (vn);
***************
*** 763,776 ****
--- 784,812 ----
                /* fall through */
        case MOD_SHUTDOWN:
                for (;;) {
+                       dev_t dev, next;
                        vn = SLIST_FIRST(&vn_list);
                        if (!vn)
                                break;
                        SLIST_REMOVE_HEAD(&vn_list, sc_list);
                        if (vn->sc_flags & VNF_INITED)
                                vnclear(vn);
+ 
+                       /*
+                        * Cleanup all the dev_t's that refer to this vn
+                        * unit, and destroy_dev the canonical device
+                        * created with make_dev
+                        */
+                       for (dev = vn->sc_devlist; dev; dev = next) {
+                               next = dev->si_drv2;
+                               dev->si_drv1 = dev->si_drv2 = 0;
+ 
+                               if (next == 0)
+                                       destroy_dev(dev);
+                       }
                        free(vn, M_DEVBUF);
                }
+               cdevsw_remove(&vn_cdevsw);
                break;
        default:
                break;