Date: Wed, 12 Mar 2014 12:30:09 +0200 From: Konstantin Belousov <kostikbel@gmail.com> To: Conrad Meyer <cemeyer@uw.edu> Cc: freebsd-hackers@freebsd.org, Jeffrey Roberson <jeff@freebsd.org>, Conrad Meyer <conrad.meyer@isilon.com> Subject: Re: [PATCH 1/5] vm/device_pager.c: dev_pager_alloc: 'size' must be non-zero Message-ID: <20140312103009.GT24664@kib.kiev.ua> In-Reply-To: <1394583583-19023-2-git-send-email-conrad.meyer@isilon.com> References: <1394583583-19023-1-git-send-email-conrad.meyer@isilon.com> <1394583583-19023-2-git-send-email-conrad.meyer@isilon.com>
next in thread | previous in thread | raw e-mail | index | archive | help
--qAYG3HKTbif+kJGg
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
Thank you for the submission, I committed four patches, except this one.
On Tue, Mar 11, 2014 at 05:19:39PM -0700, Conrad Meyer wrote:
> If size is zero, paddr is used uninitialized when assigning
> object1->pg_color.
So the issue there is only with non-managed device pager, right ?
Please note that GEM explicitely initializes color in the constructor.
I do not like the change below, it puts the policy into pager, while
currently the decision is up to managed pager consumers, e.g. GEM,
which do the similar check on its own.
I prefer a different way to shut down the warning, please see the
patch at the end of the message. Does it work for you ?
>=20
> Found with Clang static analysis.
>=20
> Signed-off-by: Conrad Meyer <conrad.meyer@isilon.com>
> ---
> sys/vm/device_pager.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>=20
> diff --git a/sys/vm/device_pager.c b/sys/vm/device_pager.c
> index 13491ba..5125d20 100644
> --- a/sys/vm/device_pager.c
> +++ b/sys/vm/device_pager.c
> @@ -135,6 +135,12 @@ cdev_pager_allocate(void *handle, enum obj_type tp, =
struct cdev_pager_ops *ops,
> if (foff & PAGE_MASK)
> return (NULL);
> =20
> + /*
> + * Size must be non-zero.
> + */
> + if (size =3D=3D 0)
> + return (NULL);
> +
> size =3D round_page(size);
> pindex =3D OFF_TO_IDX(foff + size);
> =20
diff --git a/sys/vm/device_pager.c b/sys/vm/device_pager.c
index 13491ba..4cd245a 100644
--- a/sys/vm/device_pager.c
+++ b/sys/vm/device_pager.c
@@ -414,6 +414,7 @@ old_dev_pager_ctor(void *handle, vm_ooffset_t size, vm_=
prot_t prot,
* XXX assumes VM_PROT_* =3D=3D PROT_*
*/
npages =3D OFF_TO_IDX(size);
+ paddr =3D 0; /* Make paddr initialized for the case of size =3D=3D 0. */
for (off =3D foff; npages--; off +=3D PAGE_SIZE) {
if (csw->d_mmap(dev, off, &paddr, (int)prot, &dummy) !=3D 0) {
dev_relthread(dev, ref);
--qAYG3HKTbif+kJGg
Content-Type: application/pgp-signature
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (FreeBSD)
iQIcBAEBAgAGBQJTIDcwAAoJEJDCuSvBvK1BZmcP/RBC1gG939uUsEHgz93CuY0L
BQ65Lm8rj4Eiq8zdSnwOWJY75tK41XvCL9HQsdu0EzUKB6jktsx79CKnNvmqTHh3
K5nPpejBbGPTUEHiDtr2LnQoaQzZxxpJ0/FQ4QlwipJ6TYKcv+CBn+aZmvTvDAUq
k3s5087toE7DTRC8USsbWF3YsdcVxwY3bhDd2ayNHGhxS7rN/NLUP3a2PP1XhQ6w
m9lV1JbuDunEt8ajpJIKUYWuih4n8vgXRwPnAiZE2ge5n5svt3lokW3eAKhmm6wo
/4VOzZmJKyfkQQhc3BDdA0h8JAMmqc4zpFRtsEwSuQPD9bUTpoQXE+wVv8Dd93ui
Vlx4oQZZz/Sv9qDE0m94Kj3BVRoUsK5EDN2cgonYCE9/YQlJVUM0BGGpc3VpY6IE
b6Qy4GbwcNVBR5hU+6mbN+GiWWW0cTvGYYdMg/eDC5aWn6+cfbTVc/s33fJDrall
AGy6LNXbsVoQN51psj5Anx8ZQjqAVafoDaLvgsEP65YVUPoo+QSB/zTqQDIR8BGV
EKjgIodoPXup32bg7OVsctEiKiVhohOcFWt1ccAOAeY+hb/WuLDtHKAOnLh6UeZG
bOgmix3WPyPM2LVi98ckOA0RxJo0FVLp2aJ/BYx9xAs0DsTLIQF+cjsQAjM1+iAW
zKtNrFO/IYCoI8menKBe
=25sT
-----END PGP SIGNATURE-----
--qAYG3HKTbif+kJGg--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20140312103009.GT24664>