sockets: poll clean ups

This makes the following poll cleanups:

 1) Add LWIP_ERROR in lwip_poll to check for invalid fds/nfds combinations.
    This fixes a possible a NULL fds dereference in lwip_poll_scan()
 2) Use has_ copies of the socket events in lwip_poll_should_wake() rather
    passing the sock pointer and accessing socket after leaving the critical
    section
This commit is contained in:
Joel Cunningham 2017-10-08 10:38:01 -05:00
parent 653a1e7778
commit 0882e0ba89

View File

@ -290,7 +290,7 @@ static struct lwip_select_cb *select_cb_list;
#if LWIP_SOCKET_SELECT || LWIP_SOCKET_POLL #if LWIP_SOCKET_SELECT || LWIP_SOCKET_POLL
static void event_callback(struct netconn *conn, enum netconn_evt evt, u16_t len); static void event_callback(struct netconn *conn, enum netconn_evt evt, u16_t len);
#define DEFAULT_SOCKET_EVENTCB event_callback #define DEFAULT_SOCKET_EVENTCB event_callback
static void select_check_waiters(int s, int has_recvevent, int has_sendevent, int has_errevent, struct lwip_sock *sock); static void select_check_waiters(int s, int has_recvevent, int has_sendevent, int has_errevent);
#else #else
#define DEFAULT_SOCKET_EVENTCB NULL #define DEFAULT_SOCKET_EVENTCB NULL
#endif #endif
@ -2261,6 +2261,8 @@ lwip_poll(struct pollfd *fds, nfds_t nfds, int timeout)
LWIP_DEBUGF(SOCKETS_DEBUG, ("lwip_poll(%p, %d, %d)\n", LWIP_DEBUGF(SOCKETS_DEBUG, ("lwip_poll(%p, %d, %d)\n",
(void*)fds, (int)nfds, timeout)); (void*)fds, (int)nfds, timeout));
LWIP_ERROR("lwip_poll: invalid fds", ((fds != NULL && nfds > 0) || (fds == NULL && nfds == 0)),
set_errno(EINVAL); return -1;);
lwip_poll_inc_sockets_used(fds, nfds); lwip_poll_inc_sockets_used(fds, nfds);
@ -2367,7 +2369,7 @@ return_success:
* lwip_poll. * lwip_poll.
*/ */
static int static int
lwip_poll_should_wake(const struct lwip_select_cb *scb, int fd, struct lwip_sock *sock) lwip_poll_should_wake(const struct lwip_select_cb *scb, int fd, int has_recvevent, int has_sendevent, int has_errevent)
{ {
nfds_t fdi; nfds_t fdi;
for (fdi = 0; fdi < scb->poll_nfds; fdi++) { for (fdi = 0; fdi < scb->poll_nfds; fdi++) {
@ -2376,13 +2378,13 @@ lwip_poll_should_wake(const struct lwip_select_cb *scb, int fd, struct lwip_sock
/* Do not update pollfd->revents right here; /* Do not update pollfd->revents right here;
that would be a data race because lwip_pollscan that would be a data race because lwip_pollscan
accesses revents without protecting. */ accesses revents without protecting. */
if (sock->rcvevent > 0 && (pollfd->events & POLLIN) != 0) { if (has_recvevent && (pollfd->events & POLLIN) != 0) {
return 1; return 1;
} }
if (sock->sendevent != 0 && (pollfd->events & POLLOUT) != 0) { if (has_sendevent && (pollfd->events & POLLOUT) != 0) {
return 1; return 1;
} }
if (sock->errevent != 0) { if (has_errevent) {
/* POLLERR is output only. */ /* POLLERR is output only. */
return 1; return 1;
} }
@ -2482,7 +2484,7 @@ event_callback(struct netconn *conn, enum netconn_evt evt, u16_t len)
has_errevent = sock->errevent != 0; has_errevent = sock->errevent != 0;
SYS_ARCH_UNPROTECT(lev); SYS_ARCH_UNPROTECT(lev);
/* Check any select calls waiting on this socket */ /* Check any select calls waiting on this socket */
select_check_waiters(s, has_recvevent, has_sendevent, has_errevent, sock); select_check_waiters(s, has_recvevent, has_sendevent, has_errevent);
} else { } else {
SYS_ARCH_UNPROTECT(lev); SYS_ARCH_UNPROTECT(lev);
} }
@ -2502,7 +2504,7 @@ event_callback(struct netconn *conn, enum netconn_evt evt, u16_t len)
* select_cb_list during our UNPROTECT/PROTECT. We use a generational counter to * select_cb_list during our UNPROTECT/PROTECT. We use a generational counter to
* detect this change and restart the list walk. The list is expected to be small * detect this change and restart the list walk. The list is expected to be small
*/ */
static void select_check_waiters(int s, int has_recvevent, int has_sendevent, int has_errevent, struct lwip_sock *sock) static void select_check_waiters(int s, int has_recvevent, int has_sendevent, int has_errevent)
{ {
struct lwip_select_cb *scb; struct lwip_select_cb *scb;
#if !LWIP_TCPIP_CORE_LOCKING #if !LWIP_TCPIP_CORE_LOCKING
@ -2522,10 +2524,7 @@ again:
int do_signal = 0; int do_signal = 0;
#if LWIP_SOCKET_POLL #if LWIP_SOCKET_POLL
if (scb->poll_fds != NULL) { if (scb->poll_fds != NULL) {
LWIP_UNUSED_ARG(has_recvevent); do_signal = lwip_poll_should_wake(scb, s, has_recvevent, has_sendevent, has_errevent);
LWIP_UNUSED_ARG(has_sendevent);
LWIP_UNUSED_ARG(has_errevent);
do_signal = lwip_poll_should_wake(scb, s, sock);
} }
#endif /* LWIP_SOCKET_POLL */ #endif /* LWIP_SOCKET_POLL */
#if LWIP_SOCKET_SELECT && LWIP_SOCKET_POLL #if LWIP_SOCKET_SELECT && LWIP_SOCKET_POLL
@ -2533,7 +2532,6 @@ again:
#endif /* LWIP_SOCKET_SELECT && LWIP_SOCKET_POLL */ #endif /* LWIP_SOCKET_SELECT && LWIP_SOCKET_POLL */
#if LWIP_SOCKET_SELECT #if LWIP_SOCKET_SELECT
{ {
LWIP_UNUSED_ARG(sock);
/* Test this select call for our socket */ /* Test this select call for our socket */
if (has_recvevent) { if (has_recvevent) {
if (scb->readset && FD_ISSET(s, scb->readset)) { if (scb->readset && FD_ISSET(s, scb->readset)) {