Date: Thu, 30 Jul 2020 01:52:35 +0000 From: Rick Macklem <rmacklem@uoguelph.ca> To: Brooks Davis <brooks@FreeBSD.org>, "src-committers@freebsd.org" <src-committers@freebsd.org>, "svn-src-all@freebsd.org" <svn-src-all@freebsd.org>, "svn-src-stable@freebsd.org" <svn-src-stable@freebsd.org>, "svn-src-stable-12@freebsd.org" <svn-src-stable-12@freebsd.org> Subject: Re: svn commit: r363625 - stable/12/usr.sbin/mountd Message-ID: <QB1PR01MB3364334927A6502C76338B42DD710@QB1PR01MB3364.CANPRD01.PROD.OUTLOOK.COM> In-Reply-To: <202007272318.06RNIFjV005206@repo.freebsd.org> References: <202007272318.06RNIFjV005206@repo.freebsd.org>
index | next in thread | previous in thread | raw e-mail
Brooks Davis wrote: >Author: brooks >Date: Mon Jul 27 23:18:14 2020 >New Revision: 363625 >URL: https://svnweb.freebsd.org/changeset/base/363625 > >Log: > MFC r363439: > > Correct a type-mismatch between xdr_long and the variable "bad". > > Way back in r28911 (August 1997, CVS rev 1.22) we imported a NetBSD > information leak fix via OpenBSD. Unfortunatly we failed to track the > followup commit that fixed the type of the error code. Apply the change > from int to long now. I don't think this is correct. RFC-1813 defines the error return for a MNT RPC as a 32bit quantity. Way back when this stuff was written it was in K&R days and "long" was always a 32bit integer. If you look at head/lib/libc/xdr/xdr.c you'll see "long" used to refer to 32bit numbers throughout it. Look near the end, where it does a "longlong" (64bits) using 2 longs. The good news w.r.t. this ancient code is that XDR_PUTLONG() assumes 32bits. Also, note that xdr_int() and xdr_long() do exactly the same thing. I support int32_t would be preferred to "int" to make sure "bad" is 32bits and then you can use xdr_int32_t(), which does exactly the same thing as xdr_int() and about the same thing as xdr_long(). { They all assume a "long" is 32bits. Scary to look at now that "long" isn't always 32bits. } --> I can't see how the xdr.c code would work for a machine that is BIG_ENDIAN and where "long" is 64bits, but we don't have any of those. I don't think "int bad" was wrong and "long bad" definitely seems wrong for 64bit systems, although the xdr.c code simply ends up putting the low order 32bits on the wire, I think? rick Reviewed by: emaste Reported by: CHERI Obtained from: CheriBSD Sponsored by: DARPA Differential Revision: https://reviews.freebsd.org/D25779 Modified: stable/12/usr.sbin/mountd/mountd.c Directory Properties: stable/12/ (props changed) Modified: stable/12/usr.sbin/mountd/mountd.c ============================================================================== --- stable/12/usr.sbin/mountd/mountd.c Mon Jul 27 21:19:41 2020 (r363624) +++ stable/12/usr.sbin/mountd/mountd.c Mon Jul 27 23:18:14 2020 (r363625) @@ -1087,7 +1087,8 @@ mntsrv(struct svc_req *rqstp, SVCXPRT *transp) struct sockaddr *saddr; u_short sport; char rpcpath[MNTPATHLEN + 1], dirpath[MAXPATHLEN]; - int bad = 0, defset, hostset; + int defset, hostset; + long bad = 0; sigset_t sighup_mask; int numsecflavors, *secflavorsp;help
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?QB1PR01MB3364334927A6502C76338B42DD710>