Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 20 Jun 2019 19:15:27 -0400
From:      Shawn Webb <shawn.webb@hardenedbsd.org>
To:        Scott Long <scottl@samsco.org>
Cc:        Alexander Motin <mav@FreeBSD.org>, svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org
Subject:   Re: svn commit: r349243 - head/sys/cam
Message-ID:  <20190620231527.pe2vhkvtcppao6ib@mutt-hbsd>
In-Reply-To: <6B266DFC-87FF-4742-ACEE-0D37EABEFD7A@samsco.org>
References:  <201906202029.x5KKThjn017867@repo.freebsd.org> <6B266DFC-87FF-4742-ACEE-0D37EABEFD7A@samsco.org>

next in thread | previous in thread | raw e-mail | index | archive | help

--ku7fsvwot5gtyq57
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

+1 for M_ZERO.

--=20
Shawn Webb
Cofounder / Security Engineer
HardenedBSD

Tor-ified Signal:    +1 443-546-8752
Tor+XMPP+OTR:        lattera@is.a.hacker.sx
GPG Key ID:          0xFF2E67A277F8E1FA
GPG Key Fingerprint: D206 BB45 15E0 9C49 0CF9  3633 C85B 0AF8 AB23 0FB2

On Thu, Jun 20, 2019 at 04:59:48PM -0600, Scott Long wrote:
> Hi Alexander,
>=20
> I???m not a fan of removing the M_ZERO.  I understand your argument that
> lengths are provided elsewhere, but having the buffer be zero???d is defe=
nsive
> against future bugs that might leak buffer contents.  GETATTR isn???t typ=
ically
> in a performance path, is there any measurable benefit to this?
>=20
> Thanks,
> Scott
>=20
>=20
> > On Jun 20, 2019, at 2:29 PM, Alexander Motin <mav@FreeBSD.org> wrote:
> >=20
> > Author: mav
> > Date: Thu Jun 20 20:29:42 2019
> > New Revision: 349243
> > URL: https://svnweb.freebsd.org/changeset/base/349243
> >=20
> > Log:
> >  Optimize xpt_getattr().
> >=20
> >  Do not allocate temporary buffer for attributes we are going to return
> >  as-is, just make sure to NUL-terminate them.  Do not zero temporary 64=
KB
> >  buffer for CDAI_TYPE_SCSI_DEVID, XPT tells us how much data it filled
> >  and there are also length fields inside the returned data also.
> >=20
> >  MFC after:	2 weeks
> >  Sponsored by:	iXsystems, Inc.
> >=20
> > Modified:
> >  head/sys/cam/cam_xpt.c
> >=20
> > Modified: head/sys/cam/cam_xpt.c
> > =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
> > --- head/sys/cam/cam_xpt.c	Thu Jun 20 20:06:19 2019	(r349242)
> > +++ head/sys/cam/cam_xpt.c	Thu Jun 20 20:29:42 2019	(r349243)
> > @@ -1253,6 +1253,7 @@ xpt_getattr(char *buf, size_t len, const char *at=
tr, s
> > 	cdai.ccb_h.func_code =3D XPT_DEV_ADVINFO;
> > 	cdai.flags =3D CDAI_FLAG_NONE;
> > 	cdai.bufsiz =3D len;
> > +	cdai.buf =3D buf;
> >=20
> > 	if (!strcmp(attr, "GEOM::ident"))
> > 		cdai.buftype =3D CDAI_TYPE_SERIAL_NUM;
> > @@ -1262,14 +1263,14 @@ xpt_getattr(char *buf, size_t len, const char *=
attr, s
> > 		 strcmp(attr, "GEOM::lunname") =3D=3D 0) {
> > 		cdai.buftype =3D CDAI_TYPE_SCSI_DEVID;
> > 		cdai.bufsiz =3D CAM_SCSI_DEVID_MAXLEN;
> > +		cdai.buf =3D malloc(cdai.bufsiz, M_CAMXPT, M_NOWAIT);
> > +		if (cdai.buf =3D=3D NULL) {
> > +			ret =3D ENOMEM;
> > +			goto out;
> > +		}
> > 	} else
> > 		goto out;
> >=20
> > -	cdai.buf =3D malloc(cdai.bufsiz, M_CAMXPT, M_NOWAIT|M_ZERO);
> > -	if (cdai.buf =3D=3D NULL) {
> > -		ret =3D ENOMEM;
> > -		goto out;
> > -	}
> > 	xpt_action((union ccb *)&cdai); /* can only be synchronous */
> > 	if ((cdai.ccb_h.status & CAM_DEV_QFRZN) !=3D 0)
> > 		cam_release_devq(cdai.ccb_h.path, 0, 0, 0, FALSE);
> > @@ -1334,13 +1335,15 @@ xpt_getattr(char *buf, size_t len, const char *=
attr, s
> > 				ret =3D EFAULT;
> > 		}
> > 	} else {
> > -		ret =3D 0;
> > -		if (strlcpy(buf, cdai.buf, len) >=3D len)
> > +		if (cdai.provsiz < len) {
> > +			cdai.buf[cdai.provsiz] =3D 0;
> > +			ret =3D 0;
> > +		} else
> > 			ret =3D EFAULT;
> > 	}
> >=20
> > out:
> > -	if (cdai.buf !=3D NULL)
> > +	if ((char *)cdai.buf !=3D buf)
> > 		free(cdai.buf, M_CAMXPT);
> > 	return ret;
> > }
> >=20
>=20
> _______________________________________________
> svn-src-all@freebsd.org mailing list
> https://lists.freebsd.org/mailman/listinfo/svn-src-all
> To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"

--ku7fsvwot5gtyq57
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAEBCAAdFiEEA6TL67gupaZ9nzhT/y5nonf44foFAl0ME4oACgkQ/y5nonf4
4fqioQ/+OCI8wWidt7sCNaPaHAJu9zKddev6a/evFMq8hqKG5JSjZ4As9aSFcfVZ
HwWIrxxvcaS0vibuw+xbcSgktiIRG9+P0ryX+8XcL07bOxJ3eHWIV7aidO1t5q3o
qSmZGxDzOXMWzQpQq+G0zz2FDag7xsCiqGosCnN1bKaEVAY83Nb9wQS8rq66jTGk
yQsGoPOFYB3FLpqjdlmrsrcWHDuQa9rzsHS9r8txTwjfMSI/xhzvsYJPmz3Y6Zxh
rA3uxRM1Do+2Y/cCnCr3LTI3GqEiQ3PS9Jzh/AeoDF2OImFnbRvr22dAjiD6iHCR
nq4IaVbDOl6jd9nodHC3X4YQF5k+gLhBkNiUPfErrjDVhwqzQ6Jn1yf4TDkWGh0C
efKxOai12X5m23SeZ1WAdJvCJ/Q6AF/tA/x3oRd+ckSYULRCAhuGqrY+XH1xgQgH
zi34yyZEBHd3wmDTzE26axCFQV3QGT9gZMq3EaLnYH+83hMxBjYHKWfnsoSXAoxL
zeGrTx7TWlGn8XW+1tn3/OYkP3I6YzNx+wZrkJHY5Xhii0Muq0F/okFyeDHRgz0k
hI+bmiYgfFS77u3D8VD6vBAAte9CKoQpgbGZLGix/lL4CnJ5plaKJp+AgIKFtAyo
qnjPt9oZyC2XQ6ORufWUexwJjeiGC6uHRAMa5k3LaeygzNznrWE=
=dbMz
-----END PGP SIGNATURE-----

--ku7fsvwot5gtyq57--



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