Message ID | 20230911113614.1412887-1-marcandre.lureau@redhat.com |
---|---|
State | New |
Headers | show |
Series | tpm: fix crash when FD >= 1024 | expand |
11.09.2023 14:36, marcandre.lureau@redhat.com: > From: Marc-André Lureau <marcandre.lureau@redhat.com> > > 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 > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > --- > backends/tpm/tpm_util.c | 12 +++--------- > 1 file changed, 3 insertions(+), 9 deletions(-) > > diff --git a/backends/tpm/tpm_util.c b/backends/tpm/tpm_util.c > index a6e6d3e72f..5f4c9f5b6f 100644 > --- a/backends/tpm/tpm_util.c > +++ b/backends/tpm/tpm_util.c > @@ -112,12 +112,9 @@ 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, > - }; > + int timeout = 1000; You don't need a variable for this in poll(). Besides, this is clear in the context of this patch, which says tv_sec=1. Without this context, it becomes suspicious and catches an eye: too long timeout? > n = write(fd, request, requestlen); > if (n < 0) { > @@ -127,11 +124,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, timeout)); It's much better IMHO to use "1000" directly here, esp. since the comment says about a second. > if (n != 1) { > return -errno; > } Other than that, Reviewed-by: Michael Tokarev <mjt@tls.msk.ru> /mjt
On 9/11/23 07:36, marcandre.lureau@redhat.com wrote: > From: Marc-Andr Lureau <marcandre.lureau@redhat.com> > > 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 The description there seems wrong. It's a limit of the POSIX API not the vTPM device driver. > > Signed-off-by: Marc-Andr Lureau <marcandre.lureau@redhat.com> Reviewed-by: Stefan Berger <stefanb@linux.ibm.com> > --- > backends/tpm/tpm_util.c | 12 +++--------- > 1 file changed, 3 insertions(+), 9 deletions(-) > > diff --git a/backends/tpm/tpm_util.c b/backends/tpm/tpm_util.c > index a6e6d3e72f..5f4c9f5b6f 100644 > --- a/backends/tpm/tpm_util.c > +++ b/backends/tpm/tpm_util.c > @@ -112,12 +112,9 @@ 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, > - }; > + int timeout = 1000; > > n = write(fd, request, requestlen); > if (n < 0) { > @@ -127,11 +124,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, timeout)); > if (n != 1) { > return -errno; > }
diff --git a/backends/tpm/tpm_util.c b/backends/tpm/tpm_util.c index a6e6d3e72f..5f4c9f5b6f 100644 --- a/backends/tpm/tpm_util.c +++ b/backends/tpm/tpm_util.c @@ -112,12 +112,9 @@ 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, - }; + int timeout = 1000; n = write(fd, request, requestlen); if (n < 0) { @@ -127,11 +124,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, timeout)); if (n != 1) { return -errno; }