[PULL,v1,1/1] tpm: fix crash when FD >= 1024

Commit Message

Stefan Berger Sept. 12, 2023, 9:41 p.m. UTC

From: Marc-Andr޸ Lureau <[e-mail]>

Replace select() with poll() to fix a crash when QEMU has a large number
of FDs.

Fixes:
https://bugzilla.redhat.com/show_bug.cgi?id=2020133

Cc: [e-mail]
Fixes: 56a3c24ffc ("tpm: Probe for connected TPM 1.2 or TPM 2")
Signed-off-by: Marc-Andr޸ Lureau <[e-mail]>
Reviewed-by: Michael Tokarev <[e-mail]>
Reviewed-by: Stefan Berger <[e-mail]>
Signed-off-by: Stefan Berger <[e-mail]>
---
 backends/tpm/tpm_util.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

Comments

Michael Tokarev Sept. 13, 2023, 6:41 a.m. UTC | #1

13.09.2023 00:41, Stefan Berger wrote:
> From: Marc-Andr޸ Lureau <[e-mail]>
> 
> Replace select() with poll() to fix a crash when QEMU has a large number
> of FDs.
> 
> Fixes:
> https://bugzilla.redhat.com/show_bug.cgi?id=2020133

Please keep these on the same line next time.

And there's an UTF8 issue with Marc-André's name writing
as Phil already pointed out (fixing in Cc of this email).
Git usually DTRT here, dunno how that can happen.

BTW, it would also be good to mention newly introduced
usage of non-EINTR operation, - before this change, EINTR
were returned to the caller, now it is not anymore.

I'm sorry for late notes..

/mjt

Daniel P. Berrangé Sept. 13, 2023, 8:09 a.m. UTC | #2

On Wed, Sep 13, 2023 at 09:41:48AM +0300, Michael Tokarev wrote:
> 13.09.2023 00:41, Stefan Berger wrote:
> > From: Marc-Andr޸ Lureau <[e-mail]>
> > 
> > Replace select() with poll() to fix a crash when QEMU has a large number
> > of FDs.
> > 
> > Fixes:
> > https://bugzilla.redhat.com/show_bug.cgi?id=2020133
> 
> Please keep these on the same line next time.
> 
> And there's an UTF8 issue with Marc-André's name writing
> as Phil already pointed out (fixing in Cc of this email).
> Git usually DTRT here, dunno how that can happen.

It has often happened in the past when people have (mistakenly)
configured the tool used to send patches to use a non-UTF8
charset.


With regards,
Daniel

Patch

diff --git a/backends/tpm/tpm_util.c b/backends/tpm/tpm_util.c
index a6e6d3e72f..1856589c3b 100644
--- a/backends/tpm/tpm_util.c
+++ b/backends/tpm/tpm_util.c
@@ -112,12 +112,8 @@  static int tpm_util_request(int fd,
                             void *response,
                             size_t responselen)
 {
-    fd_set readfds;
+    GPollFD fds[1] = { {.fd = fd, .events = G_IO_IN } };
     int n;
-    struct timeval tv = {
-        .tv_sec = 1,
-        .tv_usec = 0,
-    };
 
     n = write(fd, request, requestlen);
     if (n < 0) {
@@ -127,11 +123,8 @@  static int tpm_util_request(int fd,
         return -EFAULT;
     }
 
-    FD_ZERO(&readfds);
-    FD_SET(fd, &readfds);
-
     /* wait for a second */
-    n = select(fd + 1, &readfds, NULL, NULL, &tv);
+    n = RETRY_ON_EINTR(g_poll(fds, 1, 1000));
     if (n != 1) {
         return -errno;
     }