* 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
* Re: [musl] [PATCH] stdio: Fix fdopen bug
2020-02-19 6:47 [musl] [PATCH] stdio: Fix fdopen bug 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 [musl] [PATCH] stdio: Fix fdopen bug 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 2:37 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
* [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
end of thread, other threads:[~2020-02-19 14:21 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-19 6:47 [musl] [PATCH] stdio: Fix fdopen bug zhangtianci
2020-02-19 14:16 ` Rich Felker
2020-02-19 14:20 ` Jens Gustedt
-- strict thread matches above, loose matches on Subject: below --
2020-02-19 2:37 Zhang Tianci
2020-02-19 3:44 ` Rich Felker
Code repositories for project(s) associated with this public inbox
https://git.vuxu.org/mirror/musl/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).