Given a partially misaligned memory read for a large number of bytes
(e.g., we allocate data at addr [0, 16) but read addr [2, 18)), the
address sanitizer (asan) would flag the error as an 'unknown-crash'
instead of a 'stack-buffer-overflow' when compiled with gcc.

This is due to a flawed heuristic in asan_errors.cpp, which won't
always locate the appropriate shadow byte that would indicate a
buffer overflow for any 8 byte or 16+ byte reads.

This asan bug impacts gcc, but not clang, as they seem to report
errors differently - gcc-compiled binaries report the starting
address of the read attempt to asan, while clang-compiled binaries
highlight the first byte access that overflows the buffer. This
appears to have always been the case at least as far back as gcc-7.

This patch resolves this issue via a linear scan of applicable
shadow bytes (instead of the original heuristic, which, at best, only
increments the shadow byte address by 1 for these scenarios). It also
adds appropriate tests to cover for these cases.
---
 .../c-c++-common/asan/stack-overflow-1.c      |  2 ++
 .../stack-overflow-multi-byte-partial-1.c     | 27 +++++++++++++++++++
 .../stack-overflow-multi-byte-partial-2.c     | 27 +++++++++++++++++++
 .../stack-overflow-multi-byte-partial-3.c     | 27 +++++++++++++++++++
 .../stack-overflow-multi-byte-partial-4.c     | 27 +++++++++++++++++++
 .../stack-overflow-multi-byte-partial-5.c     | 27 +++++++++++++++++++
 .../stack-overflow-multi-byte-partial-6.c     | 27 +++++++++++++++++++
 .../stack-overflow-multi-byte-partial-7.c     | 27 +++++++++++++++++++
 .../stack-overflow-multi-byte-partial-8.c     | 27 +++++++++++++++++++
 libsanitizer/asan/asan_errors.cpp             |  7 +++--
 10 files changed, 223 insertions(+), 2 deletions(-)
 create mode 100644 
gcc/testsuite/c-c++-common/asan/stack-overflow-multi-byte-partial-1.c
 create mode 100644 
gcc/testsuite/c-c++-common/asan/stack-overflow-multi-byte-partial-2.c
 create mode 100644 
gcc/testsuite/c-c++-common/asan/stack-overflow-multi-byte-partial-3.c
 create mode 100644 
gcc/testsuite/c-c++-common/asan/stack-overflow-multi-byte-partial-4.c
 create mode 100644 
gcc/testsuite/c-c++-common/asan/stack-overflow-multi-byte-partial-5.c
 create mode 100644 
gcc/testsuite/c-c++-common/asan/stack-overflow-multi-byte-partial-6.c
 create mode 100644 
gcc/testsuite/c-c++-common/asan/stack-overflow-multi-byte-partial-7.c
 create mode 100644 
gcc/testsuite/c-c++-common/asan/stack-overflow-multi-byte-partial-8.c

diff --git a/gcc/testsuite/c-c++-common/asan/stack-overflow-1.c 
b/gcc/testsuite/c-c++-common/asan/stack-overflow-1.c
index 7eab79c6514..c76294b6765 100644
--- a/gcc/testsuite/c-c++-common/asan/stack-overflow-1.c
+++ b/gcc/testsuite/c-c++-common/asan/stack-overflow-1.c
@@ -17,7 +17,9 @@ int main() {
   return res;
 }

+/* { dg-output "ERROR: AddressSanitizer:? stack-buffer-overflow on 
address\[^\n\r]*(\n|\r\n|\r)" } */
 /* { dg-output "READ of size 1 at 0x\[0-9a-f\]+ thread 
T0\[^\n\r]*(\n|\r\n|\r)" } */
 /* { dg-output "    #0 0x\[0-9a-f\]+ +(in _*main 
(\[^\n\r]*stack-overflow-1.c:16|\[^\n\r]*:0|\[^\n\r]*\\+0x\[0-9a-z\]*)|\[(\]).*(\n|\r\n|\r)"
 } */
 /* { dg-output "\[^\n\r]*Address 0x\[0-9a-f\]+ is located in stack of thread 
T0.*(\n|\r\n|\r)" */
+/* { dg-output ".*Memory access at offset \[0-9\]+ overflows this 
variable.*(\n|\r\n|\r)" } */
 /* { dg-output "\[^\n\r]*in main.*stack-overflow-1.c.*(\n|\r\n|\r)" */
diff --git 
a/gcc/testsuite/c-c++-common/asan/stack-overflow-multi-byte-partial-1.c 
b/gcc/testsuite/c-c++-common/asan/stack-overflow-multi-byte-partial-1.c
new file mode 100644
index 00000000000..8a4fef2be32
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/asan/stack-overflow-multi-byte-partial-1.c
@@ -0,0 +1,27 @@
+/* { dg-do run } */
+/* { dg-shouldfail "asan" } */
+
+#define STACK_ALLOC_SIZE 8
+#define READ_SIZE 8
+#define OFFSET 1
+
+struct X {
+  char bytes[READ_SIZE];
+};
+
+__attribute__((noinline)) struct X out_of_bounds() {
+  volatile char bytes[STACK_ALLOC_SIZE];
+  struct X* x_ptr = (struct X*)(bytes + OFFSET);
+  return *x_ptr;
+}
+
+int main() {
+  struct X x = out_of_bounds();
+  return x.bytes[0];
+}
+
+/* { dg-output "ERROR: AddressSanitizer:? stack-buffer-overflow on 
address\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "READ of size 8 at 0x\[0-9a-f\]+ thread 
T0\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*Address 0x\[0-9a-f\]+ is located in stack of thread 
T0.*(\n|\r\n|\r)" */
+/* { dg-output ".*Memory access at offset \[0-9\]+ partially overflows this 
variable.*(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*in 
out_of_bounds.*stack-overflow-multi-byte-partial-1.c.*(\n|\r\n|\r)" */
diff --git 
a/gcc/testsuite/c-c++-common/asan/stack-overflow-multi-byte-partial-2.c 
b/gcc/testsuite/c-c++-common/asan/stack-overflow-multi-byte-partial-2.c
new file mode 100644
index 00000000000..ddc86a6ddfc
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/asan/stack-overflow-multi-byte-partial-2.c
@@ -0,0 +1,27 @@
+/* { dg-do run } */
+/* { dg-shouldfail "asan" } */
+
+#define STACK_ALLOC_SIZE 8
+#define READ_SIZE 5
+#define OFFSET 5
+
+struct X {
+  char bytes[READ_SIZE];
+};
+
+__attribute__((noinline)) struct X out_of_bounds() {
+  volatile char bytes[STACK_ALLOC_SIZE];
+  struct X* x_ptr = (struct X*)(bytes + OFFSET);
+  return *x_ptr;
+}
+
+int main() {
+  struct X x = out_of_bounds();
+  return x.bytes[0];
+}
+
+/* { dg-output "ERROR: AddressSanitizer:? stack-buffer-overflow on 
address\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "READ of size 5 at 0x\[0-9a-f\]+ thread 
T0\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*Address 0x\[0-9a-f\]+ is located in stack of thread 
T0.*(\n|\r\n|\r)" */
+/* { dg-output ".*Memory access at offset \[0-9\]+ partially overflows this 
variable.*(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*in 
out_of_bounds.*stack-overflow-multi-byte-partial-2.c.*(\n|\r\n|\r)" */
diff --git 
a/gcc/testsuite/c-c++-common/asan/stack-overflow-multi-byte-partial-3.c 
b/gcc/testsuite/c-c++-common/asan/stack-overflow-multi-byte-partial-3.c
new file mode 100644
index 00000000000..8f6cf6a1f50
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/asan/stack-overflow-multi-byte-partial-3.c
@@ -0,0 +1,27 @@
+/* { dg-do run } */
+/* { dg-shouldfail "asan" } */
+
+#define STACK_ALLOC_SIZE 8
+#define READ_SIZE 8
+#define OFFSET 7
+
+struct X {
+  char bytes[READ_SIZE];
+};
+
+__attribute__((noinline)) struct X out_of_bounds() {
+  volatile char bytes[STACK_ALLOC_SIZE];
+  struct X* x_ptr = (struct X*)(bytes + OFFSET);
+  return *x_ptr;
+}
+
+int main() {
+  struct X x = out_of_bounds();
+  return x.bytes[0];
+}
+
+/* { dg-output "ERROR: AddressSanitizer:? stack-buffer-overflow on 
address\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "READ of size 8 at 0x\[0-9a-f\]+ thread 
T0\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*Address 0x\[0-9a-f\]+ is located in stack of thread 
T0.*(\n|\r\n|\r)" */
+/* { dg-output ".*Memory access at offset \[0-9\]+ partially overflows this 
variable.*(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*in 
out_of_bounds.*stack-overflow-multi-byte-partial-3.c.*(\n|\r\n|\r)" */
diff --git 
a/gcc/testsuite/c-c++-common/asan/stack-overflow-multi-byte-partial-4.c 
b/gcc/testsuite/c-c++-common/asan/stack-overflow-multi-byte-partial-4.c
new file mode 100644
index 00000000000..14d989619cc
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/asan/stack-overflow-multi-byte-partial-4.c
@@ -0,0 +1,27 @@
+/* { dg-do run } */
+/* { dg-shouldfail "asan" } */
+
+#define STACK_ALLOC_SIZE 16
+#define READ_SIZE 16
+#define OFFSET 7
+
+struct X {
+  char bytes[READ_SIZE];
+};
+
+__attribute__((noinline)) struct X out_of_bounds() {
+  volatile char bytes[STACK_ALLOC_SIZE];
+  struct X* x_ptr = (struct X*)(bytes + OFFSET);
+  return *x_ptr;
+}
+
+int main() {
+  struct X x = out_of_bounds();
+  return x.bytes[0];
+}
+
+/* { dg-output "ERROR: AddressSanitizer:? stack-buffer-overflow on 
address\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "READ of size 16 at 0x\[0-9a-f\]+ thread 
T0\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*Address 0x\[0-9a-f\]+ is located in stack of thread 
T0.*(\n|\r\n|\r)" */
+/* { dg-output ".*Memory access at offset \[0-9\]+ partially overflows this 
variable.*(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*in 
out_of_bounds.*stack-overflow-multi-byte-partial-4.c.*(\n|\r\n|\r)" */
diff --git 
a/gcc/testsuite/c-c++-common/asan/stack-overflow-multi-byte-partial-5.c 
b/gcc/testsuite/c-c++-common/asan/stack-overflow-multi-byte-partial-5.c
new file mode 100644
index 00000000000..aafb3ca0f06
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/asan/stack-overflow-multi-byte-partial-5.c
@@ -0,0 +1,27 @@
+/* { dg-do run } */
+/* { dg-shouldfail "asan" } */
+
+#define STACK_ALLOC_SIZE 16
+#define READ_SIZE 16
+#define OFFSET 8
+
+struct X {
+  char bytes[READ_SIZE];
+};
+
+__attribute__((noinline)) struct X out_of_bounds() {
+  volatile char bytes[STACK_ALLOC_SIZE];
+  struct X* x_ptr = (struct X*)(bytes + OFFSET);
+  return *x_ptr;
+}
+
+int main() {
+  struct X x = out_of_bounds();
+  return x.bytes[0];
+}
+
+/* { dg-output "ERROR: AddressSanitizer:? stack-buffer-overflow on 
address\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "READ of size 16 at 0x\[0-9a-f\]+ thread 
T0\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*Address 0x\[0-9a-f\]+ is located in stack of thread 
T0.*(\n|\r\n|\r)" */
+/* { dg-output ".*Memory access at offset \[0-9\]+ partially overflows this 
variable.*(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*in 
out_of_bounds.*stack-overflow-multi-byte-partial-4.c.*(\n|\r\n|\r)" */
diff --git 
a/gcc/testsuite/c-c++-common/asan/stack-overflow-multi-byte-partial-6.c 
b/gcc/testsuite/c-c++-common/asan/stack-overflow-multi-byte-partial-6.c
new file mode 100644
index 00000000000..c2626e7b706
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/asan/stack-overflow-multi-byte-partial-6.c
@@ -0,0 +1,27 @@
+/* { dg-do run } */
+/* { dg-shouldfail "asan" } */
+
+#define STACK_ALLOC_SIZE 16
+#define READ_SIZE 15
+#define OFFSET 2
+
+struct X {
+  char bytes[READ_SIZE];
+};
+
+__attribute__((noinline)) struct X out_of_bounds() {
+  volatile char bytes[STACK_ALLOC_SIZE];
+  struct X* x_ptr = (struct X*)(bytes + OFFSET);
+  return *x_ptr;
+}
+
+int main() {
+  struct X x = out_of_bounds();
+  return x.bytes[0];
+}
+
+/* { dg-output "ERROR: AddressSanitizer:? stack-buffer-overflow on 
address\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "READ of size 15 at 0x\[0-9a-f\]+ thread 
T0\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*Address 0x\[0-9a-f\]+ is located in stack of thread 
T0.*(\n|\r\n|\r)" */
+/* { dg-output ".*Memory access at offset \[0-9\]+ partially overflows this 
variable.*(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*in 
out_of_bounds.*stack-overflow-multi-byte-partial-6.c.*(\n|\r\n|\r)" */
diff --git 
a/gcc/testsuite/c-c++-common/asan/stack-overflow-multi-byte-partial-7.c 
b/gcc/testsuite/c-c++-common/asan/stack-overflow-multi-byte-partial-7.c
new file mode 100644
index 00000000000..99f993f708d
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/asan/stack-overflow-multi-byte-partial-7.c
@@ -0,0 +1,27 @@
+/* { dg-do run } */
+/* { dg-shouldfail "asan" } */
+
+#define STACK_ALLOC_SIZE 24
+#define READ_SIZE 23
+#define OFFSET 15
+
+struct X {
+  char bytes[READ_SIZE];
+};
+
+__attribute__((noinline)) struct X out_of_bounds() {
+  volatile char bytes[STACK_ALLOC_SIZE];
+  struct X* x_ptr = (struct X*)(bytes + OFFSET);
+  return *x_ptr;
+}
+
+int main() {
+  struct X x = out_of_bounds();
+  return x.bytes[0];
+}
+
+/* { dg-output "ERROR: AddressSanitizer:? stack-buffer-overflow on 
address\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "READ of size 23 at 0x\[0-9a-f\]+ thread 
T0\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*Address 0x\[0-9a-f\]+ is located in stack of thread 
T0.*(\n|\r\n|\r)" */
+/* { dg-output ".*Memory access at offset \[0-9\]+ partially overflows this 
variable.*(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*in 
out_of_bounds.*stack-overflow-multi-byte-partial-7.c.*(\n|\r\n|\r)" */
diff --git 
a/gcc/testsuite/c-c++-common/asan/stack-overflow-multi-byte-partial-8.c 
b/gcc/testsuite/c-c++-common/asan/stack-overflow-multi-byte-partial-8.c
new file mode 100644
index 00000000000..b0b700d85c0
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/asan/stack-overflow-multi-byte-partial-8.c
@@ -0,0 +1,27 @@
+/* { dg-do run } */
+/* { dg-shouldfail "asan" } */
+
+#define STACK_ALLOC_SIZE 24
+#define READ_SIZE 24
+#define OFFSET 20
+
+struct X {
+  char bytes[READ_SIZE];
+};
+
+__attribute__((noinline)) struct X out_of_bounds() {
+  volatile char bytes[STACK_ALLOC_SIZE];
+  struct X* x_ptr = (struct X*)(bytes + OFFSET);
+  return *x_ptr;
+}
+
+int main() {
+  struct X x = out_of_bounds();
+  return x.bytes[0];
+}
+
+/* { dg-output "ERROR: AddressSanitizer:? stack-buffer-overflow on 
address\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "READ of size 24 at 0x\[0-9a-f\]+ thread 
T0\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*Address 0x\[0-9a-f\]+ is located in stack of thread 
T0.*(\n|\r\n|\r)" */
+/* { dg-output ".*Memory access at offset \[0-9\]+ partially overflows this 
variable.*(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*in 
out_of_bounds.*stack-overflow-multi-byte-partial-8.c.*(\n|\r\n|\r)" */
diff --git a/libsanitizer/asan/asan_errors.cpp 
b/libsanitizer/asan/asan_errors.cpp
index 4f112cc5d1b..99facaa8c82 100644
--- a/libsanitizer/asan/asan_errors.cpp
+++ b/libsanitizer/asan/asan_errors.cpp
@@ -435,8 +435,11 @@ ErrorGeneric::ErrorGeneric(u32 tid, uptr pc_, uptr bp_, 
uptr sp_, uptr addr,
     bug_descr = "unknown-crash";
     if (AddrIsInMem(addr)) {
       u8 *shadow_addr = (u8 *)MemToShadow(addr);
-      // If we are accessing 16 bytes, look at the second shadow byte.
-      if (*shadow_addr == 0 && access_size > ASAN_SHADOW_GRANULARITY)
+      u8 *shadow_addr_upper_bound =
+          shadow_addr + (1 + ((access_size - 1) / ASAN_SHADOW_GRANULARITY));
+      // If the read could span multiple shadow bytes,
+      // do a sequential scan and look for the first bad shadow byte.
+      while (*shadow_addr == 0 && shadow_addr < shadow_addr_upper_bound)
         shadow_addr++;
       // If we are in the partial right redzone, look at the next shadow byte.
       if (*shadow_addr > 0 && *shadow_addr < 128) shadow_addr++;
--
2.34.1


This e-mail and any attachments may contain information that is confidential 
and proprietary and otherwise protected from disclosure. If you are not the 
intended recipient of this e-mail, do not read, duplicate or redistribute it by 
any means. Please immediately delete it and any attachments and notify the 
sender that you have received it by mistake. Unintended recipients are 
prohibited from taking action on the basis of information in this e-mail or any 
attachments. The DRW Companies make no representations that this e-mail or any 
attachments are free of computer viruses or other defects.

Reply via email to