* [PATCH 01/13] ia64: use get_unused_fd_flags(0) instead of get_unused_fd()
2013-07-02 16:39 [PATCH 00/13] Getting rid of get_unused_fd() Yann Droneaud
@ 2013-07-02 16:39 ` Yann Droneaud
2013-07-02 16:39 ` [PATCH 02/13] ppc/cell: " Yann Droneaud
` (11 subsequent siblings)
12 siblings, 0 replies; 27+ messages in thread
From: Yann Droneaud @ 2013-07-02 16:39 UTC (permalink / raw)
To: linux-kernel, linux-ia64; +Cc: Yann Droneaud
Macro get_unused_fd() is used to allocate a file descriptor with
default flags. Those default flags (0) can be "unsafe":
O_CLOEXEC must be used by default to not leak file descriptor
across exec().
Instead of macro get_unused_fd(), functions anon_inode_getfd()
or get_unused_fd_flags() should be used with flags given by userspace.
If not possible, flags should be set to O_CLOEXEC to provide userspace
with a default safe behavor.
In a further patch, get_unused_fd() will be removed so that
new code start using anon_inode_getfd() or get_unused_fd_flags()
with correct flags.
This patch replaces calls to get_unused_fd() with equivalent call to
get_unused_fd_flags(0) to preserve current behavor for existing code.
The hard coded flag value (0) should be reviewed on a per-subsystem basis,
and, if possible, set to O_CLOEXEC.
Signed-off-by: Yann Droneaud <ydroneaud@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 5a9ff1c..64757c1 100644
--- a/arch/ia64/kernel/perfmon.c
+++ b/arch/ia64/kernel/perfmon.c
@@ -2668,7 +2668,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.8.3.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH 02/13] ppc/cell: use get_unused_fd_flags(0) instead of get_unused_fd()
2013-07-02 16:39 [PATCH 00/13] Getting rid of get_unused_fd() Yann Droneaud
2013-07-02 16:39 ` [PATCH 01/13] ia64: use get_unused_fd_flags(0) instead " Yann Droneaud
@ 2013-07-02 16:39 ` Yann Droneaud
2014-01-12 23:06 ` Benjamin Herrenschmidt
2013-07-02 16:39 ` [PATCH 03/13] infiniband: " Yann Droneaud
` (10 subsequent siblings)
12 siblings, 1 reply; 27+ messages in thread
From: Yann Droneaud @ 2013-07-02 16:39 UTC (permalink / raw)
To: linux-kernel, linuxppc-dev, cbe-oss-dev; +Cc: Yann Droneaud
Macro get_unused_fd() is used to allocate a file descriptor with
default flags. Those default flags (0) can be "unsafe":
O_CLOEXEC must be used by default to not leak file descriptor
across exec().
Instead of macro get_unused_fd(), functions anon_inode_getfd()
or get_unused_fd_flags() should be used with flags given by userspace.
If not possible, flags should be set to O_CLOEXEC to provide userspace
with a default safe behavor.
In a further patch, get_unused_fd() will be removed so that
new code start using anon_inode_getfd() or get_unused_fd_flags()
with correct flags.
This patch replaces calls to get_unused_fd() with equivalent call to
get_unused_fd_flags(0) to preserve current behavor for existing code.
The hard coded flag value (0) should be reviewed on a per-subsystem basis,
and, if possible, set to O_CLOEXEC.
Signed-off-by: Yann Droneaud <ydroneaud@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 f390042..88df441 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.8.3.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH 02/13] ppc/cell: use get_unused_fd_flags(0) instead of get_unused_fd()
2013-07-02 16:39 ` [PATCH 02/13] ppc/cell: " Yann Droneaud
@ 2014-01-12 23:06 ` Benjamin Herrenschmidt
2014-01-13 9:30 ` Yann Droneaud
0 siblings, 1 reply; 27+ messages in thread
From: Benjamin Herrenschmidt @ 2014-01-12 23:06 UTC (permalink / raw)
To: Yann Droneaud; +Cc: linux-kernel, linuxppc-dev, cbe-oss-dev
On Tue, 2013-07-02 at 18:39 +0200, Yann Droneaud wrote:
> Macro get_unused_fd() is used to allocate a file descriptor with
> default flags. Those default flags (0) can be "unsafe":
> O_CLOEXEC must be used by default to not leak file descriptor
> across exec().
>
> Instead of macro get_unused_fd(), functions anon_inode_getfd()
> or get_unused_fd_flags() should be used with flags given by userspace.
> If not possible, flags should be set to O_CLOEXEC to provide userspace
> with a default safe behavor.
>
> In a further patch, get_unused_fd() will be removed so that
> new code start using anon_inode_getfd() or get_unused_fd_flags()
> with correct flags.
>
> This patch replaces calls to get_unused_fd() with equivalent call to
> get_unused_fd_flags(0) to preserve current behavor for existing code.
>
> The hard coded flag value (0) should be reviewed on a per-subsystem basis,
> and, if possible, set to O_CLOEXEC.
>
> Signed-off-by: Yann Droneaud <ydroneaud@opteya.com>
Should I merge this (v5 on patchwork) or let Al do it ?
Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
> 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 f390042..88df441 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;
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 02/13] ppc/cell: use get_unused_fd_flags(0) instead of get_unused_fd()
2014-01-12 23:06 ` Benjamin Herrenschmidt
@ 2014-01-13 9:30 ` Yann Droneaud
2014-01-20 17:01 ` Yann Droneaud
0 siblings, 1 reply; 27+ messages in thread
From: Yann Droneaud @ 2014-01-13 9:30 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linux-kernel, linuxppc-dev, cbe-oss-dev
Hi Benjamin,
Le lundi 13 janvier 2014 à 10:06 +1100, Benjamin Herrenschmidt a écrit :
> On Tue, 2013-07-02 at 18:39 +0200, Yann Droneaud wrote:
> > Macro get_unused_fd() is used to allocate a file descriptor with
> > default flags. Those default flags (0) can be "unsafe":
> > O_CLOEXEC must be used by default to not leak file descriptor
> > across exec().
> >
> > Instead of macro get_unused_fd(), functions anon_inode_getfd()
> > or get_unused_fd_flags() should be used with flags given by userspace.
> > If not possible, flags should be set to O_CLOEXEC to provide userspace
> > with a default safe behavor.
> >
> > In a further patch, get_unused_fd() will be removed so that
> > new code start using anon_inode_getfd() or get_unused_fd_flags()
> > with correct flags.
> >
> > This patch replaces calls to get_unused_fd() with equivalent call to
> > get_unused_fd_flags(0) to preserve current behavor for existing code.
> >
> > The hard coded flag value (0) should be reviewed on a per-subsystem basis,
> > and, if possible, set to O_CLOEXEC.
> >
> > Signed-off-by: Yann Droneaud <ydroneaud@opteya.com>
>
> Should I merge this (v5 on patchwork) or let Al do it ?
>
Please merge it directly: patches from the previous patchsets were
picked individually by each subsystem maintainer after proper review
regarding setting close on exec flag by default.
> Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>
Thanks a lot.
> > ---
> > 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 f390042..88df441 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;
> >
>
>
Note:
latest patch (from v5 patchset) is at
http://lkml.kernel.org/r/fe27abcfab5563d36a3e5e58ff36e5500c39be6a.1388952061.git.ydroneaud@opteya.com
v5 patchset is at
http://lkml.kernel.org/r/cover.1388952061.git.ydroneaud@opteya.com
Regards.
--
Yann Droneaud
OPTEYA
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 02/13] ppc/cell: use get_unused_fd_flags(0) instead of get_unused_fd()
2014-01-13 9:30 ` Yann Droneaud
@ 2014-01-20 17:01 ` Yann Droneaud
0 siblings, 0 replies; 27+ messages in thread
From: Yann Droneaud @ 2014-01-20 17:01 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: linux-kernel, linuxppc-dev, cbe-oss-dev, Yann Droneaud
Hi,
Le lundi 13 janvier 2014 à 10:30 +0100, Yann Droneaud a écrit :
> Le lundi 13 janvier 2014 à 10:06 +1100, Benjamin Herrenschmidt a écrit :
> > On Tue, 2013-07-02 at 18:39 +0200, Yann Droneaud wrote:
> > > Macro get_unused_fd() is used to allocate a file descriptor with
> > > default flags. Those default flags (0) can be "unsafe":
> > > O_CLOEXEC must be used by default to not leak file descriptor
> > > across exec().
> > >
> > > Instead of macro get_unused_fd(), functions anon_inode_getfd()
> > > or get_unused_fd_flags() should be used with flags given by userspace.
> > > If not possible, flags should be set to O_CLOEXEC to provide userspace
> > > with a default safe behavor.
> > >
> > > In a further patch, get_unused_fd() will be removed so that
> > > new code start using anon_inode_getfd() or get_unused_fd_flags()
> > > with correct flags.
> > >
> > > This patch replaces calls to get_unused_fd() with equivalent call to
> > > get_unused_fd_flags(0) to preserve current behavor for existing code.
> > >
> > > The hard coded flag value (0) should be reviewed on a per-subsystem basis,
> > > and, if possible, set to O_CLOEXEC.
> > >
> > > Signed-off-by: Yann Droneaud <ydroneaud@opteya.com>
> >
> > Should I merge this (v5 on patchwork) or let Al do it ?
> >
>
> Please merge it directly: patches from the previous patchsets were
> picked individually by each subsystem maintainer after proper review
> regarding setting close on exec flag by default.
>
> > Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> >
>
> Thanks a lot.
> Note:
> latest patch (from v5 patchset) is at
> http://lkml.kernel.org/r/fe27abcfab5563d36a3e5e58ff36e5500c39be6a.1388952061.git.ydroneaud@opteya.com
> v5 patchset is at
> http://lkml.kernel.org/r/cover.1388952061.git.ydroneaud@opteya.com
>
I have not yet seen the patch in your trees at
https://git.kernel.org/cgit/linux/kernel/git/benh/powerpc.git/
I hope you would pick the patch, if possible in its latest version
(unfortunately I'm not able to give the link to the astest patch in
patchwork).
Regards.
--
Yann Droneaud
OPTEYA
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 03/13] infiniband: use get_unused_fd_flags(0) instead of get_unused_fd()
2013-07-02 16:39 [PATCH 00/13] Getting rid of get_unused_fd() Yann Droneaud
2013-07-02 16:39 ` [PATCH 01/13] ia64: use get_unused_fd_flags(0) instead " Yann Droneaud
2013-07-02 16:39 ` [PATCH 02/13] ppc/cell: " Yann Droneaud
@ 2013-07-02 16:39 ` Yann Droneaud
2013-07-08 18:23 ` Roland Dreier
2013-07-02 16:39 ` [PATCH 04/13] android/sw_sync: " Yann Droneaud
` (9 subsequent siblings)
12 siblings, 1 reply; 27+ messages in thread
From: Yann Droneaud @ 2013-07-02 16:39 UTC (permalink / raw)
To: linux-kernel, linux-rdma; +Cc: Yann Droneaud
Macro get_unused_fd() is used to allocate a file descriptor with
default flags. Those default flags (0) can be "unsafe":
O_CLOEXEC must be used by default to not leak file descriptor
across exec().
Instead of macro get_unused_fd(), functions anon_inode_getfd()
or get_unused_fd_flags() should be used with flags given by userspace.
If not possible, flags should be set to O_CLOEXEC to provide userspace
with a default safe behavor.
In a further patch, get_unused_fd() will be removed so that
new code start using anon_inode_getfd() or get_unused_fd_flags()
with correct flags.
This patch replaces calls to get_unused_fd() with equivalent call to
get_unused_fd_flags(0) to preserve current behavor for existing code.
The hard coded flag value (0) should be reviewed on a per-subsystem basis,
and, if possible, set to O_CLOEXEC.
Signed-off-by: Yann Droneaud <ydroneaud@opteya.com>
---
drivers/infiniband/core/uverbs_cmd.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
index a7d00f6..9c893bc 100644
--- a/drivers/infiniband/core/uverbs_cmd.c
+++ b/drivers/infiniband/core/uverbs_cmd.c
@@ -334,7 +334,7 @@ ssize_t ib_uverbs_get_context(struct ib_uverbs_file *file,
resp.num_comp_vectors = file->device->num_comp_vectors;
- ret = get_unused_fd();
+ ret = get_unused_fd_flags(0);
if (ret < 0)
goto err_free;
resp.async_fd = ret;
@@ -1184,7 +1184,7 @@ ssize_t ib_uverbs_create_comp_channel(struct ib_uverbs_file *file,
if (copy_from_user(&cmd, buf, sizeof cmd))
return -EFAULT;
- ret = get_unused_fd();
+ ret = get_unused_fd_flags(0);
if (ret < 0)
return ret;
resp.fd = ret;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH 03/13] infiniband: use get_unused_fd_flags(0) instead of get_unused_fd()
2013-07-02 16:39 ` [PATCH 03/13] infiniband: " Yann Droneaud
@ 2013-07-08 18:23 ` Roland Dreier
2013-07-08 21:26 ` Yann Droneaud
0 siblings, 1 reply; 27+ messages in thread
From: Roland Dreier @ 2013-07-08 18:23 UTC (permalink / raw)
To: Yann Droneaud; +Cc: LKML, linux-rdma@vger.kernel.org
Thanks, I just applied a patch to convert to
get_unused_fd_flags(O_CLOEXEC) in uverbs, since there isn't anything
useful that can be done with uverbs fds across an exec.
- R.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 03/13] infiniband: use get_unused_fd_flags(0) instead of get_unused_fd()
2013-07-08 18:23 ` Roland Dreier
@ 2013-07-08 21:26 ` Yann Droneaud
0 siblings, 0 replies; 27+ messages in thread
From: Yann Droneaud @ 2013-07-08 21:26 UTC (permalink / raw)
To: Roland Dreier; +Cc: LKML, linux-rdma, ydroneaud
Le 08.07.2013 20:23, Roland Dreier a écrit :
> Thanks, I just applied a patch to convert to
> get_unused_fd_flags(O_CLOEXEC) in uverbs, since there isn't anything
> useful that can be done with uverbs fds across an exec.
>
Thanks.
In fact, InfiniBand was my main target and I kept this change (setting
O_CLOEXEC)
for another batch.
It's following my patch on libibverbs "[PATCH] open files with close on
exec flag"
http://thread.gmane.org/gmane.linux.drivers.rdma/15727
http://marc.info/?l=linux-rdma&m=136908991102575&w=2
This patch was already quoting Dan Walsh, "Excuse me son, but your code
is leaking !!!"
http://danwalsh.livejournal.com/53603.html but I couldn't resist to post
it again.
BTW, I was working on the rationnal/commit message for setting flags to
O_CLOEXEC
on kernel side, please find the draft if revelant.
--------------------8<----------------------
InfiniBand verbs/RDMA: use O_CLOEXEC
This subsystem is allocating new file descriptor through the InfiniBand
verbs / RDMA API.
Thoses file descriptors are created after a write() from userspace to a
special device file.
No read operation is needed to get the file descriptor: it is returned
to userspace
in a buffer whose address was stored as part of the buffer passed to
write().
If the write() succeed, the response buffer is updated and the new file
descriptor is available.
But such file descriptors are mostly hidden to application developpers
by libibverbs / librdma_cm libraries API.
In fact, application developpers could use InfiniBand verbs / RDMA
without
using directly the file descriptors.
Here's how are created the two file descriptors (using mlx4 as example):
- ibv_context.async_fd:
---- kernel ----
ib_uverbs_get_context() : linux/drivers/infiniband/core/uverbs_cmd.c
uverbs_cmd_table[IB_USER_VERBS_CMD_GET_CONTEXT]() :
linux/drivers/infiniband/core/uverbs_main.c
ib_uverbs_write() : linux/drivers/infiniband/core/uverbs_main.c
uverbs_mmap_fops.write : linux/drivers/infiniband/core/uverbs_main.c
---- userspace ----
ibv_cmd_get_context() : libibverbs/src/cmd.c
mlx4_alloc_context() : libmlx4/src/mlx4.c
mlx4_dev_ops.alloc_context : libmlx4/src/mlx4.c
__ibv_open_device() : libibverbs/src/device.c
- ibv_comp_channel.fd:
---- kernel ----
ib_uverbs_create_comp_channel() :
linux/drivers/infiniband/core/uverbs_cmd.c
uverbs_cmd_table[IB_USER_VERBS_CMD_CREATE_COMP_CHANNEL]() :
linux/drivers/infiniband/core/uverbs_main.c
ib_uverbs_write() : linux/drivers/infiniband/core/uverbs_main.c
uverbs_mmap_fops.write : linux/drivers/infiniband/core/uverbs_main.c
---- userspace ----
ibv_create_comp_channel() : libibverbs/src/verbs.c
But those file descriptors are of no use for another program executed
through exec():
- without the memory mappings for special memory pages,
the file descriptor are of no use ...
- the userland libraries/drivers are not ready to
found the devices already opened.
[ In fact, supporting fork() is already a challenge for thoses API. ]
So those file descriptors can safely be opened with O_CLOEXEC without
disturbing users of InfiniBand verbs /RDMA
--------------------8<----------------------
Regards.
--
Yann Droneaud
OPTEYA
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 04/13] android/sw_sync: use get_unused_fd_flags(0) instead of get_unused_fd()
2013-07-02 16:39 [PATCH 00/13] Getting rid of get_unused_fd() Yann Droneaud
` (2 preceding siblings ...)
2013-07-02 16:39 ` [PATCH 03/13] infiniband: " Yann Droneaud
@ 2013-07-02 16:39 ` Yann Droneaud
2013-07-02 22:22 ` Erik Gilling
2013-07-02 16:39 ` [PATCH 05/13] android/sync: " Yann Droneaud
` (8 subsequent siblings)
12 siblings, 1 reply; 27+ messages in thread
From: Yann Droneaud @ 2013-07-02 16:39 UTC (permalink / raw)
To: linux-kernel, devel; +Cc: Yann Droneaud
Macro get_unused_fd() is used to allocate a file descriptor with
default flags. Those default flags (0) can be "unsafe":
O_CLOEXEC must be used by default to not leak file descriptor
across exec().
Instead of macro get_unused_fd(), functions anon_inode_getfd()
or get_unused_fd_flags() should be used with flags given by userspace.
If not possible, flags should be set to O_CLOEXEC to provide userspace
with a default safe behavor.
In a further patch, get_unused_fd() will be removed so that
new code start using anon_inode_getfd() or get_unused_fd_flags()
with correct flags.
This patch replaces calls to get_unused_fd() with equivalent call to
get_unused_fd_flags(0) to preserve current behavor for existing code.
The hard coded flag value (0) should be reviewed on a per-subsystem basis,
and, if possible, set to O_CLOEXEC.
Signed-off-by: Yann Droneaud <ydroneaud@opteya.com>
---
drivers/staging/android/sw_sync.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/android/sw_sync.c b/drivers/staging/android/sw_sync.c
index 765c757..a4ed0b9 100644
--- a/drivers/staging/android/sw_sync.c
+++ b/drivers/staging/android/sw_sync.c
@@ -163,7 +163,7 @@ static int sw_sync_release(struct inode *inode, struct file *file)
static long sw_sync_ioctl_create_fence(struct sw_sync_timeline *obj,
unsigned long arg)
{
- int fd = get_unused_fd();
+ int fd = get_unused_fd_flags(0);
int err;
struct sync_pt *pt;
struct sync_fence *fence;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH 05/13] android/sync: use get_unused_fd_flags(0) instead of get_unused_fd()
2013-07-02 16:39 [PATCH 00/13] Getting rid of get_unused_fd() Yann Droneaud
` (3 preceding siblings ...)
2013-07-02 16:39 ` [PATCH 04/13] android/sw_sync: " Yann Droneaud
@ 2013-07-02 16:39 ` Yann Droneaud
2013-07-02 22:22 ` Erik Gilling
2013-07-02 16:39 ` [PATCH 06/13] vfio: " Yann Droneaud
` (7 subsequent siblings)
12 siblings, 1 reply; 27+ messages in thread
From: Yann Droneaud @ 2013-07-02 16:39 UTC (permalink / raw)
To: linux-kernel, devel; +Cc: Yann Droneaud
Macro get_unused_fd() is used to allocate a file descriptor with
default flags. Those default flags (0) can be "unsafe":
O_CLOEXEC must be used by default to not leak file descriptor
across exec().
Instead of macro get_unused_fd(), functions anon_inode_getfd()
or get_unused_fd_flags() should be used with flags given by userspace.
If not possible, flags should be set to O_CLOEXEC to provide userspace
with a default safe behavor.
In a further patch, get_unused_fd() will be removed so that
new code start using anon_inode_getfd() or get_unused_fd_flags()
with correct flags.
This patch replaces calls to get_unused_fd() with equivalent call to
get_unused_fd_flags(0) to preserve current behavor for existing code.
The hard coded flag value (0) should be reviewed on a per-subsystem basis,
and, if possible, set to O_CLOEXEC.
Signed-off-by: Yann Droneaud <ydroneaud@opteya.com>
---
drivers/staging/android/sync.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c
index 2996077..c40c743 100644
--- a/drivers/staging/android/sync.c
+++ b/drivers/staging/android/sync.c
@@ -697,7 +697,7 @@ static long sync_fence_ioctl_wait(struct sync_fence *fence, unsigned long arg)
static long sync_fence_ioctl_merge(struct sync_fence *fence, unsigned long arg)
{
- int fd = get_unused_fd();
+ int fd = get_unused_fd_flags(0);
int err;
struct sync_fence *fence2, *fence3;
struct sync_merge_data data;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH 06/13] vfio: use get_unused_fd_flags(0) instead of get_unused_fd()
2013-07-02 16:39 [PATCH 00/13] Getting rid of get_unused_fd() Yann Droneaud
` (4 preceding siblings ...)
2013-07-02 16:39 ` [PATCH 05/13] android/sync: " Yann Droneaud
@ 2013-07-02 16:39 ` Yann Droneaud
2013-07-02 16:39 ` [PATCH 07/13] binfmt_misc: " Yann Droneaud
` (6 subsequent siblings)
12 siblings, 0 replies; 27+ messages in thread
From: Yann Droneaud @ 2013-07-02 16:39 UTC (permalink / raw)
To: linux-kernel, kvm; +Cc: Yann Droneaud
Macro get_unused_fd() is used to allocate a file descriptor with
default flags. Those default flags (0) can be "unsafe":
O_CLOEXEC must be used by default to not leak file descriptor
across exec().
Instead of macro get_unused_fd(), functions anon_inode_getfd()
or get_unused_fd_flags() should be used with flags given by userspace.
If not possible, flags should be set to O_CLOEXEC to provide userspace
with a default safe behavor.
In a further patch, get_unused_fd() will be removed so that
new code start using anon_inode_getfd() or get_unused_fd_flags()
with correct flags.
This patch replaces calls to get_unused_fd() with equivalent call to
get_unused_fd_flags(0) to preserve current behavor for existing code.
The hard coded flag value (0) should be reviewed on a per-subsystem basis,
and, if possible, set to O_CLOEXEC.
Signed-off-by: Yann Droneaud <ydroneaud@opteya.com>
---
drivers/vfio/vfio.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
index c488da5..bb4e9fd 100644
--- a/drivers/vfio/vfio.c
+++ b/drivers/vfio/vfio.c
@@ -1126,7 +1126,7 @@ static int vfio_group_get_device_fd(struct vfio_group *group, char *buf)
* We can't use anon_inode_getfd() because we need to modify
* the f_mode flags directly to allow more than just ioctls
*/
- ret = get_unused_fd();
+ ret = get_unused_fd_flags(0);
if (ret < 0) {
device->ops->release(device->device_data);
break;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH 07/13] binfmt_misc: use get_unused_fd_flags(0) instead of get_unused_fd()
2013-07-02 16:39 [PATCH 00/13] Getting rid of get_unused_fd() Yann Droneaud
` (5 preceding siblings ...)
2013-07-02 16:39 ` [PATCH 06/13] vfio: " Yann Droneaud
@ 2013-07-02 16:39 ` Yann Droneaud
2013-07-02 16:39 ` [PATCH 08/13] file: " Yann Droneaud
` (5 subsequent siblings)
12 siblings, 0 replies; 27+ messages in thread
From: Yann Droneaud @ 2013-07-02 16:39 UTC (permalink / raw)
To: linux-kernel, linux-fsdevel; +Cc: Yann Droneaud
Macro get_unused_fd() is used to allocate a file descriptor with
default flags. Those default flags (0) can be "unsafe":
O_CLOEXEC must be used by default to not leak file descriptor
across exec().
Instead of macro get_unused_fd(), functions anon_inode_getfd()
or get_unused_fd_flags() should be used with flags given by userspace.
If not possible, flags should be set to O_CLOEXEC to provide userspace
with a default safe behavor.
In a further patch, get_unused_fd() will be removed so that
new code start using anon_inode_getfd() or get_unused_fd_flags()
with correct flags.
This patch replaces calls to get_unused_fd() with equivalent call to
get_unused_fd_flags(0) to preserve current behavor for existing code.
The hard coded flag value (0) should be reviewed on a per-subsystem basis,
and, if possible, set to O_CLOEXEC.
Signed-off-by: Yann Droneaud <ydroneaud@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 1c740e1..052f6dc 100644
--- a/fs/binfmt_misc.c
+++ b/fs/binfmt_misc.c
@@ -138,7 +138,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.8.3.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH 08/13] file: use get_unused_fd_flags(0) instead of get_unused_fd()
2013-07-02 16:39 [PATCH 00/13] Getting rid of get_unused_fd() Yann Droneaud
` (6 preceding siblings ...)
2013-07-02 16:39 ` [PATCH 07/13] binfmt_misc: " Yann Droneaud
@ 2013-07-02 16:39 ` Yann Droneaud
2013-07-02 16:39 ` [PATCH 09/13] fanotify: " Yann Droneaud
` (4 subsequent siblings)
12 siblings, 0 replies; 27+ messages in thread
From: Yann Droneaud @ 2013-07-02 16:39 UTC (permalink / raw)
To: linux-kernel, linux-fsdevel; +Cc: Yann Droneaud
Macro get_unused_fd() is used to allocate a file descriptor with
default flags. Those default flags (0) can be "unsafe":
O_CLOEXEC must be used by default to not leak file descriptor
across exec().
Instead of macro get_unused_fd(), functions anon_inode_getfd()
or get_unused_fd_flags() should be used with flags given by userspace.
If not possible, flags should be set to O_CLOEXEC to provide userspace
with a default safe behavor.
In a further patch, get_unused_fd() will be removed so that
new code start using anon_inode_getfd() or get_unused_fd_flags()
with correct flags.
This patch replaces calls to get_unused_fd() with equivalent call to
get_unused_fd_flags(0) to preserve current behavor for existing code.
The hard coded flag value (0) should be reviewed on a per-subsystem basis,
and, if possible, set to O_CLOEXEC.
Signed-off-by: Yann Droneaud <ydroneaud@opteya.com>
---
fs/file.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/file.c b/fs/file.c
index 4a78f98..1420d28 100644
--- a/fs/file.c
+++ b/fs/file.c
@@ -897,7 +897,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.8.3.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH 09/13] fanotify: use get_unused_fd_flags(0) instead of get_unused_fd()
2013-07-02 16:39 [PATCH 00/13] Getting rid of get_unused_fd() Yann Droneaud
` (7 preceding siblings ...)
2013-07-02 16:39 ` [PATCH 08/13] file: " Yann Droneaud
@ 2013-07-02 16:39 ` Yann Droneaud
2013-07-02 16:39 ` [PATCH 10/13] xfs: " Yann Droneaud
` (3 subsequent siblings)
12 siblings, 0 replies; 27+ messages in thread
From: Yann Droneaud @ 2013-07-02 16:39 UTC (permalink / raw)
To: linux-kernel, Eric Paris; +Cc: Yann Droneaud
Macro get_unused_fd() is used to allocate a file descriptor with
default flags. Those default flags (0) can be "unsafe":
O_CLOEXEC must be used by default to not leak file descriptor
across exec().
Instead of macro get_unused_fd(), functions anon_inode_getfd()
or get_unused_fd_flags() should be used with flags given by userspace.
If not possible, flags should be set to O_CLOEXEC to provide userspace
with a default safe behavor.
In a further patch, get_unused_fd() will be removed so that
new code start using anon_inode_getfd() or get_unused_fd_flags()
with correct flags.
This patch replaces calls to get_unused_fd() with equivalent call to
get_unused_fd_flags(0) to preserve current behavor for existing code.
The hard coded flag value (0) should be reviewed on a per-subsystem basis,
and, if possible, set to O_CLOEXEC.
Signed-off-by: Yann Droneaud <ydroneaud@opteya.com>
---
fs/notify/fanotify/fanotify_user.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
index e44cb64..644b9a7 100644
--- a/fs/notify/fanotify/fanotify_user.c
+++ b/fs/notify/fanotify/fanotify_user.c
@@ -69,7 +69,7 @@ static int create_fd(struct fsnotify_group *group,
pr_debug("%s: group=%p event=%p\n", __func__, group, event);
- client_fd = get_unused_fd();
+ client_fd = get_unused_fd_flags(0);
if (client_fd < 0)
return client_fd;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH 10/13] xfs: use get_unused_fd_flags(0) instead of get_unused_fd()
2013-07-02 16:39 [PATCH 00/13] Getting rid of get_unused_fd() Yann Droneaud
` (8 preceding siblings ...)
2013-07-02 16:39 ` [PATCH 09/13] fanotify: " Yann Droneaud
@ 2013-07-02 16:39 ` Yann Droneaud
2013-07-08 22:41 ` Ben Myers
2013-07-02 16:39 ` [PATCH 11/13] events: " Yann Droneaud
` (2 subsequent siblings)
12 siblings, 1 reply; 27+ messages in thread
From: Yann Droneaud @ 2013-07-02 16:39 UTC (permalink / raw)
To: linux-kernel, xfs; +Cc: Yann Droneaud
Macro get_unused_fd() is used to allocate a file descriptor with
default flags. Those default flags (0) can be "unsafe":
O_CLOEXEC must be used by default to not leak file descriptor
across exec().
Instead of macro get_unused_fd(), functions anon_inode_getfd()
or get_unused_fd_flags() should be used with flags given by userspace.
If not possible, flags should be set to O_CLOEXEC to provide userspace
with a default safe behavor.
In a further patch, get_unused_fd() will be removed so that
new code start using anon_inode_getfd() or get_unused_fd_flags()
with correct flags.
This patch replaces calls to get_unused_fd() with equivalent call to
get_unused_fd_flags(0) to preserve current behavor for existing code.
The hard coded flag value (0) should be reviewed on a per-subsystem basis,
and, if possible, set to O_CLOEXEC.
Signed-off-by: Yann Droneaud <ydroneaud@opteya.com>
---
fs/xfs/xfs_ioctl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index 5e99968..dc5b659 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -248,7 +248,7 @@ xfs_open_by_handle(
goto out_dput;
}
- fd = get_unused_fd();
+ fd = get_unused_fd_flags(0);
if (fd < 0) {
error = fd;
goto out_dput;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH 10/13] xfs: use get_unused_fd_flags(0) instead of get_unused_fd()
2013-07-02 16:39 ` [PATCH 10/13] xfs: " Yann Droneaud
@ 2013-07-08 22:41 ` Ben Myers
2013-07-09 20:53 ` Ben Myers
0 siblings, 1 reply; 27+ messages in thread
From: Ben Myers @ 2013-07-08 22:41 UTC (permalink / raw)
To: Yann Droneaud; +Cc: linux-kernel, xfs
On Tue, Jul 02, 2013 at 06:39:34PM +0200, Yann Droneaud wrote:
> Macro get_unused_fd() is used to allocate a file descriptor with
> default flags. Those default flags (0) can be "unsafe":
> O_CLOEXEC must be used by default to not leak file descriptor
> across exec().
>
> Instead of macro get_unused_fd(), functions anon_inode_getfd()
> or get_unused_fd_flags() should be used with flags given by userspace.
> If not possible, flags should be set to O_CLOEXEC to provide userspace
> with a default safe behavor.
>
> In a further patch, get_unused_fd() will be removed so that
> new code start using anon_inode_getfd() or get_unused_fd_flags()
> with correct flags.
>
> This patch replaces calls to get_unused_fd() with equivalent call to
> get_unused_fd_flags(0) to preserve current behavor for existing code.
>
> The hard coded flag value (0) should be reviewed on a per-subsystem basis,
> and, if possible, set to O_CLOEXEC.
>
> Signed-off-by: Yann Droneaud <ydroneaud@opteya.com>
>
> ---
> fs/xfs/xfs_ioctl.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> index 5e99968..dc5b659 100644
> --- a/fs/xfs/xfs_ioctl.c
> +++ b/fs/xfs/xfs_ioctl.c
> @@ -248,7 +248,7 @@ xfs_open_by_handle(
> goto out_dput;
> }
>
> - fd = get_unused_fd();
> + fd = get_unused_fd_flags(0);
O_CLOEXEC should be fine in this case.
Reviewed-by: Ben Myers <bpm@sgi.com>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 10/13] xfs: use get_unused_fd_flags(0) instead of get_unused_fd()
2013-07-08 22:41 ` Ben Myers
@ 2013-07-09 20:53 ` Ben Myers
2013-07-10 10:00 ` Yann Droneaud
0 siblings, 1 reply; 27+ messages in thread
From: Ben Myers @ 2013-07-09 20:53 UTC (permalink / raw)
To: Yann Droneaud; +Cc: linux-kernel, xfs
Yann,
On Mon, Jul 08, 2013 at 05:41:33PM -0500, Ben Myers wrote:
> On Tue, Jul 02, 2013 at 06:39:34PM +0200, Yann Droneaud wrote:
> > Macro get_unused_fd() is used to allocate a file descriptor with
> > default flags. Those default flags (0) can be "unsafe":
> > O_CLOEXEC must be used by default to not leak file descriptor
> > across exec().
> >
> > Instead of macro get_unused_fd(), functions anon_inode_getfd()
> > or get_unused_fd_flags() should be used with flags given by userspace.
> > If not possible, flags should be set to O_CLOEXEC to provide userspace
> > with a default safe behavor.
> >
> > In a further patch, get_unused_fd() will be removed so that
> > new code start using anon_inode_getfd() or get_unused_fd_flags()
> > with correct flags.
> >
> > This patch replaces calls to get_unused_fd() with equivalent call to
> > get_unused_fd_flags(0) to preserve current behavor for existing code.
> >
> > The hard coded flag value (0) should be reviewed on a per-subsystem basis,
> > and, if possible, set to O_CLOEXEC.
> >
> > Signed-off-by: Yann Droneaud <ydroneaud@opteya.com>
> >
> > ---
> > fs/xfs/xfs_ioctl.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> > index 5e99968..dc5b659 100644
> > --- a/fs/xfs/xfs_ioctl.c
> > +++ b/fs/xfs/xfs_ioctl.c
> > @@ -248,7 +248,7 @@ xfs_open_by_handle(
> > goto out_dput;
> > }
> >
> > - fd = get_unused_fd();
> > + fd = get_unused_fd_flags(0);
>
> O_CLOEXEC should be fine in this case.
>
> Reviewed-by: Ben Myers <bpm@sgi.com>
Applied at git://oss.sgi.com/xfs/xfs.git. Looks like I was wrong about
O_CLOEXEC being ok here. There may be applications which open_by_handle then
fork/exec and expect to still be able to use that file descriptor.
Thanks,
Ben
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 10/13] xfs: use get_unused_fd_flags(0) instead of get_unused_fd()
2013-07-09 20:53 ` Ben Myers
@ 2013-07-10 10:00 ` Yann Droneaud
2013-07-11 0:36 ` Dave Chinner
0 siblings, 1 reply; 27+ messages in thread
From: Yann Droneaud @ 2013-07-10 10:00 UTC (permalink / raw)
To: Ben Myers; +Cc: linux-kernel, xfs, ydroneaud
Hi,
Le 09.07.2013 22:53, Ben Myers a écrit :
> On Mon, Jul 08, 2013 at 05:41:33PM -0500, Ben Myers wrote:
>> On Tue, Jul 02, 2013 at 06:39:34PM +0200, Yann Droneaud wrote:
>> > diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
>> > index 5e99968..dc5b659 100644
>> > --- a/fs/xfs/xfs_ioctl.c
>> > +++ b/fs/xfs/xfs_ioctl.c
>> > @@ -248,7 +248,7 @@ xfs_open_by_handle(
>> > goto out_dput;
>> > }
>> >
>> > - fd = get_unused_fd();
>> > + fd = get_unused_fd_flags(0);
>>
>> O_CLOEXEC should be fine in this case.
>>
>> Reviewed-by: Ben Myers <bpm@sgi.com>
>
> Applied at git://oss.sgi.com/xfs/xfs.git. Looks like I was wrong about
> O_CLOEXEC being ok here. There may be applications which
> open_by_handle then
> fork/exec and expect to still be able to use that file descriptor.
>
OK, it's very important to not cause regression here.
For the record, xfs_open_by_handle() is not related to
open_by_handle_at() syscall.
It's an ioctl (XFS_IOC_OPEN_BY_HANDLE) which is used by xfsprogs's
libhandle
in functions open_by_fshandle() and open_by_handle().
http://sources.debian.net/src/xfsprogs/3.1.9/libhandle/handle.c?hl=284#L284
http://sources.debian.net/src/xfsprogs/3.1.9/libhandle/handle.c?hl=308#L308
According to codesearch.debian.org, libhandle's open_by_handle() is only
used
by xfsdump
http://sources.debian.net/src/xfsdump/3.1.1/restore/tree.c?hl=2534#L2534
So there's no many *known* users of this features ... but it's more
important
not to break *unknown* users of it.
BTW, thanks for the review and applying.
Regards.
--
Yann Droneaud
OPTEYA
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 10/13] xfs: use get_unused_fd_flags(0) instead of get_unused_fd()
2013-07-10 10:00 ` Yann Droneaud
@ 2013-07-11 0:36 ` Dave Chinner
0 siblings, 0 replies; 27+ messages in thread
From: Dave Chinner @ 2013-07-11 0:36 UTC (permalink / raw)
To: Yann Droneaud; +Cc: Ben Myers, linux-kernel, xfs
On Wed, Jul 10, 2013 at 12:00:57PM +0200, Yann Droneaud wrote:
> Hi,
>
> Le 09.07.2013 22:53, Ben Myers a écrit :
> >On Mon, Jul 08, 2013 at 05:41:33PM -0500, Ben Myers wrote:
> >>On Tue, Jul 02, 2013 at 06:39:34PM +0200, Yann Droneaud wrote:
>
> >>> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> >>> index 5e99968..dc5b659 100644
> >>> --- a/fs/xfs/xfs_ioctl.c
> >>> +++ b/fs/xfs/xfs_ioctl.c
> >>> @@ -248,7 +248,7 @@ xfs_open_by_handle(
> >>> goto out_dput;
> >>> }
> >>>
> >>> - fd = get_unused_fd();
> >>> + fd = get_unused_fd_flags(0);
> >>
> >>O_CLOEXEC should be fine in this case.
> >>
> >>Reviewed-by: Ben Myers <bpm@sgi.com>
> >
> >Applied at git://oss.sgi.com/xfs/xfs.git. Looks like I was wrong about
> >O_CLOEXEC being ok here. There may be applications which
> >open_by_handle then
> >fork/exec and expect to still be able to use that file descriptor.
> >
>
> OK, it's very important to not cause regression here.
>
> For the record, xfs_open_by_handle() is not related to
> open_by_handle_at() syscall.
>
> It's an ioctl (XFS_IOC_OPEN_BY_HANDLE) which is used by xfsprogs's
> libhandle
> in functions open_by_fshandle() and open_by_handle().
>
> http://sources.debian.net/src/xfsprogs/3.1.9/libhandle/handle.c?hl=284#L284
> http://sources.debian.net/src/xfsprogs/3.1.9/libhandle/handle.c?hl=308#L308
>
> According to codesearch.debian.org, libhandle's open_by_handle() is
> only used
> by xfsdump
>
> http://sources.debian.net/src/xfsdump/3.1.1/restore/tree.c?hl=2534#L2534
>
> So there's no many *known* users of this features ... but it's more
> important
> not to break *unknown* users of it.
There are commercial products (i.e. proprietary, closed source) that
use it. SGI has one (DMF) and there are a couple of other backup
programs that have used it in the past, too.
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 11/13] events: use get_unused_fd_flags(0) instead of get_unused_fd()
2013-07-02 16:39 [PATCH 00/13] Getting rid of get_unused_fd() Yann Droneaud
` (9 preceding siblings ...)
2013-07-02 16:39 ` [PATCH 10/13] xfs: " Yann Droneaud
@ 2013-07-02 16:39 ` Yann Droneaud
2013-07-02 16:39 ` [PATCH 12/13] sctp: " Yann Droneaud
2013-07-02 16:39 ` [PATCH 13/13] file: remove get_unused_fd() Yann Droneaud
12 siblings, 0 replies; 27+ messages in thread
From: Yann Droneaud @ 2013-07-02 16:39 UTC (permalink / raw)
To: linux-kernel, Peter Zijlstra, Paul Mackerras, Ingo Molnar,
Arnaldo Carvalho de Melo
Cc: Yann Droneaud
Macro get_unused_fd() is used to allocate a file descriptor with
default flags. Those default flags (0) can be "unsafe":
O_CLOEXEC must be used by default to not leak file descriptor
across exec().
Instead of macro get_unused_fd(), functions anon_inode_getfd()
or get_unused_fd_flags() should be used with flags given by userspace.
If not possible, flags should be set to O_CLOEXEC to provide userspace
with a default safe behavor.
In a further patch, get_unused_fd() will be removed so that
new code start using anon_inode_getfd() or get_unused_fd_flags()
with correct flags.
This patch replaces calls to get_unused_fd() with equivalent call to
get_unused_fd_flags(0) to preserve current behavor for existing code.
The hard coded flag value (0) should be reviewed on a per-subsystem basis,
and, if possible, set to O_CLOEXEC.
Signed-off-by: Yann Droneaud <ydroneaud@opteya.com>
---
kernel/events/core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 47bb0f7..c2feeb5 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -6810,7 +6810,7 @@ SYSCALL_DEFINE5(perf_event_open,
if ((flags & PERF_FLAG_PID_CGROUP) && (pid == -1 || cpu == -1))
return -EINVAL;
- event_fd = get_unused_fd();
+ event_fd = get_unused_fd_flags(0);
if (event_fd < 0)
return event_fd;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH 12/13] sctp: use get_unused_fd_flags(0) instead of get_unused_fd()
2013-07-02 16:39 [PATCH 00/13] Getting rid of get_unused_fd() Yann Droneaud
` (10 preceding siblings ...)
2013-07-02 16:39 ` [PATCH 11/13] events: " Yann Droneaud
@ 2013-07-02 16:39 ` Yann Droneaud
2013-07-02 17:50 ` Vlad Yasevich
2013-07-02 16:39 ` [PATCH 13/13] file: remove get_unused_fd() Yann Droneaud
12 siblings, 1 reply; 27+ messages in thread
From: Yann Droneaud @ 2013-07-02 16:39 UTC (permalink / raw)
To: linux-kernel, linux-sctp, netdev; +Cc: Yann Droneaud
Macro get_unused_fd() is used to allocate a file descriptor with
default flags. Those default flags (0) can be "unsafe":
O_CLOEXEC must be used by default to not leak file descriptor
across exec().
Instead of macro get_unused_fd(), functions anon_inode_getfd()
or get_unused_fd_flags() should be used with flags given by userspace.
If not possible, flags should be set to O_CLOEXEC to provide userspace
with a default safe behavor.
In a further patch, get_unused_fd() will be removed so that
new code start using anon_inode_getfd() or get_unused_fd_flags()
with correct flags.
This patch replaces calls to get_unused_fd() with equivalent call to
get_unused_fd_flags(0) to preserve current behavor for existing code.
The hard coded flag value (0) should be reviewed on a per-subsystem basis,
and, if possible, set to O_CLOEXEC.
Signed-off-by: Yann Droneaud <ydroneaud@opteya.com>
---
net/sctp/socket.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 66fcdcf..caa5919 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -4320,7 +4320,7 @@ static int sctp_getsockopt_peeloff(struct sock *sk, int len, char __user *optval
goto out;
/* Map the socket to an unused fd that can be returned to the user. */
- retval = get_unused_fd();
+ retval = get_unused_fd_flags(0);
if (retval < 0) {
sock_release(newsock);
goto out;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH 12/13] sctp: use get_unused_fd_flags(0) instead of get_unused_fd()
2013-07-02 16:39 ` [PATCH 12/13] sctp: " Yann Droneaud
@ 2013-07-02 17:50 ` Vlad Yasevich
2013-07-02 23:14 ` David Miller
0 siblings, 1 reply; 27+ messages in thread
From: Vlad Yasevich @ 2013-07-02 17:50 UTC (permalink / raw)
To: Yann Droneaud; +Cc: linux-kernel, linux-sctp, netdev
On 07/02/2013 12:39 PM, Yann Droneaud wrote:
> Macro get_unused_fd() is used to allocate a file descriptor with
> default flags. Those default flags (0) can be "unsafe":
> O_CLOEXEC must be used by default to not leak file descriptor
> across exec().
>
> Instead of macro get_unused_fd(), functions anon_inode_getfd()
> or get_unused_fd_flags() should be used with flags given by userspace.
> If not possible, flags should be set to O_CLOEXEC to provide userspace
> with a default safe behavor.
>
> In a further patch, get_unused_fd() will be removed so that
> new code start using anon_inode_getfd() or get_unused_fd_flags()
> with correct flags.
>
> This patch replaces calls to get_unused_fd() with equivalent call to
> get_unused_fd_flags(0) to preserve current behavor for existing code.
>
> The hard coded flag value (0) should be reviewed on a per-subsystem basis,
> and, if possible, set to O_CLOEXEC.
>
> Signed-off-by: Yann Droneaud <ydroneaud@opteya.com>
Acked-by: Vlad Yasevich <vyasevich@gmail.com>
-vlad
> ---
> net/sctp/socket.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index 66fcdcf..caa5919 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -4320,7 +4320,7 @@ static int sctp_getsockopt_peeloff(struct sock *sk, int len, char __user *optval
> goto out;
>
> /* Map the socket to an unused fd that can be returned to the user. */
> - retval = get_unused_fd();
> + retval = get_unused_fd_flags(0);
> if (retval < 0) {
> sock_release(newsock);
> goto out;
>
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 12/13] sctp: use get_unused_fd_flags(0) instead of get_unused_fd()
2013-07-02 17:50 ` Vlad Yasevich
@ 2013-07-02 23:14 ` David Miller
0 siblings, 0 replies; 27+ messages in thread
From: David Miller @ 2013-07-02 23:14 UTC (permalink / raw)
To: vyasevich; +Cc: ydroneaud, linux-kernel, linux-sctp, netdev
From: Vlad Yasevich <vyasevich@gmail.com>
Date: Tue, 02 Jul 2013 13:50:32 -0400
> On 07/02/2013 12:39 PM, Yann Droneaud wrote:
>> Macro get_unused_fd() is used to allocate a file descriptor with
>> default flags. Those default flags (0) can be "unsafe":
>> O_CLOEXEC must be used by default to not leak file descriptor
>> across exec().
>>
>> Instead of macro get_unused_fd(), functions anon_inode_getfd()
>> or get_unused_fd_flags() should be used with flags given by userspace.
>> If not possible, flags should be set to O_CLOEXEC to provide userspace
>> with a default safe behavor.
>>
>> In a further patch, get_unused_fd() will be removed so that
>> new code start using anon_inode_getfd() or get_unused_fd_flags()
>> with correct flags.
>>
>> This patch replaces calls to get_unused_fd() with equivalent call to
>> get_unused_fd_flags(0) to preserve current behavor for existing code.
>>
>> The hard coded flag value (0) should be reviewed on a per-subsystem
>> basis,
>> and, if possible, set to O_CLOEXEC.
>>
>> Signed-off-by: Yann Droneaud <ydroneaud@opteya.com>
>
> Acked-by: Vlad Yasevich <vyasevich@gmail.com>
Applied to net-next, thanks.
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 13/13] file: remove get_unused_fd()
2013-07-02 16:39 [PATCH 00/13] Getting rid of get_unused_fd() Yann Droneaud
` (11 preceding siblings ...)
2013-07-02 16:39 ` [PATCH 12/13] sctp: " Yann Droneaud
@ 2013-07-02 16:39 ` Yann Droneaud
12 siblings, 0 replies; 27+ messages in thread
From: Yann Droneaud @ 2013-07-02 16:39 UTC (permalink / raw)
To: linux-kernel, linux-fsdevel; +Cc: Yann Droneaud
Macro get_unused_fd() allocates a file descriptor without O_CLOEXEC flag.
This can be seen as an unsafe default: in most case O_CLOEXEC
must be used to not leak file descriptor across exec().
Using O_CLOEXEC by default allows userspace to choose, without race,
if the file descriptor is going to be inherited across exec().
This patch removes get_unused_fd() so that newer kernel code use
anon_inode_getfd() or get_unused_fd_flags() with flags provided
by userspace. If flags cannot be given by userspace,
O_CLOEXEC must be the default flag.
Signed-off-by: Yann Droneaud <ydroneaud@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 cbacf4f..8666002 100644
--- a/include/linux/file.h
+++ b/include/linux/file.h
@@ -63,7 +63,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.8.3.1
^ permalink raw reply related [flat|nested] 27+ messages in thread