BBlack has uploaded a new change for review. https://gerrit.wikimedia.org/r/298350
Change subject: Add nginx.org ubsan shift patches ...................................................................... Add nginx.org ubsan shift patches Change-Id: I9ff99b4c16b3a55e6f9c64ff2303511d8f52c08b --- A debian/patches/0090-shift1.patch A debian/patches/0091-shift2.patch M debian/patches/series 3 files changed, 202 insertions(+), 0 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/operations/software/nginx refs/changes/50/298350/1 diff --git a/debian/patches/0090-shift1.patch b/debian/patches/0090-shift1.patch new file mode 100644 index 0000000..c4caf92 --- /dev/null +++ b/debian/patches/0090-shift1.patch @@ -0,0 +1,168 @@ +commit 6299f5e9149483251bbbcc8ad26cf29b6109e75c +Author: Sergey Kandaurov <[email protected]> +Date: Thu Jul 7 21:02:28 2016 +0300 + + Avoid left-shifting integers into the sign bit, which is undefined. + + Found with UndefinedBehaviorSanitizer. + +diff --git a/src/core/ngx_string.c b/src/core/ngx_string.c +index cf665a4..7a73ef5 100644 +--- a/src/core/ngx_string.c ++++ b/src/core/ngx_string.c +@@ -1563,7 +1563,7 @@ ngx_escape_uri(u_char *dst, u_char *src, size_t size, ngx_uint_t type) + n = 0; + + while (size) { +- if (escape[*src >> 5] & (1 << (*src & 0x1f))) { ++ if (escape[*src >> 5] & (1U << (*src & 0x1f))) { + n++; + } + src++; +@@ -1574,7 +1574,7 @@ ngx_escape_uri(u_char *dst, u_char *src, size_t size, ngx_uint_t type) + } + + while (size) { +- if (escape[*src >> 5] & (1 << (*src & 0x1f))) { ++ if (escape[*src >> 5] & (1U << (*src & 0x1f))) { + *dst++ = '%'; + *dst++ = hex[*src >> 4]; + *dst++ = hex[*src & 0xf]; +diff --git a/src/http/modules/ngx_http_log_module.c b/src/http/modules/ngx_http_log_module.c +index df9424f..c42fb08 100644 +--- a/src/http/modules/ngx_http_log_module.c ++++ b/src/http/modules/ngx_http_log_module.c +@@ -1000,7 +1000,7 @@ ngx_http_log_escape(u_char *dst, u_char *src, size_t size) + n = 0; + + while (size) { +- if (escape[*src >> 5] & (1 << (*src & 0x1f))) { ++ if (escape[*src >> 5] & (1U << (*src & 0x1f))) { + n++; + } + src++; +@@ -1011,7 +1011,7 @@ ngx_http_log_escape(u_char *dst, u_char *src, size_t size) + } + + while (size) { +- if (escape[*src >> 5] & (1 << (*src & 0x1f))) { ++ if (escape[*src >> 5] & (1U << (*src & 0x1f))) { + *dst++ = '\\'; + *dst++ = 'x'; + *dst++ = hex[*src >> 4]; +diff --git a/src/http/modules/ngx_http_userid_filter_module.c b/src/http/modules/ngx_http_userid_filter_module.c +index 1487c09..0dbacba 100644 +--- a/src/http/modules/ngx_http_userid_filter_module.c ++++ b/src/http/modules/ngx_http_userid_filter_module.c +@@ -836,7 +836,7 @@ ngx_http_userid_init_worker(ngx_cycle_t *cycle) + ngx_gettimeofday(&tp); + + /* use the most significant usec part that fits to 16 bits */ +- start_value = ((tp.tv_usec / 20) << 16) | ngx_pid; ++ start_value = (((uint32_t) tp.tv_usec / 20) << 16) | ngx_pid; + + return NGX_OK; + } +diff --git a/src/http/ngx_http_parse.c b/src/http/ngx_http_parse.c +index 59aa1fe..bd6c9c9 100644 +--- a/src/http/ngx_http_parse.c ++++ b/src/http/ngx_http_parse.c +@@ -481,7 +481,7 @@ ngx_http_parse_request_line(ngx_http_request_t *r, ngx_buf_t *b) + /* check "/.", "//", "%", and "\" (Win32) in URI */ + case sw_after_slash_in_uri: + +- if (usual[ch >> 5] & (1 << (ch & 0x1f))) { ++ if (usual[ch >> 5] & (1U << (ch & 0x1f))) { + state = sw_check_uri; + break; + } +@@ -540,7 +540,7 @@ ngx_http_parse_request_line(ngx_http_request_t *r, ngx_buf_t *b) + /* check "/", "%" and "\" (Win32) in URI */ + case sw_check_uri: + +- if (usual[ch >> 5] & (1 << (ch & 0x1f))) { ++ if (usual[ch >> 5] & (1U << (ch & 0x1f))) { + break; + } + +@@ -626,7 +626,7 @@ ngx_http_parse_request_line(ngx_http_request_t *r, ngx_buf_t *b) + /* URI */ + case sw_uri: + +- if (usual[ch >> 5] & (1 << (ch & 0x1f))) { ++ if (usual[ch >> 5] & (1U << (ch & 0x1f))) { + break; + } + +@@ -1131,7 +1131,7 @@ ngx_http_parse_uri(ngx_http_request_t *r) + /* check "/.", "//", "%", and "\" (Win32) in URI */ + case sw_after_slash_in_uri: + +- if (usual[ch >> 5] & (1 << (ch & 0x1f))) { ++ if (usual[ch >> 5] & (1U << (ch & 0x1f))) { + state = sw_check_uri; + break; + } +@@ -1179,7 +1179,7 @@ ngx_http_parse_uri(ngx_http_request_t *r) + /* check "/", "%" and "\" (Win32) in URI */ + case sw_check_uri: + +- if (usual[ch >> 5] & (1 << (ch & 0x1f))) { ++ if (usual[ch >> 5] & (1U << (ch & 0x1f))) { + break; + } + +@@ -1228,7 +1228,7 @@ ngx_http_parse_uri(ngx_http_request_t *r) + /* URI */ + case sw_uri: + +- if (usual[ch >> 5] & (1 << (ch & 0x1f))) { ++ if (usual[ch >> 5] & (1U << (ch & 0x1f))) { + break; + } + +@@ -1289,7 +1289,7 @@ ngx_http_parse_complex_uri(ngx_http_request_t *r, ngx_uint_t merge_slashes) + + case sw_usual: + +- if (usual[ch >> 5] & (1 << (ch & 0x1f))) { ++ if (usual[ch >> 5] & (1U << (ch & 0x1f))) { + *u++ = ch; + ch = *p++; + break; +@@ -1358,7 +1358,7 @@ ngx_http_parse_complex_uri(ngx_http_request_t *r, ngx_uint_t merge_slashes) + + case sw_slash: + +- if (usual[ch >> 5] & (1 << (ch & 0x1f))) { ++ if (usual[ch >> 5] & (1U << (ch & 0x1f))) { + state = sw_usual; + *u++ = ch; + ch = *p++; +@@ -1401,7 +1401,7 @@ ngx_http_parse_complex_uri(ngx_http_request_t *r, ngx_uint_t merge_slashes) + + case sw_dot: + +- if (usual[ch >> 5] & (1 << (ch & 0x1f))) { ++ if (usual[ch >> 5] & (1U << (ch & 0x1f))) { + state = sw_usual; + *u++ = ch; + ch = *p++; +@@ -1442,7 +1442,7 @@ ngx_http_parse_complex_uri(ngx_http_request_t *r, ngx_uint_t merge_slashes) + + case sw_dot_dot: + +- if (usual[ch >> 5] & (1 << (ch & 0x1f))) { ++ if (usual[ch >> 5] & (1U << (ch & 0x1f))) { + state = sw_usual; + *u++ = ch; + ch = *p++; +@@ -1836,7 +1836,7 @@ ngx_http_parse_unsafe_uri(ngx_http_request_t *r, ngx_str_t *uri, + continue; + } + +- if (usual[ch >> 5] & (1 << (ch & 0x1f))) { ++ if (usual[ch >> 5] & (1U << (ch & 0x1f))) { + continue; + } + diff --git a/debian/patches/0091-shift2.patch b/debian/patches/0091-shift2.patch new file mode 100644 index 0000000..8c71bc3 --- /dev/null +++ b/debian/patches/0091-shift2.patch @@ -0,0 +1,32 @@ +commit 586ef968f98c379153fea0e7e80119b149380dc8 +Author: Sergey Kandaurov <[email protected]> +Date: Thu Jul 7 21:03:21 2016 +0300 + + HTTP/2: avoid left-shifting signed integer into the sign bit. + + On non-aligned platforms, properly cast argument before left-shifting it in + ngx_http_v2_parse_uint32 that is used with u_char. Otherwise it propagates + to int to hold the value and can step over the sign bit. Usually, on known + compilers, this results in negation. Furthermore, a subsequent store into a + wider type, that is ngx_uint_t on 64-bit platforms, results in sign-extension. + + In practice, this can be observed in debug log as a very large exclusive bit + value, when client sent PRIORITY frame with exclusive bit set: + + : *14 http2 PRIORITY frame sid:5 on 1 excl:8589934591 weight:17 + + Found with UndefinedBehaviorSanitizer. + +diff --git a/src/http/v2/ngx_http_v2.h b/src/http/v2/ngx_http_v2.h +index 9e738aa..d712d38 100644 +--- a/src/http/v2/ngx_http_v2.h ++++ b/src/http/v2/ngx_http_v2.h +@@ -298,7 +298,7 @@ size_t ngx_http_v2_huff_encode(u_char *src, size_t len, u_char *dst, + + #define ngx_http_v2_parse_uint16(p) ((p)[0] << 8 | (p)[1]) + #define ngx_http_v2_parse_uint32(p) \ +- ((p)[0] << 24 | (p)[1] << 16 | (p)[2] << 8 | (p)[3]) ++ ((uint32_t) (p)[0] << 24 | (p)[1] << 16 | (p)[2] << 8 | (p)[3]) + + #endif + diff --git a/debian/patches/series b/debian/patches/series index 04b971f..3f26e44 100644 --- a/debian/patches/series +++ b/debian/patches/series @@ -1,2 +1,4 @@ perl-use-dpkg-buildflags.patch 0002-Make-sure-signature-stays-the-same-in-all-nginx-buil.patch +0090-shift1.patch +0091-shift2.patch -- To view, visit https://gerrit.wikimedia.org/r/298350 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I9ff99b4c16b3a55e6f9c64ff2303511d8f52c08b Gerrit-PatchSet: 1 Gerrit-Project: operations/software/nginx Gerrit-Branch: wmf-1.11.2 Gerrit-Owner: BBlack <[email protected]> _______________________________________________ MediaWiki-commits mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
