--- v1
+++ v4
@@ -1,86 +1,103 @@
Hi,
-Macro get_unused_fd() is a shortcut to call function get_unused_fd_flags(),
-to allocate a file descriptor.
+Please find the fourth revision of my patchset to remove get_unused_fd()
+macro in order to encourage subsystems to use get_unused_fd_flags() or
+anon_inode_getfd() with open flags set to O_CLOEXEC were appropriate.
-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.
+The patchset 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):
+Not using O_CLOEXEC by default or not letting userspace provide the
+"open" flags should be considered bad practice from security point
+of view: in most case O_CLOEXEC must be used to not leak file descriptor
+across exec().
+
+Using O_CLOEXEC by default when flags are not provided by userspace
+allows userspace to set, using fcntl(), without any risk of race,
+if the file descriptor is going to be inherited or not across exec().
+
+Status:
+
+In linux-next tag 20131029, they're currently:
+
+- 32 calls to fd_install()
+- 23 calls to get_unused_fd_flags()
+- 11 calls to anon_inode_getfd()
+- 7 calls to get_unused_fd()
+
+Changes from patchset v3 [PATCHSETv3]:
+
+- industrialio: use anon_inode_getfd() with O_CLOEXEC flag
+ DROPPED: applied upstream
+
+Changes from patchset v2 [PATCHSETv2]:
+
+- android/sw_sync: use get_unused_fd_flags(O_CLOEXEC) instead of get_unused_fd()
+ DROPPED: applied upstream
+
+- android/sync: use get_unused_fd_flags(O_CLOEXEC) instead of get_unused_fd()
+ DROPPED: applied upstream
+
+- vfio: use get_unused_fd_flags(0) instead of get_unused_fd()
+ DROPPED: applied upstream.
+ Additionally subsystem maintainer applied another patch on top
+ to set the flags to O_CLOEXEC.
+
+- industrialio: use anon_inode_getfd() with O_CLOEXEC flag
+ NEW: propose to use O_CLOEXEC as default flag.
+
+Changes from patchset v1 [PATCHSETv1]:
+
+- explicitly added subsystem maintainers as mail recepients.
+
+- infiniband: use get_unused_fd_flags(0) instead of get_unused_fd()
+ DROPPED: subsystem maintainer applied another patch
+ using get_unused_fd_flags(O_CLOEXEC) as suggested.
+
+- android/sw_sync: use get_unused_fd_flags(0) instead of get_unused_fd()
+ MODIFIED: use get_unused_fd_flags(O_CLOEXEC) as suggested
+
+- android/sync: use get_unused_fd_flags(0) instead of get_unused_fd()
+ MODIFIED: use get_unused_fd_flags(O_CLOEXEC) as suggested
+
+- xfs: use get_unused_fd_flags(0) instead of get_unused_fd()
+ DROPPED: applied asis by subsystem maintainer.
+
+- sctp: use get_unused_fd_flags(0) instead of get_unused_fd()
+ DROPPED: applied asis by subsystem maintainer.
+
+Links:
+
+[PATCHSETv3]
+ http://lkml.kernel.org/r/cover.1378460926.git.ydroneaud@opteya.com
+
+[PATCHSETv2]
+ http://lkml.kernel.org/r/cover.1376327678.git.ydroneaud@opteya.com
+
+[PATCHSETv1]
+ http://lkml.kernel.org/r/cover.1372777600.git.ydroneaud@opteya.com
+
+Yann Droneaud (7):
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(-)
+ 7 files changed, 7 insertions(+), 8 deletions(-)
--
1.8.3.1