Hi Paul,
On Wed, Nov 14, 2018 at 03:33:02PM +0000, Paul Martin wrote:
> Atomic operations on aarch64 (arm64) have to be aligned to 8 byte
> boundaries (same size as a pointer type), otherwise a SIGBUS is raised.
>
> Because the variable ts here isn't guaranteed to be aligned due to the
> various data_size adjustments, make sure that data_size is always
> incremented by a minimum of sizeof(int *) rather than sizeof(int).
>
> Program received signal SIGBUS, Bus error.
> 0x0000aaaaaab1176c in process_store_rules (s=s@entry=0xaaaaaae01060,
> rep=0xaaaaaae010d0, rep=0xaaaaaae010d0, an_bit=8388608)
> at src/stream.c:1609
> 1609 HA_RWLOCK_WRLOCK(STK_SESS_LOCK, &ts->lock);
> (gdb) bt
> %0 0x0000aaaaaab1176c in process_store_rules (s=s@entry=0xaaaaaae01060,
> rep=0xaaaaaae010d0, rep=0xaaaaaae010d0, an_bit=8388608)
> at src/stream.c:1609
> %1 0x0000aaaaaab18898 in process_stream (t=<optimized out>,
> context=0xaaaaaae01060, state=<optimized out>) at src/stream.c:2054
> %2 0x0000aaaaaabb0220 in process_runnable_tasks () at src/task.c:421
> %3 0x0000aaaaaab51b40 in run_poll_loop () at src/haproxy.c:2609
> %4 run_thread_poll_loop (data=<optimized out>) at src/haproxy.c:2674
> %5 0x0000aaaaaaac715c in main (argc=<optimized out>, argv=0xfffffffff290)
> at src/haproxy.c:3286
> ---
> include/proto/stick_table.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/proto/stick_table.h b/include/proto/stick_table.h
> index 40bb8ca6..6e39ad47 100644
> --- a/include/proto/stick_table.h
> +++ b/include/proto/stick_table.h
> @@ -64,7 +64,7 @@ static inline int stktable_type_size(int type)
> switch(type) {
> case STD_T_SINT:
> case STD_T_UINT:
> - return sizeof(int);
> + return sizeof(int *);
> case STD_T_ULL:
> return sizeof(unsigned long long);
> case STD_T_FRQP:
Oops, you're right indeed.
I'm not sure I'm a big fan of special-casing STD_T_UINT. For example,
STD_T_FRQP is probably 12bytes too, so it'd be a problem.
Can you test the (untested, but hopefully right) patch attached ?
Thanks a lot !
Olivier
>From 50027352049d64b874e1116758400580541ea49f Mon Sep 17 00:00:00 2001
From: Olivier Houchard <ohouch...@haproxy.com>
Date: Wed, 14 Nov 2018 17:54:36 +0100
Subject: [PATCH] BUG/MEDIUM: Make sure stksess is properly aligned.
When we allocate struct stksess, we also allocate memory to store the
associated data before the struct itself.
As the data can be of different types, they can have different size. However,
we need the struct stksess to be properly aligned, as it can do 64bits
load/store (including atomic load/stores) on 64bits platforms, and some of
them doesn't support unaligned access.
So, when allocating the struct stksess, round the size up to the next
multiple of sizeof(void *), and make sure the struct stksess itself is
properly aligned.
Many thanks to Paul Martin for investigating and reporting that bug.
---
src/stick_table.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/src/stick_table.c b/src/stick_table.c
index 7f141631..4ec7861e 100644
--- a/src/stick_table.c
+++ b/src/stick_table.c
@@ -45,6 +45,7 @@
/* structure used to return a table key built from a sample */
static THREAD_LOCAL struct stktable_key static_table_key;
+#define round_ptr_size(i) ((i + (sizeof(void *) - 1)) &~ (sizeof(void *) - 1))
/*
* Free an allocated sticky session <ts>, and decrease sticky sessions counter
* in table <t>.
@@ -52,7 +53,7 @@ static THREAD_LOCAL struct stktable_key static_table_key;
void __stksess_free(struct stktable *t, struct stksess *ts)
{
t->current--;
- pool_free(t->pool, (void *)ts - t->data_size);
+ pool_free(t->pool, (void *)ts - round_ptr_size(t->data_size));
}
/*
@@ -230,7 +231,7 @@ struct stksess *__stksess_new(struct stktable *t, struct
stktable_key *key)
ts = pool_alloc(t->pool);
if (ts) {
t->current++;
- ts = (void *)ts + t->data_size;
+ ts = (void *)ts + round_ptr_size(t->data_size);
__stksess_init(t, ts);
if (key)
stksess_setkey(t, ts, key);
@@ -598,7 +599,7 @@ int stktable_init(struct stktable *t)
t->updates = EB_ROOT_UNIQUE;
HA_SPIN_INIT(&t->lock);
- t->pool = create_pool("sticktables", sizeof(struct stksess) +
t->data_size + t->key_size, MEM_F_SHARED);
+ t->pool = create_pool("sticktables", sizeof(struct stksess) +
round_ptr_size(t->data_size) + t->key_size, MEM_F_SHARED);
t->exp_next = TICK_ETERNITY;
if ( t->expire ) {
--
2.17.1
