Discussion:
[PATCHv9 4/5] file: trivial: replace get_unused_fd() by get_unused_fd_flags(0)
(too old to reply)
Yann Droneaud
2014-10-13 19:31:01 UTC
Permalink
Raw Message
This patch replaces calls to get_unused_fd() with equivalent call to
get_unused_fd_flags(0) to preserve current behavor for existing code.

In a further patch, get_unused_fd() will be removed so that new code
start using get_unused_fd_flags(), with the hope O_CLOEXEC could be
used, either by default or choosen by userspace.

Link: http://lkml.kernel.org/r/***@opteya.com
Cc: Al Viro <***@zeniv.linux.org.uk>
Cc: Andrew Morton <***@linux-foundation.org>
Cc: ***@kernel.org
Signed-off-by: Yann Droneaud <***@opteya.com>
---
fs/file.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/file.c b/fs/file.c
index ab3eb6a88239..ee738ea028fa 100644
--- a/fs/file.c
+++ b/fs/file.c
@@ -869,7 +869,7 @@ SYSCALL_DEFINE1(dup, unsigned int, fildes)
struct file *file = fget_raw(fildes);

if (file) {
- ret = get_unused_fd();
+ ret = get_unused_fd_flags(0);
if (ret >= 0)
fd_install(ret, file);
else
--
1.9.3

--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Yann Droneaud
2014-10-13 19:31:00 UTC
Permalink
Raw Message
This patch replaces calls to get_unused_fd() with equivalent call to
get_unused_fd_flags(0) to preserve current behavor for existing code.

In a further patch, get_unused_fd() will be removed so that new code
start using get_unused_fd_flags(), with the hope O_CLOEXEC could be
used, either by default or choosen by userspace.

Link: http://lkml.kernel.org/r/***@opteya.com
Cc: Al Viro <***@zeniv.linux.org.uk>
Cc: Andrew Morton <***@linux-foundation.org>
Cc: ***@kernel.org
Signed-off-by: Yann Droneaud <***@opteya.com>
---
fs/binfmt_misc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c
index fd8beb9657a2..1cc5377ba955 100644
--- a/fs/binfmt_misc.c
+++ b/fs/binfmt_misc.c
@@ -153,7 +153,7 @@ static int load_misc_binary(struct linux_binprm *bprm)
/* if the binary should be opened on behalf of the
* interpreter than keep it open and assign descriptor
* to it */
- fd_binary = get_unused_fd();
+ fd_binary = get_unused_fd_flags(0);
if (fd_binary < 0) {
retval = fd_binary;
goto _ret;
--
1.9.3

--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Yann Droneaud
2014-10-13 19:31:02 UTC
Permalink
Raw Message
Macro get_unused_fd() is used to allocate a file descriptor
with default flags. Those default flags (0) don't enable
close-on-exec.

This can be seen as an unsafe default: in most case close-on-exec
should be enabled to not leak file descriptor across exec().

It would be better to have a "safer" default set of flags, eg.
O_CLOEXEC must be used to enable close-on-exec.

Instead this patch removes get_unused_fd() so that out of tree
modules won't be affect by a runtime behavor change which might
introduce other kind of bugs: it's better to catch the change at
build time, making it easier to fix.

Removing the macro will also promote use of get_unused_fd_flags()
(or anon_inode_getfd()) with flags provided by userspace. Or, if
flags cannot be given by userspace, with flags set to O_CLOEXEC
by default.

Link: http://lkml.kernel.org/r/***@opteya.com
Cc: Al Viro <***@zeniv.linux.org.uk>
Cc: Andrew Morton <***@linux-foundation.org>
Signed-off-by: Yann Droneaud <***@opteya.com>
---
include/linux/file.h | 1 -
1 file changed, 1 deletion(-)

diff --git a/include/linux/file.h b/include/linux/file.h
index 4d69123377a2..f87d30882a24 100644
--- a/include/linux/file.h
+++ b/include/linux/file.h
@@ -66,7 +66,6 @@ extern void set_close_on_exec(unsigned int fd, int flag);
extern bool get_close_on_exec(unsigned int fd);
extern void put_filp(struct file *);
extern int get_unused_fd_flags(unsigned flags);
-#define get_unused_fd() get_unused_fd_flags(0)
extern void put_unused_fd(unsigned int fd);

extern void fd_install(unsigned int fd, struct file *file);
--
1.9.3

--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Yann Droneaud
2014-10-13 19:30:58 UTC
Permalink
Raw Message
This patch replaces calls to get_unused_fd() with equivalent call to
get_unused_fd_flags(0) to preserve current behavor for existing code.

In a further patch, get_unused_fd() will be removed so that new code
start using get_unused_fd_flags(), with the hope O_CLOEXEC could be
used, either by default or choosen by userspace.

Link: http://lkml.kernel.org/r/***@opteya.com
Cc: Al Viro <***@zeniv.linux.org.uk>
Cc: Andrew Morton <***@linux-foundation.org>
Cc: ***@kernel.org
Signed-off-by: Yann Droneaud <***@opteya.com>
---
arch/ia64/kernel/perfmon.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/ia64/kernel/perfmon.c b/arch/ia64/kernel/perfmon.c
index 5845ffea67c3..dc063fe6646a 100644
--- a/arch/ia64/kernel/perfmon.c
+++ b/arch/ia64/kernel/perfmon.c
@@ -2662,7 +2662,7 @@ pfm_context_create(pfm_context_t *ctx, void *arg, int count, struct pt_regs *reg

ret = -ENOMEM;

- fd = get_unused_fd();
+ fd = get_unused_fd_flags(0);
if (fd < 0)
return fd;
--
1.9.3

--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Yann Droneaud
2014-10-13 19:30:59 UTC
Permalink
Raw Message
This patch replaces calls to get_unused_fd() with equivalent call to
get_unused_fd_flags(0) to preserve current behavor for existing code.

In a further patch, get_unused_fd() will be removed so that new code
start using get_unused_fd_flags(), with the hope O_CLOEXEC could be
used, either by default or choosen by userspace.

Link: http://lkml.kernel.org/r/***@opteya.com
Cc: Al Viro <***@zeniv.linux.org.uk>
Cc: Andrew Morton <***@linux-foundation.org>
Cc: ***@kernel.org
Signed-off-by: Yann Droneaud <***@opteya.com>
---
arch/powerpc/platforms/cell/spufs/inode.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/cell/spufs/inode.c b/arch/powerpc/platforms/cell/spufs/inode.c
index 87ba7cf99cd7..51effcec30d8 100644
--- a/arch/powerpc/platforms/cell/spufs/inode.c
+++ b/arch/powerpc/platforms/cell/spufs/inode.c
@@ -301,7 +301,7 @@ static int spufs_context_open(struct path *path)
int ret;
struct file *filp;

- ret = get_unused_fd();
+ ret = get_unused_fd_flags(0);
if (ret < 0)
return ret;

@@ -518,7 +518,7 @@ static int spufs_gang_open(struct path *path)
int ret;
struct file *filp;

- ret = get_unused_fd();
+ ret = get_unused_fd_flags(0);
if (ret < 0)
return ret;
--
1.9.3

--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Michael Ellerman
2014-10-14 01:57:12 UTC
Permalink
Raw Message
Post by Yann Droneaud
This patch replaces calls to get_unused_fd() with equivalent call to
get_unused_fd_flags(0) to preserve current behavor for existing code.
In a further patch, get_unused_fd() will be removed so that new code
start using get_unused_fd_flags(), with the hope O_CLOEXEC could be
used, either by default or choosen by userspace.
This is fine by me, do you want an ack, or do you want us to take it via the
powerpc tree?

If the former:

Acked-by: Michael Ellerman <***@ellerman.id.au>


cheers


--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Yann Droneaud
2014-10-16 09:10:24 UTC
Permalink
Raw Message
Hi,

Le mardi 14 octobre 2014 =C3=A0 12:57 +1100, Michael Ellerman a =C3=A9c=
This patch replaces calls to get_unused_fd() with equivalent call t=
o
get_unused_fd_flags(0) to preserve current behavor for existing cod=
e.
=20
In a further patch, get_unused_fd() will be removed so that new cod=
e
start using get_unused_fd_flags(), with the hope O_CLOEXEC could be
used, either by default or choosen by userspace.
=20
a.com
=20
This is fine by me, do you want an ack, or do you want us to take it =
via the
powerpc tree?
=20
The patch was added in -mm by Andrew, so I guess the patch will be
merged sooner or later.

Anyway, you could investigate to check if O_CLOEXEC could be used
instead of 0 in call to get_unused_fd_flags().
=20
=20
Thanks a lot.

Regards.

--=20
Yann Droneaud
OPTEYA

Loading...