Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date: Sat, 27 Sep 2014 12:56:14 -0400
From: "Anthony G. Basile" <basile@...nsource.dyc.edu>
To: musl@...ts.openwall.com
Subject: Re: fdopen/fflush problem

On 09/27/14 09:38, Rich Felker wrote:
> On Sat, Sep 27, 2014 at 09:29:20AM -0400, Anthony G. Basile wrote:
>> On 09/26/14 03:23, u-wsnj@...ey.se wrote:
>>> On Fri, Sep 26, 2014 at 03:05:48AM -0400, Rich Felker wrote:
>>>>      unspecified if the flag argument contains more than the following
>>>>      flags:"
>>>>
>>>> The list that follows includes just O_APPEND, O_CLOEXEC, and O_*SYNC,
>>>> so including O_WRONLY has unspecified behavior. However, since this is
>>>> just unspecified, not undefined, it seems bad for something "horribly
>>>> wrong" to happen, like a file with no access like we're getting now.
>>>> Indeed, POSIX will define an optional error:
>>>>
>>>>      "The mkostemp( ) function may fail if:
>>>>
>>>>      [EINVAL] The value of the flag argument is invalid."
>>>>
>>>> So I think we should either make this error be detected, or silently
>>>> mask off the bad access mode. My leaning would be towards reporting it
>>>> as an error. Opinions?
>>>
>>> +1 (reporting an error)
>>>
>>> Rune
>>>
>>
>> Both uclibc [1] and glibc are okay with the flag combination
>> O_WRONLY|O_CLOEXEC.  With unspecified behaviour you really can't say
>> which way to go here for portability.  The correct thing to do would
>> be to get this behaviour specified (in SUSv4 or something??) since
>> mkostemp is currently a GNU-ism.
>
> Except that it's not; it's accepted for POSIX and you can read the
> text for it here:
>
> http://austingroupbugs.net/view.php?id=411

Thanks.  I didn't know that.

>
> It seems the unspecified behavior is intentional. But I don't see
> anything you could meaningfully do with the access mode since it's not
> mandatory; an explicit mode of O_RDONLY would be indistinguishable
> from a request for the default (O_RDWR), which is not a big problem
> since O_RDONLY doesn't make sense for a new file you're creating, but
> this only works because O_RDONLY happens to be the one with value 0.
> If O_WRONLY had the value zero, you couldn't honor it. So really, the
> only viable implementation choices seem to be silently ignoring the
> mode, or throwing an error if any mode bits are specified. And since
> we can't detect all modes (e.g. O_RDONLY can't be detected since it's
> 0) I think it somewhat makes sense, from a consistency standpoint, to
> just ignore the access mode bits...

Yeah, makes sense.

>
>> [1] In uclibc libc/stdlib/mkostemp.c calls __gen_tempname in
>> libc/misc/internals/tempname.c with kind = __GT_FILE and opens the
>> file with
>>
>>      fd = open (tmpl, O_RDWR | O_CREAT | O_EXCL, mode);
>>
>> which is different from how musl does it in __mkostemps in
>> src/temp/mkostemps.c
>>
>>     fd = open(template, flags | O_RDWR | O_CREAT | O_EXCL, 0600);
>>
>> Looks like uclibc completely ignores O_APPEND, O_CLOEXEC, and O_*SYNC.
>
> Wow. So they just made a fake version of this function which ignores
> the whole purpose it was created for -- atomic close-on-exec? Care to
> report this?
>
> Rich
>

Yes, I am definitely going to report this.  Defeating the atomic 
O_CLOEXEC is painful.  This is the second time atomicity was overlooked 
in uclibc (that I know of), the other being pread/pwrite implemented as 
open/lseek/{read,write}.  I'm not sure if this is legacy cruft going 
back to time when linux kernels didn't provide the support or what.


-- 
Anthony G. Basile, Ph. D.
Chair of Information Technology
D'Youville College
Buffalo, NY 14201
(716) 829-8197

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.