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
[-- Attachment #1 --] +1 for M_ZERO. -- 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, > > 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 defensive > against future bugs that might leak buffer contents. GETATTR isn???t typically > in a performance path, is there any measurable benefit to this? > > Thanks, > Scott > > > > On Jun 20, 2019, at 2:29 PM, Alexander Motin <mav@FreeBSD.org> wrote: > > > > Author: mav > > Date: Thu Jun 20 20:29:42 2019 > > New Revision: 349243 > > URL: https://svnweb.freebsd.org/changeset/base/349243 > > > > Log: > > Optimize xpt_getattr(). > > > > 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 64KB > > 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. > > > > MFC after: 2 weeks > > Sponsored by: iXsystems, Inc. > > > > Modified: > > head/sys/cam/cam_xpt.c > > > > Modified: head/sys/cam/cam_xpt.c > > ============================================================================== > > --- 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 *attr, s > > cdai.ccb_h.func_code = XPT_DEV_ADVINFO; > > cdai.flags = CDAI_FLAG_NONE; > > cdai.bufsiz = len; > > + cdai.buf = buf; > > > > if (!strcmp(attr, "GEOM::ident")) > > cdai.buftype = CDAI_TYPE_SERIAL_NUM; > > @@ -1262,14 +1263,14 @@ xpt_getattr(char *buf, size_t len, const char *attr, s > > strcmp(attr, "GEOM::lunname") == 0) { > > cdai.buftype = CDAI_TYPE_SCSI_DEVID; > > cdai.bufsiz = CAM_SCSI_DEVID_MAXLEN; > > + cdai.buf = malloc(cdai.bufsiz, M_CAMXPT, M_NOWAIT); > > + if (cdai.buf == NULL) { > > + ret = ENOMEM; > > + goto out; > > + } > > } else > > goto out; > > > > - cdai.buf = malloc(cdai.bufsiz, M_CAMXPT, M_NOWAIT|M_ZERO); > > - if (cdai.buf == NULL) { > > - ret = ENOMEM; > > - goto out; > > - } > > xpt_action((union ccb *)&cdai); /* can only be synchronous */ > > if ((cdai.ccb_h.status & CAM_DEV_QFRZN) != 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 = EFAULT; > > } > > } else { > > - ret = 0; > > - if (strlcpy(buf, cdai.buf, len) >= len) > > + if (cdai.provsiz < len) { > > + cdai.buf[cdai.provsiz] = 0; > > + ret = 0; > > + } else > > ret = EFAULT; > > } > > > > out: > > - if (cdai.buf != NULL) > > + if ((char *)cdai.buf != buf) > > free(cdai.buf, M_CAMXPT); > > return ret; > > } > > > > _______________________________________________ > 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" [-- Attachment #2 --] -----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-----
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20190620231527.pe2vhkvtcppao6ib>