Re: [Xen-devel] [Qemu-devel] [PATCH] Fix issues affecting Xen 9pfs discovered by Coverity

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Mon, 8 May 2017, Stefano Stabellini wrote:
> On Mon, 8 May 2017, Eric Blake wrote:
> > On 05/08/2017 03:45 PM, Stefano Stabellini wrote:
> > > Fix two resource leaks on error paths, discovered by Coverity.
> > > Check for errors returned by fcntl, also found by Coverity.
> > > 
> > > CID:1374836
> > > CID:1374831
> > > 
> > 
> > > @@ -378,7 +380,10 @@ static int xen_9pfs_connect(struct XenDevice *xendev)
> > >          if (xen_9pdev->rings[i].evtchndev == NULL) {
> > >              goto out;
> > >          }
> > > -        fcntl(xenevtchn_fd(xen_9pdev->rings[i].evtchndev), F_SETFD, 
> > > FD_CLOEXEC);
> > > +        if (fcntl(xenevtchn_fd(xen_9pdev->rings[i].evtchndev),
> > > +                  F_SETFD, FD_CLOEXEC) == -1) {
> > > +            goto out;
> > 
> > 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 other
> two fixes, fixing 1374836 only.

Unless you would be fine with:

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 = fcntl(fd, F_GETFD);
-    fcntl(fd, F_SETFD, f | FD_CLOEXEC);
+    assert(f != -1);
+    f = fcntl(fd, F_SETFD, f | FD_CLOEXEC);
+    assert(f != -1);
 }
 
 /*

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel