mailing list of musl libc
 help / color / Atom feed
* [musl] [PATCH] stdio: Fix fdopen bug
@ 2020-02-19  2:37 Zhang Tianci
  2020-02-19  3:44 ` Rich Felker
  0 siblings, 1 reply; 5+ messages in thread
From: Zhang Tianci @ 2020-02-19  2:37 UTC (permalink / raw)
  To: musl; +Cc: zhangtianci1, yunlong.song

Currently, in musl the fdopen doesn't check the consistence between
fd's mode and corresponding file's mode.

For example,

int fd = open("file1", O_RDONLY);
FILE *f = fdopen(fd, "W")

In musl, above code will be Okay.
While according to POSIX, above code (fdopen) will return EINVAL.

Signed-off-by: Zhang Tianci <zhangtianci1@huawei.com>
---
 src/stdio/__fdopen.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/src/stdio/__fdopen.c b/src/stdio/__fdopen.c
index 116e78e..23c4ffd 100644
--- a/src/stdio/__fdopen.c
+++ b/src/stdio/__fdopen.c
@@ -26,6 +26,16 @@ FILE *__fdopen(int fd, const char *mode)
 	/* Impose mode restrictions */
 	if (!strchr(mode, '+')) f->flags = (*mode == 'r') ? F_NOWR : F_NORD;
 
+	int fd_flag = __syscall(SYS_fcntl, fd, F_GETFL);
+
+	if (fd_flag == -1) return 0;
+
+	if (((fd_flag & O_ACCMODE) == O_RDONLY && !(f->flags & F_NORD)) ||
+	    ((fd_flag & O_ACCMODE) == O_WRONLY && !(f->flags & F_NOWR))) {
+		errno = EINVAL;
+		return 0;
+	}
+
 	/* Apply close-on-exec flag */
 	if (strchr(mode, 'e')) __syscall(SYS_fcntl, fd, F_SETFD, FD_CLOEXEC);
 
-- 
2.17.1


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

* Re: [musl] [PATCH] stdio: Fix fdopen bug
  2020-02-19  2:37 [musl] [PATCH] stdio: Fix fdopen bug Zhang Tianci
@ 2020-02-19  3:44 ` Rich Felker
  0 siblings, 0 replies; 5+ messages in thread
From: Rich Felker @ 2020-02-19  3:44 UTC (permalink / raw)
  To: Zhang Tianci; +Cc: musl, yunlong.song

On Wed, Feb 19, 2020 at 10:37:29AM +0800, Zhang Tianci wrote:
> Currently, in musl the fdopen doesn't check the consistence between
> fd's mode and corresponding file's mode.
> 
> For example,
> 
> int fd = open("file1", O_RDONLY);
> FILE *f = fdopen(fd, "W")
> 
> In musl, above code will be Okay.
> While according to POSIX, above code (fdopen) will return EINVAL.
> 
> Signed-off-by: Zhang Tianci <zhangtianci1@huawei.com>
> ---
>  src/stdio/__fdopen.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/src/stdio/__fdopen.c b/src/stdio/__fdopen.c
> index 116e78e..23c4ffd 100644
> --- a/src/stdio/__fdopen.c
> +++ b/src/stdio/__fdopen.c
> @@ -26,6 +26,16 @@ FILE *__fdopen(int fd, const char *mode)
>  	/* Impose mode restrictions */
>  	if (!strchr(mode, '+')) f->flags = (*mode == 'r') ? F_NOWR : F_NORD;
>  
> +	int fd_flag = __syscall(SYS_fcntl, fd, F_GETFL);
> +
> +	if (fd_flag == -1) return 0;
> +
> +	if (((fd_flag & O_ACCMODE) == O_RDONLY && !(f->flags & F_NORD)) ||
> +	    ((fd_flag & O_ACCMODE) == O_WRONLY && !(f->flags & F_NOWR))) {
> +		errno = EINVAL;
> +		return 0;
> +	}
> +
>  	/* Apply close-on-exec flag */
>  	if (strchr(mode, 'e')) __syscall(SYS_fcntl, fd, F_SETFD, FD_CLOEXEC);
>  
> -- 
> 2.17.1

Per POSIX this is a "may fail" not a "shall fail". Testing for this is
more costly (see added code/syscalls in the patch) and serves no
purpose, which is why it's not done.

Rich

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

* Re: [musl] [PATCH] stdio: Fix fdopen bug
  2020-02-19  6:47 zhangtianci
  2020-02-19 14:16 ` Rich Felker
@ 2020-02-19 14:20 ` Jens Gustedt
  1 sibling, 0 replies; 5+ messages in thread
From: Jens Gustedt @ 2020-02-19 14:20 UTC (permalink / raw)
  To: zhangtianci; +Cc: musl, Songyunlong (Euler)

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

zhangtianci,

>POSIX's require on fdopen:
>
>     The application shall ensure that the mode of the stream as
> expressed by the mode argument is allowed by the file access mode of
> the open file description to which fildes refers.
>
>So I think the example above should return EINVAL.

No, the "application" is the user code, so this is a requirement for
you, not musl.

Otherwise it would say "The *implementation* shall ensure …"

Jens

-- 
:: INRIA Nancy Grand Est ::: Camus ::::::: ICube/ICPS :::
:: ::::::::::::::: office Strasbourg : +33 368854536   ::
:: :::::::::::::::::::::: gsm France : +33 651400183   ::
:: ::::::::::::::: gsm international : +49 15737185122 ::
:: http://icube-icps.unistra.fr/index.php/Jens_Gustedt ::

[-- Attachment #2: Digitale Signatur von OpenPGP --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

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

* Re: [musl] [PATCH] stdio: Fix fdopen bug
  2020-02-19  6:47 zhangtianci
@ 2020-02-19 14:16 ` Rich Felker
  2020-02-19 14:20 ` Jens Gustedt
  1 sibling, 0 replies; 5+ messages in thread
From: Rich Felker @ 2020-02-19 14:16 UTC (permalink / raw)
  To: musl

On Wed, Feb 19, 2020 at 06:47:53AM +0000, zhangtianci wrote:
> > On Wed, Feb 19, 2020 at 10:37:29AM +0800, Zhang Tianci wrote:
> > > Currently, in musl the fdopen doesn't check the consistence between
> > > fd's mode and corresponding file's mode.
> > >
> > > For example,
> > >
> > > int fd = open("file1", O_RDONLY);
> > > FILE *f = fdopen(fd, "W")
> > >
> > > In musl, above code will be Okay.
> > > While according to POSIX, above code (fdopen) will return EINVAL.
> > >
> > > Signed-off-by: Zhang Tianci <zhangtianci1@huawei.com>
> > > ---
> > >  src/stdio/__fdopen.c | 10 ++++++++++
> > >  1 file changed, 10 insertions(+)
> > >
> > > diff --git a/src/stdio/__fdopen.c b/src/stdio/__fdopen.c index
> > > 116e78e..23c4ffd 100644
> > > --- a/src/stdio/__fdopen.c
> > > +++ b/src/stdio/__fdopen.c
> > > @@ -26,6 +26,16 @@ FILE *__fdopen(int fd, const char *mode)
> > >  	/* Impose mode restrictions */
> > >  	if (!strchr(mode, '+')) f->flags = (*mode == 'r') ? F_NOWR : F_NORD;
> > >
> > > +	int fd_flag = __syscall(SYS_fcntl, fd, F_GETFL);
> > > +
> > > +	if (fd_flag == -1) return 0;
> > > +
> > > +	if (((fd_flag & O_ACCMODE) == O_RDONLY && !(f->flags & F_NORD))
> > ||
> > > +	    ((fd_flag & O_ACCMODE) == O_WRONLY && !(f->flags &
> > F_NOWR))) {
> > > +		errno = EINVAL;
> > > +		return 0;
> > > +	}
> > > +
> > >  	/* Apply close-on-exec flag */
> > >  	if (strchr(mode, 'e')) __syscall(SYS_fcntl, fd, F_SETFD,
> > > FD_CLOEXEC);
> > >
> > > --
> > > 2.17.1
> > 
> > Per POSIX this is a "may fail" not a "shall fail". Testing for this is more costly
> > (see added code/syscalls in the patch) and serves no purpose, which is why
> > it's not done.
> > 
> > Rich
> 
> POSIX's require on fdopen:
> 
>      The application shall ensure that the mode of the stream as expressed by the
       ^^^^^^^^^^^^^^^
>      mode argument is allowed by the file access mode of the open file description 
>      to which fildes refers.
> 
> So I think the example above should return EINVAL.

The text you're quoting is placing a requirement on the application,
not the implementation. If the application fails to meet a "shall", it
has undefined behavior and there are no obligations whatsoever on the
implementation.

The error is clearly a "may fail" if you read the ERRORS section of
the specification.

Rich

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

* Re: [musl] [PATCH] stdio: Fix fdopen bug
@ 2020-02-19  6:47 zhangtianci
  2020-02-19 14:16 ` Rich Felker
  2020-02-19 14:20 ` Jens Gustedt
  0 siblings, 2 replies; 5+ messages in thread
From: zhangtianci @ 2020-02-19  6:47 UTC (permalink / raw)
  To: musl; +Cc: Songyunlong (Euler)

> On Wed, Feb 19, 2020 at 10:37:29AM +0800, Zhang Tianci wrote:
> > Currently, in musl the fdopen doesn't check the consistence between
> > fd's mode and corresponding file's mode.
> >
> > For example,
> >
> > int fd = open("file1", O_RDONLY);
> > FILE *f = fdopen(fd, "W")
> >
> > In musl, above code will be Okay.
> > While according to POSIX, above code (fdopen) will return EINVAL.
> >
> > Signed-off-by: Zhang Tianci <zhangtianci1@huawei.com>
> > ---
> >  src/stdio/__fdopen.c | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> >
> > diff --git a/src/stdio/__fdopen.c b/src/stdio/__fdopen.c index
> > 116e78e..23c4ffd 100644
> > --- a/src/stdio/__fdopen.c
> > +++ b/src/stdio/__fdopen.c
> > @@ -26,6 +26,16 @@ FILE *__fdopen(int fd, const char *mode)
> >  	/* Impose mode restrictions */
> >  	if (!strchr(mode, '+')) f->flags = (*mode == 'r') ? F_NOWR : F_NORD;
> >
> > +	int fd_flag = __syscall(SYS_fcntl, fd, F_GETFL);
> > +
> > +	if (fd_flag == -1) return 0;
> > +
> > +	if (((fd_flag & O_ACCMODE) == O_RDONLY && !(f->flags & F_NORD))
> ||
> > +	    ((fd_flag & O_ACCMODE) == O_WRONLY && !(f->flags &
> F_NOWR))) {
> > +		errno = EINVAL;
> > +		return 0;
> > +	}
> > +
> >  	/* Apply close-on-exec flag */
> >  	if (strchr(mode, 'e')) __syscall(SYS_fcntl, fd, F_SETFD,
> > FD_CLOEXEC);
> >
> > --
> > 2.17.1
> 
> Per POSIX this is a "may fail" not a "shall fail". Testing for this is more costly
> (see added code/syscalls in the patch) and serves no purpose, which is why
> it's not done.
> 
> Rich

POSIX's require on fdopen:

     The application shall ensure that the mode of the stream as expressed by the
     mode argument is allowed by the file access mode of the open file description 
     to which fildes refers.

So I think the example above should return EINVAL.

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

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-19  2:37 [musl] [PATCH] stdio: Fix fdopen bug Zhang Tianci
2020-02-19  3:44 ` Rich Felker
2020-02-19  6:47 zhangtianci
2020-02-19 14:16 ` Rich Felker
2020-02-19 14:20 ` Jens Gustedt

mailing list of musl libc

Archives are clonable: git clone --mirror http://inbox.vuxu.org/musl

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://inbox.vuxu.org/vuxu.archive.musl


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