Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20170830013036.GC1627@brightrain.aerifal.cx>
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[0] = s[0] = 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.