mailing list of musl libc
 help / color / mirror / Atom feed
* [musl] errno on writing to read-only files
@ 2021-10-13  1:21 (GalaxyMaster)
  2021-10-13  7:25 ` Dmitry V. Levin
  2021-10-13 14:01 ` Rich Felker
  0 siblings, 2 replies; 5+ messages in thread
From: (GalaxyMaster) @ 2021-10-13  1:21 UTC (permalink / raw)
  To: musl

Hello,

I am observing the following on musl and I am not sure that this is the way it
should be:
===
galaxy@archlinux:~/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@archlinux:~/musl-tests $ gcc -o fput-to-readonly fput-to-readonly.c 
galaxy@archlinux:~/musl-tests $ ./fput-to-readonly 
i = -1 (should be negative [EOF = -1])
errno = 0
galaxy@archlinux:~/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@archlinux musl-tests]$ ./fput-to-readonly 
i = -1 (should be negative [EOF = -1])
errno = 9
[galaxy@archlinux 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) */
===

-- 
(GM)

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [musl] errno on writing to read-only files
  2021-10-13  1:21 [musl] errno on writing to read-only files (GalaxyMaster)
@ 2021-10-13  7:25 ` Dmitry V. Levin
  2021-10-13  8:06   ` (GalaxyMaster)
  2021-10-13 14:01 ` Rich Felker
  1 sibling, 1 reply; 5+ messages in thread
From: Dmitry V. Levin @ 2021-10-13  7:25 UTC (permalink / raw)
  To: musl

Hi,

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@archlinux:~/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);

Note that the first "printf" invocation is allowed to clobber errno
and this is not musl-specific.


-- 
ldv

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [musl] errno on writing to read-only files
  2021-10-13  7:25 ` Dmitry V. Levin
@ 2021-10-13  8:06   ` (GalaxyMaster)
  0 siblings, 0 replies; 5+ messages in thread
From: (GalaxyMaster) @ 2021-10-13  8:06 UTC (permalink / raw)
  To: musl

Dmitry,

On Wed, Oct 13, 2021 at 10:25:12AM +0300, Dmitry V. Levin wrote:
> > 	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);
> 
> Note that the first "printf" invocation is allowed to clobber errno
> and this is not musl-specific.

Thanks, noted.  I will ensure that in my future test cases I will put
printing errno right after the function I am checking.

-- 
(GM)

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [musl] errno on writing to read-only files
  2021-10-13  1:21 [musl] errno on writing to read-only files (GalaxyMaster)
  2021-10-13  7:25 ` Dmitry V. Levin
@ 2021-10-13 14:01 ` Rich Felker
  2021-10-16  6:30   ` A. Wilcox
  1 sibling, 1 reply; 5+ messages in thread
From: Rich Felker @ 2021-10-13 14:01 UTC (permalink / raw)
  To: (GalaxyMaster); +Cc: musl

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@archlinux:~/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@archlinux:~/musl-tests $ gcc -o fput-to-readonly fput-to-readonly.c 
> galaxy@archlinux:~/musl-tests $ ./fput-to-readonly 
> i = -1 (should be negative [EOF = -1])
> errno = 0
> galaxy@archlinux:~/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@archlinux musl-tests]$ ./fput-to-readonly 
> i = -1 (should be negative [EOF = -1])
> errno = 9
> [galaxy@archlinux 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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [musl] errno on writing to read-only files
  2021-10-13 14:01 ` Rich Felker
@ 2021-10-16  6:30   ` A. Wilcox
  0 siblings, 0 replies; 5+ messages in thread
From: A. Wilcox @ 2021-10-16  6:30 UTC (permalink / raw)
  To: musl; +Cc: (GalaxyMaster)

[-- Attachment #1: Type: text/plain, Size: 3212 bytes --]

On Oct 13, 2021, at 9:02 AM, Rich Felker <dalias@libc.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@archlinux:~/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@archlinux:~/musl-tests $ gcc -o fput-to-readonly fput-to-readonly.c 
>> galaxy@archlinux:~/musl-tests $ ./fput-to-readonly 
>> i = -1 (should be negative [EOF = -1])
>> errno = 0
>> galaxy@archlinux:~/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@archlinux musl-tests]$ ./fput-to-readonly 
>> i = -1 (should be negative [EOF = -1])
>> errno = 9
>> [galaxy@archlinux 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

[-- Attachment #2: Type: text/html, Size: 6755 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-10-16  6:30 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-13  1:21 [musl] errno on writing to read-only files (GalaxyMaster)
2021-10-13  7:25 ` Dmitry V. Levin
2021-10-13  8:06   ` (GalaxyMaster)
2021-10-13 14:01 ` Rich Felker
2021-10-16  6:30   ` A. Wilcox

mailing list of musl libc

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.vuxu.org/musl

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V1 musl musl/ https://inbox.vuxu.org/musl \
		musl@inbox.vuxu.org
	public-inbox-index musl

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.vuxu.org/vuxu.archive.musl


code repositories for the project(s) associated with this inbox:

	https://git.vuxu.org/mirror/musl/

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git