* [Qemu-devel] [PULL v2 0/1] Merge IO 2017/11/06
@ 2017-11-07 14:36 Daniel P. Berrange
2017-11-07 14:36 ` [Qemu-devel] [PULL v2 1/1] sockets: avoid leak of listen file descriptor Daniel P. Berrange
2017-11-07 15:22 ` [Qemu-devel] [PULL v2 0/1] Merge IO 2017/11/06 Peter Maydell
0 siblings, 2 replies; 4+ messages in thread
From: Daniel P. Berrange @ 2017-11-07 14:36 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Daniel P. Berrange
The following changes since commit 5853e92207193e967abf5e4c25b4a551c7604725:
Merge remote-tracking branch 'remotes/pmaydell/tags/pull-cocoa-20171107' into staging (2017-11-07 12:19:48 +0000)
are available in the git repository at:
git://github.com/berrange/qemu pull-2017-11-06-2
for you to fetch changes up to 10a7b7e6fd9f250f0506568345d7b4d2ab52889d:
sockets: avoid leak of listen file descriptor (2017-11-07 14:10:20 +0000)
----------------------------------------------------------------
Pull IO 2017/11/06 v2
----------------------------------------------------------------
Daniel P. Berrange (1):
sockets: avoid leak of listen file descriptor
util/qemu-sockets.c | 52 +++++++++++++++++++++++++++++-----------------------
1 file changed, 29 insertions(+), 23 deletions(-)
--
2.13.6
^ permalink raw reply [flat|nested] 4+ messages in thread
* [Qemu-devel] [PULL v2 1/1] sockets: avoid leak of listen file descriptor
2017-11-07 14:36 [Qemu-devel] [PULL v2 0/1] Merge IO 2017/11/06 Daniel P. Berrange
@ 2017-11-07 14:36 ` Daniel P. Berrange
2017-11-07 15:22 ` [Qemu-devel] [PULL v2 0/1] Merge IO 2017/11/06 Peter Maydell
1 sibling, 0 replies; 4+ messages in thread
From: Daniel P. Berrange @ 2017-11-07 14:36 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Daniel P. Berrange
If we iterate over the full port range without successfully binding+listening
on the socket, we'll try the next address, whereupon we overwrite the slisten
file descriptor variable without closing it.
Rather than having two places where we open + close socket FDs on different
iterations of nested for loops, re-arrange the code to always open+close
within the same loop iteration.
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
util/qemu-sockets.c | 52 +++++++++++++++++++++++++++++-----------------------
1 file changed, 29 insertions(+), 23 deletions(-)
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index b47fb45885..8b75541ce4 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -207,7 +207,7 @@ static int inet_listen_saddr(InetSocketAddress *saddr,
char uaddr[INET6_ADDRSTRLEN+1];
char uport[33];
int rc, port_min, port_max, p;
- int slisten = 0;
+ int slisten = -1;
int saved_errno = 0;
bool socket_created = false;
Error *err = NULL;
@@ -267,31 +267,42 @@ static int inet_listen_saddr(InetSocketAddress *saddr,
uaddr,INET6_ADDRSTRLEN,uport,32,
NI_NUMERICHOST | NI_NUMERICSERV);
- slisten = create_fast_reuse_socket(e);
- if (slisten < 0) {
- continue;
- }
-
- socket_created = true;
port_min = inet_getport(e);
port_max = saddr->has_to ? saddr->to + port_offset : port_min;
for (p = port_min; p <= port_max; p++) {
inet_setport(e, p);
- rc = try_bind(slisten, saddr, e);
- if (rc) {
- if (errno == EADDRINUSE) {
+
+ slisten = create_fast_reuse_socket(e);
+ if (slisten < 0) {
+ /* First time we expect we might fail to create the socket
+ * eg if 'e' has AF_INET6 but ipv6 kmod is not loaded.
+ * Later iterations should always succeed if first iteration
+ * worked though, so treat that as fatal.
+ */
+ if (p == port_min) {
continue;
} else {
- error_setg_errno(errp, errno, "Failed to bind socket");
+ error_setg_errno(errp, errno,
+ "Failed to recreate failed listening socket");
goto listen_failed;
}
}
- if (!listen(slisten, 1)) {
- goto listen_ok;
- }
- if (errno != EADDRINUSE) {
- error_setg_errno(errp, errno, "Failed to listen on socket");
- goto listen_failed;
+ socket_created = true;
+
+ rc = try_bind(slisten, saddr, e);
+ if (rc < 0) {
+ if (errno != EADDRINUSE) {
+ error_setg_errno(errp, errno, "Failed to bind socket");
+ goto listen_failed;
+ }
+ } else {
+ if (!listen(slisten, 1)) {
+ goto listen_ok;
+ }
+ if (errno != EADDRINUSE) {
+ error_setg_errno(errp, errno, "Failed to listen on socket");
+ goto listen_failed;
+ }
}
/* Someone else managed to bind to the same port and beat us
* to listen on it! Socket semantics does not allow us to
@@ -299,12 +310,7 @@ static int inet_listen_saddr(InetSocketAddress *saddr,
* socket to allow bind attempts for subsequent ports:
*/
closesocket(slisten);
- slisten = create_fast_reuse_socket(e);
- if (slisten < 0) {
- error_setg_errno(errp, errno,
- "Failed to recreate failed listening socket");
- goto listen_failed;
- }
+ slisten = -1;
}
}
error_setg_errno(errp, errno,
--
2.13.6
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PULL v2 0/1] Merge IO 2017/11/06
2017-11-07 14:36 [Qemu-devel] [PULL v2 0/1] Merge IO 2017/11/06 Daniel P. Berrange
2017-11-07 14:36 ` [Qemu-devel] [PULL v2 1/1] sockets: avoid leak of listen file descriptor Daniel P. Berrange
@ 2017-11-07 15:22 ` Peter Maydell
2017-11-07 15:25 ` Daniel P. Berrange
1 sibling, 1 reply; 4+ messages in thread
From: Peter Maydell @ 2017-11-07 15:22 UTC (permalink / raw)
To: Daniel P. Berrange; +Cc: QEMU Developers
On 7 November 2017 at 14:36, Daniel P. Berrange <berrange@redhat.com> wrote:
> The following changes since commit 5853e92207193e967abf5e4c25b4a551c7604725:
>
> Merge remote-tracking branch 'remotes/pmaydell/tags/pull-cocoa-20171107' into staging (2017-11-07 12:19:48 +0000)
>
> are available in the git repository at:
>
> git://github.com/berrange/qemu pull-2017-11-06-2
>
> for you to fetch changes up to 10a7b7e6fd9f250f0506568345d7b4d2ab52889d:
>
> sockets: avoid leak of listen file descriptor (2017-11-07 14:10:20 +0000)
>
> ----------------------------------------------------------------
> Pull IO 2017/11/06 v2
>
> ----------------------------------------------------------------
> Daniel P. Berrange (1):
> sockets: avoid leak of listen file descriptor
>
> util/qemu-sockets.c | 52 +++++++++++++++++++++++++++++-----------------------
> 1 file changed, 29 insertions(+), 23 deletions(-)
Applied, thanks. (My test script expected your pullreq to
say "git://github.com/berrange/qemu tags/pull-2017-11-06-2"
but that might be the script not being as smart as it could be.)
-- PMM
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PULL v2 0/1] Merge IO 2017/11/06
2017-11-07 15:22 ` [Qemu-devel] [PULL v2 0/1] Merge IO 2017/11/06 Peter Maydell
@ 2017-11-07 15:25 ` Daniel P. Berrange
0 siblings, 0 replies; 4+ messages in thread
From: Daniel P. Berrange @ 2017-11-07 15:25 UTC (permalink / raw)
To: Peter Maydell; +Cc: QEMU Developers
On Tue, Nov 07, 2017 at 03:22:02PM +0000, Peter Maydell wrote:
> On 7 November 2017 at 14:36, Daniel P. Berrange <berrange@redhat.com> wrote:
> > The following changes since commit 5853e92207193e967abf5e4c25b4a551c7604725:
> >
> > Merge remote-tracking branch 'remotes/pmaydell/tags/pull-cocoa-20171107' into staging (2017-11-07 12:19:48 +0000)
> >
> > are available in the git repository at:
> >
> > git://github.com/berrange/qemu pull-2017-11-06-2
> >
> > for you to fetch changes up to 10a7b7e6fd9f250f0506568345d7b4d2ab52889d:
> >
> > sockets: avoid leak of listen file descriptor (2017-11-07 14:10:20 +0000)
> >
> > ----------------------------------------------------------------
> > Pull IO 2017/11/06 v2
> >
> > ----------------------------------------------------------------
> > Daniel P. Berrange (1):
> > sockets: avoid leak of listen file descriptor
> >
> > util/qemu-sockets.c | 52 +++++++++++++++++++++++++++++-----------------------
> > 1 file changed, 29 insertions(+), 23 deletions(-)
>
> Applied, thanks. (My test script expected your pullreq to
> say "git://github.com/berrange/qemu tags/pull-2017-11-06-2"
> but that might be the script not being as smart as it could be.)
Oh my bad. I forgot to actually push the tag to github before generating
the cover letter text, and I guess that caused git to leave off the 'tags/'
bit
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-11-07 15:25 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-07 14:36 [Qemu-devel] [PULL v2 0/1] Merge IO 2017/11/06 Daniel P. Berrange
2017-11-07 14:36 ` [Qemu-devel] [PULL v2 1/1] sockets: avoid leak of listen file descriptor Daniel P. Berrange
2017-11-07 15:22 ` [Qemu-devel] [PULL v2 0/1] Merge IO 2017/11/06 Peter Maydell
2017-11-07 15:25 ` Daniel P. Berrange
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.