* Another bug, in dup3
@ 2012-10-09 9:15 Richard W.M. Jones
2012-10-09 9:25 ` Richard W.M. Jones
0 siblings, 1 reply; 4+ messages in thread
From: Richard W.M. Jones @ 2012-10-09 9:15 UTC (permalink / raw)
To: Al Viro, linux-fsdevel, Jim Meyering, Linus Torvalds
Al:
A second problem we noticed from the gnulib test is that this
assertion fails with the latest upstream kernel:
/* Assigning to self is invalid. */
errno = 0;
ASSERT (dup3 (fd, fd, o_flags) == -1); <--- fails
ASSERT (errno == EINVAL);
dup3 is not currently specified by POSIX, so technically it is fine
for dup3 on Linux to do anything it likes.
*However* the proposal for dup3 in POSIX:
http://austingroupbugs.net/view.php?id=411
says:
"The dup3( ) function shall be equivalent to the dup2( ) function if
the flag argument is 0, except that it shall be an error if fildes
is equal to fildes2."
So it's probably a good idea for Linux not to regress here.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-p2v converts physical machines to virtual machines. Boot with a
live CD or over the network (PXE) and turn machines into Xen guests.
http://et.redhat.com/~rjones/virt-p2v
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Another bug, in dup3
2012-10-09 9:15 Another bug, in dup3 Richard W.M. Jones
@ 2012-10-09 9:25 ` Richard W.M. Jones
2012-10-09 14:27 ` [PATCH regression] dup3: Return an error when oldfd == newfd Richard W.M. Jones
0 siblings, 1 reply; 4+ messages in thread
From: Richard W.M. Jones @ 2012-10-09 9:25 UTC (permalink / raw)
To: Al Viro, linux-fsdevel, Jim Meyering, Linus Torvalds
[-- Attachment #1: Type: text/plain, Size: 304 bytes --]
Simple reproducer attached.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-p2v converts physical machines to virtual machines. Boot with a
live CD or over the network (PXE) and turn machines into Xen guests.
http://et.redhat.com/~rjones/virt-p2v
[-- Attachment #2: dup3.c --]
[-- Type: text/plain, Size: 423 bytes --]
/* Test if dup3 works right.
* by Richard W.M. Jones.
*/
#define _GNU_SOURCE
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <fcntl.h>
#include <assert.h>
int
main ()
{
char *file = "/tmp/dup3.tmp";
int fd = creat (file, 0600);
assert (fd >= 0);
if (dup3 (fd, fd, 0) != -1) {
fprintf (stderr, "dup3 (fd, fd, 0) didn't fail!\n");
exit (EXIT_FAILURE);
}
exit (EXIT_SUCCESS);
}
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH regression] dup3: Return an error when oldfd == newfd.
2012-10-09 9:25 ` Richard W.M. Jones
@ 2012-10-09 14:27 ` Richard W.M. Jones
2012-10-09 14:52 ` Al Viro
0 siblings, 1 reply; 4+ messages in thread
From: Richard W.M. Jones @ 2012-10-09 14:27 UTC (permalink / raw)
To: Al Viro, linux-fsdevel, Jim Meyering, Linus Torvalds
I have tested the attached patch to fix the dup3 regression.
Rich.
>From 0944e30e12dec6544b3602626b60ff412375c78f Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones@redhat.com>
Date: Tue, 9 Oct 2012 14:42:45 +0100
Subject: [PATCH] dup3: Return an error when oldfd == newfd.
The following commit:
commit fe17f22d7fd0e344ef6447238f799bb49f670c6f
Author: Al Viro <viro@zeniv.linux.org.uk>
Date: Tue Aug 21 11:48:11 2012 -0400
take purely descriptor-related stuff from fcntl.c to file.c
was supposed to be just code motion, but it dropped the following two
lines:
if (unlikely(oldfd == newfd))
return -EINVAL;
from the dup3 system call. dup3 is not specified by POSIX, so Linux
can do what it likes. However the POSIX proposal for dup3 [1] states
that it should return an error if oldfd == newfd.
[1] http://austingroupbugs.net/view.php?id=411
Signed-off-by: Richard W.M. Jones <rjones@redhat.com>
Tested-by: Richard W.M. Jones <rjones@redhat.com>
---
fs/file.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/fs/file.c b/fs/file.c
index 0f1bda4..d3b5fa8 100644
--- a/fs/file.c
+++ b/fs/file.c
@@ -922,6 +922,9 @@ SYSCALL_DEFINE3(dup3, unsigned int, oldfd, unsigned int, newfd, int, flags)
if ((flags & ~O_CLOEXEC) != 0)
return -EINVAL;
+ if (unlikely(oldfd == newfd))
+ return -EINVAL;
+
if (newfd >= rlimit(RLIMIT_NOFILE))
return -EMFILE;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH regression] dup3: Return an error when oldfd == newfd.
2012-10-09 14:27 ` [PATCH regression] dup3: Return an error when oldfd == newfd Richard W.M. Jones
@ 2012-10-09 14:52 ` Al Viro
0 siblings, 0 replies; 4+ messages in thread
From: Al Viro @ 2012-10-09 14:52 UTC (permalink / raw)
To: Richard W.M. Jones; +Cc: linux-fsdevel, Jim Meyering, Linus Torvalds
On Tue, Oct 09, 2012 at 03:27:43PM +0100, Richard W.M. Jones wrote:
> I have tested the attached patch to fix the dup3 regression.
>
> Rich.
>
> From 0944e30e12dec6544b3602626b60ff412375c78f Mon Sep 17 00:00:00 2001
> From: "Richard W.M. Jones" <rjones@redhat.com>
> Date: Tue, 9 Oct 2012 14:42:45 +0100
> Subject: [PATCH] dup3: Return an error when oldfd == newfd.
>
> The following commit:
>
> commit fe17f22d7fd0e344ef6447238f799bb49f670c6f
> Author: Al Viro <viro@zeniv.linux.org.uk>
> Date: Tue Aug 21 11:48:11 2012 -0400
>
> take purely descriptor-related stuff from fcntl.c to file.c
>
> was supposed to be just code motion, but it dropped the following two
> lines:
>
> if (unlikely(oldfd == newfd))
> return -EINVAL;
*grumble*
So it has; applied, will push to Linus. But now I really want to check
if I'd lost anything else in those code moves (and you are right, it
was supposed to be just that).
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2012-10-09 14:52 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-10-09 9:15 Another bug, in dup3 Richard W.M. Jones
2012-10-09 9:25 ` Richard W.M. Jones
2012-10-09 14:27 ` [PATCH regression] dup3: Return an error when oldfd == newfd Richard W.M. Jones
2012-10-09 14:52 ` Al Viro
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.