svn commit: r349243 - head/sys/cam

[Shawn Webb]

+1 for M_ZERO.

-- 
Shawn Webb
Cofounder / Security Engineer
HardenedBSD

Tor-ified Signal:    +1 443-546-8752
Tor+XMPP+OTR:        lattera at 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 at 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 at freebsd.org mailing list
> https://lists.freebsd.org/mailman/listinfo/svn-src-all
> To unsubscribe, send any mail to "svn-src-all-unsubscribe at freebsd.org"
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.freebsd.org/pipermail/svn-src-head/attachments/20190620/43d0372a/attachment.sig>