<div dir="ltr">Nick, if there is not patch yet, I am going to create one.</div><br><div class="gmail_quote"><div dir="ltr">On Fri, Nov 11, 2016 at 5:24 PM Kostya Serebryany <<a href="mailto:kcc@google.com">kcc@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr" class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_extra gmail_msg"><br class="gmail_msg"><div class="gmail_quote gmail_msg">On Fri, Nov 11, 2016 at 5:21 PM, Kostya Serebryany <span dir="ltr" class="gmail_msg"><<a href="mailto:kcc@google.com" class="gmail_msg" target="_blank">kcc@google.com</a>></span> wrote:<br class="gmail_msg"><blockquote class="gmail_quote gmail_msg" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr" class="gmail_msg">btw, Nick, do you have a test where this fails? </div><div class="m_4988204927796374810HOEnZb gmail_msg"><div class="m_4988204927796374810h5 gmail_msg"><div class="gmail_extra gmail_msg"><br class="gmail_msg"><div class="gmail_quote gmail_msg">On Tue, Nov 8, 2016 at 5:51 PM, Nick Lewycky <span dir="ltr" class="gmail_msg"><<a href="mailto:nlewycky@google.com" class="gmail_msg" target="_blank">nlewycky@google.com</a>></span> wrote:<br class="gmail_msg"><blockquote class="gmail_quote gmail_msg" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr" class="gmail_msg"><div class="gmail_extra gmail_msg"><div class="gmail_quote gmail_msg"><div class="gmail_msg"><div class="m_4988204927796374810m_-6893603495633923300h5 gmail_msg">On 8 November 2016 at 17:20, Kostya Serebryany <span dir="ltr" class="gmail_msg"><<a href="mailto:kcc@google.com" class="gmail_msg" target="_blank">kcc@google.com</a>></span> wrote:<br class="gmail_msg"><blockquote class="gmail_quote gmail_msg" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr" class="gmail_msg"><br class="gmail_msg"><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_extra gmail_msg"><br class="gmail_msg"><div class="gmail_quote gmail_msg"><div class="gmail_msg"><div class="m_4988204927796374810m_-6893603495633923300m_5717655743882326564gmail-h5 gmail_msg">On Tue, Nov 8, 2016 at 2:30 PM, Nick Lewycky <span dir="ltr" class="gmail_msg"><<a href="mailto:nlewycky@google.com" class="gmail_msg" target="_blank">nlewycky@google.com</a>></span> wrote:<br class="gmail_msg"><blockquote class="gmail_quote gmail_msg" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr" class="gmail_msg"><div class="gmail_extra gmail_msg"><div class="gmail_quote gmail_msg"><span class="m_4988204927796374810m_-6893603495633923300m_5717655743882326564gmail-m_-4476846632077403236gmail- gmail_msg">On 21 October 2016 at 16:52, Kostya Serebryany via llvm-commits <span dir="ltr" class="gmail_msg"><<a href="mailto:llvm-commits@lists.llvm.org" class="gmail_msg" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br class="gmail_msg"></span><span class="m_4988204927796374810m_-6893603495633923300m_5717655743882326564gmail-m_-4476846632077403236gmail- gmail_msg"><blockquote class="gmail_quote gmail_msg" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Author: kcc<br class="gmail_msg">
Date: Fri Oct 21 18:52:26 2016<br class="gmail_msg">
New Revision: 284901<br class="gmail_msg">
<br class="gmail_msg">
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=284901&view=rev" rel="noreferrer" class="gmail_msg" target="_blank">http://llvm.org/viewvc/llvm-project?rev=284901&view=rev</a><br class="gmail_msg">
Log:<br class="gmail_msg">
[sanitizers] support strict_string_checks for strncmp<br class="gmail_msg">
<br class="gmail_msg">
Added:<br class="gmail_msg">
compiler-rt/trunk/test/asan/TestCases/strncmp_strict.c<br class="gmail_msg">
Modified:<br class="gmail_msg">
compiler-rt/trunk/lib/sanitizer_common/sanitizer_common_interceptors.inc<br class="gmail_msg">
<br class="gmail_msg">
Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_common_interceptors.inc<br class="gmail_msg">
URL: <a href="http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_common_interceptors.inc?rev=284901&r1=284900&r2=284901&view=diff" rel="noreferrer" class="gmail_msg" target="_blank">http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_common_interceptors.inc?rev=284901&r1=284900&r2=284901&view=diff</a><br class="gmail_msg">
==============================================================================<br class="gmail_msg">
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_common_interceptors.inc (original)<br class="gmail_msg">
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_common_interceptors.inc Fri Oct 21 18:52:26 2016<br class="gmail_msg">
@@ -304,8 +304,8 @@ INTERCEPTOR(int, strncmp, const char *s1<br class="gmail_msg">
c2 = (unsigned char)s2[i];<br class="gmail_msg">
if (c1 != c2 || c1 == '\0') break;<br class="gmail_msg">
}<br class="gmail_msg">
- COMMON_INTERCEPTOR_READ_RANGE(ctx, s1, Min(i + 1, size));<br class="gmail_msg">
- COMMON_INTERCEPTOR_READ_RANGE(ctx, s2, Min(i + 1, size));<br class="gmail_msg">
+ COMMON_INTERCEPTOR_READ_STRING(ctx, s1, Min(i + 1, size));<br class="gmail_msg">
+ COMMON_INTERCEPTOR_READ_STRING(ctx, s2, Min(i + 1, size));<br class="gmail_msg"></blockquote><div class="gmail_msg"><br class="gmail_msg"></div></span><div class="gmail_msg">I think this change is wrong, but please double check my analysis. I think arguments to strncmp don't need to be NUL terminated like arguments to strcmp do.</div><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">C99 7.1.1 defines: "A string is a contiguous sequence of characters terminated by and including the first null character." Then strcmp is defined as a function which "compares the strings" in 7.21.4.2 while strncmp is defined to operate over two "possibly null-terminated array"s in 7.21.4.4. The bug manifests in COMMON_INTERCEPTOR_READ_STRING calling REAL(strlen) on these arguments which aren't guaranteed to be NUL terminated.</div><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">What do you think?</div></div></div></div></blockquote><div class="gmail_msg"><br class="gmail_msg"></div></div></div><div class="gmail_msg">Mmmm. Are these parts equivalent in C99 and C++ standards? </div><div class="gmail_msg">At least here <a href="http://www.cplusplus.com/reference/cstring/strncmp/" class="gmail_msg" target="_blank">http://www.cplusplus.com/reference/cstring/strncmp/</a> I read: </div><div class="gmail_msg"><dl><dt style="background-color:rgb(240,240,240);font-family:monospace;margin-top:5px;color:rgb(0,0,0);font-size:12px" class="gmail_msg">str1</dt><dd style="margin-bottom:10px;color:rgb(0,0,0);font-family:verdana,arial,helvetica,sans-serif;font-size:12px" class="gmail_msg">C string to be compared.</dd><dt style="background-color:rgb(240,240,240);font-family:monospace;margin-top:5px;color:rgb(0,0,0);font-size:12px" class="gmail_msg">str2</dt><dd style="margin-bottom:10px;color:rgb(0,0,0);font-family:verdana,arial,helvetica,sans-serif;font-size:12px" class="gmail_msg">C string to be compared.</dd></dl></div></div></div></div></blockquote></div></div><div class="gmail_msg">I think they're simplifying. But <a href="http://cppreference.com" class="gmail_msg" target="_blank">cppreference.com</a> also disagrees with me: <a href="http://en.cppreference.com/w/cpp/string/byte/strncmp" class="gmail_msg" target="_blank">http://en.cppreference.com/w/cpp/string/byte/strncmp</a> . They have separate entries for strncmp in C and for C++ and they differ, for C++ it says they compare two null-terminated strings, while in C it says they compare two possibly null-terminated strings.</div><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg"><div class="gmail_msg">The C++ standard doesn't define either strcmp or strncmp. I think the normative text is in 21.8 [c.strings]:</div></div><div class="gmail_msg">"1 Tables 74, 75, 76, 77, 78, and 79 describe headers <cctype>, <cwctype>, <cstring>, <cwchar>, <cstdlib></div><div class="gmail_msg">(character conversions), and <cuchar>, respectively."</div><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">The tables list all the various functions, table 76 includes both strcmp and strncmp.<br class="gmail_msg"></div><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">"2 The contents of these headers shall be the same as the Standard C Library headers <ctype.h>, <wctype.h>,</div><div class="gmail_msg"><string.h>, <wchar.h>, and <stdlib.h> and the C Unicode TR header <uchar.h>, respectively, with the</div><div class="gmail_msg">following modifications:"</div><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">The modifications that follow affect types char16_t, char32_t and wchar_t and functions strchr, strpbrk, strrchr, strstr, memchr, wcschr, wcspbrk, wcsrchr, wcsstr, wmemchr, strerror, strtok, as well as anything that takes an mbstate_t*.</div><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">I can't see anything else that would make the semantics different between C and C++.</div><span class="gmail_msg"><div class="gmail_msg"><br class="gmail_msg"></div><blockquote class="gmail_quote gmail_msg" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr" class="gmail_msg"><div class="gmail_extra gmail_msg"><div class="gmail_quote gmail_msg"><div class="gmail_msg">I probably still need to replace this with something like </div><div class="gmail_msg"><div class="gmail_msg">COMMON_INTERCEPTOR_READ_RANGE((ctx), (s), \</div><div class="gmail_msg"> common_flags()->strict_string_checks ? size : (Min(i + 1, size)) )</div></div><div class="gmail_msg">to avoid calling strlen</div></div></div></div></blockquote><div class="gmail_msg"><br class="gmail_msg"></div></span><div class="gmail_msg">I don't think that's quite right either. strncmp will read up to the first NUL at which point it stops, or it reads up to size. That makes it:</div><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg"><div class="gmail_msg"> unsigned char c1 = 0, c2 = 0;</div><div class="gmail_msg"> uptr i;</div><div class="gmail_msg"> for (i = 0; i < size; i++) {</div><div class="gmail_msg"> c1 = (unsigned char)s1[i];</div><span class="gmail_msg"><div class="gmail_msg"> c2 = (unsigned char)s2[i];</div></span><div class="gmail_msg"> if (c1 != c2 || c1 == '\0') {</div><div class="gmail_msg"> ++i; // include trailing NUL</div><div class="gmail_msg"> break;</div><div class="gmail_msg"> }</div><div class="gmail_msg"> }</div></div><div class="gmail_msg"> assert(i <= size);</div><div class="gmail_msg"> COMMON_INTERCEPTOR_READ_RANGE((ctx), (s1), i)<br class="gmail_msg"></div><div class="gmail_msg"><div class="gmail_msg"> COMMON_INTERCEPTOR_READ_RANGE((ctx), (s2), i)<br class="gmail_msg"></div></div><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">and there's no need to check for strict string handling?</div><span class="m_4988204927796374810m_-6893603495633923300HOEnZb gmail_msg"><font color="#888888" class="gmail_msg"><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">Nick</div></font></span><div class="gmail_msg"><div class="m_4988204927796374810m_-6893603495633923300h5 gmail_msg"><div class="gmail_msg"><br class="gmail_msg"></div><blockquote class="gmail_quote gmail_msg" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr" class="gmail_msg"><div class="gmail_extra gmail_msg"><div class="gmail_quote gmail_msg"><div class="m_4988204927796374810m_-6893603495633923300m_5717655743882326564gmail-h5 gmail_msg"><blockquote class="gmail_quote gmail_msg" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr" class="gmail_msg"><div class="gmail_extra gmail_msg"><div class="gmail_quote gmail_msg"><span class="m_4988204927796374810m_-6893603495633923300m_5717655743882326564gmail-m_-4476846632077403236gmail-HOEnZb gmail_msg"><font color="#888888" class="gmail_msg"><div class="gmail_msg">Nick<br class="gmail_msg"></div></font></span><div class="gmail_msg"><div class="m_4988204927796374810m_-6893603495633923300m_5717655743882326564gmail-m_-4476846632077403236gmail-h5 gmail_msg"><div class="gmail_msg"><br class="gmail_msg"></div><blockquote class="gmail_quote gmail_msg" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
int result = CharCmpX(c1, c2);<br class="gmail_msg">
CALL_WEAK_INTERCEPTOR_HOOK(__sanitizer_weak_hook_strncmp, GET_CALLER_PC(), s1,<br class="gmail_msg">
s2, size, result);<br class="gmail_msg">
<br class="gmail_msg">
Added: compiler-rt/trunk/test/asan/TestCases/strncmp_strict.c<br class="gmail_msg">
URL: <a href="http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/test/asan/TestCases/strncmp_strict.c?rev=284901&view=auto" rel="noreferrer" class="gmail_msg" target="_blank">http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/test/asan/TestCases/strncmp_strict.c?rev=284901&view=auto</a><br class="gmail_msg">
==============================================================================<br class="gmail_msg">
--- compiler-rt/trunk/test/asan/TestCases/strncmp_strict.c (added)<br class="gmail_msg">
+++ compiler-rt/trunk/test/asan/TestCases/strncmp_strict.c Fri Oct 21 18:52:26 2016<br class="gmail_msg">
@@ -0,0 +1,26 @@<br class="gmail_msg">
+// Test strict_string_checks option in strncmp function<br class="gmail_msg">
+// RUN: %clang_asan %s -o %t && %run %t 2>&1<br class="gmail_msg">
+// RUN: %env_asan_opts=strict_string_checks=false %run %t 2>&1<br class="gmail_msg">
+// RUN: %env_asan_opts=strict_string_checks=true not %run %t 2>&1 | FileCheck %s<br class="gmail_msg">
+<br class="gmail_msg">
+#include <assert.h><br class="gmail_msg">
+#include <stdlib.h><br class="gmail_msg">
+#include <string.h><br class="gmail_msg">
+<br class="gmail_msg">
+int main(int argc, char **argv) {<br class="gmail_msg">
+ size_t size = 100;<br class="gmail_msg">
+ char fill = 'o';<br class="gmail_msg">
+ char *s1 = (char*)malloc(size);<br class="gmail_msg">
+ memset(s1, fill, size);<br class="gmail_msg">
+ char *s2 = (char*)malloc(size);<br class="gmail_msg">
+ memset(s2, fill, size);<br class="gmail_msg">
+ s1[size - 1] = 'z';<br class="gmail_msg">
+ s2[size - 1] = 'x';<br class="gmail_msg">
+ int r = strncmp(s1, s2, size + 1);<br class="gmail_msg">
+ // CHECK: {{.*ERROR: AddressSanitizer: heap-buffer-overflow on address}}<br class="gmail_msg">
+ // CHECK: READ of size 101<br class="gmail_msg">
+ assert(r == 1);<br class="gmail_msg">
+ free(s1);<br class="gmail_msg">
+ free(s2);<br class="gmail_msg">
+ return 0;<br class="gmail_msg">
+}<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
_______________________________________________<br class="gmail_msg">
llvm-commits mailing list<br class="gmail_msg">
<a href="mailto:llvm-commits@lists.llvm.org" class="gmail_msg" target="_blank">llvm-commits@lists.llvm.org</a><br class="gmail_msg">
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" class="gmail_msg" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br class="gmail_msg">
</blockquote></div></div></div><br class="gmail_msg"><br class="gmail_msg"></div></div>
</blockquote></div></div><br class="gmail_msg"></div></div>
</blockquote></div></div></div><br class="gmail_msg"></div></div>
</blockquote></div><br class="gmail_msg"></div>
</div></div></blockquote></div><br class="gmail_msg"></div>
</blockquote></div>