Skip to content

Commit

Permalink
Use poll() instead of select()
Browse files Browse the repository at this point in the history
From [Pcsclite-muscle] select()-induced crashes (and attached tentative fix)
http:https://lists.infradead.org/pipermail/pcsclite-muscle/2019-November/001199.html

" Hello all,
we tracked down some crashes in Firefox [1] to the use of select() and
its related macros in the libpcslite library. Recent versions of glibc
added checks to ensure that the values of the file descriptors passed to
the FD_SET(), FD_CLR() and FD_ISSET() macros have values lower than the
FD_SETSIZE constant. If the file descriptor value is found to be higher
than FD_SETSIZE then abort() gets called which is ultimately what we're
seeing in Firefox.

I have attached a patch that replaces the select() calls with poll()
which does not suffer from this problem. Unfortunately I don't have a
smartcard reader on hand so I can't test the patch myself.

Cheers,

 Gabriele Svelto

[1] select() crashes in libpcslite
    https://bugzilla.mozilla.org/show_bug.cgi?id=1591876
"

Fixes github issue #51
"Using 'select' in libpcsclite can be problematic for application opening a large number of file descriptors #51"
  • Loading branch information
gabrielesvelto authored and LudovicRousseau committed Nov 8, 2019
1 parent 864f955 commit 20385ef
Showing 1 changed file with 31 additions and 30 deletions.
61 changes: 31 additions & 30 deletions src/winscard_msg.c
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
#include <sys/un.h>
#include <sys/ioctl.h>
#include <errno.h>
#include <poll.h>
#include <stdio.h>
#include <time.h>
#include <string.h>
Expand Down Expand Up @@ -212,38 +213,36 @@ INTERNAL LONG MessageReceiveTimeout(uint32_t command, void *buffer_void,
/* repeat until we get the whole message */
while (remaining > 0)
{
fd_set read_fd;
struct timeval timeout, now;
int selret;
struct pollfd read_fd;
struct timeval now;
int pollret;
long delta;

gettimeofday(&now, NULL);
delta = time_sub(&now, &start);
delta = time_sub(&now, &start) / 1000;

if (delta > timeOut*1000)
if (delta > timeOut)
{
/* we already timed out */
retval = SCARD_E_TIMEOUT;
break;
}

/* remaining time to wait */
delta = timeOut*1000 - delta;
delta = timeOut - delta;

FD_ZERO(&read_fd);
FD_SET(filedes, &read_fd);
read_fd.fd = filedes;
read_fd.events = POLLIN;
read_fd.revents = 0;

timeout.tv_sec = delta/1000000;
timeout.tv_usec = delta - timeout.tv_sec*1000000;

selret = select(filedes + 1, &read_fd, NULL, NULL, &timeout);
pollret = poll(&read_fd, 1, delta);

/* try to read only when socket is readable */
if (selret > 0)
if (pollret > 0)
{
int readed;

if (!FD_ISSET(filedes, &read_fd))
if (!(read_fd.revents & POLLIN))
{
/* very strange situation. it should be an assert really */
retval = SCARD_F_COMM_ERROR;
Expand Down Expand Up @@ -271,7 +270,7 @@ INTERNAL LONG MessageReceiveTimeout(uint32_t command, void *buffer_void,
break;
}
}
} else if (selret == 0)
} else if (pollret == 0)
{
/* is the daemon still there? */
retval = SCardCheckDaemonAvailability();
Expand Down Expand Up @@ -369,20 +368,21 @@ INTERNAL LONG MessageSend(void *buffer_void, uint64_t buffer_size,
/* repeat until all data is written */
while (remaining > 0)
{
fd_set write_fd;
int selret;
struct pollfd write_fd;
int pollret;

FD_ZERO(&write_fd);
FD_SET(filedes, &write_fd);
write_fd.fd = filedes;
write_fd.events = POLLOUT;
write_fd.revents = 0;

selret = select(filedes + 1, NULL, &write_fd, NULL, NULL);
pollret = poll(&write_fd, 1, -1);

/* try to write only when the file descriptor is writable */
if (selret > 0)
if (pollret > 0)
{
int written;

if (!FD_ISSET(filedes, &write_fd))
if (!(write_fd.revents & POLLOUT))
{
/* very strange situation. it should be an assert really */
retval = SCARD_F_COMM_ERROR;
Expand Down Expand Up @@ -419,7 +419,7 @@ INTERNAL LONG MessageSend(void *buffer_void, uint64_t buffer_size,
break;
}
}
} else if (selret == 0)
} else if (pollret == 0)
{
/* timeout */
retval = SCARD_E_TIMEOUT;
Expand Down Expand Up @@ -468,20 +468,21 @@ INTERNAL LONG MessageReceive(void *buffer_void, uint64_t buffer_size,
/* repeat until we get the whole message */
while (remaining > 0)
{
fd_set read_fd;
int selret;
struct pollfd read_fd;
int pollret;

FD_ZERO(&read_fd);
FD_SET(filedes, &read_fd);
read_fd.fd = filedes;
read_fd.events = POLLIN;
read_fd.revents = 0;

selret = select(filedes + 1, &read_fd, NULL, NULL, NULL);
pollret = poll(&read_fd, 1 , -1);

/* try to read only when socket is readable */
if (selret > 0)
if (pollret > 0)
{
int readed;

if (!FD_ISSET(filedes, &read_fd))
if (!(read_fd.revents & POLLIN))
{
/* very strange situation. it should be an assert really */
retval = SCARD_F_COMM_ERROR;
Expand Down

0 comments on commit 20385ef

Please sign in to comment.