Hi,
The attached patch attempts to fix the way we handle the update_mask bit.
Clearing that bit in fd_insert() may lead to entries being inserted twice
in fd_updt. Instead, make sure the pollers clear that bit once they are
done with the entry.
Regards,
Olivier
>From 348ce4601eb92b01c098b54e7fadb9822fd8d15f Mon Sep 17 00:00:00 2001
From: Olivier Houchard <ohouch...@haproxy.com>
Date: Tue, 3 Apr 2018 19:06:18 +0200
Subject: [PATCH] BUG/MINOR: fd: Don't clear the update_mask in fd_insert.
Clearing the update_mask bit in fd_insert may lead to duplicate insertion
of fd in fd_updt, that could lead to a write past the end of the array.
Instead, make sure the update_mask bit is cleared by the pollers no matter
what.
This should big backported to 1.8.
---
include/proto/fd.h | 1 -
src/ev_epoll.c | 2 +-
src/ev_kqueue.c | 2 +-
src/ev_poll.c | 2 +-
src/ev_select.c | 2 +-
5 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/include/proto/fd.h b/include/proto/fd.h
index 0f59df661..6c9cfe701 100644
--- a/include/proto/fd.h
+++ b/include/proto/fd.h
@@ -445,7 +445,6 @@ static inline void fd_insert(int fd, void *owner, void
(*iocb)(int fd), unsigned
fdtab[fd].owner = owner;
fdtab[fd].iocb = iocb;
fdtab[fd].ev = 0;
- fdtab[fd].update_mask &= ~tid_bit;
fdtab[fd].linger_risk = 0;
fdtab[fd].cloned = 0;
fdtab[fd].thread_mask = thread_mask;
diff --git a/src/ev_epoll.c b/src/ev_epoll.c
index b98ca8ca8..a8e57973f 100644
--- a/src/ev_epoll.c
+++ b/src/ev_epoll.c
@@ -74,13 +74,13 @@ REGPRM2 static void _do_poll(struct poller *p, int exp)
for (updt_idx = 0; updt_idx < fd_nbupdt; updt_idx++) {
fd = fd_updt[updt_idx];
+ HA_ATOMIC_AND(&fdtab[fd].update_mask, ~tid_bit);
if (!fdtab[fd].owner) {
activity[tid].poll_drop++;
continue;
}
en = fdtab[fd].state;
- HA_ATOMIC_AND(&fdtab[fd].update_mask, ~tid_bit);
if (fdtab[fd].polled_mask & tid_bit) {
if (!(fdtab[fd].thread_mask & tid_bit) || !(en &
FD_EV_POLLED_RW)) {
diff --git a/src/ev_kqueue.c b/src/ev_kqueue.c
index cc96307b0..a103ece9d 100644
--- a/src/ev_kqueue.c
+++ b/src/ev_kqueue.c
@@ -47,13 +47,13 @@ REGPRM2 static void _do_poll(struct poller *p, int exp)
for (updt_idx = 0; updt_idx < fd_nbupdt; updt_idx++) {
fd = fd_updt[updt_idx];
+ HA_ATOMIC_AND(&fdtab[fd].update_mask, ~tid_bit);
if (!fdtab[fd].owner) {
activity[tid].poll_drop++;
continue;
}
en = fdtab[fd].state;
- HA_ATOMIC_AND(&fdtab[fd].update_mask, ~tid_bit);
if (!(fdtab[fd].thread_mask & tid_bit) || !(en &
FD_EV_POLLED_RW)) {
if (!(fdtab[fd].polled_mask & tid_bit)) {
diff --git a/src/ev_poll.c b/src/ev_poll.c
index edcfe6565..6093b652b 100644
--- a/src/ev_poll.c
+++ b/src/ev_poll.c
@@ -65,13 +65,13 @@ REGPRM2 static void _do_poll(struct poller *p, int exp)
for (updt_idx = 0; updt_idx < fd_nbupdt; updt_idx++) {
fd = fd_updt[updt_idx];
+ HA_ATOMIC_AND(&fdtab[fd].update_mask, ~tid_bit);
if (!fdtab[fd].owner) {
activity[tid].poll_drop++;
continue;
}
en = fdtab[fd].state;
- HA_ATOMIC_AND(&fdtab[fd].update_mask, ~tid_bit);
/* we have a single state for all threads, which is why we
* don't check the tid_bit. First thread to see the update
diff --git a/src/ev_select.c b/src/ev_select.c
index 6e834678d..163a45839 100644
--- a/src/ev_select.c
+++ b/src/ev_select.c
@@ -57,13 +57,13 @@ REGPRM2 static void _do_poll(struct poller *p, int exp)
for (updt_idx = 0; updt_idx < fd_nbupdt; updt_idx++) {
fd = fd_updt[updt_idx];
+ HA_ATOMIC_AND(&fdtab[fd].update_mask, ~tid_bit);
if (!fdtab[fd].owner) {
activity[tid].poll_drop++;
continue;
}
en = fdtab[fd].state;
- HA_ATOMIC_AND(&fdtab[fd].update_mask, ~tid_bit);
/* we have a single state for all threads, which is why we
* don't check the tid_bit. First thread to see the update
--
2.14.3
