From owner-freebsd-current Mon Jan 24 23:25:56 2000 Delivered-To: freebsd-current@freebsd.org Received: from apollo.backplane.com (apollo.backplane.com [216.240.41.2]) by hub.freebsd.org (Postfix) with ESMTP id D75E414A1F for ; Mon, 24 Jan 2000 23:25:52 -0800 (PST) (envelope-from dillon@apollo.backplane.com) Received: (from dillon@localhost) by apollo.backplane.com (8.9.3/8.9.1) id XAA99663; Mon, 24 Jan 2000 23:25:52 -0800 (PST) (envelope-from dillon) Date: Mon, 24 Jan 2000 23:25:52 -0800 (PST) From: Matthew Dillon Message-Id: <200001250725.XAA99663@apollo.backplane.com> To: current@FreeBSD.ORG Subject: nqnfs bug PR kern/15883, patch to review Sender: owner-freebsd-current@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG This patch is designed to fix PR kern/15883. This PR is very easy to reproduce. http://www.freebsd.org/cgi/query-pr.cgi?pr=15883 Unfortunately the patch is not entirely trivial. NQNFS has a horrendous bug in the code in question -- it's accessing a structure pointer from union elements that are not properly initialized when you use a UDP NQNFS mount. TCP NQNFS mounts are not effected. There are probably only a handful of people that understand this code well enough to review it, unfortunately. If I don't get any responses I'll get a special dispensation to commit the beast because UDP NQNFS mounts are dangerous without it (and NQNFS is considered highly experimental anyway). I've emailed the author of the PR but may not get a response quickly enough. -Matt Index: nfs_nqlease.c =================================================================== RCS file: /home/ncvs/src/sys/nfs/nfs_nqlease.c,v retrieving revision 1.48 diff -u -r1.48 nfs_nqlease.c --- nfs_nqlease.c 1999/12/19 01:55:37 1.48 +++ nfs_nqlease.c 2000/01/25 07:01:01 @@ -387,6 +387,7 @@ return; } nsso = slp->ns_so; + lph->lph_slp = slp; if (nsso && nsso->so_proto->pr_protocol == IPPROTO_UDP) { saddr = (struct sockaddr_in *)nam; lph->lph_flag |= (LC_VALID | LC_UDP); @@ -399,7 +400,6 @@ #endif } else { lph->lph_flag |= (LC_VALID | LC_SREF); - lph->lph_slp = slp; slp->ns_sref++; } } @@ -506,7 +506,6 @@ register int siz; struct nqm *lphnext = lp->lc_morehosts; struct mbuf *m, *mreq, *mb, *mb2, *mheadend; - struct socket *so; struct sockaddr *nam2; struct sockaddr_in *saddr; nfsfh_t nfh; @@ -514,12 +513,16 @@ caddr_t bpos, cp; u_int32_t xid, *tl; int len = 1, ok = 1, i = 0; - int sotype, *solockp; while (ok && (lph->lph_flag & LC_VALID)) { - if (nqsrv_cmpnam(slp, nam, lph)) + if (nqsrv_cmpnam(slp, nam, lph)) { lph->lph_flag |= LC_VACATED; - else if ((lph->lph_flag & (LC_LOCAL | LC_VACATED)) == 0) { + } else if ((lph->lph_flag & (LC_LOCAL | LC_VACATED)) == 0) { + struct socket *so; + int sotype; + int *solockp = NULL; + + so = lph->lph_slp->ns_so; if (lph->lph_flag & LC_UDP) { MALLOC(nam2, struct sockaddr *, sizeof *nam2, M_SONAME, M_WAITOK); @@ -528,20 +531,16 @@ saddr->sin_family = AF_INET; saddr->sin_addr.s_addr = lph->lph_inetaddr; saddr->sin_port = lph->lph_port; - so = lph->lph_slp->ns_so; } else if (lph->lph_flag & LC_CLTP) { nam2 = lph->lph_nam; - so = lph->lph_slp->ns_so; } else if (lph->lph_slp->ns_flag & SLP_VALID) { nam2 = (struct sockaddr *)0; - so = lph->lph_slp->ns_so; - } else + } else { goto nextone; + } sotype = so->so_type; if (so->so_proto->pr_flags & PR_CONNREQUIRED) solockp = &lph->lph_slp->ns_solock; - else - solockp = (int *)0; nfsm_reqhead((struct vnode *)0, NQNFSPROC_EVICTED, NFSX_V3FH + NFSX_UNSIGNED); fhp = &nfh.fh_generic; @@ -576,11 +575,11 @@ * nfs_sndlock if PR_CONNREQUIRED XXX */ - if (((lph->lph_flag & (LC_UDP | LC_CLTP)) == 0 && - (lph->lph_slp->ns_flag & SLP_VALID) == 0) || - (nfs_slplock(lph->lph_slp, 0) == 0)) + if ((lph->lph_flag & (LC_UDP | LC_CLTP)) == 0 && + ((lph->lph_slp->ns_flag & SLP_VALID) == 0 || + nfs_slplock(lph->lph_slp, 0) == 0)) { m_freem(m); - else { + } else { (void) nfs_send(so, nam2, m, (struct nfsreq *)0); if (solockp) Index: nqnfs.h =================================================================== RCS file: /home/ncvs/src/sys/nfs/nqnfs.h,v retrieving revision 1.20 diff -u -r1.20 nqnfs.h --- nqnfs.h 1999/12/29 04:54:55 1.20 +++ nqnfs.h 2000/01/25 07:02:20 @@ -87,31 +87,26 @@ #define LC_MOREHOSTSIZ 10 struct nqhost { + u_int16_t lph_flag; + u_int16_t lph_port; + struct nfssvc_sock *lph_slp; + union { struct { - u_int16_t udp_flag; - u_int16_t udp_port; union nethostaddr udp_haddr; } un_udp; struct { - u_int16_t connless_flag; - u_int16_t connless_spare; union nethostaddr connless_haddr; } un_connless; struct { - u_int16_t conn_flag; - u_int16_t conn_spare; - struct nfssvc_sock *conn_slp; + int dummy; } un_conn; } lph_un; }; -#define lph_flag lph_un.un_udp.udp_flag -#define lph_port lph_un.un_udp.udp_port #define lph_haddr lph_un.un_udp.udp_haddr #define lph_inetaddr lph_un.un_udp.udp_haddr.had_inetaddr #define lph_claddr lph_un.un_connless.connless_haddr #define lph_nam lph_un.un_connless.connless_haddr.had_nam -#define lph_slp lph_un.un_conn.conn_slp struct nqlease { LIST_ENTRY(nqlease) lc_hash; /* Fhandle hash list */ @@ -123,7 +118,7 @@ char lc_fiddata[MAXFIDSZ]; struct vnode *lc_vp; /* Soft reference to associated vnode */ }; -#define lc_flag lc_host.lph_un.un_udp.udp_flag +#define lc_flag lc_host.lph_flag /* lc_flag bits */ #define LC_VALID 0x0001 /* Host address valid */ To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-current" in the body of the message