From owner-svn-src-all@freebsd.org Thu Jun 20 22:59:58 2019 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id AD3BD15C9F55; Thu, 20 Jun 2019 22:59:58 +0000 (UTC) (envelope-from scottl@samsco.org) Received: from out3-smtp.messagingengine.com (out3-smtp.messagingengine.com [66.111.4.27]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 0910D6B357; Thu, 20 Jun 2019 22:59:58 +0000 (UTC) (envelope-from scottl@samsco.org) Received: from compute6.internal (compute6.nyi.internal [10.202.2.46]) by mailout.nyi.internal (Postfix) with ESMTP id 968E0220E2; Thu, 20 Jun 2019 18:59:51 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute6.internal (MEProxy); Thu, 20 Jun 2019 18:59:51 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=samsco.org; h= content-type:mime-version:subject:from:in-reply-to:date:cc :content-transfer-encoding:message-id:references:to; s=fm3; bh=D jrFj6mQ9cl5xX59rzKd+x8OH5RseHzM/p6bxXdiYgc=; b=s12McLPrsp1cijw9P cN9BdCy+14cekWf8BWljvk8nso0MV/D2Xau27ncttx1CybRfhKEiE9NXRn576Pjc Kc1PtzsJiaz9BO0IrcDDZzCE1vteyRidZ1p61qMqSX7InlvDJoroWtDussasE1Cp 2akrlSmTwCmZAstkJbFS4QpCMFEm/FxD0UabrlCWlwQEF7mOBjFrB8Z9yNqaEZEh nSnr9r5lPN/6v8aCImSQmFSgt5wVMdHfrw+RxID177NuzQEP81VnywRqnIB9kj9E Y818w2sHJXM+1Ait30D0DthgnSGF1hAXtaUnBjWN/SR7T/VBnPsCHcrjImqElLK2 xIeVw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm3; bh=DjrFj6mQ9cl5xX59rzKd+x8OH5RseHzM/p6bxXdiY gc=; b=PVWA/i88d00wptGi+lOROjGt2jwCqyrvkzCrzrJQ06X93v/TvgLARjZhh yukOr0ydwWFbOyKdWzYNBzldPeVz2RrjisqCvFrhLjuHKDW1K2wTe9x9tiiK3+Pc VF7M6yUPDIBkpbUsUQ8+f+FmwfGUt1dw1dwo1JFd/MWJ8JxMGM8nrr/vximvSaQa RX4Dst5aVGBteqHUZ/gKc+Pa4Hlr05x5ZwhrrN4CGPq3TztLCud9FKxsS2Oxbu/F kMj5EhwPWsgswTJ1++zwtoYq87aZSOP/2v37PZAMyWCPf1Zc2tlLeCqHXYQkymU9 w/2gJ/4Tzd1qPMxnBsOvmsDyYemoQ== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeduvddrtdehgdduiecutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecunecujfgurheptggguffhjgffgffkfhfvofesthhqmh dthhdtjeenucfhrhhomhepufgtohhtthcunfhonhhguceoshgtohhtthhlsehsrghmshgt ohdrohhrgheqnecuffhomhgrihhnpehfrhgvvggsshgurdhorhhgnecukfhppeduledvrd ehhedrheegrdehleenucfrrghrrghmpehmrghilhhfrhhomhepshgtohhtthhlsehsrghm shgtohdrohhrghenucevlhhushhtvghrufhiiigvpedt X-ME-Proxy: Received: from [10.178.24.12] (unknown [192.55.54.59]) by mail.messagingengine.com (Postfix) with ESMTPA id 9490D80065; Thu, 20 Jun 2019 18:59:50 -0400 (EDT) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 12.4 \(3445.104.11\)) Subject: Re: svn commit: r349243 - head/sys/cam From: Scott Long In-Reply-To: <201906202029.x5KKThjn017867@repo.freebsd.org> Date: Thu, 20 Jun 2019 16:59:48 -0600 Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Content-Transfer-Encoding: quoted-printable Message-Id: <6B266DFC-87FF-4742-ACEE-0D37EABEFD7A@samsco.org> References: <201906202029.x5KKThjn017867@repo.freebsd.org> To: Alexander Motin X-Mailer: Apple Mail (2.3445.104.11) X-Rspamd-Queue-Id: 0910D6B357 X-Spamd-Bar: ------ Authentication-Results: mx1.freebsd.org X-Spamd-Result: default: False [-6.94 / 15.00]; NEURAL_HAM_MEDIUM(-1.00)[-0.999,0]; NEURAL_HAM_SHORT(-0.95)[-0.945,0]; REPLY(-4.00)[]; NEURAL_HAM_LONG(-1.00)[-1.000,0] X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 20 Jun 2019 22:59:58 -0000 Hi Alexander, I=E2=80=99m not a fan of removing the M_ZERO. I understand your = argument that lengths are provided elsewhere, but having the buffer be zero=E2=80=99d = is defensive against future bugs that might leak buffer contents. GETATTR isn=E2=80=99= 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 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 = 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. >=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 = *attr, 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