[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] Fix issues affecting Xen 9pfs discovered by Cov
From: |
Stefano Stabellini |
Subject: |
Re: [Qemu-devel] [PATCH] Fix issues affecting Xen 9pfs discovered by Coverity |
Date: |
Mon, 8 May 2017 15:00:24 -0700 (PDT) |
User-agent: |
Alpine 2.10 (DEB 1266 2009-07-14) |
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);
}
/*