Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <65AB4B12-2015-450E-BAD8-06398498D57A@trust-in-soft.com>
Date: Thu, 9 Nov 2017 21:19:03 +0000
From: Pascal Cuoq <cuoq@...st-in-soft.com>
To: "musl@...ts.openwall.com" <musl@...ts.openwall.com>
Subject: misc minor undefined behaviors in musl

Hello,

this message contains a collection of three issues that we have found running musl's source code inside tis-interpreter. They are all very minor, so it seems like the best format may be for me to put all of them in a single message, so that subscribers that aren't interested by C minutiae ignore the discussion more easily.

I can provide more detail on each of them, but again perhaps I'll only do this if someone thinks the individual issue deserves it. Unlike UB that the musl developers are certainly aware of but that involve difficult trade-offs (e.g. HASZERO in string functions), these seemed to be easy to fix if one deems it worth it, and a fix is suggested each time.

a) pointer arithmetic on NULL pointer in __fputwc_unlocked

• f->wpos may be NULL when reaching the test:
 } else if (f->wpos + MB_LEN_MAX < f->wend) {
 Pointer arithmetic using a null pointer is UB.

• fix: add a f->wpos != NULL test.

b) __pthread_tsd_run_dtors does not have a prototype

• in cleanup_fromsig, __pthread_tsd_run_dtors is called with an argument:
https://git.musl-libc.org/cgit/musl/tree/src/time/timer_create.c?id=1b9406b03c0a94ebe2076a8fc1746a8c45e78a83#n27
It is defined without a prototype here:
https://git.musl-libc.org/cgit/musl/tree/src/thread/pthread_key_create.c?id=1b9406b03c0a94ebe2076a8fc1746a8c45e78a83#n38
It is also called without an argument here:
https://git.musl-libc.org/cgit/musl/tree/src/thread/pthread_create.c?id=1b9406b03c0a94ebe2076a8fc1746a8c45e78a83#n38
and weak_alias'd to dummy_0 here:
https://git.musl-libc.org/cgit/musl/tree/src/thread/pthread_create.c?id=1b9406b03c0a94ebe2076a8fc1746a8c45e78a83#n18

• it seems like the above is on purpose, especially the definition of __pthread_tsd_run_dtors without a prototype, which is still valid in C11and does not mean the same thing as expecting void. Still, for the sake of simplicity, would it be possible to uniformize to __pthread_tsd_run_dtors always taking an argument? Can the invocation without an argument in pthread_create.c end up being linked with an implementation of __pthread_tsd_run_dtors that takes an argument and actually uses it?

c) definition of MAP_FAILED

• tis-interpreter does not like the definition of MAP_FAILED as (void*)-1. While mmap is a bit outside the scope of what it was initially designed to do, some simple uses of mmap can be emulated pretty well on a case-by-case basis by making mmap return the address of a preallocated array of chars. When we do this, then the comparison to MAP_FAILED at the call site becomes the next problem, because unlike (void *)0, nothing guarantees that (void*)-1 does not happen to be, by bad luck, the address of the array.

• we fixed this by defining MAP_FAILED as the address of a const char variable defined for this purpose. Since everyone else is defining MAP_FAILED as (void*)-1, this will never be wrong, but I wonder whether someone can see ways in which our solution is better or worse than the original.


Best regards,

Pascal






Content of type "text/html" skipped

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.