Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-Id: <BFBA6A32-4445-4831-9838-86F71F80E6F7@adelielinux.org>
Date: Sat, 16 Oct 2021 01:30:19 -0500
From: "A. Wilcox" <awilfox@...lielinux.org>
To: musl@...ts.openwall.com
Cc: "(GalaxyMaster)" <galaxy@...nwall.com.au>
Subject: Re: errno on writing to read-only files

On Oct 13, 2021, at 9:02 AM, Rich Felker <dalias@...c.org> wrote:
> 
> On Wed, Oct 13, 2021 at 01:21:31AM +0000, (GalaxyMaster) wrote:
>> Hello,
>> 
>> I am observing the following on musl and I am not sure that this is the way it
>> should be:
>> ===
>> galaxy@...hlinux:~/musl-tests $ cat fput-to-readonly.c 
>> #include <stdio.h>
>> #include <errno.h>
>> 
>> int main() {
>>    FILE *f;
>>    int i = 0;
>>    f = fopen("fput-to-readonly.c", "r");
>>    errno = 0;
>>    i = fputs("should not be written", f);
>>    printf("i = %d (should be negative [EOF = %d])\n", i, EOF);
>>    printf("errno = %d\n", errno);
>>    return 0;
>> }
>> galaxy@...hlinux:~/musl-tests $ gcc -o fput-to-readonly fput-to-readonly.c 
>> galaxy@...hlinux:~/musl-tests $ ./fput-to-readonly 
>> i = -1 (should be negative [EOF = -1])
>> errno = 0
>> galaxy@...hlinux:~/musl-tests $
>> ===
>> 
>> Logically, I would expect the errno variable to be set to something since there
>> was clearly an error and the data has not been written to the destination.
>> 
>> Glibc returns EBADF (9) in this case:
>> ===
>> [galaxy@...hlinux musl-tests]$ ./fput-to-readonly 
>> i = -1 (should be negative [EOF = -1])
>> errno = 9
>> [galaxy@...hlinux musl-tests]$
>> ===
>> 
>> Should not we do the same?  It kind of makes sense since the descriptor we are
>> asked to write to is read-only.  I think it would be just one line added to
>> src/stdio/__towrite.c, something like:
>> ===
>> --- musl-b76f37fd5625d038141b52184956fb4b7838e9a5.orig/src/stdio/__towrite.c    2021-09-24 00:09:22.000000000 +0000
>> +++ musl-b76f37fd5625d038141b52184956fb4b7838e9a5/src/stdio/__towrite.c    2021-10-13 01:16:04.713069382 +0000
>> @@ -5,6 +5,7 @@ int __towrite(FILE *f)
>>    f->mode |= f->mode-1;
>>    if (f->flags & F_NOWR) {
>>        f->flags |= F_ERR;
>> +        errno = EBADF;
>>        return EOF;
>>    }
>>    /* Clear read buffer (easier than summoning nasal demons) */
>> ===
> 
> This is a duplicate of:
> https://www.openwall.com/lists/musl/2020/10/08/1
> 
> As noted then, it's undefined behavior to call stdio functions on a
> stream that's not the appropriate type. We do the check quoted above
> to avoid blowing up and corrupting buffer state by trying to do the
> wrong type of operation, but don't set an errno because there isn't
> one specified for this (since it's UB). Arguably it would be better
> and more consistent with what we do elsewhere to crash, but for
> whatever reason that wasn't done.
> 
> EBADF is specified for when the underlying fd is in the wrong mode,
> which is a different condition (and only can happen when you inherited
> it as stdin/out/err, used fdopen, or dup2'd over an existing FILE's
> fd).
> 
> Rich

Hi there,

The last (relevant) message in the thread of the duplicate, at https://www.openwall.com/lists/musl/2020/10/08/3, mentions a potential fix by setting errno in __towrite()/__toread().  For clarity, would that be something acceptable?

I doubt it, but was just curious, since no one replied to that suggestion either way.

Best,
-arw

--
A. Wilcox (Sent from my iPhone)
Mac, iOS, Linux software engineer
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.