Date: Tue, 29 Aug 2017 21:30:36 -0400 From: Rich Felker <dalias@...c.org> To: musl@...ts.openwall.com Subject: Re: open issues On Sun, Aug 27, 2017 at 06:36:06AM +0200, Szabolcs Nagy wrote: > list of issues since last release that don't seem to be resolved. > first ones that looked like bugs to me, then other issues. > list goes backward in time, some issues may be missed. > > bug: > - update contributors in copyright file Will do at release time. Thanks for the reminder though. > - fflush(0) does not lock f > http://git.musl-libc.org/cgit/musl/tree/src/stdio/fflush.c#n12 Fixing: commit 670d6d01f53b4e85be6b333bf8a137e2be6d3fc3 > - getenv/setenv/putenv ub > http://www.openwall.com/lists/musl/2017/08/21/1 > http://www.openwall.com/lists/musl/2016/03/13/7 Merging this is way overdue. It's larger than the patches I'm merging while reading through this post right now but I'll come back to it. > - fix ioctl on mips, add SIOCGSTAMPNS > http://www.openwall.com/lists/musl/2017/08/13/4 > http://www.openwall.com/lists/musl/2017/08/13/5 Committing above patches. > - ipc/ftok.c overflowing shift > http://www.openwall.com/lists/musl/2017/08/12/5 Already committed but I forgot to push. > - __progname fallbacks so it's never 0 > http://www.openwall.com/lists/musl/2017/07/28/6 Committing the patch. > - mbsnrtowcs and mbsnrtowcs confuses byte and wchar counts > http://www.openwall.com/lists/musl/2017/08/09/1 I know the current code is wrong but I'm not sure the patches are fully right either; probably needs testing. But if we don't get to it, just merging them would be better than leaving the code dangerously broken. > - memset ub because s = s = c > http://www.openwall.com/lists/musl/2017/07/10/7 > http://www.openwall.com/lists/musl/2017/07/06/2 Applying patch manually with a short message crediting Pascal Cuoq. The reason I didn't get to this earlier was that something botched the email attachment and the patch was corrupt (DOS newlines); I should have replied and mentioned it at the time. Fix is in commit 9d4c902c42b3fda368d7ea64bb9575c46228fa7f > - handle whitespace before %% in scanf > http://www.openwall.com/lists/musl/2017/07/11/1 Unless I missed something, I'm waiting for a different form of the patch that simplifies code rather than making it more complex. > - mips64 utime issue? > "tar binary can't fix the modification/access times on any extracted symbolic links," > http://www.openwall.com/lists/musl/2017/07/06/1 Needs more investigation. > - oob reads in memmem (and signed << ub) > http://www.openwall.com/lists/musl/2017/06/29/6 I think either proposed fix is probably okay, but I'm kinda waiting for some tests to show that the problem is fixed and that no obvious new bugs are introduced. > - use-after-free in __unlock of pthread struct > http://www.openwall.com/lists/musl/2017/06/01/7 This could be fixed by merging Jens' new lock implementation or with a local fix for use of exitlock. I'm not sure which is more work but either is more than I can do in a couple minutes going through this list to tick off the easy things. > - newly created thread may run with signals blocked > < sergei> there seems to be a race condition in pthread_create.c between lines 134 and 298 > < sergei> if line 298 is executed before 134 (assuming syscall returned 0), startlock will be overwritten with zero, the condition will be evaluated to false and __restore_sigs will not be executed > < sergei> the newly created thread will run with all signals blocked > < sergei> i have a patch that fixes the issue for me: https://pastebin.com/T5QSd0C9 I don't really like this patch, but it might be the best short-term solution. Ideally all of the wacky random logic with atomics should just be ripped out and replaced by idiomatic locks of some sort. It doesn't help that startlock is being used both as a lock and to reflect other flags (whether the new thread needs to restore its signal mask and whether it needs to detach+abort). There really should just be some sort of semaphore here (possibly the existing startlock, but without overloaded meaning) followed by (after passing it) code to check and act on the signal and abort conditions _without_ caring whether the semaphore ever blocked forward progress at first. > - scanf, wrong types in va_arg > http://www.openwall.com/lists/musl/2017/04/10/3 It's not clear what can be done here. Any fix is doomed to be incomplete because POSIX allows missing specifiers (assuming all pointers have same variadic representation as void *). When there are no missing specifiers, we can fix it, but with a lot of "spurious" code. > - missed underflow in fma > http://www.openwall.com/lists/musl/2017/03/19/6 > new fma, depends on a_clz_64 > http://www.openwall.com/lists/musl/2017/04/23/10 IIRC I was plenty convinced that your proposed new fma is a better approach, but wasn't too fond of adding new a_* primitives (a_clz_64). Maybe we should just do it for now though. > - fix nftw when called with paths ending in slash > http://www.openwall.com/lists/musl/2017/03/07/1 This is another one that needs more attention that I'm giving items on the list right now. > - fix syscall number differences compared to linux uapi > http://www.openwall.com/lists/musl/2017/02/18/1 I don't recall whether we checked that the patch doesn't mess up musl's internal use of the syscall numbers (especially the remapping logic in src/internal/syscall.h). If that's checked this is probably ok. > - getservbyport(_r) should not report numeric ports > http://www.openwall.com/lists/musl/2017/02/06/5 This is probably correct, and my comments on simplifying it in the original thread look wrong. I do wonder if it's inconsistent to make this change without changing the opposite-direction lookup not to accept numeric strings though. Thoughts? > - add s390x and powerpc64 to supported arches > http://www.openwall.com/lists/musl/2017/02/01/2 Does s390x need any special notes in INSTALL like most other archs have? I think probably not. > - define IPPORT_RESERVED in netinet/in.h and netdb.h > http://www.openwall.com/lists/musl/2017/01/31/4 Fixed in commit 5f7efb87a28a311ad377dd26adf53715dedb096d > - GLOB_PERIOD is inconsistent with glibc > http://www.openwall.com/lists/musl/2017/01/12/5 This needs further consideration of what the right fix is, but I agree the current behavior is wrong. > - mmap should not return EPERM when it means ENOMEM > http://www.openwall.com/lists/musl/2017/01/12/1 Probably need to work out the specific logic for when the replacement should be done. > - getopt_long does not report failure correctly > http://www.openwall.com/lists/musl/2017/01/07/4 I'm missing this thread from my copy of the list mail due to downtime from fire. I think the issue was already fixed correctly (vs with weird internal-state hacks) in commit 786fda875a901dc1807289c940338487854cd3ba from a few days earlier. > - make dlsym and reloc time lookup consistent > http://www.openwall.com/lists/musl/2017/02/16/1 At least part of this was already merged in commit c9783e4d32e786c4b76bf77c6030111d9e79dbb7. I think the other part had sufficient conflicts with other changes made that it wasn't easy for me to resolve them. > - ldso ctor dependency ordering and recursive dlopen fix > http://www.openwall.com/lists/musl/2017/01/03/6 Yes, I think both of these need to wait til after release since they involve significant amounts of new development & design that have been error-prone so far... > - align arm hwcap.h with glibc (nsz) I think I'm missing this patch...? > feature request: I'm skipping all these for now, but appreciate having them listed. Rich
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.