Skip site navigation (1)Skip section navigation (2)
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>

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



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?QB1PR01MB3364334927A6502C76338B42DD710>