Merge ~ahasenack/ubuntu/+source/haproxy:cosmic-haproxy-arm-sigbus-1804069 into ubuntu/+source/haproxy:ubuntu/cosmic-devel

[Andreas Hasenack]

Fix a crash error on arm64. The patch was taken as-is from upstream with a tiny offset of 1 line. I checked and it applies correctly, therefore I didn't change it other than added a few dep3 headers.

The linked bug contains testing instructions and has the SRU template filled out.

Bileto ticket, still in progress as I write this: https://bileto.ubuntu.com/#/ticket/3610 (for some reason, tests haven't even started yet).

If you need arm64 hardware to test, ping me here or in telegram if I'm away, it's easy to arrange if you have vpn access to http://10.229.32.21.

[Andreas Hasenack (ahasenack)]

sil2100 is troubleshooting the bileto issues:
<sil2100> ahasenack: hey! hm, weird, let me take a look
<sil2100> argh
<sil2100> 2019-01-29 10:59:45.746494 Updating ticket 3610: 401 UNAUTHORIZED
<sil2100> ahasenack: let me try getting that fixed, but in the meantime it seems that 3610 was passing: https://objectstorage.prodstack4-5.canonical.com/v1/AUTH_39a8dbb93caf4ec889f8a1b7f69885db/bileto-3610-excuses/2019-01-29_10:20:01/3610_cosmic_excuses.html
<ahasenack> thanks
<sil2100> I'll quickly look at the other two and then try to find a way to make Bileto update the tickets contents again

3610, which represents this MP, is green: https://objectstorage.prodstack4-5.canonical.com/v1/AUTH_39a8dbb93caf4ec889f8a1b7f69885db/bileto-3610-excuses/2019-01-29_10:20:01/3610_cosmic_excuses.html

[Christian Ehrhardt  (paelzer)]

Change LGTM, small but effective.
I tried the case on x86 and arm64 and agree to the issue, the fix and the SRU template.
Please feel free to go on as-is if you want.

OTOH as I outlined on the Disco MP - I'd encourage you to bundle the Dep8 test that you added there with the SRU.

review: Approve

[Andreas Hasenack (ahasenack)]

Going to add the new dep8 test from disco.

[Andreas Hasenack (ahasenack)]

Triggered a bileto run with the new cmp based dep8 test, copied from disco. All green:

https://bileto.ubuntu.com/excuses/3610/cosmic.html

haproxy (1.8.13-2ubuntu0.1 to 1.8.13-2ubuntu0.2~ppa3)
Maintainer: Ubuntu Developers
0 days old
autopkgtest for haproxy/1.8.13-2ubuntu0.2~ppa3: amd64: Pass [artifacts], armhf: Pass [artifacts], i386: Pass [artifacts], ppc64el: Pass [artifacts], s390x: Pass [artifacts]
autopkgtest for neutron/2:13.0.2-0ubuntu1: amd64: Pass [artifacts], armhf: Pass [artifacts], i386: Pass [artifacts], ppc64el: Pass [artifacts], s390x: Pass [artifacts]
autopkgtest for neutron-lbaas/2:13.0.0-0ubuntu1: amd64: Pass [artifacts], armhf: Pass [artifacts], i386: Pass [artifacts], ppc64el: Pass [artifacts], s390x: Pass [artifacts]
Valid candidate

[Christian Ehrhardt  (paelzer)]

Fix was already good, the test matches what we reivewed for Disco.
Thanks for making sure it works on the SRU target release!

Overall +1

[Christian Ehrhardt  (paelzer)]

review: Approve

[Andreas Hasenack (ahasenack)]

Tagged and uploaded:
$ git push pkg upload/1.8.13-2ubuntu0.2
Enumerating objects: 29, done.
Counting objects: 100% (29/29), done.
Delta compression using up to 4 threads
Compressing objects: 100% (21/21), done.
Writing objects: 100% (22/22), 4.04 KiB | 4.04 MiB/s, done.
Total 22 (delta 13), reused 4 (delta 1)
To ssh://git.launchpad.net/~usd-import-team/ubuntu/+source/haproxy
 * [new tag] upload/1.8.13-2ubuntu0.2 -> upload/1.8.13-2ubuntu0.2

$ dput ubuntu ../haproxy_1.8.13-2ubuntu0.2_source.changes
Checking signature on .changes
gpg: ../haproxy_1.8.13-2ubuntu0.2_source.changes: Valid signature from AC983EB5BF6BCBA9
Checking signature on .dsc
gpg: ../haproxy_1.8.13-2ubuntu0.2.dsc: Valid signature from AC983EB5BF6BCBA9
Uploading to ubuntu (via ftp to upload.ubuntu.com):
  Uploading haproxy_1.8.13-2ubuntu0.2.dsc: done.
  Uploading haproxy_1.8.13-2ubuntu0.2.debian.tar.xz: done.
  Uploading haproxy_1.8.13-2ubuntu0.2_source.buildinfo: done.
  Uploading haproxy_1.8.13-2ubuntu0.2_source.changes: done.
Successfully uploaded packages.

diff --git a/debian/changelog b/debian/changelog
index 1759af2..ae6c7c9 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,12 @@
+haproxy (1.8.13-2ubuntu0.2) cosmic; urgency=medium
+
+  * d/p/stksess-align.patch: Make sure stksess is properly aligned.
+    (LP: #1804069)
+  * d/t/control, d/t/proxy-localhost: simple DEP8 test to actually
+    generate traffic through haproxy.
+
+ -- Andreas Hasenack <[e-mail]>  Wed, 23 Jan 2019 17:24:30 -0200
+
 haproxy (1.8.13-2ubuntu0.1) cosmic-security; urgency=medium
 
   * SECURITY UPDATE: Out-of-bounds read
diff --git a/debian/patches/series b/debian/patches/series
index 6044ff4..1dfbfa2 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -9,3 +9,4 @@ CVE-2018-14645
 CVE-2018-20102.patch
 CVE-2018-20103.patch
 CVE-2018-20615.patch
+stksess-align.patch
diff --git a/debian/patches/stksess-align.patch b/debian/patches/stksess-align.patch
new file mode 100644
index 0000000..0c3d9ce
--- /dev/null
+++ b/debian/patches/stksess-align.patch
@@ -0,0 +1,64 @@
+From 52dabbc4fad338233c7f0c96f977a43f8f81452a Mon Sep 17 00:00:00 2001
+From: Olivier Houchard <[e-mail]>
+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.
+
+This should be backported to earlier releases.
+
+Origin: https://github.com/haproxy/haproxy/commit/52dabbc4fad338233c7f0c96f977a43f8f81452a
+Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/haproxy/+bug/1804069
+Last-Update: 2019-01-23
+---
+ 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..5e55b7a9 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 ) {
diff --git a/debian/tests/control b/debian/tests/control
index 13837c2..70649e3 100644
--- a/debian/tests/control
+++ b/debian/tests/control
@@ -1,3 +1,7 @@
 Tests: cli
 Depends: haproxy, socat
 Restrictions: needs-root
+
+Tests: proxy-localhost
+Depends: haproxy, wget, apache2
+Restrictions: needs-root, allow-stderr, isolation-container
diff --git a/debian/tests/proxy-localhost b/debian/tests/proxy-localhost
new file mode 100644
index 0000000..2824287
--- /dev/null
+++ b/debian/tests/proxy-localhost
@@ -0,0 +1,48 @@
+#!/bin/sh
+
+set -eux
+
+cat > /etc/haproxy/haproxy.cfg <<EOF
+global
+        chroot /var/lib/haproxy
+        user haproxy
+        group haproxy
+        daemon
+        maxconn 4096
+
+defaults
+        log global
+        option dontlognull
+        option redispatch
+        retries 3
+        timeout client 50s
+        timeout connect 10s
+        timeout http-request 5s
+        timeout server 50s
+        maxconn 4096
+
+frontend test-front
+    bind *:8080
+    mode http
+    default_backend test-back
+
+backend test-back
+    mode http
+    stick store-request src
+    stick-table type ip size 256k expire 30m
+    server test-1 localhost:80
+EOF
+
+systemctl restart haproxy
+
+# index.html is shipped with apache2
+# Download it via haproxy and compare
+if wget -t1 http://localhost:8080 -O- | cmp /var/www/html/index.html -; then
+    echo "OK: index.html downloaded via haproxy matches the source file."
+else
+    echo "FAIL: downloaded index.html via haproxy is different from the"
+    echo "      file delivered by apache."
+    exit 1
+fi
+
+exit 0