From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eric Blake Subject: Re: [Qemu-devel] [PATCH] Fix issues affecting Xen 9pfs discovered by Coverity Date: Mon, 8 May 2017 17:05:01 -0500 Message-ID: <356752db-bd70-a908-141e-eec99f4c15e2@redhat.com> References: <1f0bf90b-a706-01c8-cad1-6dea6620deef@redhat.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============6729642245629704698==" Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" To: Stefano Stabellini Cc: anthony.perard@citrix.com, groug@kaod.org, xen-devel@lists.xensource.com, aneesh.kumar@linux.vnet.ibm.com, qemu-devel@nongnu.org List-Id: xen-devel@lists.xenproject.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --===============6729642245629704698== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="gjiOIWEhbDnMrvrswwQKPE39C1tk9Kl7N" This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --gjiOIWEhbDnMrvrswwQKPE39C1tk9Kl7N Content-Type: multipart/mixed; boundary="1IObXG6KVDAGAP13mMDqH2upkLd3rKRlh"; protected-headers="v1" From: Eric Blake To: Stefano Stabellini Cc: anthony.perard@citrix.com, xen-devel@lists.xensource.com, aneesh.kumar@linux.vnet.ibm.com, qemu-devel@nongnu.org, groug@kaod.org Message-ID: <356752db-bd70-a908-141e-eec99f4c15e2@redhat.com> Subject: Re: [Qemu-devel] [PATCH] Fix issues affecting Xen 9pfs discovered by Coverity References: <1f0bf90b-a706-01c8-cad1-6dea6620deef@redhat.com> In-Reply-To: --1IObXG6KVDAGAP13mMDqH2upkLd3rKRlh Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: quoted-printable On 05/08/2017 05:00 PM, Stefano Stabellini wrote: >>> Directly calling fcntl(F_SETFD) without first reading fcntl(F_GETFD) = is >>> (theoretically) incorrect. Better might be using qemu_set_cloexec() >>> instead of open-coding something. >> >> Makes sense but the unchecked return of fcntl, discovered by Coverity,= >> would remain unfixed by calling qemu_set_cloexec here. I don't think I= >> am up for fixing all the call sites of qemu_set_cloexec. >> >> I am going to drop this change, and resend this patch was only the oth= er >> two fixes, fixing 1374836 only. >=20 > Unless you would be fine with: >=20 > diff --git a/util/oslib-posix.c b/util/oslib-posix.c > index 4d9189e..16894ad 100644 > --- a/util/oslib-posix.c > +++ b/util/oslib-posix.c > @@ -182,7 +182,9 @@ void qemu_set_cloexec(int fd) > { > int f; > f =3D fcntl(fd, F_GETFD); > - fcntl(fd, F_SETFD, f | FD_CLOEXEC); > + assert(f !=3D -1); > + f =3D fcntl(fd, F_SETFD, f | FD_CLOEXEC); > + assert(f !=3D -1); Seems reasonable to me, but I don't know if anyone else would object. Changes semantics if someone ever calls qemu_set_cloexec(-1) (previously it would ignore the EBADF failures, now it will abort) - such callers are arguably broken, so that's okay by me. --=20 Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org --1IObXG6KVDAGAP13mMDqH2upkLd3rKRlh-- --gjiOIWEhbDnMrvrswwQKPE39C1tk9Kl7N Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJZEOuNAAoJEKeha0olJ0NqcLQH/RafKZ5gkrROCygz3mcGUN6E vOffrprotbepD4fnloyqRmkdjpsXjeWY4/pohqGfDOGr+JzuGB4z44tYsi2Ty3hP R3ywaotARt3Al7akkM7GkpnSkNj4fU7ur+IBsrH35NBvmA2l3jjav3DmphO8zBEf jh+dYvy2OkVgM9Yc6IWfa0EhQpJXkv6+0sV/YRxP7TA7Fa0Y8Kbq5vf2IX873K4x /ztYt/zgEloXAhcjOfAGUAJyR8edNfOkVZiuiJE3ueLefXhvic/H+gaFPMBdtf4S outoELbjZGZ0BxToT72HiGVNDzpA/Tj2MF2o3ewGID5eva6tWqnshItyjEjiRPE= =9aJj -----END PGP SIGNATURE----- --gjiOIWEhbDnMrvrswwQKPE39C1tk9Kl7N-- --===============6729642245629704698== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KWGVuLWRldmVs IG1haWxpbmcgbGlzdApYZW4tZGV2ZWxAbGlzdHMueGVuLm9yZwpodHRwczovL2xpc3RzLnhlbi5v cmcveGVuLWRldmVsCg== --===============6729642245629704698==--