Date: Mon, 24 Jan 2000 23:25:52 -0800 (PST) From: Matthew Dillon <dillon@apollo.backplane.com> To: current@FreeBSD.ORG Subject: nqnfs bug PR kern/15883, patch to review Message-ID: <200001250725.XAA99663@apollo.backplane.com>
next in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200001250725.XAA99663>