Hi,
Macro get_unused_fd() is a shortcut to call function get_unused_fd_flags(),
to allocate a file descriptor.
The macro use 0 as flags, so the file descriptor is created
without O_CLOEXEC flag.
This can be seen as an unsafe default eg. in most case O_CLOEXEC
must be used to not leak file descriptor across exec().
Newer kernel code should 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.
Using O_CLOEXEC by default allows userspace to choose, without race,
if the file descriptor is going to be inherited across exec().
They are two ways to achieve this:
- makes get_unused_fd() use O_CLOEXEC by default
It's difficult to get it right: every code using of get_unused_fd()
must take this change into account and be fixed as soon as
macro get_unused_fd() do the switch. Non updated code will have
unexpected behavor and it's likely going to break API contract.
- remove get_unused_fd()
It's going to break some out of tree, not yet upstream kernel code,
but it's easy to notice and fix. Anyway, newer code should use
anon_inode_getfd() or get_unused_fd_flags().
The latter option was choosen to ensure no unexpected behavor
for out of tree, not yet upstream code. Removing the macro is the safest
choice: it's better to break build than trying to make get_unused_fd()
use O_CLOEXEC by default and get all user of get_unused_fd() update.
Additionnaly, removing the macro is not going to break modules ABI.
In linux-next tag 20130702, they're currently:
- 15 calls to get_unused_fd_flags()
not counting get_unused_fd() and anon_inode_getfd()
- 14 calls to get_unused_fd()
- 11 calls to anon_inode_getfd()
The following patchset try to convert all calls to get_unused_fd()
to get_unused_fd_flags(0) before removing get_unused_fd() macro.
Without get_unused_fd() macro, more subsystems are likely to use
anon_inode_getfd() and be teached to provide an API that let userspace
choose the opening flags of the file descriptor.
Yann Droneaud (13):
ia64: use get_unused_fd_flags(0) instead of get_unused_fd()
ppc/cell: use get_unused_fd_flags(0) instead of get_unused_fd()
infiniband: use get_unused_fd_flags(0) instead of get_unused_fd()
android/sw_sync: use get_unused_fd_flags(0) instead of get_unused_fd()
android/sync: use get_unused_fd_flags(0) instead of get_unused_fd()
vfio: use get_unused_fd_flags(0) instead of get_unused_fd()
binfmt_misc: use get_unused_fd_flags(0) instead of get_unused_fd()
file: use get_unused_fd_flags(0) instead of get_unused_fd()
fanotify: use get_unused_fd_flags(0) instead of get_unused_fd()
xfs: use get_unused_fd_flags(0) instead of get_unused_fd()
events: use get_unused_fd_flags(0) instead of get_unused_fd()
sctp: use get_unused_fd_flags(0) instead of get_unused_fd()
file: remove get_unused_fd()
arch/ia64/kernel/perfmon.c | 2 +-
arch/powerpc/platforms/cell/spufs/inode.c | 4 ++--
drivers/infiniband/core/uverbs_cmd.c | 4 ++--
drivers/staging/android/sw_sync.c | 2 +-
drivers/staging/android/sync.c | 2 +-
drivers/vfio/vfio.c | 2 +-
fs/binfmt_misc.c | 2 +-
fs/file.c | 2 +-
fs/notify/fanotify/fanotify_user.c | 2 +-
fs/xfs/xfs_ioctl.c | 2 +-
include/linux/file.h | 1 -
kernel/events/core.c | 2 +-
net/sctp/socket.c | 2 +-
13 files changed, 14 insertions(+), 15 deletions(-)
--
1.8.3.1
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 <redacted>
---
arch/powerpc/platforms/cell/spufs/inode.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2014-01-12 23:06:22
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 <redacted>
Should I merge this (v5 on patchwork) or let Al do it ?
Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
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:
quoted
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 <redacted>
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>
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 :
quoted
On Tue, 2013-07-02 at 18:39 +0200, Yann Droneaud wrote:
quoted
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 <redacted>
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.
quoted
Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
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