clang's memory sanitizer triggers in freea()

[Tim Rühsen]

Hi,

I am trying to use clang's address sanitizer on libidn2.

It finds the use of uninitialized stack memory in malloca.c/freea()
(latest gnulib sources).

It is this line which causes problems:

if (((int *) p)[-1] == MAGIC_NUMBER)


Is there anything that can/should be done in the code ?

Maybe using clang's __attribute__((no_sanitize("memory"))) (see
https://clang.llvm.org/docs/MemorySanitizer.html#handling-external-code) ?

Or is it even a false positive that should be addressed at llvm/clang ?


The trace is


==685==WARNING: MemorySanitizer: use-of-uninitialized-value
#0 0x7fcb2c841ba8 in freea
/usr/oms/src/libidn2/unistring/malloca.c:134:11
#1 0x7fcb2c851b44 in mem_iconveha
/usr/oms/src/libidn2/unistring/striconveha.c:253:7
#2 0x7fcb2c854ea2 in u8_conv_from_encoding
/usr/oms/src/libidn2/unistring/uniconv/u8-conv-from-enc.c:98:11
#3 0x7fcb2c855336 in u8_strconv_from_encoding
/usr/oms/src/libidn2/unistring/uniconv/u-strconv-from-enc.h:35:5
#4 0x7fcb2c8557a4 in u8_strconv_from_locale
/usr/oms/src/libidn2/unistring/uniconv/u8-strconv-from-locale.c:37:10
#5 0x7fcb2c82e546 in idn2_register_ul
/usr/oms/src/libidn2/lib/register.c:245:20
#6 0x48f256 in main /usr/oms/src/libidn2/tests/test-register.c:187:13
#7 0x7fcb2b9592b0 in __libc_start_main
(/lib/x86_64-linux-gnu/libc.so.6+0x202b0)
#8 0x419b79 in _start
(/usr/oms/src/libidn2/tests/test-register+0x419b79)

SUMMARY: MemorySanitizer: use-of-uninitialized-value
/usr/oms/src/libidn2/unistring/malloca.c:134:11 in freea
Exiting


With Best Regards, Tim

[Eric Blake]

Post by Tim Rühsen
Hi,

[you signed your mail with an expired gpg key...]

Post by Tim Rühsen
I am trying to use clang's address sanitizer on libidn2.
It finds the use of uninitialized stack memory in malloca.c/freea()
(latest gnulib sources).
if (((int *) p)[-1] == MAGIC_NUMBER)
Is there anything that can/should be done in the code ?

The access really is undefined per C rules, but safe in practice because
of the size of the stack (the access may be out of bounds but is
guaranteed not to fault), and while our decision based on the comparison
can very-occasionally hit a false positive, our slow path code should
correctly handle that.

So the best we can do is annotate the code to state that we are
intentionally stepping outside the bounds of safe C.

Post by Tim Rühsen
Maybe using clang's __attribute__((no_sanitize("memory"))) (see
https://clang.llvm.org/docs/MemorySanitizer.html#handling-external-code) ?

If it works. Patches welcome (I don't use clang enough myself to easily
test the idea).

Post by Tim Rühsen
Or is it even a false positive that should be addressed at llvm/clang ?

No, this is one case where the compiler IS flagging a real violation of
the C standard (where we are instead relying on our knowledge of how
hardware really behaves to still get reliable behavior).

[Tim Rühsen]

Post by Eric Blake

Post by Tim Rühsen
Hi,

[you signed your mail with an expired gpg key...]

I update the expiration time every two years, your local cache is maybe
updated (just reload the key from the key server). My key is still valid for
~1 year.

Post by Eric Blake

Post by Tim Rühsen
I am trying to use clang's address sanitizer on libidn2.
It finds the use of uninitialized stack memory in malloca.c/freea()
(latest gnulib sources).
if (((int *) p)[-1] == MAGIC_NUMBER)
Is there anything that can/should be done in the code ?

The access really is undefined per C rules, but safe in practice because
of the size of the stack (the access may be out of bounds but is
guaranteed not to fault), and while our decision based on the comparison
can very-occasionally hit a false positive, our slow path code should
correctly handle that.
So the best we can do is annotate the code to state that we are
intentionally stepping outside the bounds of safe C.

Thanks for clarification !

Post by Eric Blake

Post by Tim Rühsen
Maybe using clang's __attribute__((no_sanitize("memory"))) (see
https://clang.llvm.org/docs/MemorySanitizer.html#handling-external-code) ?

If it works. Patches welcome (I don't use clang enough myself to easily
test the idea).

If it works here, I'll make up a patch in the next days.

Regards, Tim

[Eric Blake]

Post by Tim Rühsen

Post by Eric Blake

Hi,

[you signed your mail with an expired gpg key...]

I update the expiration time every two years, your local cache is maybe
updated (just reload the key from the key server). My key is still valid for
~1 year.

Indeed. Annoying that gpg can't automatically figure that sort of thing
out.

[Tim Rühsen]

Post by Eric Blake

Post by Tim Rühsen
I am trying to use clang's address sanitizer on libidn2.
It finds the use of uninitialized stack memory in malloca.c/freea()
(latest gnulib sources).
if (((int *) p)[-1] == MAGIC_NUMBER)
Is there anything that can/should be done in the code ?

The access really is undefined per C rules, but safe in practice because
of the size of the stack (the access may be out of bounds but is
guaranteed not to fault), and while our decision based on the comparison
can very-occasionally hit a false positive, our slow path code should
correctly handle that.
So the best we can do is annotate the code to state that we are
intentionally stepping outside the bounds of safe C.

Post by Tim Rühsen
Maybe using clang's __attribute__((no_sanitize("memory"))) (see
https://clang.llvm.org/docs/MemorySanitizer.html#handling-external-code) ?

If it works. Patches welcome (I don't use clang enough myself to easily
test the idea).

Here we go. The patch is simple and tested with clang-5.0 and gcc-6 and Debian
unstable. Works for me ;-)

Regards, Tim

[Bruno Haible]

Post by Tim Rühsen
Here we go. The patch is simple and tested with clang-5.0 and gcc-6 and Debian
unstable. Works for me ;-)

Thanks! I've applied it, with minor stylistic tweaks (gnulib's indentation style
for #if lines, put generic macro definitions near the beginning of the file) and
a ChangeLog entry.

Bruno