[libvirt] [PATCH] virNetClientSetTLSSession: Restore original signal mask
Currently, we use pthread_sigmask(SIG_BLOCK, ...) prior to calling poll(). This is okay, as we don't want poll() to be interrupted. However, then - immediately as we fall out from the poll() - we try to restore the original sigmask - again using SIG_BLOCK. But as the man page says, SIG_BLOCK adds signals to the signal mask: SIG_BLOCK The set of blocked signals is the union of the current set and the set argument. Therefore, when restoring the original mask, we need to completely overwrite the one we set earlier and hence we should be using: SIG_SETMASK The set of blocked signals is set to the argument set. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- Notes: This is a very old bug. It's worth backporting onto all maint branches we have. src/rpc/virnetclient.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c index b288b74..255997b 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c @@ -792,7 +792,7 @@ int virNetClientSetTLSSession(virNetClientPtr client, if (ret < 0 && (errno == EAGAIN || errno == EINTR)) goto repoll; - ignore_value(pthread_sigmask(SIG_BLOCK, &oldmask, NULL)); + ignore_value(pthread_sigmask(SIG_SETMASK, &oldmask, NULL)); } ret = virNetTLSContextCheckCertificate(tls, client->tls); @@ -816,7 +816,7 @@ int virNetClientSetTLSSession(virNetClientPtr client, if (ret < 0 && (errno == EAGAIN || errno == EINTR)) goto repoll2; - ignore_value(pthread_sigmask(SIG_BLOCK, &oldmask, NULL)); + ignore_value(pthread_sigmask(SIG_SETMASK, &oldmask, NULL)); len = virNetTLSSessionRead(client->tls, buf, 1); if (len < 0 && errno != ENOMSG) { -- 1.9.0
On Wed, Mar 19, 2014 at 06:29:01PM +0100, Michal Privoznik wrote:
Currently, we use pthread_sigmask(SIG_BLOCK, ...) prior to calling poll(). This is okay, as we don't want poll() to be interrupted. However, then - immediately as we fall out from the poll() - we try to restore the original sigmask - again using SIG_BLOCK. But as the man page says, SIG_BLOCK adds signals to the signal mask:
SIG_BLOCK The set of blocked signals is the union of the current set and the set argument.
Therefore, when restoring the original mask, we need to completely overwrite the one we set earlier and hence we should be using:
SIG_SETMASK The set of blocked signals is set to the argument set.
And indeed we do that on all other places.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> ---
ACK.
Notes: This is a very old bug. It's worth backporting onto all maint branches we have.
Go ahead (but only the active ones ;-) ) Martin
On 03/19/2014 11:29 AM, Michal Privoznik wrote:
Currently, we use pthread_sigmask(SIG_BLOCK, ...) prior to calling poll(). This is okay, as we don't want poll() to be interrupted. However, then - immediately as we fall out from the poll() - we try to restore the original sigmask - again using SIG_BLOCK. But as the man page says, SIG_BLOCK adds signals to the signal mask:
SIG_BLOCK The set of blocked signals is the union of the current set and the set argument.
Therefore, when restoring the original mask, we need to completely overwrite the one we set earlier and hence we should be using:
SIG_SETMASK The set of blocked signals is set to the argument set.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> ---
Notes: This is a very old bug. It's worth backporting onto all maint branches we have.
src/rpc/virnetclient.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
ACK. And yes, it needs to be backported. Bug was introduced in virnetclient.c at its creation in commit 434de30 (v0.9.3); that code was generalized from src/remote/remote_driver.c, but at that time, remote_driver.c was doing things correctly. Not sure how I missed the corruption (obviously, the refactoring to add the rpc client code wasn't quite a copy and paste, for that typo to have snuck in). -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 03/19/2014 11:42 AM, Eric Blake wrote:
On 03/19/2014 11:29 AM, Michal Privoznik wrote:
Currently, we use pthread_sigmask(SIG_BLOCK, ...) prior to calling poll(). This is okay, as we don't want poll() to be interrupted. However, then - immediately as we fall out from the poll() - we try to restore the original sigmask - again using SIG_BLOCK. But as the man page says, SIG_BLOCK adds signals to the signal mask:
SIG_BLOCK The set of blocked signals is the union of the current set and the set argument.
Therefore, when restoring the original mask, we need to completely overwrite the one we set earlier and hence we should be using:
SIG_SETMASK The set of blocked signals is set to the argument set.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> ---
Notes: This is a very old bug. It's worth backporting onto all maint branches we have.
src/rpc/virnetclient.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
ACK. And yes, it needs to be backported.
Now done to all the active -maint branches. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 20.03.2014 18:13, Eric Blake wrote:
On 03/19/2014 11:42 AM, Eric Blake wrote:
On 03/19/2014 11:29 AM, Michal Privoznik wrote:
Currently, we use pthread_sigmask(SIG_BLOCK, ...) prior to calling poll(). This is okay, as we don't want poll() to be interrupted. However, then - immediately as we fall out from the poll() - we try to restore the original sigmask - again using SIG_BLOCK. But as the man page says, SIG_BLOCK adds signals to the signal mask:
SIG_BLOCK The set of blocked signals is the union of the current set and the set argument.
Therefore, when restoring the original mask, we need to completely overwrite the one we set earlier and hence we should be using:
SIG_SETMASK The set of blocked signals is set to the argument set.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> ---
Notes: This is a very old bug. It's worth backporting onto all maint branches we have.
src/rpc/virnetclient.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
ACK. And yes, it needs to be backported.
Now done to all the active -maint branches.
Thank you, I was planning to do that later, but got distracted ... Michal
On Wed, Mar 19, 2014 at 06:29:01PM +0100, Michal Privoznik wrote:
Currently, we use pthread_sigmask(SIG_BLOCK, ...) prior to calling poll(). This is okay, as we don't want poll() to be interrupted. However, then - immediately as we fall out from the poll() - we try to restore the original sigmask - again using SIG_BLOCK. But as the man page says, SIG_BLOCK adds signals to the signal mask:
SIG_BLOCK The set of blocked signals is the union of the current set and the set argument.
Therefore, when restoring the original mask, we need to completely overwrite the one we set earlier and hence we should be using:
SIG_SETMASK The set of blocked signals is set to the argument set.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> ---
Notes: This is a very old bug. It's worth backporting onto all maint branches we have.
src/rpc/virnetclient.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c index b288b74..255997b 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c @@ -792,7 +792,7 @@ int virNetClientSetTLSSession(virNetClientPtr client, if (ret < 0 && (errno == EAGAIN || errno == EINTR)) goto repoll;
- ignore_value(pthread_sigmask(SIG_BLOCK, &oldmask, NULL)); + ignore_value(pthread_sigmask(SIG_SETMASK, &oldmask, NULL)); }
ret = virNetTLSContextCheckCertificate(tls, client->tls); @@ -816,7 +816,7 @@ int virNetClientSetTLSSession(virNetClientPtr client, if (ret < 0 && (errno == EAGAIN || errno == EINTR)) goto repoll2;
- ignore_value(pthread_sigmask(SIG_BLOCK, &oldmask, NULL)); + ignore_value(pthread_sigmask(SIG_SETMASK, &oldmask, NULL));
len = virNetTLSSessionRead(client->tls, buf, 1); if (len < 0 && errno != ENOMSG) {
ACK, I see other places in our code get this right already Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
participants (4)
-
Daniel P. Berrange -
Eric Blake -
Martin Kletzander -
Michal Privoznik