Re: [PATCH 2/2] ppc: kvm: use anon_inode_getfd() with O_CLOEXEC flag
Le 26.08.2013 09:39, Paolo Bonzini a écrit : Il 25/08/2013 17:04, Alexander Graf ha scritto: On 24.08.2013, at 21:14, Yann Droneaud wrote: This patch set O_CLOEXEC flag on all file descriptors created with anon_inode_getfd() to not leak file descriptors across exec(). Signed-off-by: Yann Droneaud Link: http://lkml.kernel.org/r/cover.1377372576.git.ydrone...@opteya.com Reviewed-by: Alexander Graf Would it make sense to simply inherit the O_CLOEXEC flag from the parent kvm fd instead? That would give user space the power to keep fds across exec() if it wants to. Does it make sense to use non-O_CLOEXEC file descriptors with KVM at all? Besides fork() not being supported by KVM, as described in Documentation/virtual/kvm/api.txt, the VMAs of the parent process go away as soon as you exec(). I'm not sure how you can use the inherited file descriptor in a sensible way after exec(). Sounds a lot like InfiniBand subsystem behavor: IB file descriptors are of no use accross exec() since memory mappings tied to those fds won't be available in the new process: https://lkml.org/lkml/2013/7/8/380 http://mid.gmane.org/f58540dc64fec1ac0e496dfcd3cc1...@meuh.org Regards. -- Yann Droneaud OPTEYA -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 03/13] infiniband: use get_unused_fd_flags(0) instead of get_unused_fd()
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 10/13] xfs: use get_unused_fd_flags(0) instead of get_unused_fd()
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 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 8/8] industrialio: use anon_inode_getfd() with O_CLOEXEC flag
Hi, Le dimanche 15 septembre 2013 à 17:26 +0100, Jonathan Cameron a écrit : > On 09/06/13 11:39, Yann Droneaud wrote: [...] > > http://lkml.kernel.org/r/cover.1377372576.git.ydrone...@opteya.com > > > > Signed-off-by: Yann Droneaud > > Link: http://lkml.kernel.org/r/cover.1378460926.git.ydrone...@opteya.com > > Thanks Yann. I had no idea about this issue but your well supported > description > above made this easy to review. > > Applied to the togreg branch of > git://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git > as it's not a regression but if there is support for pushing this back to > stable > I personally am not against it. > It's not a -stable materiel. So let it flow to v3.12 or v3.13. Thanks for the review. Regards. -- Yann Droneaud OPTEYA -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v3 0/8] Getting rid of get_unused_fd() / use O_CLOEXEC
Hi, With help from subsystem maintainers, I've managed to get some of get_unused_fd() calls converted to use get_unused_fd_flags(O_CLOEXEC) instead. [ANDROID][IB][VFIO] KVM subsystem maintainers helped me to change calls to anon_inode_getfd() to use O_CLOEXEC by default. [KVM] Some maintainers applied my patches to convert get_unused_fd() to get_unused_fd_flags(0) were using O_CLOEXEC wasn't possible without breaking ABI. [SCTP][XFS] So, still in the hope to get rid of get_unused_fd(), please find a another attempt to remove get_unused_fd() macro and encourage subsystems to use get_unused_fd_flags(O_CLOEXEC) or anon_inode_getfd(O_CLOEXEC) by default were appropriate. 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. Additionally I'm suggesting Industrial IO (IIO) subsystem to use anon_inode_getfd(O_CLOEXEC): it's the only subsystem using anon_inode_getfd() with a fixed set of flags not including O_CLOEXEC. 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 it to choose, without race, if the file descriptor is going to be inherited across exec(). Status: In linux-next tag 20130906, they're currently: - 22 calls to get_unused_fd_flags() (+3) not counting get_unused_fd() and anon_inode_getfd() - 7 calls to get_unused_fd() (-3) - 11 calls to anon_inode_getfd()(0) 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. Links: [ANDROID] http://lkml.kernel.org/r/cacsp8sjxgmk2_kx_+rgzqqqwqkernvf1wt3k5tw991w5dfa...@mail.gmail.com http://lkml.kernel.org/r/CACSP8SjZcpcpEtQHzcGYhf-MP7QGo0XpN7-uN7rmD=vNtopG=w...@mail.gmail.com [IB] http://lkml.kernel.org/r/CAL1RGDXV1_BjSLrQDFdVQ1_D75+bMtOtikHOUp8VBiy_OJUf=w...@mail.gmail.com [VFIO] http://lkml.kernel.org/r/20130822171744.1297.13711.st...@bling.home [KVM] http://lkml.kernel.org/r/5219a8fc.8090...@redhat.com http://lkml.kernel.org/r/3557ef65-4327-4dae-999a-b0ee13c43...@suse.de http://lkml.kernel.org/r/20130826102023.ga8...@redhat.com [SCTP] http://lkml.kernel.org/r/51d312e8.6090...@gmail.com http://lkml.kernel.org/r/20130702.161428.1703028286206350504.da...@davemloft.net [XFS] http://lkml.kernel.org/r/20130709205321.gv20...@sgi.com [PATCHSETv2] http://lkml.kernel.org/r/cover.1376327678.git.ydrone...@opteya.com Yann Droneaud (8): 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() 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() events: use get_unused_fd_flags(0) instead of get_unused_fd() file: remove get_unused_fd() industrialio: use anon_inode_getfd() with O_CLOEXEC flag arch/ia64/kernel/perfmon.c| 2 +- arch/powerpc/platforms/cell/spufs/inode.c | 4 ++-- drivers/iio/industrialio-event.c | 2 +- fs/binfmt_misc.c | 2 +- fs/file.c | 2 +- fs/notify/fanotify/fanotify_user.c| 2 +- include/linux/file.h | 1 - kernel/events/core.c | 2 +- 8 files changed, 8 insertions(+), 9 deletions(-) -- 1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v3 1/8] ia64: use get_unused_fd_flags(0) instead of get_unused_fd()
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 Link: http://lkml.kernel.org/r/cover.1378460926.git.ydrone...@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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v3 7/8] file: remove get_unused_fd()
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 Link: http://lkml.kernel.org/r/cover.1378460926.git.ydrone...@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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v3 2/8] ppc/cell: use get_unused_fd_flags(0) instead of get_unused_fd()
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 Link: http://lkml.kernel.org/r/cover.1378460926.git.ydrone...@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 87ba7cf..51effce 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v3 6/8] events: use get_unused_fd_flags(0) instead of get_unused_fd()
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 Link: http://lkml.kernel.org/r/cover.1378460926.git.ydrone...@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 2207efc..b12fdd3 100644 --- a/kernel/events/core.c +++ b/kernel/events/core.c @@ -6935,7 +6935,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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v3 5/8] fanotify: use get_unused_fd_flags(0) instead of get_unused_fd()
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
Link: http://lkml.kernel.org/r/cover.1378460926.git.ydrone...@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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v3 4/8] file: use get_unused_fd_flags(0) instead of get_unused_fd()
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
Link: http://lkml.kernel.org/r/cover.1378460926.git.ydrone...@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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v3 8/8] industrialio: use anon_inode_getfd() with O_CLOEXEC flag
IIO uses anon_inode_get() to allocate file descriptors as part
of its ioctls. But those ioctls are lacking a flag argument
allowing userspace to choose options for the newly opened file
descriptor.
In such case it's advised to use O_CLOEXEC by default so that
userspace is allowed to choose, without race, if the file descriptor
is going to be inherited across exec().
KVM usage of anon_inode_getfd() was fixed in a previous patchset [1],
so IIO is the only subsystem using anon_inode_getfd() with a fixed set
of flags not including O_CLOEXEC.
This patch set O_CLOEXEC flag on the event file descriptor created
with anon_inode_getfd() to not leak file descriptors across exec().
Links:
- Secure File Descriptor Handling (Ulrich Drepper, 2008)
http://udrepper.livejournal.com/20407.html
- Excuse me son, but your code is leaking !!! (Dan Walsh, March 2012)
http://danwalsh.livejournal.com/53603.html
- [1] kvm: use anon_inode_getfd() with O_CLOEXEC flag
http://lkml.kernel.org/r/cover.1377372576.git.ydrone...@opteya.com
Signed-off-by: Yann Droneaud
Link: http://lkml.kernel.org/r/cover.1378460926.git.ydrone...@opteya.com
---
drivers/iio/industrialio-event.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iio/industrialio-event.c b/drivers/iio/industrialio-event.c
index 10aa9ef..2390e3d 100644
--- a/drivers/iio/industrialio-event.c
+++ b/drivers/iio/industrialio-event.c
@@ -159,7 +159,7 @@ int iio_event_getfd(struct iio_dev *indio_dev)
}
spin_unlock_irq(&ev_int->wait.lock);
fd = anon_inode_getfd("iio:event",
- &iio_event_chrdev_fileops, ev_int, O_RDONLY);
+ &iio_event_chrdev_fileops, ev_int, O_RDONLY |
O_CLOEXEC);
if (fd < 0) {
spin_lock_irq(&ev_int->wait.lock);
__clear_bit(IIO_BUSY_BIT_POS, &ev_int->flags);
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v3 3/8] binfmt_misc: use get_unused_fd_flags(0) instead of get_unused_fd()
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
Link: http://lkml.kernel.org/r/cover.1378460926.git.ydrone...@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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] MAINTAINERS: x...@oss.sgi.com is a list
This patch changes type of x...@oss.sgi.com The output of ./scripts/get_maintainer.pl is modified to report x...@oss.sgi.com as a list: Ben Myers (supporter:XFS FILESYSTEM) Alex Elder (supporter:XFS FILESYSTEM) -x...@oss.sgi.com (supporter:XFS FILESYSTEM) +x...@oss.sgi.com (open list:XFS FILESYSTEM) linux-kernel@vger.kernel.org (open list) Signed-off-by: Yann Droneaud --- MAINTAINERS | 1 - 1 file changed, 1 deletion(-) diff --git a/MAINTAINERS b/MAINTAINERS index ad7e322..7b6ab51 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -9139,7 +9139,6 @@ XFS FILESYSTEM P: Silicon Graphics Inc M: Ben Myers M: Alex Elder -M: x...@oss.sgi.com L: x...@oss.sgi.com W: http://oss.sgi.com/projects/xfs T: git git://oss.sgi.com/xfs/xfs.git -- 1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] MAINTAINERS: dm-de...@redhat.com is a list
This patch changes type of dm-de...@redhat.com The output of ./scripts/get_maintainer.pl is modified to report dm-de...@redhat.com as a list: Alasdair Kergon (maintainer:DEVICE-MAPPER (LVM)) -dm-de...@redhat.com (maintainer:DEVICE-MAPPER (LVM)) Neil Brown (supporter:SOFTWARE RAID (Mu...) +dm-de...@redhat.com (open list:DEVICE-MAPPER (LVM)) linux-r...@vger.kernel.org (open list:SOFTWARE RAID (Mu...) linux-kernel@vger.kernel.org (open list) Signed-off-by: Yann Droneaud --- MAINTAINERS | 1 - 1 file changed, 1 deletion(-) diff --git a/MAINTAINERS b/MAINTAINERS index 7b6ab51..fefd17f 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2527,7 +2527,6 @@ S:Maintained DEVICE-MAPPER (LVM) M: Alasdair Kergon -M: dm-de...@redhat.com L: dm-de...@redhat.com W: http://sources.redhat.com/dm Q: http://patchwork.kernel.org/project/dm-devel/list/ -- 1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 00/13] Getting rid of get_unused_fd()
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 13/13] file: remove get_unused_fd()
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 --- 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 12/13] sctp: use get_unused_fd_flags(0) instead of get_unused_fd()
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
---
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 11/13] events: use get_unused_fd_flags(0) instead of get_unused_fd()
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 --- 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 06/13] vfio: use get_unused_fd_flags(0) instead of get_unused_fd()
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
---
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 08/13] file: use get_unused_fd_flags(0) instead of get_unused_fd()
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
---
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 03/13] infiniband: use get_unused_fd_flags(0) instead of get_unused_fd()
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 --- 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 02/13] ppc/cell: use get_unused_fd_flags(0) instead of get_unused_fd()
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 --- 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 10/13] xfs: use get_unused_fd_flags(0) instead of get_unused_fd()
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
---
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 04/13] android/sw_sync: use get_unused_fd_flags(0) instead of get_unused_fd()
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
---
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 09/13] fanotify: use get_unused_fd_flags(0) instead of get_unused_fd()
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
---
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 01/13] ia64: use get_unused_fd_flags(0) instead of get_unused_fd()
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 --- 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 05/13] android/sync: use get_unused_fd_flags(0) instead of get_unused_fd()
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
---
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 07/13] binfmt_misc: use get_unused_fd_flags(0) instead of get_unused_fd()
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
---
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] MAINTAINERS: x...@oss.sgi.com is a list
Hi, Le 03.07.2013 08:40, Dave Chinner a écrit : On Tue, Jul 02, 2013 at 05:00:47PM +0200, Yann Droneaud wrote: This patch changes type of x...@oss.sgi.com The output of ./scripts/get_maintainer.pl is modified to report x...@oss.sgi.com as a list: What's the problem with that? All XFS patches and problem reports should be sent to the x...@oss.sgi.com list. There are far more people than just the maintainer that can triage problems, answer questions and review patches... It was just disturbing: I was looking for a list for XFS and found only maintainers. Except dm-devel, no others public mailing list is used as a maintainer. So I thought it was a misinterpretation after following commit: commit 18caa67ad41212e6f82a675c40f461ffb45f098e Author: Ben Myers Date: Thu Apr 12 17:05:05 2012 + MAINTAINERS: retire xfs-mast...@oss.sgi.com xfs-mast...@oss.sgi.com will be retired in favor of x...@oss.sgi.com sometime soon. Signed-off-by: Ben Myers Reviewed-by: Christoph Hellwig diff --git a/MAINTAINERS b/MAINTAINERS index 2dcfca8..12b0445 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -7564,7 +7564,7 @@ XFS FILESYSTEM P: Silicon Graphics Inc M: Ben Myers M: Alex Elder -M: xfs-mast...@oss.sgi.com +M: x...@oss.sgi.com L: x...@oss.sgi.com W: http://oss.sgi.com/projects/xfs T: git git://oss.sgi.com/xfs/xfs.git Regards. -- Yann Droneaud OPTEYA -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] MAINTAINERS: x...@oss.sgi.com is a list
Le 03.07.2013 11:24, Dave Chinner a écrit : On Wed, Jul 03, 2013 at 10:14:41AM +0200, Yann Droneaud wrote: Le 03.07.2013 08:40, Dave Chinner a écrit : >On Tue, Jul 02, 2013 at 05:00:47PM +0200, Yann Droneaud wrote: >>This patch changes type of x...@oss.sgi.com >> >>The output of ./scripts/get_maintainer.pl is >>modified to report x...@oss.sgi.com as a list: > >What's the problem with that? All XFS patches and problem >reports should be sent to the x...@oss.sgi.com list. There are far >more people than just the maintainer that can triage problems, >answer questions and review patches... > It was just disturbing: I was looking for a list for XFS and found only maintainers. That's what the: L: x...@oss.sgi.com entry is, yes? In the output of ./scripts/get_maintainer.pl: Ben Myers (supporter:XFS FILESYSTEM) Alex Elder (supporter:XFS FILESYSTEM) x...@oss.sgi.com (supporter:XFS FILESYSTEM) linux-kernel@vger.kernel.org (open list) [If having the mailing list listed as the maintainer/supporter is correct, could ./scripts/get_maintainer.pl also report the mailing list as a list ?] Regards. -- Yann Droneaud OPTEYA -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] MAINTAINERS: x...@oss.sgi.com is a list
Le 03.07.2013 11:50, Dave Chinner a écrit : On Wed, Jul 03, 2013 at 11:36:39AM +0200, Yann Droneaud wrote: Le 03.07.2013 11:24, Dave Chinner a écrit : >On Wed, Jul 03, 2013 at 10:14:41AM +0200, Yann Droneaud wrote: >>Le 03.07.2013 08:40, Dave Chinner a écrit : >>>On Tue, Jul 02, 2013 at 05:00:47PM +0200, Yann Droneaud wrote: >>>>This patch changes type of x...@oss.sgi.com >>>> >>>>The output of ./scripts/get_maintainer.pl is >>>>modified to report x...@oss.sgi.com as a list: >>> >>>What's the problem with that? All XFS patches and problem >>>reports should be sent to the x...@oss.sgi.com list. There are far >>>more people than just the maintainer that can triage problems, >>>answer questions and review patches... >>> >> >>It was just disturbing: I was looking for a list for XFS >>and found only maintainers. > >That's what the: > >L: x...@oss.sgi.com > >entry is, yes? > In the output of ./scripts/get_maintainer.pl: Ben Myers (supporter:XFS FILESYSTEM) Alex Elder (supporter:XFS FILESYSTEM) x...@oss.sgi.com (supporter:XFS FILESYSTEM) linux-kernel@vger.kernel.org (open list) You're smarter than a dumb script. If the information that the script parses is correct and the dumb script doesn't give you the right information, then what needs fixing? And what about: If you are the only one[*] to abuse ./scripts/get_maintainer.pl, then what needs fixing ? [*] I have found three other examples of duplicating mailing list as maintainer: $ sed -n 's/^L:[[:space:]]*//p' MAINTAINERS | sort | uniq > L $ sed -n 's/^M:[[:space:]]*//p' MAINTAINERS | sort | uniq > M $ comm -12 L M ceph-de...@vger.kernel.org dm-de...@redhat.com x...@oss.sgi.com I've sent a patch for x...@oss.sgi.com and dm-de...@redhat.com, in my first pass I've missed ceph-de...@vger.kernel.org Seriously, all you are proving is the old adage that scripts/get_maintainer.pl should be considered harmful because people use it without first engaging their brain. Just thinking about the whole picture. In this case ./scripts/get_maintainer.pl is going to be right most of time except for 3 subsystems on about 1160. So having the same address for maintainer and list is a rather uncommon case. Which could be misleading for most. Regards. -- Yann Droneaud OPTEYA -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 01/10] ia64: use get_unused_fd_flags(0) instead of get_unused_fd()
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 Link: http://lkml.kernel.org/r/cover.1376327678.git.ydrone...@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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 02/10] ppc/cell: use get_unused_fd_flags(0) instead of get_unused_fd()
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 Link: http://lkml.kernel.org/r/cover.1376327678.git.ydrone...@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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 05/10] vfio: use get_unused_fd_flags(0) instead of get_unused_fd()
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
Link: http://lkml.kernel.org/r/cover.1376327678.git.ydrone...@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 d3cb342..75c16cc 100644
--- a/drivers/vfio/vfio.c
+++ b/drivers/vfio/vfio.c
@@ -1109,7 +1109,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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 04/10] android/sync: use get_unused_fd_flags(O_CLOEXEC) instead of get_unused_fd()
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 call to
get_unused_fd_flags(O_CLOEXEC) following advice from Erik Gilling.
Signed-off-by: Yann Droneaud
Cc: Erik Gilling
Cc: Colin Cross
Link:
http://lkml.kernel.org/r/cacsp8sjxgmk2_kx_+rgzqqqwqkernvf1wt3k5tw991w5dfa...@mail.gmail.com
Link: http://lkml.kernel.org/r/cover.1376327678.git.ydrone...@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..38e5d3b 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(O_CLOEXEC);
int err;
struct sync_fence *fence2, *fence3;
struct sync_merge_data data;
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 07/10] file: use get_unused_fd_flags(0) instead of get_unused_fd()
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
Link: http://lkml.kernel.org/r/cover.1376327678.git.ydrone...@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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 08/10] fanotify: use get_unused_fd_flags(0) instead of get_unused_fd()
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
Link: http://lkml.kernel.org/r/cover.1376327678.git.ydrone...@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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 03/10] android/sw_sync: use get_unused_fd_flags(O_CLOEXEC) instead of get_unused_fd()
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 call to
get_unused_fd_flags(O_CLOEXEC) following advice from Erik Gilling.
Signed-off-by: Yann Droneaud
Cc: Erik Gilling
Cc: Colin Cross
Link:
http://lkml.kernel.org/r/CACSP8SjZcpcpEtQHzcGYhf-MP7QGo0XpN7-uN7rmD=vNtopG=w...@mail.gmail.com
Link: http://lkml.kernel.org/r/cover.1376327678.git.ydrone...@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..f24493a 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(O_CLOEXEC);
int err;
struct sync_pt *pt;
struct sync_fence *fence;
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 06/10] binfmt_misc: use get_unused_fd_flags(0) instead of get_unused_fd()
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
Link: http://lkml.kernel.org/r/cover.1376327678.git.ydrone...@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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 10/10] file: remove get_unused_fd()
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 Link: http://lkml.kernel.org/r/cover.1376327678.git.ydrone...@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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 00/10] Getting rid of get_unused_fd_flags()
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 20130815, they're currently: - 19 calls to get_unused_fd_flags() (+4) not counting get_unused_fd() and anon_inode_getfd() - 10 calls to get_unused_fd() (-4) - 11 calls to anon_inode_getfd()(0) 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. Changes from v1 <http://lkml.kernel.org/r/cover.1372777600.git.ydrone...@opteya.com>: - 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 by <http://lkml.kernel.org/r/cacsp8sjxgmk2_kx_+rgzqqqwqkernvf1wt3k5tw991w5dfa...@mail.gmail.com> - android/sync: use get_unused_fd_flags(0) instead of get_unused_fd() MODIFIED: use get_unused_fd_flags(O_CLOEXEC) as suggested by <http://lkml.kernel.org/r/CACSP8SjZcpcpEtQHzcGYhf-MP7QGo0XpN7-uN7rmD=vNtopG=w...@mail.gmail.com> - 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. Yann Droneaud (10): 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() android/sw_sync: use get_unused_fd_flags(O_CLOEXEC) instead of get_unused_fd() android/sync: use get_unused_fd_flags(O_CLOEXEC) 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() events: 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/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 +- include/linux/file.h | 1 - kernel/events/core.c | 2 +- 10 files changed, 10 insertions(+), 11 deletions(-) -- 1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 09/10] events: use get_unused_fd_flags(0) instead of get_unused_fd()
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 Link: http://lkml.kernel.org/r/cover.1376327678.git.ydrone...@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 f25ce6d..a224a14 100644 --- a/kernel/events/core.c +++ b/kernel/events/core.c @@ -6891,7 +6891,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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] USB: serial: fix stringify operator in usb-serial-simple
From: Yann Droneaud
usb-serial-simple uses an unknown stringify macro that make
all drivers being named "stringify(vendor)".
This can be a problem when two drivers have the same (wrong) name:
kernel: usbcore: registered new interface driver usb_serial_simple
kernel: usbserial: USB Serial support registered for stringify(vendor)
kernel Error: Driver 'stringify(vendor)' is already registered, aborting...
kernel: usbserial: problem -16 when registering driver stringify(vendor)
kernel: usbserial: USB Serial deregistering driver stringify(vendor)
kernel: usbcore: deregistering interface driver usb_serial_simple
Before the fix:
$ strings drivers/usb/serial/usb-serial-simple.o
usb_serial_simple
stringify(vendor)
After the fix:
$ strings drivers/usb/serial/usb-serial-simple.o
usb_serial_simple
funsoft
flashloader
vivopay
moto_modem
hp4x
suunto
siemens_mpi
This patch makes usb-serial-simple use the correct stringify operator.
Signed-off-by: Yann Droneaud
---
drivers/usb/serial/usb-serial-simple.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/serial/usb-serial-simple.c
b/drivers/usb/serial/usb-serial-simple.c
index 6a06131..677c08c 100644
--- a/drivers/usb/serial/usb-serial-simple.c
+++ b/drivers/usb/serial/usb-serial-simple.c
@@ -29,7 +29,7 @@ static const struct usb_device_id vendor##_id_table[] = {
\
static struct usb_serial_driver vendor##_device = {\
.driver = { \
.owner =THIS_MODULE,\
- .name = "stringify(vendor)",\
+ .name = #vendor,\
}, \
.id_table = vendor##_id_table, \
.num_ports =1, \
--
1.8.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] USB: serial: fix stringify operator in usb-serial-simple
Le 18.08.2013 21:40, Greg Kroah-Hartman a écrit : On Sun, Aug 18, 2013 at 09:29:00PM +0200, Yann Droneaud wrote: Ugh, sorry about that, I thought there used to be a stringify() macro that used to do this. Nice patch, I'll queue it up. That's __stringify() which is defined in but: 1) inside a string (eg "__stringify(vendor)") it's gonna never work; 2) it's not required here, __stringify(vendor) would be needed if vendor was itself a macro and the macro content was to be converted to a string. Regards. -- Yann Droneaud OPTEYA -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/2] kvm: use anon_inode_getfd() with O_CLOEXEC flag
KVM uses anon_inode_get() to allocate file descriptors as part
of some of its ioctls. But those ioctls are lacking a flag argument
allowing userspace to choose options for the newly opened file descriptor.
In such case it's advised to use O_CLOEXEC by default so that
userspace is allowed to choose, without race, if the file descriptor
is going to be inherited across exec().
This patch set O_CLOEXEC flag on all file descriptors created
with anon_inode_getfd() to not leak file descriptors across exec().
Signed-off-by: Yann Droneaud
Link: http://lkml.kernel.org/r/cover.1377372576.git.ydrone...@opteya.com
---
virt/kvm/kvm_main.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 89f74d1..d65cc0c 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -1896,7 +1896,7 @@ static struct file_operations kvm_vcpu_fops = {
*/
static int create_vcpu_fd(struct kvm_vcpu *vcpu)
{
- return anon_inode_getfd("kvm-vcpu", &kvm_vcpu_fops, vcpu, O_RDWR);
+ return anon_inode_getfd("kvm-vcpu", &kvm_vcpu_fops, vcpu, O_RDWR |
O_CLOEXEC);
}
/*
@@ -2306,7 +2306,7 @@ static int kvm_ioctl_create_device(struct kvm *kvm,
return ret;
}
- ret = anon_inode_getfd(ops->name, &kvm_device_fops, dev, O_RDWR);
+ ret = anon_inode_getfd(ops->name, &kvm_device_fops, dev, O_RDWR |
O_CLOEXEC);
if (ret < 0) {
ops->destroy(dev);
return ret;
@@ -2590,7 +2590,7 @@ static int kvm_dev_ioctl_create_vm(unsigned long type)
return r;
}
#endif
- r = anon_inode_getfd("kvm-vm", &kvm_vm_fops, kvm, O_RDWR);
+ r = anon_inode_getfd("kvm-vm", &kvm_vm_fops, kvm, O_RDWR | O_CLOEXEC);
if (r < 0)
kvm_put_kvm(kvm);
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 0/2] kvm: use anon_inode_getfd() with O_CLOEXEC flag
Hi, Following a patchset asking to change calls to get_unused_flag() [1] to use O_CLOEXEC, Alex Williamson [2][3] decided to change VFIO to use the flag. Since it's a related subsystem to KVM, using O_CLOEXEC for file descriptors created by KVM might be applicable too. I'm suggesting to change calls to anon_inode_getfd() to use O_CLOEXEC as default flag. This patchset should be reviewed to not break existing userspace program. BTW, if it's not applicable, I would suggest that new ioctls be added to KVM subsystem, those ioctls would have a "flag" field added to their arguments. Such "flag" would let userspace choose the open flag to use. See for example other APIs using anon_inode_getfd() such as fanotify, inotify, signalfd and timerfd. You might be interested to read: - Secure File Descriptor Handling (Ulrich Drepper, 2008) http://udrepper.livejournal.com/20407.html - Excuse me son, but your code is leaking !!! (Dan Walsh, March 2012) http://danwalsh.livejournal.com/53603.html Regards. [1] http://lkml.kernel.org/r/cover.1376327678.git.ydrone...@opteya.com [2] http://lkml.kernel.org/r/1377186804.25163.17.ca...@ul30vt.home [3] http://lkml.kernel.org/r/20130822171744.1297.13711.st...@bling.home Yann Droneaud (2): kvm: use anon_inode_getfd() with O_CLOEXEC flag ppc: kvm: use anon_inode_getfd() with O_CLOEXEC flag arch/powerpc/kvm/book3s_64_mmu_hv.c | 2 +- arch/powerpc/kvm/book3s_64_vio.c| 2 +- arch/powerpc/kvm/book3s_hv.c| 2 +- virt/kvm/kvm_main.c | 6 +++--- 4 files changed, 6 insertions(+), 6 deletions(-) -- 1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/2] ppc: kvm: use anon_inode_getfd() with O_CLOEXEC flag
KVM uses anon_inode_get() to allocate file descriptors as part
of some of its ioctls. But those ioctls are lacking a flag argument
allowing userspace to choose options for the newly opened file descriptor.
In such case it's advised to use O_CLOEXEC by default so that
userspace is allowed to choose, without race, if the file descriptor
is going to be inherited across exec().
This patch set O_CLOEXEC flag on all file descriptors created
with anon_inode_getfd() to not leak file descriptors across exec().
Signed-off-by: Yann Droneaud
Link: http://lkml.kernel.org/r/cover.1377372576.git.ydrone...@opteya.com
---
arch/powerpc/kvm/book3s_64_mmu_hv.c | 2 +-
arch/powerpc/kvm/book3s_64_vio.c| 2 +-
arch/powerpc/kvm/book3s_hv.c| 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_64_mmu_hv.c
b/arch/powerpc/kvm/book3s_64_mmu_hv.c
index 710d313..f7c9e8a 100644
--- a/arch/powerpc/kvm/book3s_64_mmu_hv.c
+++ b/arch/powerpc/kvm/book3s_64_mmu_hv.c
@@ -1579,7 +1579,7 @@ int kvm_vm_ioctl_get_htab_fd(struct kvm *kvm, struct
kvm_get_htab_fd *ghf)
ctx->first_pass = 1;
rwflag = (ghf->flags & KVM_GET_HTAB_WRITE) ? O_WRONLY : O_RDONLY;
- ret = anon_inode_getfd("kvm-htab", &kvm_htab_fops, ctx, rwflag);
+ ret = anon_inode_getfd("kvm-htab", &kvm_htab_fops, ctx, rwflag |
O_CLOEXEC);
if (ret < 0) {
kvm_put_kvm(kvm);
return ret;
diff --git a/arch/powerpc/kvm/book3s_64_vio.c b/arch/powerpc/kvm/book3s_64_vio.c
index b2d3f3b..54cf9bc 100644
--- a/arch/powerpc/kvm/book3s_64_vio.c
+++ b/arch/powerpc/kvm/book3s_64_vio.c
@@ -136,7 +136,7 @@ long kvm_vm_ioctl_create_spapr_tce(struct kvm *kvm,
mutex_unlock(&kvm->lock);
return anon_inode_getfd("kvm-spapr-tce", &kvm_spapr_tce_fops,
- stt, O_RDWR);
+ stt, O_RDWR | O_CLOEXEC);
fail:
if (stt) {
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index e8d51cb..3503829 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -1556,7 +1556,7 @@ long kvm_vm_ioctl_allocate_rma(struct kvm *kvm, struct
kvm_allocate_rma *ret)
if (!ri)
return -ENOMEM;
- fd = anon_inode_getfd("kvm-rma", &kvm_rma_fops, ri, O_RDWR);
+ fd = anon_inode_getfd("kvm-rma", &kvm_rma_fops, ri, O_RDWR | O_CLOEXEC);
if (fd < 0)
kvm_release_rma(ri);
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
strange bug, alloca suspected
Hi,
I found a strange bug with modprobe/glibc
I supposed this is a bad interaction between gcc alloca(), glibc and modprobe.
Modprobe don't use alloca() correctly, then glibc failed. (stack corruption ?)
This need more investigation.
This mail is sent to glibc, gcc and modutils maintainers.
>Submitter-Id: net
>Originator:Yann Droneaud <[EMAIL PROTECTED]>
>Organization: no others than mine
>Confidential: no
>Synopsis: undetermined
>Severity: non-critical
>Priority: low
>Category: libc
>Class: sw-bug
>Release: libc-2.1.3
>Environment:
Host type: i586-pc-linux-gnu
System: Linux Corwin.Ambre 2.4.0-prerelease #1 Mon Jan 1 23:11:11 CET 2001 i586
unknown
Architecture: i586
Addons: crypt linuxthreads
Build CFLAGS: -g -O2 -march=i586
Build CC: gcc
Compiler version: 2.95.2 19991024 (release)
Kernel headers: 2.4.1
Symbol versioning: yes
Build static: yes
Build shared: yes
Build pic-default: no
Build profile: yes
Build omitfp: no
Build bounded: no
Build static-nss: no
Stdio: libio
compiled against linux 2.3.99-pre5 with gcc-2.95.2
Others:
modutils-2.4.2
configured with './configure --prefix=/usr --exec-prefix= --disable-combined
--enable-combined-rmmod --enable-combined-lsmod --disable-strip'
gcc 2.95.2
bash 2.03.0(1)-release
>Description:
>How-To-Repeat:
How to reproduce bug
on shell prompt, as root type:
# /sbin/modprobe --help
just display help, no problem.
try this now:
# modprobe --help
display help,
but finish by a Segmentation Fault
What a strange behaviour, isn't it ?
>Fix:
A little taste of debugging:
I decide to debug modprobe.
I added a signal handler for SIGSEGV.
The handler only wait for the debugger.
0x400951f1 in __libc_nanosleep () from /lib/libc.so.6
(gdb) bt
#0 0x400951f1 in __libc_nanosleep () from /lib/libc.so.6
#1 0x40095188 in __sleep (seconds=1) at ../sysdeps/unix/sysv/linux/sleep.c:82
#2 0x804ba3a in sighandler ()
#3
#4 tdestroy_recurse (root=0x0, freefct=0x400f1d20 <_IO_2_1_stderr_>) at
tsearch.c:637
#5 0x100 in ?? ()
(gdb) directory /tmp/glibc-2.1.3/misc
Source directories searched: /tmp/glibc-2.1.3/misc:$cdir:$cwd
(gdb) frame 4
#4 tdestroy_recurse (root=0x0, freefct=0x400f1d20 <_IO_2_1_stderr_>) at
tsearch.c:637
637 if (root->left != NULL)
(gdb) list
632tree cannot be removed easily. We provide a function to do this. */
633 static void
634 internal_function
635 tdestroy_recurse (node root, __free_fn_t freefct)
636 {
637 if (root->left != NULL)
638 tdestroy_recurse (root->left, freefct);
639 if (root->right != NULL)
640 tdestroy_recurse (root->right, freefct);
641 (*freefct) ((void *) root->key);
(gdb) print root
$1 = 0x0
I wasn't able to find how/where this function could be call with a NULL
argument.
A little patch for glibc 2.1.3, misc/tsearch.c
=
--- tsearch.c Thu Nov 6 00:18:09 1997
+++ tsearch-meuh.c Sun Feb 11 23:54:37 2001
@@ -634,13 +634,22 @@
internal_function
tdestroy_recurse (node root, __free_fn_t freefct)
{
- if (root->left != NULL)
-tdestroy_recurse (root->left, freefct);
- if (root->right != NULL)
-tdestroy_recurse (root->right, freefct);
- (*freefct) ((void *) root->key);
- /* Free the node itself. */
- free (root);
+ if (root != NULL)
+{
+ if (root->left != NULL)
+ tdestroy_recurse (root->left, freefct);
+ if (root->right != NULL)
+ tdestroy_recurse (root->right, freefct);
+ (*freefct) ((void *) root->key);
+ /* Free the node itself. */
+ free (root);
+}
+#ifdef DEBUGGING
+ else
+{
+ assert(root != NULL);
+}
+#endif /* DEBUGGING */
}
void
===
For modprobe
I review the code before the getopt_long(), I found the alloca() call did not
reserve room for the last NUL char
The only thing important is the l++;
so the patch:
--- modutils-2.4.2/insmod/modprobe.cFri Jan 19 07:26:33 2001
+++ modutils-2.4.2-debug/insmod/modprobe.c Mon Feb 12 00:00:58 2001
@@ -33,6 +33,9 @@
#include
#include
#include
+#include
#include "module.h"
#include "obj.h"
#include "modstat.h"
@@ -1476,6 +1479,25 @@
);
}
+#ifdef DEBUG
+void sighandler(int sig)
+{
+ int i;
+ static int wait_for_gdb;
+ printf("%d: signal SIGSEGV: %d, waiting for debugger\n", getpid(), sig);
+ while (wait_for_gdb == 0)
+sleep(1);
+ /* wait about 10 seconds */
+ for(i = 0; i < 10 ; i++)
+sleep(1);
+ exit(1);
+}
+#endif
int main(int argc, char *argv[])
{
int ret = 0;
@@ -1506,10 +1528,18 @@
int i, l = 0;
char *command;
check_dis(c|k)_change(|d): duplicated code
I was looking up linux code to find why /proc/partitions report 'hdc' instead of 'ide/host0/bus1/target0/lun0/cd' example: major minor #blocks name 8 01048575 scsi/host0/bus0/target6/lun0/disc 3 03140928 ide/host0/bus0/target0/lun0/disc 3 1 4000 ide/host0/bus0/target0/lun0/part1 3 23132864 ide/host0/bus0/target0/lun0/part2 3643167640 ide/host0/bus0/target1/lun0/disc 365 1 ide/host0/bus0/target1/lun0/part1 367 899136 ide/host0/bus0/target1/lun0/part3 368 165312 ide/host0/bus0/target1/lun0/part4 3691052289 ide/host0/bus0/target1/lun0/part5 370 983776 ide/host0/bus0/target1/lun0/part6 371 64480 ide/host0/bus0/target1/lun0/part7 22 0 648180 hdc I found a strange thing in 'fs/block_dev.c' there's a function 'check_disk_change' and in 'fs/devfs/base.c' there's a function 'check_disc_changed' They done exactly the same job. I think that one of these must be rewritted to call the other and it's not to me to tell you which one :-). -- Yann Droneaud <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
fs block/devfs check disc change
This is a re-post I found in 'fs/block_dev.c' there's a function 'check_disk_change' and in 'fs/devfs/base.c' there's a function 'check_disc_changed' They done exactly the same job. Their source code are quite the same. I think that one of these must be rewritted to call/use the other and it's not to me to tell you which one :-). -- Yann Droneaud <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/3] coccinelle: also catch kzfree() issues
Hi,
Le mardi 21 juin 2016 à 11:43 +0200, Michal Marek a écrit :
> Dne 20.6.2016 v 22:21 Julia Lawall napsal(a):
> > On Mon, 20 Jun 2016, Michal Marek wrote:
> > On 2016-05-23 17:18, Julia Lawall wrote:
> > > > On Mon, 23 May 2016, Yann Droneaud wrote:
> > > >
> > > > > Since commit 3ef0e5ba4673 ('slab: introduce kzfree()'),
> > > > > kfree() is no more the only function to be considered:
> > > > > kzfree() should be recognized too.
> > > > >
> > > > > In particular, kzfree() must not be called on memory
> > > > > allocated through devm_*() functions.
> > > > >
> > > > > Cc: Johannes Weiner
> > > > > Signed-off-by: Yann Droneaud
> > > >
> > > > Acked-by: Julia Lawall
> > >
> > > Hi Julia,
> > >
> > > does your ACK apply to the other two patches as well?
> >
> > Sorry, I seem to have missed the other two. I have reviewed them
> > now, and the ack applies to all three. Thanks for checking on it.
>
Back in February, those two other patches were already Acked-by:
http://lkml.kernel.org/r/alpine.DEB.2.10.1602161818100.2685@hadrien
http://lkml.kernel.org/r/alpine.DEB.2.10.1602161819340.2685@hadrien
I've (re)sent them with added Acked-by:, and thought it would not
require further Acked-by:.
Anyway, this one was new and required proper review. Thanks a lot.
> Np. Applied to kbuild.git#misc now.
>
Thanks a lot.
--
Yann Droneaud
OPTEYA
Re: [kernel-hardening] [RFC patch 1/6] random: Simplify API for random address requests
Hi,
Le mardi 26 juillet 2016 à 03:01 +, Jason Cooper a écrit :
> To date, all callers of randomize_range() have set the length to 0,
> and check for a zero return value. For the current callers, the only
> way to get zero returned is if end <= start. Since they are all
> adding a constant to the start address, this is unnecessary.
>
I agree.
> We can remove a bunch of needless checks by simplifying the API to do
> just what everyone wants, return an address between [start, start +
> range].
>
I agree.
For the record:
http://lkml.kernel.org/r/cover.1390770607.git.ydrone...@opteya.com
> While we're here, s/get_random_int/get_random_long/. No current call
> site is adversely affected by get_random_int(), since all current
> range requests are < MAX_UINT. However, we should match caller
> expectations to avoid coming up short (ha!) in the future.
>
> Signed-off-by: Jason Cooper
> ---
> drivers/char/random.c | 17 -
> include/linux/random.h | 2 +-
> 2 files changed, 5 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/char/random.c b/drivers/char/random.c
> index 0158d3bff7e5..1251cb2cbab2 100644
> --- a/drivers/char/random.c
> +++ b/drivers/char/random.c
> @@ -1822,22 +1822,13 @@ unsigned long get_random_long(void)
> EXPORT_SYMBOL(get_random_long);
>
> /*
> - * randomize_range() returns a start address such that
> - *
> - *[.. .]
> - * start end
> - *
> - * a with size "len" starting at the return value is inside
> in the
> - * area defined by [start, end], but is otherwise randomized.
> + * randomize_addr() returns a page aligned address within [start,
> start +
> + * range]
> */
> unsigned long
> -randomize_range(unsigned long start, unsigned long end, unsigned
> long len)
> +randomize_addr(unsigned long start, unsigned long range)
> {
> - unsigned long range = end - len - start;
> -
> - if (end <= start + len)
> - return 0;
> - return PAGE_ALIGN(get_random_int() % range + start);
> + return PAGE_ALIGN(get_random_long() % range + start);
> }
>
> /* Interface for in-kernel drivers of true hardware RNGs.
> diff --git a/include/linux/random.h b/include/linux/random.h
> index e47e533742b5..1ad877a98186 100644
> --- a/include/linux/random.h
> +++ b/include/linux/random.h
> @@ -34,7 +34,7 @@ extern const struct file_operations random_fops,
> urandom_fops;
>
> unsigned int get_random_int(void);
> unsigned long get_random_long(void);
> -unsigned long randomize_range(unsigned long start, unsigned long
> end, unsigned long len);
> +unsigned long randomize_addr(unsigned long start, unsigned long
> range);
>
> u32 prandom_u32(void);
> void prandom_bytes(void *buf, size_t nbytes);
Re: [PATCH 1/7] random: Simplify API for random address requests
Hi,
Le jeudi 28 juillet 2016 à 20:47 +, Jason Cooper a écrit :
> To date, all callers of randomize_range() have set the length to 0,
> and check for a zero return value. For the current callers, the only
> way to get zero returned is if end <= start. Since they are all
> adding a constant to the start address, this is unnecessary.
>
> We can remove a bunch of needless checks by simplifying the API to do
> just what everyone wants, return an address between [start, start +
> range).
>
> While we're here, s/get_random_int/get_random_long/. No current call
> site is adversely affected by get_random_int(), since all current
> range requests are < UINT_MAX. However, we should match caller
> expectations to avoid coming up short (ha!) in the future.
>
> Address generation within [start, start + range) behavior is
> preserved.
>
> All current callers to randomize_range() chose to use the start
> address if randomize_range() failed. Therefore, we simplify things
> by just returning the start address on error.
>
> randomize_range() will be removed once all callers have been
> converted over to randomize_addr().
>
> Signed-off-by: Jason Cooper
> ---
> drivers/char/random.c | 26 ++
> include/linux/random.h | 1 +
> 2 files changed, 27 insertions(+)
>
> diff --git a/drivers/char/random.c b/drivers/char/random.c
> index 0158d3bff7e5..3610774bcc53 100644
> --- a/drivers/char/random.c
> +++ b/drivers/char/random.c
> @@ -1840,6 +1840,32 @@ randomize_range(unsigned long start, unsigned
> long end, unsigned long len)
> return PAGE_ALIGN(get_random_int() % range + start);
> }
>
> +/**
> + * randomize_addr - Generate a random, page aligned address
> + * @start: The smallest acceptable address the caller will take.
> + * @range: The size of the area, starting at @start, within which the
> + * random address must fall.
> + *
> + * Before page alignment, the random address generated can be any value from
> + * @start, to @start + @range - 1 inclusive.
> + *
> + * If @start + @range would overflow, @range is capped.
> + *
> + * Return: A page aligned address within [start, start + range).
PAGE_ALIGN(start + range - 1) can be greater than start + range ..
In the worst case, when start = 0, range = ULONG_MAX, the result would
be 0.
In order to stay in the bounds, the start address must be rounded up,
and the random offset must be rounded down.
Something I haven't found the time to send was looking like this:
unsigned long base = PAGE_ALIGN(start);
range -= (base - start);
range >>= PAGE_SHIFT;
return base + ((get_random_int() % range) << PAGE_SHIFT);
> On error,
> + * @start is returned.
> + */
> +unsigned long
> +randomize_addr(unsigned long start, unsigned long range)
> +{
> + if (range == 0)
> + return start;
> +
> + if (start > ULONG_MAX - range)
> + range = ULONG_MAX - start;
> +
> + return PAGE_ALIGN(get_random_long() % range + start);
> +}
> +
> /* Interface for in-kernel drivers of true hardware RNGs.
> * Those devices may produce endless random bits and will be throttled
> * when our pool is full.
> diff --git a/include/linux/random.h b/include/linux/random.h
> index e47e533742b5..f1ca2fa4c071 100644
> --- a/include/linux/random.h
> +++ b/include/linux/random.h
> @@ -35,6 +35,7 @@ extern const struct file_operations random_fops,
> urandom_fops;
> unsigned int get_random_int(void);
> unsigned long get_random_long(void);
> unsigned long randomize_range(unsigned long start, unsigned long end,
> unsigned long len);
> +unsigned long randomize_addr(unsigned long start, unsigned long range);
>
> u32 prandom_u32(void);
> void prandom_bytes(void *buf, size_t nbytes);
Regards.
--
Yann Droneaud
OPTEYA
Re: [PATCH] timerfd: expose uapi header
Hi,
Le lundi 05 octobre 2015 à 16:53 +0200, Gabriel Laskar a écrit :
> this patch expose the timerfd apis to the userland. It is already in
> glibc header sys/timerfd.h but not synchronised, and missing the
> ioctl
> number definition.
>
> Signed-off-by: Gabriel Laskar
> ---
> include/linux/timerfd.h | 20 +---
> include/uapi/linux/Kbuild| 1 +
> include/uapi/linux/timerfd.h | 31 +++
> 3 files changed, 33 insertions(+), 19 deletions(-)
> create mode 100644 include/uapi/linux/timerfd.h
>
> diff --git a/include/linux/timerfd.h b/include/linux/timerfd.h
> index bd36ce4..bab0b1a 100644
> --- a/include/linux/timerfd.h
> +++ b/include/linux/timerfd.h
> @@ -8,23 +8,7 @@
> #ifndef _LINUX_TIMERFD_H
> #define _LINUX_TIMERFD_H
>
> -/* For O_CLOEXEC and O_NONBLOCK */
> -#include
> -
> -/* For _IO helpers */
> -#include
> -
> -/*
> - * CAREFUL: Check include/asm-generic/fcntl.h when defining
> - * new flags, since they might collide with O_* ones. We want
> - * to re-use O_* flags that couldn't possibly have a meaning
> - * from eventfd, in order to leave a free define-space for
> - * shared O_* flags.
> - */
> -#define TFD_TIMER_ABSTIME (1 << 0)
> -#define TFD_TIMER_CANCEL_ON_SET (1 << 1)
> -#define TFD_CLOEXEC O_CLOEXEC
> -#define TFD_NONBLOCK O_NONBLOCK
> +#include
>
> #define TFD_SHARED_FCNTL_FLAGS (TFD_CLOEXEC | TFD_NONBLOCK)
> /* Flags for timerfd_create. */
> @@ -32,6 +16,4 @@
> /* Flags for timerfd_settime. */
> #define TFD_SETTIME_FLAGS (TFD_TIMER_ABSTIME |
> TFD_TIMER_CANCEL_ON_SET)
>
> -#define TFD_IOC_SET_TICKS_IOW('T', 0, u64)
> -
> #endif /* _LINUX_TIMERFD_H */
> diff --git a/include/uapi/linux/Kbuild b/include/uapi/linux/Kbuild
> index f7b2db4..874ac3f 100644
> --- a/include/uapi/linux/Kbuild
> +++ b/include/uapi/linux/Kbuild
> @@ -395,6 +395,7 @@ header-y += tcp_metrics.h
> header-y += telephony.h
> header-y += termios.h
> header-y += thermal.h
> +header-y += tiemrfd.h
Typo here ...
That should have been caught with make headers_install
> header-y += time.h
> header-y += times.h
> header-y += timex.h
> diff --git a/include/uapi/linux/timerfd.h
> b/include/uapi/linux/timerfd.h
> new file mode 100644
> index 000..69a2f92
> --- /dev/null
> +++ b/include/uapi/linux/timerfd.h
> @@ -0,0 +1,31 @@
> +/*
> + * include/uapi/linux/timerfd.h
> + *
> + * Copyright (C) 2007 Davide Libenzi
> + *
> + */
> +
> +#ifndef _UAPI_LINUX_TIMERFD_H
> +#define _UAPI_LINUX_TIMERFD_H
> +
> +/* For O_CLOEXEC and O_NONBLOCK */
> +#include
> +
> +/* For _IO helpers */
> +#include
> +
> +/*
> + * CAREFUL: Check include/asm-generic/fcntl.h when defining
> + * new flags, since they might collide with O_* ones. We want
> + * to re-use O_* flags that couldn't possibly have a meaning
> + * from eventfd, in order to leave a free define-space for
> + * shared O_* flags.
> + */
> +#define TFD_TIMER_ABSTIME (1 << 0)
> +#define TFD_TIMER_CANCEL_ON_SET (1 << 1)
> +#define TFD_CLOEXEC O_CLOEXEC
> +#define TFD_NONBLOCK O_NONBLOCK
> +
> +#define TFD_IOC_SET_TICKS_IOW('T', 0, u64)
> +
> +#endif /* _UAPI_LINUX_TIMERFD_H */
Regards.
--
Yann Droneaud
OPTEYA
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 3/3] falloc: expose ioctl numbers into uapi
Hi,
Le mardi 06 octobre 2015 à 16:27 +0200, Gabriel Laskar a écrit :
> Signed-off-by: Gabriel Laskar
> ---
>
> v2: missed the include of
>
> include/linux/falloc.h | 18 --
> include/uapi/linux/falloc.h | 20
> 2 files changed, 20 insertions(+), 18 deletions(-)
>
> diff --git a/include/uapi/linux/falloc.h
> b/include/uapi/linux/falloc.h
> index 3e445a7..28abee4 100644
> --- a/include/uapi/linux/falloc.h
> +++ b/include/uapi/linux/falloc.h
> @@ -1,6 +1,8 @@
> #ifndef _UAPI_FALLOC_H_
> #define _UAPI_FALLOC_H_
>
> +#include
> +
You should also add #include for _IOW() macro
> #define FALLOC_FL_KEEP_SIZE 0x01 /* default is extend size */
> #define FALLOC_FL_PUNCH_HOLE 0x02 /* de-allocates range */
> #define FALLOC_FL_NO_HIDE_STALE 0x04 /* reserved codepoint */
> @@ -58,4 +60,22 @@
> */
> #define FALLOC_FL_INSERT_RANGE 0x20
>
> +
> +/*
> + * Space reservation ioctls and argument structure
> + * are designed to be compatible with the legacy XFS ioctls.
> + */
> +struct space_resv {
> + __s16 l_type;
> + __s16 l_whence;
> + __s64 l_start;
> + __s64 l_len; /* len == 0 means
> until end of file */
> + __s32 l_sysid;
> + __u32 l_pid;
> + __s32 l_pad[4]; /* reserved area */
> +};
> +
> +#define FS_IOC_RESVSP_IOW('X', 40, struct
> space_resv)
> +#define FS_IOC_RESVSP64 _IOW('X', 42, struct
> space_resv)
> +
> #endif /* _UAPI_FALLOC_H_ */
Regards.
--
Yann Droneaud
OPTEYA
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] IB/core: avoid 32-bit warning
Hi,
Le mercredi 07 octobre 2015 à 14:29 +0200, Arnd Bergmann a écrit :
> The INIT_UDATA() macro requires a pointer or unsigned long argument
> for both input and output buffer, and all callers had a cast from
> when the code was merged until a recent restructuring, so now we get
>
> core/uverbs_cmd.c: In function 'ib_uverbs_create_cq':
> core/uverbs_cmd.c:1481:66: warning: cast to pointer from integer of
> different size [-Wint-to-pointer-cast]
>
> This makes the code behave as before by adding back the cast to
> unsigned long.
>
> Signed-off-by: Arnd Bergmann
> Fixes: 565197dd8fb1 ("IB/core: Extend ib_uverbs_create_cq")
>
Reviewed-by: Yann Droneaud
> diff --git a/drivers/infiniband/core/uverbs_cmd.c
> b/drivers/infiniband/core/uverbs_cmd.c
> index be4cb9f04be3..88b3b78340f2 100644
> --- a/drivers/infiniband/core/uverbs_cmd.c
> +++ b/drivers/infiniband/core/uverbs_cmd.c
> @@ -1478,7 +1478,7 @@ ssize_t ib_uverbs_create_cq(struct
> ib_uverbs_file *file,
> if (copy_from_user(&cmd, buf, sizeof(cmd)))
> return -EFAULT;
>
> - INIT_UDATA(&ucore, buf, cmd.response, sizeof(cmd),
> sizeof(resp));
> + INIT_UDATA(&ucore, buf, (unsigned long)cmd.response,
> sizeof(cmd), sizeof(resp));
>
> INIT_UDATA(&uhw, buf + sizeof(cmd),
> (unsigned long)cmd.response + sizeof(resp),
>
Regards.
--
Yann Droneaud
OPTEYA
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] IB/core: avoid 32-bit warning
Hi,
Le mercredi 07 octobre 2015 à 16:19 +0300, Sagi Grimberg a écrit :
> On 10/7/2015 3:29 PM, Arnd Bergmann wrote:
> > The INIT_UDATA() macro requires a pointer or unsigned long argument
> > for
> > both input and output buffer, and all callers had a cast from when
> > the code was merged until a recent restructuring, so now we get
> >
> > core/uverbs_cmd.c: In function 'ib_uverbs_create_cq':
> > core/uverbs_cmd.c:1481:66: warning: cast to pointer from integer of
> > different size [-Wint-to-pointer-cast]
> >
> > This makes the code behave as before by adding back the cast to
> > unsigned long.
> >
> > Signed-off-by: Arnd Bergmann
> > Fixes: 565197dd8fb1 ("IB/core: Extend ib_uverbs_create_cq")
> >
> > diff --git a/drivers/infiniband/core/uverbs_cmd.c
> > b/drivers/infiniband/core/uverbs_cmd.c
> > index be4cb9f04be3..88b3b78340f2 100644
> > --- a/drivers/infiniband/core/uverbs_cmd.c
> > +++ b/drivers/infiniband/core/uverbs_cmd.c
> > @@ -1478,7 +1478,7 @@ ssize_t ib_uverbs_create_cq(struct
> > ib_uverbs_file *file,
> > if (copy_from_user(&cmd, buf, sizeof(cmd)))
> > return -EFAULT;
> >
> > - INIT_UDATA(&ucore, buf, cmd.response, sizeof(cmd),
> > sizeof(resp));
> > + INIT_UDATA(&ucore, buf, (unsigned long)cmd.response,
> > sizeof(cmd), sizeof(resp));
>
> Would it make sense to cast inside INIT_UDATA() and not have callers
> worry about it?
It's ... complicated. See INIT_UDATA_BUF_OR_NULL().
Awayway, I have patch to do the opposite, eg. explicitly cast u64 value
to (void __user *)(unsigned long) in the caller function instead, as it
allows some safer / new operations on the response buffer.
Regards.
--
Yann Droneaud
OPTEYA
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[-stable] perf/x86: Fix copy_from_user_nmi() return if range is not ok
Hi,
I believe commit ebf2d2689de551d90965090bb991fc640a0c0d41
("perf/x86: Fix copy_from_user_nmi() return if range is not ok")
should be applied to stable kernels starting from v3.13.y up to v4.1.y.
Regards.
--
Yann Droneaud
OPTEYA
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 0/2] perf tool: improve error handling in perf_flag_probe()
Hi,
Following the EBUSY errors reported by Jiri Olsa [1], I've tryed to
improve a bit the way perf_flag_probe() handle errors.
In case EBUSY is returned by perf_event_open(), testing the function
again without PERF_FLAG_FD_CLOEXEC is meaningless: EBUSY is not
related to close-on-exec flag, so there's nothing to confirm.
For other errors, not yet explicitly handled by perf_flag_probe(),
it's pointless to report a second error message for the same error
code: the second check should not print an error if the error is
the same as the one returned for the first check.
Changes from v1 [2]:
- resend
Changes from v0 [3]:
- rebased on top of commit 48536c9195ae ("perf tools: Fix probing for
PERF_FLAG_FD_CLOEXEC flag");
- update a bit commit message;
- the previous patchset was acked by Jiri[4], but I haven't
reported Acked-by: in this updated patchset.
[1] http://lkml.kernel.org/r/1406908014-8312-1-git-send-email-jo...@kernel.org
[2] http://lkml.kernel.org/r/cover.1425901229.git.ydrone...@opteya.com
[3] http://lkml.kernel.org/r/cover.1410595700.git.ydrone...@opteya.com
[4] http://lkml.kernel.org/r/20140920121438.gb15...@krava.brq.redhat.com
Yann Droneaud (2):
perf tools: shortcut PERF_FLAG_FD_CLOEXEC probing in case of EBUSY
error
perf tools: report PERF_FLAG_FD_CLOEXEC probing error once
tools/perf/util/cloexec.c | 27 ---
1 file changed, 16 insertions(+), 11 deletions(-)
--
2.4.3
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 1/2] perf tools: shortcut PERF_FLAG_FD_CLOEXEC probing in case of EBUSY error
This patch is a simplification of the logic introduced as part of
commit 63914aca8f7e ('perf tools: Show better error message in case we fail to
open counters due to EBUSY error'):
if EBUSY is reported by perf_event_open(), it will not be possible
to probe PERF_FLAG_FD_CLOEXEC, so it's safe to leave early.
It should be noted that -EBUSY errors are not really expected here
since commit 038fa0b9739d ('perf tools: Fix PERF_FLAG_FD_CLOEXEC flag probing
event type open counters due to EBUSY error'):
the perf event type used now should be safe to use regarding -EBUSY
error.
Cc: Adrian Hunter
Cc: David Ahern
Cc: Frederic Weisbecker
Cc: Jiri Olsa
Cc: Namhyung Kim
Cc: Paul Mackerras
Cc: Peter Zijlstra
Cc: Stephane Eranian
Cc: William Cohen
Link: http://lkml.kernel.org/r/cover.1443708092.git.ydrone...@opteya.com
Signed-off-by: Yann Droneaud
---
tools/perf/util/cloexec.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/tools/perf/util/cloexec.c b/tools/perf/util/cloexec.c
index 2babddaa2481..be226d293c91 100644
--- a/tools/perf/util/cloexec.c
+++ b/tools/perf/util/cloexec.c
@@ -56,7 +56,11 @@ static int perf_flag_probe(void)
return 1;
}
- WARN_ONCE(err != EINVAL && err != EBUSY,
+ /* ignore busy errors */
+ if (err == EBUSY)
+ return -1;
+
+ WARN_ONCE(err != EINVAL,
"perf_event_open(..., PERF_FLAG_FD_CLOEXEC) failed with
unexpected error %d (%s)\n",
err, strerror_r(err, sbuf, sizeof(sbuf)));
@@ -74,7 +78,7 @@ static int perf_flag_probe(void)
if (fd >= 0)
close(fd);
- if (WARN_ONCE(fd < 0 && err != EBUSY,
+ if (WARN_ONCE(fd < 0,
"perf_event_open(..., 0) failed unexpectedly with error
%d (%s)\n",
err, strerror_r(err, sbuf, sizeof(sbuf
return -1;
--
2.4.3
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 2/2] perf tools: report PERF_FLAG_FD_CLOEXEC probing error once
In case of failure, unrelated to PERF_FLAG_FD_CLOEXEC,
perf_flag_probe() reports the error twice. For example:
$ perf record ls
Error:
perf_event_open(..., PERF_FLAG_FD_CLOEXEC) failed with unexpected error 16
(Device or resource busy)
perf_event_open(..., 0) failed unexpectedly with error 16 (Device or resource
busy)
There's no need for the second error message, so this
patch changes the function to only report a second
error message when the two calls to perf_even_open(2)
fail with different error codes.
Cc: Adrian Hunter
Cc: David Ahern
Cc: Frederic Weisbecker
Cc: Jiri Olsa
Cc: Namhyung Kim
Cc: Paul Mackerras
Cc: Peter Zijlstra
Cc: Stephane Eranian
Cc: William Cohen
Link: http://lkml.kernel.org/r/cover.1443708092.git.ydrone...@opteya.com
Reported-by: Jiri Olsa
Signed-off-by: Yann Droneaud
---
tools/perf/util/cloexec.c | 25 +
1 file changed, 13 insertions(+), 12 deletions(-)
diff --git a/tools/perf/util/cloexec.c b/tools/perf/util/cloexec.c
index be226d293c91..552528698566 100644
--- a/tools/perf/util/cloexec.c
+++ b/tools/perf/util/cloexec.c
@@ -26,7 +26,7 @@ static int perf_flag_probe(void)
.exclude_kernel = 1,
};
int fd;
- int err;
+ int err0, err1;
int cpu;
pid_t pid = -1;
char sbuf[STRERR_BUFSIZE];
@@ -49,7 +49,7 @@ static int perf_flag_probe(void)
}
break;
}
- err = errno;
+ err0 = errno;
if (fd >= 0) {
close(fd);
@@ -57,12 +57,12 @@ static int perf_flag_probe(void)
}
/* ignore busy errors */
- if (err == EBUSY)
+ if (err0 == EBUSY)
return -1;
- WARN_ONCE(err != EINVAL,
+ WARN_ONCE(err0 != EINVAL,
"perf_event_open(..., PERF_FLAG_FD_CLOEXEC) failed with
unexpected error %d (%s)\n",
- err, strerror_r(err, sbuf, sizeof(sbuf)));
+ err0, strerror_r(err0, sbuf, sizeof(sbuf)));
/* not supported, confirm error related to PERF_FLAG_FD_CLOEXEC */
while (1) {
@@ -73,17 +73,18 @@ static int perf_flag_probe(void)
}
break;
}
- err = errno;
+ err1 = errno;
- if (fd >= 0)
+ if (fd >= 0) {
close(fd);
+ return 0;
+ }
- if (WARN_ONCE(fd < 0,
- "perf_event_open(..., 0) failed unexpectedly with error
%d (%s)\n",
- err, strerror_r(err, sbuf, sizeof(sbuf
- return -1;
+ WARN_ONCE(err0 != err1,
+ "perf_event_open(..., 0) failed unexpectedly with error %d
(%s)\n",
+ err1, strerror_r(err1, sbuf, sizeof(sbuf)));
- return 0;
+ return -1;
}
unsigned long perf_event_open_cloexec_flag(void)
--
2.4.3
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] uapi: remove i2o header in uapi
Hi,
Le jeudi 01 octobre 2015 à 17:15 +0200, Gabriel Laskar a écrit :
> Remove uapi header for i2o subsystem that has been removed by
> commit 4a72a7af462d ("staging: remove i2o subsystem")
>
> CC: Greg Kroah-Hartman
> Signed-off-by: Gabriel Laskar
> ---
Don't miss to update the Kbuild file.
diff --git a/include/uapi/linux/Kbuild b/include/uapi/linux/Kbuild
index e77707802dcc..470b2f6e2193 100644
--- a/include/uapi/linux/Kbuild
+++ b/include/uapi/linux/Kbuild
@@ -151,7 +151,6 @@ header-y += hyperv.h
header-y += hysdn_if.h
header-y += i2c-dev.h
header-y += i2c.h
-header-y += i2o-dev.h
header-y += i8k.h
header-y += icmp.h
header-y += icmpv6.h
Note, you may also use git format-patch -D to reduce the size of your
patch.
Regards.
--
Yann Droneaud
OPTEYA
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/2] perf tools: shortcut PERF_FLAG_FD_CLOEXEC probing in case of EBUSY error
Hi,
Le jeudi 01 octobre 2015 à 22:52 +0200, Jiri Olsa a écrit :
> On Thu, Oct 01, 2015 at 05:16:25PM +0200, Yann Droneaud wrote:
> > This patch is a simplification of the logic introduced as part of
> > commit 63914aca8f7e ('perf tools: Show better error message in case
> > we fail to open counters due to EBUSY error'):
> > if EBUSY is reported by perf_event_open(), it will not be possible
> > to probe PERF_FLAG_FD_CLOEXEC, so it's safe to leave early.
> >
> > It should be noted that -EBUSY errors are not really expected here
> > since commit 038fa0b9739d ('perf tools: Fix PERF_FLAG_FD_CLOEXEC
> > flag probing event type open counters due to EBUSY error'):
> > the perf event type used now should be safe to use regarding -EBUSY
> > error.
> >
> > Cc: Adrian Hunter
> > Cc: David Ahern
> > Cc: Frederic Weisbecker
> > Cc: Jiri Olsa
> > Cc: Namhyung Kim
> > Cc: Paul Mackerras
> > Cc: Peter Zijlstra
> > Cc: Stephane Eranian
> > Cc: William Cohen
> > Link:
> > http://lkml.kernel.org/r/cover.1443708092.git.ydrone...@opteya.com
> > Signed-off-by: Yann Droneaud
> > ---
> > tools/perf/util/cloexec.c | 8 ++--
> > 1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/perf/util/cloexec.c b/tools/perf/util/cloexec.c
> > index 2babddaa2481..be226d293c91 100644
> > --- a/tools/perf/util/cloexec.c
> > +++ b/tools/perf/util/cloexec.c
> > @@ -56,7 +56,11 @@ static int perf_flag_probe(void)
> > return 1;
> > }
> >
> > - WARN_ONCE(err != EINVAL && err != EBUSY,
> > + /* ignore busy errors */
> > + if (err == EBUSY)
> > + return -1;
>
> so what's now the error message in case we get EBUSY ?
>
None, just as it does currently.
perf_event_open_cloexec_flag() will still will return 0 just like
PERF_FLAG_FD_CLOEXEC was not available.
Handling -EBUSY is left to the function which will try to make use of
perf_event_open().
Regards.
--
Yann Droneaud
OPTEYA
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 2/2] perf tools: report PERF_FLAG_FD_CLOEXEC probing error once
Le vendredi 02 octobre 2015 à 15:20 +0200, Jiri Olsa a écrit : > On Thu, Oct 01, 2015 at 05:16:26PM +0200, Yann Droneaud wrote: > > In case of failure, unrelated to PERF_FLAG_FD_CLOEXEC, > > perf_flag_probe() reports the error twice. For example: > > > > $ perf record ls > > Error: > > perf_event_open(..., PERF_FLAG_FD_CLOEXEC) failed with unexpected > > error 16 (Device or resource busy) > > perf_event_open(..., 0) failed unexpectedly with error 16 (Device > > or resource busy) > > so this error message is no longer possible due to your earlier patch > right? > My bad, the error used to illustrate the behavior is not well choosed. > could you please provide current 'before&after' output into changelog > otherwise it looks ok > OK. I'm updating the message. Regards. -- Yann Droneaud OPTEYA -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] UAPI: Document auxvec AT_* namespace policy and note reservations
Hi, Le mercredi 16 mai 2018 à 15:20 +0100, Dave Martin a écrit : > There are constraints on defining AT_* auxvec tags that are not > obvious to the casual maintainer of either the global > or the arch-specific headers. This is likely > to lead to mistakes. (I certainly fell foul of it...) > > For the benefit of future maintainers, this patch collects the > relevant information in one place, documenting how the namespace > needs to be managed, and noting all the values currently in use. > > Maintaining a global list may result in some merge conflicts, but > AT_* values are not added frequently. I'm open to suggestions on > the best approach. > > I also assume that values 38 and 39 may have been used for > historical purposes, such as an architecture that is no longer > supported. If they have definitely never been used for anything, > they could be removed from the "reserved" list. > Some of those AT_* values are described in getauxval(3) man-page: http://man7.org/linux/man-pages/man3/getauxval.3.html https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/tree/man3/g etauxval.3?id=4eae8eb731386d81797d5c30365426722410874e And glibc provides with definitions for almost all AT_*, regardless of the current target architecture: https://sourceware.org/git/?p=glibc.git;a=blob;f=elf/elf.h;h=954f3266f7 11ab83996670ea504a17dcf668e061;hb=23158b08a0908f381459f273a984c6fd32836 3cb#l1135 Also, despite not being listed as a reserved namespace by POSIX, one should try to avoid name collision with other AT_ constants, those used with *at() functions (openat(), etc.): - AT_EACCESS - AT_EMPTY_PATH - AT_FDCWD - AT_NO_AUTOMOUNT - AT_REMOVEDIR - AT_STATX_DONT_SYNC - AT_STATX_FORCE_SYNC - AT_STATX_SYNC_AS_STAT - AT_SYMLINK_FOLLOW - AT_SYMLINK_NOFOLLOW http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/fcntl.h.html http://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.htm l#tag_15_02_02 https://sourceware.org/git/?p=glibc.git;a=blob;f=io/fcntl.h;h=3d239e8f0 9f7ce0a3106621be327e1ea4cd1a3e7;hb=23158b08a0908f381459f273a984c6fd3283 63cb#l142 And there's also AT_ANYNET and AT_ANYNODE from ddp (aka. AppleTalk) http://man7.org/linux/man-pages/man7/ddp.7.html Regards. -- Yann Droneaud OPTEYA
Re: [PATCH] kbuild: remove old information in headers_install.txt document
Hi, Le mardi 02 décembre 2014 à 13:28 +0900, Masahiro Yamada a écrit : > The arch header directories "include/asm-*" were moved long before. > > Signed-off-by: Masahiro Yamada > --- > > Documentation/kbuild/headers_install.txt | 5 + > 1 file changed, 1 insertion(+), 4 deletions(-) > > diff --git a/Documentation/kbuild/headers_install.txt > b/Documentation/kbuild/headers_install.txt > index 951eb9f..0d8fe5e 100644 > --- a/Documentation/kbuild/headers_install.txt > +++ b/Documentation/kbuild/headers_install.txt > @@ -28,10 +28,7 @@ optional arguments: > > ARCH indicates which architecture to produce headers for, and defaults to the > current architecture. The linux/asm directory of the exported kernel headers > -is platform-specific, to see a complete list of supported architectures use > -the command: > - > - ls -d include/asm-* | sed 's/.*-//' > +is platform-specific. > > INSTALL_HDR_PATH indicates where to install the headers. It defaults to > "./usr/include". Back in september, I've already sent a patch[1] to address this, following a discussion with Sam Ravnborg[2]. See the mail thread[3]. Regards. [1] http://lkml.kernel.org/r/b5181cf7604baa454c11f7aa92d07dd05349ce46.1410712841.git.ydrone...@opteya.com [2] http://lkml.kernel.org/r/20140713212608.gb16...@ravnborg.org [3] http://lkml.kernel.org/g/cover.1410712841.git.ydrone...@opteya.com -- Yann Droneaud OPTEYA -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: CVE-2014-8159 kernel: infiniband: uverbs: unprotected physical memory access
Hi, Le mercredi 18 mars 2015 à 17:39 +, Shachar Raindel a écrit : > Hi, > > It was found that the Linux kernel's InfiniBand/RDMA subsystem did not > properly sanitize input parameters while registering memory regions > from user space via the (u)verbs API. A local user with access to > a /dev/infiniband/uverbsX device could use this flaw to crash the > system or, potentially, escalate their privileges on the system. > > The issue has been assigned CVE-2014-8159. > > The issue exists in the InfiniBand/RDMA/iWARP drivers since Linux > Kernel version 2.6.13. > > Mellanox OFED 2.4-1.0.4 fixes the issue. Available from: > http://www.mellanox.com/page/products_dyn?product_family=26&mtag=linux_sw_drivers > > > RedHat errata: https://access.redhat.com/security/cve/CVE-2014-8159 > Canonical errata: > http://people.canonical.com/~ubuntu-security/cve/2014/CVE-2014-8159.html > Novell (Suse) bug tracking: https://bugzilla.novell.com/show_bug.cgi?id=914742 > > > The following patch fixes the issue: > > --- 8< -- > > From d4d68430d4a12c569e28b4f4468284ea2286 Mon Sep 17 00:00:00 2001 > From: Shachar Raindel > Date: Sun, 04 Jan 2015 18:30:32 +0200 > Subject: [PATCH] IB/core: Prevent integer overflow in ib_umem_get address > arithmetic > > Properly verify that the resulting page aligned end address is larger > than both the start address and the length of the memory area > requested. > > Both the start and length arguments for ib_umem_get are controlled by > the user. A misbehaving user can provide values which will cause an > integer overflow when calculating the page aligned end address. > > This overflow can cause also miscalculation of the number of pages > mapped, and additional logic issues. > > Signed-off-by: Shachar Raindel > Signed-off-by: Jack Morgenstein > Signed-off-by: Or Gerlitz > --- > > diff --git a/drivers/infiniband/core/umem.c b/drivers/infiniband/core/umem.c > index aec7a6a..8c014b5 100644 > --- a/drivers/infiniband/core/umem.c > +++ b/drivers/infiniband/core/umem.c > @@ -99,6 +99,14 @@ > if (dmasync) > dma_set_attr(DMA_ATTR_WRITE_BARRIER, &attrs); > > + /* > + * If the combination of the addr and size requested for this memory > + * region causes an integer overflow, return error. > + */ > + if ((PAGE_ALIGN(addr + size) <= size) || > + (PAGE_ALIGN(addr + size) <= addr)) > + return ERR_PTR(-EINVAL); > + Can access_ok() be used here ? if (!access_ok(writable ? VERIFY_WRITE : VERIFY_READ, addr, size)) return ERR_PTR(-EINVAL); > if (!can_do_mlock()) > return ERR_PTR(-EPERM); > Regards. -- Yann Droneaud OPTEYA -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: CVE-2014-8159 kernel: infiniband: uverbs: unprotected physical memory access
Hi, Le jeudi 02 avril 2015 à 10:52 +, Shachar Raindel a écrit : > > -Original Message- > > From: Yann Droneaud [mailto:ydrone...@opteya.com] > > Sent: Thursday, April 02, 2015 1:05 PM > > Le mercredi 18 mars 2015 à 17:39 +, Shachar Raindel a écrit : > > > + /* > > > + * If the combination of the addr and size requested for this > > memory > > > + * region causes an integer overflow, return error. > > > + */ > > > + if ((PAGE_ALIGN(addr + size) <= size) || > > > + (PAGE_ALIGN(addr + size) <= addr)) > > > + return ERR_PTR(-EINVAL); > > > + > > > > Can access_ok() be used here ? > > > > if (!access_ok(writable ? VERIFY_WRITE : VERIFY_READ, > > addr, size)) > > return ERR_PTR(-EINVAL); > > > > No, this will break the current ODP semantics. > > ODP allows the user to register memory that is not accessible yet. > This is a critical design feature, as it allows avoiding holding > a registration cache. Adding this check will break the behavior, > forcing memory to be all accessible when registering an ODP MR. > Where's the check for the range being in userspace memory space, especially for the ODP case ? For non ODP case (eg. plain old behavior), does get_user_pages() ensure the requested pages fit in userspace region on all architectures ? I think so. In ODP case, I'm not sure such check is ever done ? (Aside, does it take special mesure to protect shared mapping from being read and/or *written* ?) Regards. -- Yann Droneaud OPTEYA -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: CVE-2014-8159 kernel: infiniband: uverbs: unprotected physical memory access
Hi, Le jeudi 02 avril 2015 à 10:52 +, Shachar Raindel a écrit : > > -Original Message- > > From: Yann Droneaud [mailto:ydrone...@opteya.com] > > Sent: Thursday, April 02, 2015 1:05 PM > > Le mercredi 18 mars 2015 à 17:39 +, Shachar Raindel a écrit : ... > > > + /* > > > + * If the combination of the addr and size requested for this > > memory > > > + * region causes an integer overflow, return error. > > > + */ > > > + if ((PAGE_ALIGN(addr + size) <= size) || > > > + (PAGE_ALIGN(addr + size) <= addr)) > > > + return ERR_PTR(-EINVAL); > > > + > > > > Can access_ok() be used here ? > > > > if (!access_ok(writable ? VERIFY_WRITE : VERIFY_READ, > > addr, size)) > > return ERR_PTR(-EINVAL); > > > > No, this will break the current ODP semantics. > > ODP allows the user to register memory that is not accessible yet. > This is a critical design feature, as it allows avoiding holding > a registration cache. Adding this check will break the behavior, > forcing memory to be all accessible when registering an ODP MR. > Failed to notice previously, but since this would break ODP, and ODP is only available starting v3.19-rc1, my proposed fix might be applicable for older kernel (if not better). >From 2a3beaeb4b35d968f127cb59cfda2f12dbd87e4b Mon Sep 17 00:00:00 2001 From: Yann Droneaud Date: Thu, 2 Apr 2015 17:01:05 +0200 Subject: [RFC PATCH] IB/core: reject invalid userspace memory range registration Signed-off-by: Yann Droneaud --- drivers/infiniband/core/umem.c | 23 +++ 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/drivers/infiniband/core/umem.c b/drivers/infiniband/core/umem.c index df0c4f605a21..6758b4ac56eb 100644 --- a/drivers/infiniband/core/umem.c +++ b/drivers/infiniband/core/umem.c @@ -90,6 +90,7 @@ struct ib_umem *ib_umem_get(struct ib_ucontext *context, unsigned long addr, DEFINE_DMA_ATTRS(attrs); struct scatterlist *sg, *sg_list_start; int need_release = 0; + bool writable; if (dmasync) dma_set_attr(DMA_ATTR_WRITE_BARRIER, &attrs); @@ -97,6 +98,19 @@ struct ib_umem *ib_umem_get(struct ib_ucontext *context, unsigned long addr, if (!can_do_mlock()) return ERR_PTR(-EPERM); + /* +* We ask for writable memory if any access flags other than +* "remote read" are set. "Local write" and "remote write" +* obviously require write access. "Remote atomic" can do +* things like fetch and add, which will modify memory, and +* "MW bind" can change permissions by binding a window. +*/ + writable = !!(access & ~IB_ACCESS_REMOTE_READ); + + if (!access_ok(writable ? VERIFY_WRITE : VERIFY_READ, + (void __user *)addr, size)) + return ERR_PTR(-EFAULT); + umem = kzalloc(sizeof *umem, GFP_KERNEL); if (!umem) return ERR_PTR(-ENOMEM); @@ -106,14 +120,7 @@ struct ib_umem *ib_umem_get(struct ib_ucontext *context, unsigned long addr, umem->offset= addr & ~PAGE_MASK; umem->page_size = PAGE_SIZE; umem->pid = get_task_pid(current, PIDTYPE_PID); - /* -* We ask for writable memory if any access flags other than -* "remote read" are set. "Local write" and "remote write" -* obviously require write access. "Remote atomic" can do -* things like fetch and add, which will modify memory, and -* "MW bind" can change permissions by binding a window. -*/ - umem->writable = !!(access & ~IB_ACCESS_REMOTE_READ); + umem->writable = writable; /* We assume the memory is from hugetlb until proved otherwise */ umem->hugetlb = 1; -- 2.1.0 Regards. -- Yann Droneaud OPTEYA -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: CVE-2014-8159 kernel: infiniband: uverbs: unprotected physical memory access
Hi Haggai, Le jeudi 02 avril 2015 à 18:18 +0300, Haggai Eran a écrit : > On 02/04/2015 16:30, Yann Droneaud wrote: > > Hi, > > > > Le jeudi 02 avril 2015 à 10:52 +, Shachar Raindel a écrit : > >>> -Original Message- > >>> From: Yann Droneaud [mailto:ydrone...@opteya.com] > >>> Sent: Thursday, April 02, 2015 1:05 PM > >>> Le mercredi 18 mars 2015 à 17:39 +, Shachar Raindel a écrit : > > > >>>> +/* > >>>> + * If the combination of the addr and size requested for this > >>> memory > >>>> + * region causes an integer overflow, return error. > >>>> + */ > >>>> +if ((PAGE_ALIGN(addr + size) <= size) || > >>>> +(PAGE_ALIGN(addr + size) <= addr)) > >>>> +return ERR_PTR(-EINVAL); > >>>> + > >>> > >>> Can access_ok() be used here ? > >>> > >>> if (!access_ok(writable ? VERIFY_WRITE : VERIFY_READ, > >>> addr, size)) > >>> return ERR_PTR(-EINVAL); > >>> > >> > >> No, this will break the current ODP semantics. > >> > >> ODP allows the user to register memory that is not accessible yet. > >> This is a critical design feature, as it allows avoiding holding > >> a registration cache. Adding this check will break the behavior, > >> forcing memory to be all accessible when registering an ODP MR. > >> > > > > Where's the check for the range being in userspace memory space, > > especially for the ODP case ? > > > > For non ODP case (eg. plain old behavior), does get_user_pages() > > ensure the requested pages fit in userspace region on all > > architectures ? I think so. > > Yes, get_user_pages will return a smaller amount of pages than requested > if it encounters an unmapped region (or a region without write > permissions for write requests). If this happens, the loop in > ib_umem_get calls get_user_pages again with the next set of pages, and > this time if it the first page still cannot be mapped an error is returned. > > > > > In ODP case, I'm not sure such check is ever done ? > > In ODP, we also call get_user_pages, but only when a page fault occurs > (see ib_umem_odp_map_dma_pages()). This allows the user to pre-register > a memory region that contains unmapped virtual space, and then mmap > different files into that area without needing to re-register. > OK, thanks for the description. > > (Aside, does it take special mesure to protect shared mapping from > > being read and/or *written* ?) > > I'm not sure I understand the question. Shared mappings that the process > is allowed to read or write are also allowed for the HCA (specifically, > to local and remote operations the same process performs using the HCA), > provided the application has registered their virtual address space as a > memory region. > I was refering to description of get_user_pages(): http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/mm/gup.c?id=v4.0-rc6#n765 * @force: whether to force access even when user mapping is currently * protected (but never forces write access to shared mapping). But since ib_umem_odp_map_dma_pages() use get_user_pages() with force argument set to 0, it's OK. Another related question: as the large memory range could be registered by user space with ibv_reg_mr(pd, base, size, IB_ACCESS_ON_DEMAND), what's prevent the kernel to map a file as the result of mmap(0, ...) in this region, making it available remotely through IBV_WR_RDMA_READ / IBV_WR_RDMA_WRITE ? Again, thanks for the information I was missing to understand how ODP is checking the memory ranges. Regards. -- Yann Droneaud OPTEYA -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: CVE-2014-8159 kernel: infiniband: uverbs: unprotected physical memory access
Hi, Le jeudi 02 avril 2015 à 16:44 +, Shachar Raindel a écrit : > > -Original Message- > > From: Yann Droneaud [mailto:ydrone...@opteya.com] > > Sent: Thursday, April 02, 2015 7:35 PM > > Another related question: as the large memory range could be registered > > by user space with ibv_reg_mr(pd, base, size, IB_ACCESS_ON_DEMAND), > > what's prevent the kernel to map a file as the result of mmap(0, ...) > > in this region, making it available remotely through IBV_WR_RDMA_READ / > > IBV_WR_RDMA_WRITE ? > > > > This is not a bug. This is a feature. > > Exposing a file through RDMA, using ODP, can be done exactly like this. > Given that the application explicitly requested this behavior, I don't > see why it is a problem. If the application cannot choose what will end up in the region it has registered, it's an issue ! What might happen if one library in a program call mmap(0, size, ...) to load a file storing a secret (a private key), and that file ends up being mapped in an registered but otherwise free region (afaict, the kernel is allowed to do it) ? What might happen if one library in a program call call mmap(0, size, ..., MAP_ANONYMOUS,...) to allocate memory, call mlock(), then write in this location a secret (a passphrase), and that area ends up in the memory region registered for on demand paging ? The application haven't choose to disclose these confidential piece of information, but they are available for reading/writing by remote through RDMA given it knows the rkey of the memory region (which is a 32bits value). I hope I'm missing something, because I'm not feeling confident such behavor is a feature. > Actually, some of our tests use such flows. > The mmu notifiers mechanism allow us to do this safely. When the page is > written back to disk, it is removed from the ODP mapping. When it is > accessed by the HCA, it is brought back to RAM. > I'm not discussing about the benefit of On Demand Paging and why it's a very good feature to expose files through RDMA. I'm trying to understand how the application can choose what is exposed through RDMA if it registers a very large memory region for later use (but do not actually explicitly map something there yet): what's the consequences ? void *start = sbrk(0); size_t size = ULONG_MAX - (unsigned long)start; ibv_reg_mr(pd, start, size, IB_ACCESS_ON_DEMAND) Regards. -- Yann Droneaud OPTEYA -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: CVE-2014-8159 kernel: infiniband: uverbs: unprotected physical memory access
Hi, Le vendredi 03 avril 2015 à 08:39 +, Haggai Eran a écrit : > On Thursday, April 2, 2015 11:40 PM, Yann Droneaud > wrote: > > Le jeudi 02 avril 2015 à 16:44 +, Shachar Raindel a écrit : > >> > -Original Message- > >> > From: Yann Droneaud [mailto:ydrone...@opteya.com] > >> > Sent: Thursday, April 02, 2015 7:35 PM > > > >> > Another related question: as the large memory range could be registered > >> > by user space with ibv_reg_mr(pd, base, size, IB_ACCESS_ON_DEMAND), > >> > what's prevent the kernel to map a file as the result of mmap(0, ...) > >> > in this region, making it available remotely through IBV_WR_RDMA_READ / > >> > IBV_WR_RDMA_WRITE ? > >> > > >> > >> This is not a bug. This is a feature. > >> > >> Exposing a file through RDMA, using ODP, can be done exactly like this. > >> Given that the application explicitly requested this behavior, I don't > >> see why it is a problem. > > > > If the application cannot choose what will end up in the region it has > > registered, it's an issue ! > > > > What might happen if one library in a program call mmap(0, size, ...) to > > load a file storing a secret (a private key), and that file ends up > > being mapped in an registered but otherwise free region (afaict, the > > kernel is allowed to do it) ? > > What might happen if one library in a program call call mmap(0, > > size, ..., MAP_ANONYMOUS,...) to allocate memory, call mlock(), then > > write in this location a secret (a passphrase), and that area ends up > > in the memory region registered for on demand paging ? > > > > The application haven't choose to disclose these confidential piece of > > information, but they are available for reading/writing by remote > > through RDMA given it knows the rkey of the memory region (which is a > > 32bits value). > > > > I hope I'm missing something, because I'm not feeling confident such > > behavor is a feature. > > What we are aiming for is the possibility to register the entire process' > address > space for RDMA operations (if the process chooses to use this feature). > This is similar to multiple threads accessing the same address space. I'm > sure > you wouldn't be complaining about the ability of one thread to access the > secret > passphrase mmapped by another thread in your example. > > > I'm trying to understand how the application can choose what is exposed > > through RDMA if it registers a very large memory region for later use > > (but do not actually explicitly map something there yet): what's the > > consequences ? > > > >void *start = sbrk(0); > >size_t size = ULONG_MAX - (unsigned long)start; > > > >ibv_reg_mr(pd, start, size, IB_ACCESS_ON_DEMAND) > > The consequences are exactly as you wrote. Just as giving a non-ODP rkey > to a remote node allows the node to access the registered memory behind that > rkey, giving an ODP rkey to a remote node allows that node to access the > virtual address space behind that rkey. > There's a difference: it's impossible to give a valid non-ODP rkey that point to a memory region not already mapped (backed by a file for example), so the application *choose* the content of the memory to be made accessible remotely before making it accessible. As I understand the last explanation regarding ODP, at creation time, an ODP rkey can point to a free, unused, unallocated memory portion. At this point the kernel can happily map anything the application (and its libraries) want to map at a (almost) *random* address that could be in (or partially in) the ODP memory region. And I have a problem with such random behavior. Allowing this is seems dangerous and should be done with care. I believe the application must kept the control of what's end up in its ODP registered memory region. Especially for multi thread program: imagine one thread creating a large memory region for its future purposes, then send the rkey to a remote peer and wait for some work to be done. In the mean time another call mmap(0, ...) to map a file at a kernel chosen address, and that address happen to be in the memory region registered by the other thread: 1) the first thread is amputated from a portion of memory it was willing to use; 2) the data used by the second thread is accessible to the remote peer(s) while not expected. Speculatively registering memory seems dangerous for any use case I could think of. Regards. -- Yann Droneaud OPTEYA -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH linux-next 1/4] infiniband/ipoib: fix possible NULL pointer dereference in ipoib_get_iflink
Hi Nicolas,
Le mardi 14 avril 2015 à 17:49 +0200, Nicolas Dichtel a écrit :
> Le 14/04/2015 17:44, Honggang LI a écrit :
> > On Tue, Apr 14, 2015 at 08:34:33AM -0700, Eric Dumazet wrote:
> >> On Tue, 2015-04-14 at 23:20 +0800, Honggang Li wrote:
> >>> Starting monitoring for VG vg_rdma01: 3 logical volume(s) in volume
> >>> group "vg_rdma01" monitored
> >>> [ OK ]
> >>
> >>
> >>> CR2: 0120
> >>> ---[ end trace a8610f6e9640eb85 ]---
> >>>
> >>> Signed-off-by: Honggang Li
> >>
> >> When was this bug added ?
> >>
> >
> > Sorry, I do not know. I ran into this bug today when I'm tracing a race
> > condition issue related qib. In order to check upstream linux-next fix
> > the race condition or not, I build linux-next-20150414 on two machines. Both
> > machines panic at modprobe ib_ipoib. Do you means I need to report a
> > bug? But I do not know report to who or where.
>
> Here is the tag:
> Fixes: 5aa7add8f14b ("infiniband/ipoib: implement ndo_get_iflink")
>
Pardon me, but this patch was never submitted to
linux-r...@vger.kernel.org for review !?
Regards.
--
Yann Droneaud
OPTEYA
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: CVE-2014-8159 kernel: infiniband: uverbs: unprotected physical memory access
Hi,
Le jeudi 02 avril 2015 à 16:34 +, Shachar Raindel a écrit :
> > -Original Message-
> > From: Yann Droneaud [mailto:ydrone...@opteya.com]
> > Sent: Thursday, April 02, 2015 6:16 PM
> > Le jeudi 02 avril 2015 à 10:52 +, Shachar Raindel a écrit :
> > > > -Original Message-
> > > > From: Yann Droneaud [mailto:ydrone...@opteya.com]
> > > > Sent: Thursday, April 02, 2015 1:05 PM
> > > > Le mercredi 18 mars 2015 à 17:39 +, Shachar Raindel a écrit :
> > ...
> > > > > + /*
> > > > > + * If the combination of the addr and size requested for this
> > > > memory
> > > > > + * region causes an integer overflow, return error.
> > > > > + */
> > > > > + if ((PAGE_ALIGN(addr + size) <= size) ||
> > > > > + (PAGE_ALIGN(addr + size) <= addr))
> > > > > + return ERR_PTR(-EINVAL);
> > > > > +
> > > >
> > > > Can access_ok() be used here ?
> > > >
> > > > if (!access_ok(writable ? VERIFY_WRITE : VERIFY_READ,
> > > > addr, size))
> > > > return ERR_PTR(-EINVAL);
> > > >
> > >
> > > No, this will break the current ODP semantics.
> > >
> > > ODP allows the user to register memory that is not accessible yet.
> > > This is a critical design feature, as it allows avoiding holding
> > > a registration cache. Adding this check will break the behavior,
> > > forcing memory to be all accessible when registering an ODP MR.
> > >
> >
> > Failed to notice previously, but since this would break ODP, and ODP is
> > only available starting v3.19-rc1, my proposed fix might be applicable
> > for older kernel (if not better).
> >
>
> Can you explain how this proposed fix is better than the existing patch?
> Why do we want to push to the stable tree a patch that is not in the
> upstream? There is an existing, tested, patch that is going to the tip
> of the development. It even applies cleanly on every kernel version around.
>
access_ok() check for overflow *and* that the region is the memory range
for the current process. The later check is not done in your proposed
fix (but it should not be needed as get_user_pages() will be called
to validate the whole region for non-ODP memory registration).
Anyway, AFAIK access_ok() won't check for address being not NULL and
size not being 0, and I've noticed your proposed fix also ensure address
is not equal to NULL and, more important, that size is not equal to 0:
before v3.15-rc1 and commit eeb8461e36c9 ("IB: Refactor umem to use
linear SG table"), calling ib_umem_get() with size equal to 0 would
succeed with any arbitrary address ... who knows what might happen in
the lowlevel drivers (aka. providers) if they got an umem for a 0-sized
memory region.
This part of the changes was not detailled in your commit message: it's
an issue not related to overflow which is addressed by your patch.
So I agree my proposed patch is no better than yours: I've missed the
0-sized memory region issue and didn't take care of NULL address.
Regards.
--
Yann Droneaud
OPTEYA
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: CVE-2014-8159 kernel: infiniband: uverbs: unprotected physical memory access
Hi,
Le mercredi 08 avril 2015 à 14:19 +0200, Yann Droneaud a écrit :
> Le jeudi 02 avril 2015 à 16:34 +, Shachar Raindel a écrit :
> > > -Original Message-
> > > From: Yann Droneaud [mailto:ydrone...@opteya.com]
> > > Sent: Thursday, April 02, 2015 6:16 PM
> > > Le jeudi 02 avril 2015 à 10:52 +, Shachar Raindel a écrit :
> > > > > -----Original Message-
> > > > > From: Yann Droneaud [mailto:ydrone...@opteya.com]
> > > > > Sent: Thursday, April 02, 2015 1:05 PM
> > > > > Le mercredi 18 mars 2015 à 17:39 +, Shachar Raindel a écrit :
> > > ...
> > > > > > + /*
> > > > > > +* If the combination of the addr and size requested for this
> > > > > memory
> > > > > > +* region causes an integer overflow, return error.
> > > > > > +*/
> > > > > > + if ((PAGE_ALIGN(addr + size) <= size) ||
> > > > > > + (PAGE_ALIGN(addr + size) <= addr))
> > > > > > + return ERR_PTR(-EINVAL);
> > > > > > +
> > > > >
> > > > > Can access_ok() be used here ?
> > > > >
> > > > > if (!access_ok(writable ? VERIFY_WRITE : VERIFY_READ,
> > > > > addr, size))
> > > > > return ERR_PTR(-EINVAL);
> > > > >
> > > >
> > > > No, this will break the current ODP semantics.
> > > >
> > > > ODP allows the user to register memory that is not accessible yet.
> > > > This is a critical design feature, as it allows avoiding holding
> > > > a registration cache. Adding this check will break the behavior,
> > > > forcing memory to be all accessible when registering an ODP MR.
> > > >
> > >
> > > Failed to notice previously, but since this would break ODP, and ODP is
> > > only available starting v3.19-rc1, my proposed fix might be applicable
> > > for older kernel (if not better).
> > >
> >
> > Can you explain how this proposed fix is better than the existing patch?
> > Why do we want to push to the stable tree a patch that is not in the
> > upstream? There is an existing, tested, patch that is going to the tip
> > of the development. It even applies cleanly on every kernel version around.
> >
>
> access_ok() check for overflow *and* that the region is the memory range
> for the current process. The later check is not done in your proposed
> fix (but it should not be needed as get_user_pages() will be called
> to validate the whole region for non-ODP memory registration).
>
> Anyway, AFAIK access_ok() won't check for address being not NULL and
> size not being 0, and I've noticed your proposed fix also ensure address
> is not equal to NULL and, more important, that size is not equal to 0
It only check address not being 0 if size is already PAGE_SIZE aligned,
and it only check size not being 0 if address is already PAGE_SIZE
aligned.
> before v3.15-rc1 and commit eeb8461e36c9 ("IB: Refactor umem to use
> linear SG table"), calling ib_umem_get() with size equal to 0 would
> succeed with any arbitrary address ... who knows what might happen in
> the lowlevel drivers (aka. providers) if they got an umem for a 0-sized
> memory region.
> This part of the changes was not detailled in your commit message: it's
> an issue not related to overflow which is addressed by your patch.
>
> So I agree my proposed patch is no better than yours: I've missed the
> 0-sized memory region issue and didn't take care of NULL address.
>
Regards.
--
Yann Droneaud
OPTEYA
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: CVE-2014-8159 kernel: infiniband: uverbs: unprotected physical memory access
Hi, Le jeudi 02 avril 2015 à 18:12 +, Haggai Eran a écrit : > On Thursday, April 2, 2015 7:44 PM, Shachar Raindel wrote: > >> -Original Message- > >> From: Yann Droneaud [mailto:ydrone...@opteya.com] > >> Le jeudi 02 avril 2015 à 18:18 +0300, Haggai Eran a écrit : > >> > On 02/04/2015 16:30, Yann Droneaud wrote: > >> >> Le jeudi 02 avril 2015 à 10:52 +, Shachar Raindel a écrit : > >> >>>> -Original Message- > >> >>>> From: Yann Droneaud [mailto:ydrone...@opteya.com] > >> >>>> Sent: Thursday, April 02, 2015 1:05 PM > >> >>>> Le mercredi 18 mars 2015 à 17:39 +, Shachar Raindel a écrit : > >> >> > >> >>>>> + /* > >> >>>>> + * If the combination of the addr and size requested for this > >> >>>> memory > >> >>>>> + * region causes an integer overflow, return error. > >> >>>>> + */ > >> >>>>> + if ((PAGE_ALIGN(addr + size) <= size) || > >> >>>>> + (PAGE_ALIGN(addr + size) <= addr)) > >> >>>>> + return ERR_PTR(-EINVAL); > >> >>>>> + > >> >>>> > >> >>>> Can access_ok() be used here ? > >> >>>> > >> >>>> if (!access_ok(writable ? VERIFY_WRITE : VERIFY_READ, > >> >>>> addr, size)) > >> >>>> return ERR_PTR(-EINVAL); > >> >>>> > >> >>> > >> >>> No, this will break the current ODP semantics. > >> >>> > >> >>> ODP allows the user to register memory that is not accessible yet. > >> >>> This is a critical design feature, as it allows avoiding holding > >> >>> a registration cache. Adding this check will break the behavior, > >> >>> forcing memory to be all accessible when registering an ODP MR. > >> >>> > >> >> > >> >> Where's the check for the range being in userspace memory space, > >> >> especially for the ODP case ? > >> >> > >> >> For non ODP case (eg. plain old behavior), does get_user_pages() > >> >> ensure the requested pages fit in userspace region on all > >> >> architectures ? I think so. > >> > > >> > Yes, get_user_pages will return a smaller amount of pages than > >> requested > >> > if it encounters an unmapped region (or a region without write > >> > permissions for write requests). If this happens, the loop in > >> > ib_umem_get calls get_user_pages again with the next set of pages, and > >> > this time if it the first page still cannot be mapped an error is > >> returned. > >> > > >> >> > >> >> In ODP case, I'm not sure such check is ever done ? > >> > > >> > In ODP, we also call get_user_pages, but only when a page fault occurs > >> > (see ib_umem_odp_map_dma_pages()). This allows the user to pre- > >> register > >> > a memory region that contains unmapped virtual space, and then mmap > >> > different files into that area without needing to re-register. > >> > > >> > >> OK, thanks for the description. > >> > >> ... > >> > >> Another related question: as the large memory range could be registered > >> by user space with ibv_reg_mr(pd, base, size, IB_ACCESS_ON_DEMAND), > >> what's prevent the kernel to map a file as the result of mmap(0, ...) > >> in this region, making it available remotely through IBV_WR_RDMA_READ / > >> IBV_WR_RDMA_WRITE ? > >> > > > > This is not a bug. This is a feature. > > > > Exposing a file through RDMA, using ODP, can be done exactly like this. > > Given that the application explicitly requested this behavior, I don't > > see why it is a problem. Actually, some of our tests use such flows. > > The mmu notifiers mechanism allow us to do this safely. When the page is > > written back to disk, it is removed from the ODP mapping. When it is > > accessed by the HCA, it is brought back to RAM. > > > > I want to add that we would like to see users registering a very large > memory region (perhaps the entire process address space) for local > access, and then enabling remote access only to specific
[PATCH] x86: fix copy_from_user_nmi() return if range is not ok
Commit 0a196848ca36 ("perf: Fix arch_perf_out_copy_user default"),
changes copy_from_user_nmi() to return the number of
remaining bytes so that it behave like copy_from_user().
Unfortunately, when the range is outside of the process,
memory the return value is still the number of byte
copied, eg. 0, instead of the remaining bytes.
As all users of copy_from_user_nmi() were modified as
part of commit 0a196848ca36, the function should be
fixed to return the total number of bytes if range is
not correct.
Signed-off-by: Yann Droneaud
---
arch/x86/lib/usercopy.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/lib/usercopy.c b/arch/x86/lib/usercopy.c
index ddf9ecb53cc3..e342586db6e4 100644
--- a/arch/x86/lib/usercopy.c
+++ b/arch/x86/lib/usercopy.c
@@ -20,7 +20,7 @@ copy_from_user_nmi(void *to, const void __user *from,
unsigned long n)
unsigned long ret;
if (__range_not_ok(from, n, TASK_SIZE))
- return 0;
+ return n;
/*
* Even though this function is typically called from NMI/IRQ context
--
2.4.3
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v1 0/3] No more seq_file pre-allocation
Hi, Please find a revised patchset to remove support for passing pre-allocated struct seq_file to seq_open(). Such feature is undocumented and prone to error. In particular, if seq_release() is used in release handler, it will kfree() a pointer which was not allocated by seq_open(). So, please find a patchset that drop the support for pre-allocated struct seq_file: it's only of use in proc_namespace.c and can be easily replaced by using seq_open_private()/seq_release_private(). Additionally, it documents the use of file->private_data to hold pointer to struct seq_file by seq_open(). Changes from v0 [0]: - convert kmalloc() + memset() to kzalloc() - revised a bit commit messages [0] [PATCH 0/3] seq_file allocation in seq_open() http://lkml.kernel.org/r/cover.1430777196.git.ydrone...@opteya.com http://lkml.kernel.org/g/cover.1430777196.git.ydrone...@opteya.com Yann Droneaud (3): fs: use seq_open_private() for proc_mounts fs: allocate structure unconditionally in seq_open() fs: documents seq_open()'s usage of file->private_data fs/mount.h | 3 --- fs/namespace.c | 6 +++--- fs/proc_namespace.c | 34 -- fs/seq_file.c | 19 +++ 4 files changed, 30 insertions(+), 32 deletions(-) -- 2.4.2 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v1 2/3] fs: allocate structure unconditionally in seq_open()
Since patch described below, from v2.6.15-rc1, seq_open()
could use a struct seq_file already allocated by the
caller if the pointer to the structure is stored in
file->private_data before calling the function.
Commit 1abe77b0fc4b485927f1f798ae81a752677e1d05
Author: Al Viro
Date: Mon Nov 7 17:15:34 2005 -0500
[PATCH] allow callers of seq_open do allocation themselves
Allow caller of seq_open() to kmalloc() seq_file + whatever else they
want and set ->private_data to it. seq_open() will then abstain from
doing allocation itself.
As there's no more use for such feature, as it could
be easily replaced by calls to seq_open_private()
(see commit 39699037a5c9 ("[FS] seq_file: Introduce
the seq_open_private()")) and seq_release_private()
(see v2.6.0-test3), support for this uncommon feature
can be removed from seq_open().
Link: http://lkml.kernel.org/r/cover.1433193673.git.ydrone...@opteya.com
Signed-off-by: Yann Droneaud
---
fs/seq_file.c | 17 +
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/fs/seq_file.c b/fs/seq_file.c
index c14f6a43beb5..a909f12dad4d 100644
--- a/fs/seq_file.c
+++ b/fs/seq_file.c
@@ -51,15 +51,16 @@ static void *seq_buf_alloc(unsigned long size)
*/
int seq_open(struct file *file, const struct seq_operations *op)
{
- struct seq_file *p = file->private_data;
+ struct seq_file *p;
+
+ WARN_ON(file->private_data);
+
+ p = kzalloc(sizeof(*p), GFP_KERNEL);
+ if (!p)
+ return -ENOMEM;
+
+ file->private_data = p;
- if (!p) {
- p = kmalloc(sizeof(*p), GFP_KERNEL);
- if (!p)
- return -ENOMEM;
- file->private_data = p;
- }
- memset(p, 0, sizeof(*p));
mutex_init(&p->lock);
p->op = op;
#ifdef CONFIG_USER_NS
--
2.4.2
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v1 3/3] fs: documents seq_open()'s usage of file->private_data
seq_open() store its struct seq_file in file->private_data,
thus, it must not be modified by user of seq_file.
Link: http://lkml.kernel.org/r/cover.1433193673.git.ydrone...@opteya.com
Signed-off-by: Yann Droneaud
---
fs/seq_file.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/fs/seq_file.c b/fs/seq_file.c
index a909f12dad4d..e48ef5682bfa 100644
--- a/fs/seq_file.c
+++ b/fs/seq_file.c
@@ -48,6 +48,8 @@ static void *seq_buf_alloc(unsigned long size)
* ERR_PTR(error). In the end of sequence they return %NULL. ->show()
* returns 0 in case of success and negative number in case of error.
* Returning SEQ_SKIP means "discard this element and move on".
+ * Note: seq_open() will allocate a struct seq_file and store its
+ * pointer in @file->private_data. This pointer should not be modified.
*/
int seq_open(struct file *file, const struct seq_operations *op)
{
--
2.4.2
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v1 1/3] fs: use seq_open_private() for proc_mounts
Since patch described below, from v2.6.15-rc1, seq_open()
could use a struct seq_file already allocated by the
caller if the pointer to the structure is stored in
file->private_data before calling the function.
Commit 1abe77b0fc4b485927f1f798ae81a752677e1d05
Author: Al Viro
Date: Mon Nov 7 17:15:34 2005 -0500
[PATCH] allow callers of seq_open do allocation themselves
Allow caller of seq_open() to kmalloc() seq_file + whatever else they
want and set ->private_data to it. seq_open() will then abstain from
doing allocation itself.
Such behavior is only used by mounts_open_common().
In order to drop support for such uncommon feature,
proc_mounts is converted to use seq_open_private(),
which take care of allocating the proc_mounts structure,
making it available through ->private in struct seq_file.
Conversely, proc_mounts is converted to use seq_release_private(),
in order to release the private structure allocated by
seq_open_private().
Then, ->private is used directly instead of proc_mounts()
macro to access to the proc_mounts structure.
Link: http://lkml.kernel.org/r/cover.1433193673.git.ydrone...@opteya.com
Signed-off-by: Yann Droneaud
---
fs/mount.h | 3 ---
fs/namespace.c | 6 +++---
fs/proc_namespace.c | 34 --
3 files changed, 19 insertions(+), 24 deletions(-)
diff --git a/fs/mount.h b/fs/mount.h
index b5b8082bfa42..14db05d424f7 100644
--- a/fs/mount.h
+++ b/fs/mount.h
@@ -118,7 +118,6 @@ static inline void unlock_mount_hash(void)
}
struct proc_mounts {
- struct seq_file m;
struct mnt_namespace *ns;
struct path root;
int (*show)(struct seq_file *, struct vfsmount *);
@@ -127,8 +126,6 @@ struct proc_mounts {
loff_t cached_index;
};
-#define proc_mounts(p) (container_of((p), struct proc_mounts, m))
-
extern const struct seq_operations mounts_op;
extern bool __is_local_mountpoint(struct dentry *dentry);
diff --git a/fs/namespace.c b/fs/namespace.c
index f30c78a2b878..ecc277107e93 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -1226,7 +1226,7 @@ EXPORT_SYMBOL(replace_mount_options);
/* iterator; we want it to have access to namespace_sem, thus here... */
static void *m_start(struct seq_file *m, loff_t *pos)
{
- struct proc_mounts *p = proc_mounts(m);
+ struct proc_mounts *p = m->private;
down_read(&namespace_sem);
if (p->cached_event == p->ns->event) {
@@ -1247,7 +1247,7 @@ static void *m_start(struct seq_file *m, loff_t *pos)
static void *m_next(struct seq_file *m, void *v, loff_t *pos)
{
- struct proc_mounts *p = proc_mounts(m);
+ struct proc_mounts *p = m->private;
p->cached_mount = seq_list_next(v, &p->ns->list, pos);
p->cached_index = *pos;
@@ -1261,7 +1261,7 @@ static void m_stop(struct seq_file *m, void *v)
static int m_show(struct seq_file *m, void *v)
{
- struct proc_mounts *p = proc_mounts(m);
+ struct proc_mounts *p = m->private;
struct mount *r = list_entry(v, struct mount, mnt_list);
return p->show(m, &r->mnt);
}
diff --git a/fs/proc_namespace.c b/fs/proc_namespace.c
index 8db932da4009..8ebd9a334085 100644
--- a/fs/proc_namespace.c
+++ b/fs/proc_namespace.c
@@ -17,7 +17,8 @@
static unsigned mounts_poll(struct file *file, poll_table *wait)
{
- struct proc_mounts *p = proc_mounts(file->private_data);
+ struct seq_file *m = file->private_data;
+ struct proc_mounts *p = m->private;
struct mnt_namespace *ns = p->ns;
unsigned res = POLLIN | POLLRDNORM;
int event;
@@ -25,8 +26,8 @@ static unsigned mounts_poll(struct file *file, poll_table
*wait)
poll_wait(file, &p->ns->poll, wait);
event = ACCESS_ONCE(ns->event);
- if (p->m.poll_event != event) {
- p->m.poll_event = event;
+ if (m->poll_event != event) {
+ m->poll_event = event;
res |= POLLERR | POLLPRI;
}
@@ -92,7 +93,7 @@ static void show_type(struct seq_file *m, struct super_block
*sb)
static int show_vfsmnt(struct seq_file *m, struct vfsmount *mnt)
{
- struct proc_mounts *p = proc_mounts(m);
+ struct proc_mounts *p = m->private;
struct mount *r = real_mount(mnt);
int err = 0;
struct path mnt_path = { .dentry = mnt->mnt_root, .mnt = mnt };
@@ -126,7 +127,7 @@ out:
static int show_mountinfo(struct seq_file *m, struct vfsmount *mnt)
{
- struct proc_mounts *p = proc_mounts(m);
+ struct proc_mounts *p = m->private;
struct mount *r = real_mount(mnt);
struct super_block *sb = mnt->mnt_sb;
struct path mnt_path = { .dentry = mnt->mnt_root, .mnt = mnt };
@@ -186,7 +187,7 @@ out:
static int show_vfsstat(struct seq_file *m, struct vfsmount *mnt)
{
- struct proc_
Re: [PATCH 2/4] pid: add pidfd_open()
Le mercredi 27 mars 2019 à 17:21 +0100, Christian Brauner a écrit :
> diff --git a/kernel/pid.c b/kernel/pid.c
> index 20881598bdfa..c9e24e726aba 100644
> --- a/kernel/pid.c
> +++ b/kernel/pid.c
> @@ -26,8 +26,10 @@
> +SYSCALL_DEFINE4(pidfd_open, pid_t, pid, int, procfd, int, pidfd,
> unsigned int,
> + flags)
> +{
> + long fd = -EINVAL;
> +
> + if (flags & ~(PIDFD_TO_PROCFD | PROCFD_TO_PIDFD))
> + return -EINVAL;
> +
> + if (!flags) {
> + struct pid *pidfd_pid;
> +
> + if (pid <= 0)
> + return -EINVAL;
> +
> + if (procfd != -1 || pidfd != -1)
> + return -EINVAL;
> +
> + rcu_read_lock();
> + pidfd_pid = get_pid(find_pid_ns(pid,
> task_active_pid_ns(current)));
> + rcu_read_unlock();
> +
> + fd = pidfd_create_fd(pidfd_pid, O_CLOEXEC);
> + put_pid(pidfd_pid);
> + } else if (flags & PIDFD_TO_PROCFD) {
[...]
> + } else if (flags & PROCFD_TO_PIDFD) {
> + if (flags & ~PROCFD_TO_PIDFD)
> + return -EINVAL;
> +
> + if (pid != -1)
> + return -EINVAL;
> +
> + if (pidfd >= 0)
>
I think it can be stricter with:
if (pidfd != -1)
(and match the check done for flag == 0).
Regards.
--
Yann Droneaud
OPTEYA
[PATCH 4/4] random/trace: remove unused trace_xfer_secondary_pool()
Since commit 84df7cdfbb21 ('random: delete code to pull data into pools'),
trace_xfer_secondary_pool() is unused.
Cc: Andy Lutomirski
Cc: Theodore Ts'o
Signed-off-by: Yann Droneaud
---
include/trace/events/random.h | 29 -
1 file changed, 29 deletions(-)
diff --git a/include/trace/events/random.h b/include/trace/events/random.h
index c2256250e3ef..8cf287423571 100644
--- a/include/trace/events/random.h
+++ b/include/trace/events/random.h
@@ -139,35 +139,6 @@ TRACE_EVENT(add_disk_randomness,
MINOR(__entry->dev), __entry->input_bits)
);
-TRACE_EVENT(xfer_secondary_pool,
- TP_PROTO(const char *pool_name, int xfer_bits, int request_bits,
-int pool_entropy, int input_entropy),
-
- TP_ARGS(pool_name, xfer_bits, request_bits, pool_entropy,
- input_entropy),
-
- TP_STRUCT__entry(
- __field( const char *, pool_name )
- __field( int, xfer_bits )
- __field( int, request_bits)
- __field( int, pool_entropy)
- __field( int, input_entropy )
- ),
-
- TP_fast_assign(
- __entry->pool_name = pool_name;
- __entry->xfer_bits = xfer_bits;
- __entry->request_bits = request_bits;
- __entry->pool_entropy = pool_entropy;
- __entry->input_entropy = input_entropy;
- ),
-
- TP_printk("pool %s xfer_bits %d request_bits %d pool_entropy %d "
- "input_entropy %d", __entry->pool_name, __entry->xfer_bits,
- __entry->request_bits, __entry->pool_entropy,
- __entry->input_entropy)
-);
-
DECLARE_EVENT_CLASS(random__get_random_bytes,
TP_PROTO(int nbytes, unsigned long IP),
--
2.25.4
[PATCH 2/4] random/trace: remove unused trace_push_to_pool()
Since commit 90ea1c6436d2 ('random: remove the blocking pool')
trace_push_to_pool() is unused.
Cc: Andy Lutomirski
Cc: Theodore Ts'o
Signed-off-by: Yann Droneaud
---
include/trace/events/random.h | 22 --
1 file changed, 22 deletions(-)
diff --git a/include/trace/events/random.h b/include/trace/events/random.h
index 087ae7e8ae13..d5aa662294fe 100644
--- a/include/trace/events/random.h
+++ b/include/trace/events/random.h
@@ -85,28 +85,6 @@ TRACE_EVENT(credit_entropy_bits,
__entry->entropy_count, (void *)__entry->IP)
);
-TRACE_EVENT(push_to_pool,
- TP_PROTO(const char *pool_name, int pool_bits, int input_bits),
-
- TP_ARGS(pool_name, pool_bits, input_bits),
-
- TP_STRUCT__entry(
- __field( const char *, pool_name )
- __field( int, pool_bits )
- __field( int, input_bits )
- ),
-
- TP_fast_assign(
- __entry->pool_name = pool_name;
- __entry->pool_bits = pool_bits;
- __entry->input_bits = input_bits;
- ),
-
- TP_printk("%s: pool_bits %d input_pool_bits %d",
- __entry->pool_name, __entry->pool_bits,
- __entry->input_bits)
-);
-
TRACE_EVENT(debit_entropy,
TP_PROTO(const char *pool_name, int debit_bits),
--
2.25.4
[PATCH 0/4] random/trace: remove unused function
Hi, After simplifications brought recently to random [1], some trace functions are no more needed. This patchset removes them. [1] https://lore.kernel.org/linux-api/cover.1577088521.git.l...@kernel.org/ Yann Droneaud (4): random/trace: remove unused trace_random_read() random/trace: remove unused trace_push_to_pool() random/trace: remove unused trace_extract_entropy_user() random/trace: remove unused trace_xfer_secondary_pool() include/trace/events/random.h | 93 +-- 1 file changed, 1 insertion(+), 92 deletions(-) -- 2.25.4
[PATCH 3/4] random/trace: remove unused trace_extract_entropy_user()
Since commit 90ea1c6436d2 ('random: remove the blocking pool')
trace_extract_entropy_user() is unused.
As the result, trace_extract_entropy() is the only trace
function needed, thus there's no need for a separate event
class / event.
Cc: Andy Lutomirski
Cc: Theodore Ts'o
Signed-off-by: Yann Droneaud
---
include/trace/events/random.h | 17 +
1 file changed, 1 insertion(+), 16 deletions(-)
diff --git a/include/trace/events/random.h b/include/trace/events/random.h
index d5aa662294fe..c2256250e3ef 100644
--- a/include/trace/events/random.h
+++ b/include/trace/events/random.h
@@ -198,7 +198,7 @@ DEFINE_EVENT(random__get_random_bytes,
get_random_bytes_arch,
TP_ARGS(nbytes, IP)
);
-DECLARE_EVENT_CLASS(random__extract_entropy,
+TRACE_EVENT(extract_entropy,
TP_PROTO(const char *pool_name, int nbytes, int entropy_count,
unsigned long IP),
@@ -223,21 +223,6 @@ DECLARE_EVENT_CLASS(random__extract_entropy,
(void *)__entry->IP)
);
-
-DEFINE_EVENT(random__extract_entropy, extract_entropy,
- TP_PROTO(const char *pool_name, int nbytes, int entropy_count,
-unsigned long IP),
-
- TP_ARGS(pool_name, nbytes, entropy_count, IP)
-);
-
-DEFINE_EVENT(random__extract_entropy, extract_entropy_user,
- TP_PROTO(const char *pool_name, int nbytes, int entropy_count,
-unsigned long IP),
-
- TP_ARGS(pool_name, nbytes, entropy_count, IP)
-);
-
TRACE_EVENT(urandom_read,
TP_PROTO(int got_bits, int pool_left, int input_left),
--
2.25.4
[PATCH 1/4] random/trace: remove unused trace_random_read()
Since commit 30c08efec888 ('random: make /dev/random be almost like
/dev/urandom'),
trace_random_read() is unused.
Cc: Andy Lutomirski
Cc: Theodore Ts'o
Signed-off-by: Yann Droneaud
---
include/trace/events/random.h | 25 -
1 file changed, 25 deletions(-)
diff --git a/include/trace/events/random.h b/include/trace/events/random.h
index 32c10a515e2d..087ae7e8ae13 100644
--- a/include/trace/events/random.h
+++ b/include/trace/events/random.h
@@ -260,31 +260,6 @@ DEFINE_EVENT(random__extract_entropy, extract_entropy_user,
TP_ARGS(pool_name, nbytes, entropy_count, IP)
);
-TRACE_EVENT(random_read,
- TP_PROTO(int got_bits, int need_bits, int pool_left, int input_left),
-
- TP_ARGS(got_bits, need_bits, pool_left, input_left),
-
- TP_STRUCT__entry(
- __field( int, got_bits)
- __field( int, need_bits )
- __field( int, pool_left )
- __field( int, input_left )
- ),
-
- TP_fast_assign(
- __entry->got_bits = got_bits;
- __entry->need_bits = need_bits;
- __entry->pool_left = pool_left;
- __entry->input_left = input_left;
- ),
-
- TP_printk("got_bits %d still_needed_bits %d "
- "blocking_pool_entropy_left %d input_entropy_left %d",
- __entry->got_bits, __entry->got_bits, __entry->pool_left,
- __entry->input_left)
-);
-
TRACE_EVENT(urandom_read,
TP_PROTO(int got_bits, int pool_left, int input_left),
--
2.25.4
Re: [PATCH] events: add a flag to perf_event_open() to set O_CLOEXEC
Hi, Le jeudi 31 octobre 2013 à 19:12 +0100, Peter Zijlstra a écrit : > On Wed, Oct 30, 2013 at 10:35:50PM +0100, Yann Droneaud wrote: > > This patch adds PERF_FLAG_FD_CLOEXEC flag for > > perf_event_open() syscall. > > > > perf_event_open() creates a new file descriptor, > > but unlike open() syscall, it lack a flag to atomicaly > > set close-on-exec (O_CLOEXEC). > > > > Not using O_CLOEXEC by default and not letting userspace > > provide the "open" flags should be avoided: in most case > > O_CLOEXEC must be used to not leak file descriptor across > > exec(). > > > > Using O_CLOEXEC when creating a file descriptor allows > > userspace to set latter, using fcntl(), without any risk > > of race, if the file descriptor is going to be inherited > > or not across exec(). > > > > Link: http://lkml.kernel.org/r/94b641a81a06ba4943cf77e80bc27...@meuh.org > > Link: http://lkml.kernel.org/r/cover.1383121137.git.ydrone...@opteya.com > > Signed-off-by: Yann Droneaud > > --- > > > > Hi Peter, > > > > This patch should replaces > > "[PATCH v4 6/7] events: use get_unused_fd_flags(0) instead of > > get_unused_fd()" > > > > Please have a look. > > I'm still terminally confused as to all of this... Why does it matter > what the default is if you can change it with fcntl() ? There's a possible race between open() and fcntl()in multi-threaded program, that's why O_CLOEXEC was added to open(), and then added to many other system calls. > Also, how can you tell nobody relies on the current behaviour and its > therefore safe > to change? > If *you* cannot tell, then no one could tell, thus the default *must* not be changed because no one could ever be sure that no application rely on the current behavior. But it's rather pointless to change the default, since the caller of get_unused_fd(), eg. syscall perf_event_open(), has a flags argument that can be used to ask for O_CLOEXEC. Just like other syscall extended to support O_CLOEXEC. You might want to read "Secure File Descriptor Handling" by Ulrich Drepper [1] who is responsible to adding O_CLOEXEC on open, and probably other syscalls [1] http://udrepper.livejournal.com/20407.html Regards. -- Yann Droneaud OPTEYA -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] 6lowpan: add a license to 6lowpan_iphc module
Since commit 8df8c56a5abc, 6lowpan_iphc is a module of its own.
Unfortunately, it lacks some infrastructure to behave like a
good kernel citizen:
kernel: 6lowpan_iphc: module license 'unspecified' taints kernel.
kernel: Disabling lock debugging due to kernel taint
This patch adds the basic MODULE_LICENSE(); with GPL license:
the code was copied from net/ieee802154/6lowpan.c which is GPL
and the module exports symbol with EXPORT_SYMBOL_GPL();.
Cc: Jukka Rissanen
Cc: Alexander Aring
Cc: Marcel Holtmann
Signed-off-by: Yann Droneaud
---
net/ieee802154/6lowpan_iphc.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/net/ieee802154/6lowpan_iphc.c b/net/ieee802154/6lowpan_iphc.c
index e14fe8b2c054..860aa2d445ba 100644
--- a/net/ieee802154/6lowpan_iphc.c
+++ b/net/ieee802154/6lowpan_iphc.c
@@ -52,6 +52,7 @@
#include
#include
+#include
#include
#include
#include
@@ -797,3 +798,5 @@ int lowpan_header_compress(struct sk_buff *skb, struct
net_device *dev,
return 0;
}
EXPORT_SYMBOL_GPL(lowpan_header_compress);
+
+MODULE_LICENSE("GPL");
--
1.8.4.2
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCHv3] driver core/platform: don't leak memory allocated for dma_mask
0 */
-/* size: 736, cachelines: 12, members: 2 */
-/* padding: 7 */
+/* size: 736, cachelines: 12, members: 3 */
/* last cacheline: 32 bytes */
};
textdata bss dec hex filename
- 7100 960 4881081fac obj-x86_64/drivers/base/platform.o
+ 7020 960 4880281f5c obj-x86_64/drivers/base/platform.o
struct device {
struct device *parent; /* 0 8 */
struct device_private *p;/* 8 8 */
@@ -350,9 +350,9 @@ struct platform_device {
struct platform_object {
struct platform_device pdev; /* 0 712 */
/* --- cacheline 11 boundary (704 bytes) was 8 bytes ago --- */
-char name[1]; /* 712 1 */
+u64dma_mask; /* 712 8 */
+char name[0]; /* 720 0 */
-/* size: 720, cachelines: 12, members: 2 */
-/* padding: 7 */
+/* size: 720, cachelines: 12, members: 3 */
/* last cacheline: 16 bytes */
};
Changes from v2 [1]:
- move 'dma_mask' to platform_object so that it's always
allocated and won't leak on release; remove all previously
added support functions.
- use C99 flexible array member for 'name' to remove padding
at the end of platform_object.
Changes from v1 [2]:
- remove unneeded kfree() from error path
- add reference to author/commit adding allocation of dmamask
Changes from v0 [3]:
- small rewrite to squeeze the patch to a bare minimal
[1]
http://lkml.kernel.org/r/1389683909-17495-1-git-send-email-ydrone...@opteya.com
https://patchwork.kernel.org/patch/3484411/
[2]
http://lkml.kernel.org/r/1389649085-7365-1-git-send-email-ydrone...@opteya.com
https://patchwork.kernel.org/patch/3480961/
[3]
http://lkml.kernel.org/r/1386886207-2735-1-git-send-email-ydrone...@opteya.com
Cc: Uwe Kleine-König
Cc: Dmitry Torokhov
Cc: Greg Kroah-Hartman
Signed-off-by: Yann Droneaud
---
Hi Greg and Uwe,
I've tried to add dma_mask to platform_object,
and found it was good. Here's the patch.
Then I tried to "fix" existing drivers to remove
the no more needed allocation for dma_mask. I've used
a spatch/coccinelle semantic patch such as:
@catchall1@
identifier func;
struct platform_device * pdev;
expression E;
@@
func(...) {
<...
pdev = platform_device_alloc(...);
...
/*
TODO: use dma_mask allocated by platform_device_alloc()
pdev->dev.dma_mask = E;
*/
...>
}
I've found that some drivers use a pointer to the inner
dev.coherent_mask.
Is this could be another option to not have to allocate
a placeholder for the dma_mask ?
Regards.
--
Yann Droneaud
OPTEYA
drivers/base/platform.c | 17 +++--
1 file changed, 7 insertions(+), 10 deletions(-)
diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index bc78848dd59a..1f6d1e832e03 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -157,7 +157,8 @@ EXPORT_SYMBOL_GPL(platform_add_devices);
struct platform_object {
struct platform_device pdev;
- char name[1];
+ u64 dma_mask;
+ char name[];
};
/**
@@ -193,16 +194,20 @@ static void platform_device_release(struct device *dev)
*
* Create a platform device object which can have other objects attached
* to it, and which will have attached objects freed when it is released.
+ *
+ * The associated struct device will be set up so that its dma_mask field
+ * is a valid pointer to an u64. This pointer must not be free'd manually.
*/
struct platform_device *platform_device_alloc(const char *name, int id)
{
struct platform_object *pa;
- pa = kzalloc(sizeof(struct platform_object) + strlen(name), GFP_KERNEL);
+ pa = kzalloc(sizeof(*pa) + strlen(name) + 1, GFP_KERNEL);
if (pa) {
strcpy(pa->name, name);
pa->pdev.name = pa->name;
pa->pdev.id = id;
+ pa->pdev.dev.dma_mask = &pa->dma_mask;
device_initialize(&pa->pdev.dev);
pa->pdev.dev.release = platform_device_release;
arch_setup_pdev_archdata(&pa->pdev);
@@ -436,16 +441,9 @@ struct platform_device *platform_device_register_full(
if (pdevinfo->dma_mask) {
/*
-* This memory isn't freed when the device is put,
-* I don't have a nice idea for that though. Conceptually
* dma_mask in struct device should not be a pointer.
* See http://thread.gmane.org/gmane.linux.kernel.pci/9081
*/
- pdev->dev.dma_mask =
- kmalloc(sizeof(*pdev->dev.dma_mask), GFP_KERNEL);
-
[PATCH 0/2] Revamp randomize_range()
Hi, Please find two patches to remove unused/unwanted behavor on randomize_range() function. The first one remove the last option (len) which is unused in current kernel. The second one rewrite the function to not fail, allowing to remove the error handling code from the calling functions. I've not added the maintainers of the various arch modified by the patches, wanted first to have your opinion about the changes. Regards. Yann Droneaud (2): random: remove unused len argument in randomize_range() function random: don't return 0 in randomize_range() arch/arm/kernel/process.c | 2 +- arch/arm64/kernel/process.c | 2 +- arch/tile/mm/mmap.c | 2 +- arch/unicore32/kernel/process.c | 2 +- arch/x86/kernel/process.c | 2 +- arch/x86/kernel/sys_x86_64.c| 5 + drivers/char/random.c | 19 --- include/linux/random.h | 2 +- 8 files changed, 15 insertions(+), 21 deletions(-) -- 1.8.5.3 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/2] random: don't return 0 in randomize_range()
randomize_range() returns 0 when 'end' address is below 'start'
address: it's an error to pass an invalid range to the function.
Code using randomize_range() deals with such error silently and
use the start address instead.
This patch makes randomize_range() issue a warning with WARN_ON()
when its parameters are invalid and returns the start address, so
that code using the function doesn't have to handle an error which
is not supposed to happen. The patch also removes the code handling
the error in functions using randomize_range().
Link: http://lkml.kernel.org/r/cover.1390770607.git.ydrone...@opteya.com
Cc: Theodore Ts'o
Signed-off-by: Yann Droneaud
---
arch/arm/kernel/process.c| 2 +-
arch/tile/mm/mmap.c | 2 +-
arch/x86/kernel/process.c| 2 +-
arch/x86/kernel/sys_x86_64.c | 5 +
drivers/char/random.c| 4 +++-
5 files changed, 7 insertions(+), 8 deletions(-)
diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
index a13d456cc8d1..005a8ba04739 100644
--- a/arch/arm/kernel/process.c
+++ b/arch/arm/kernel/process.c
@@ -428,7 +428,7 @@ unsigned long get_wchan(struct task_struct *p)
unsigned long arch_randomize_brk(struct mm_struct *mm)
{
unsigned long range_end = mm->brk + 0x0200;
- return randomize_range(mm->brk, range_end) ? : mm->brk;
+ return randomize_range(mm->brk, range_end);
}
#ifdef CONFIG_MMU
diff --git a/arch/tile/mm/mmap.c b/arch/tile/mm/mmap.c
index bc29e8ce0d27..294292714f34 100644
--- a/arch/tile/mm/mmap.c
+++ b/arch/tile/mm/mmap.c
@@ -89,5 +89,5 @@ void arch_pick_mmap_layout(struct mm_struct *mm)
unsigned long arch_randomize_brk(struct mm_struct *mm)
{
unsigned long range_end = mm->brk + 0x0200;
- return randomize_range(mm->brk, range_end) ? : mm->brk;
+ return randomize_range(mm->brk, range_end);
}
diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index 2db44a7147d1..076358128404 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -466,6 +466,6 @@ unsigned long arch_align_stack(unsigned long sp)
unsigned long arch_randomize_brk(struct mm_struct *mm)
{
unsigned long range_end = mm->brk + 0x0200;
- return randomize_range(mm->brk, range_end) ? : mm->brk;
+ return randomize_range(mm->brk, range_end);
}
diff --git a/arch/x86/kernel/sys_x86_64.c b/arch/x86/kernel/sys_x86_64.c
index 5cd395f21a25..7b2a029e63fb 100644
--- a/arch/x86/kernel/sys_x86_64.c
+++ b/arch/x86/kernel/sys_x86_64.c
@@ -85,7 +85,6 @@ static void find_start_end(unsigned long flags, unsigned long
*begin,
unsigned long *end)
{
if (!test_thread_flag(TIF_ADDR32) && (flags & MAP_32BIT)) {
- unsigned long new_begin;
/* This is usually used needed to map code in small
model, so it needs to be in the first 31bit. Limit
it to that. This means we need to move the
@@ -96,9 +95,7 @@ static void find_start_end(unsigned long flags, unsigned long
*begin,
*begin = 0x4000;
*end = 0x8000;
if (current->flags & PF_RANDOMIZE) {
- new_begin = randomize_range(*begin, *begin +
0x0200);
- if (new_begin)
- *begin = new_begin;
+ *begin = randomize_range(*begin, *begin + 0x0200);
}
} else {
*begin = current->mm->mmap_legacy_base;
diff --git a/drivers/char/random.c b/drivers/char/random.c
index 115b5a5381fb..7c7c47e220ca 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -1641,8 +1641,10 @@ EXPORT_SYMBOL(get_random_int);
unsigned long
randomize_range(unsigned long start, unsigned long end)
{
+ WARN_ON(end <= start);
+
if (end <= start)
- return 0;
+ return start;
return PAGE_ALIGN(get_random_int() % (end - start) + start);
}
--
1.8.5.3
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCHv3] perf tools: enable close-on-exec flag on perf file descriptor
In commit a21b0b354d4a, flag PERF_FLAG_FD_CLOEXEC was
added to perf_event_open(2) syscall to allows userspace
to atomically enable close-on-exec behavor when creating
the file descriptor.
This patch makes perf tools use the new flag if supported
by the kernel, so that the event file descriptors got
automatically closed if perf tool exec a sub-command.
Changes from v2 [1]:
- use safest perf attr configuration in probe function,
as used in perf_evsel__fallback().
Changes from v1 [2]:
- don't probe PERF_FLAG_FD_CLOEXEC for each call to
perf_event_open_cloexec_flag(): don't forget to set
'probed' variable once flag was probed.
- call perf_event_open_cloexec_flag() only once in
util/record.c:perf_do_probe_api().
- fixed minor coding style issue (unneeded braces)
in util/cloexec.c
Changes from v0 [3]:
- add probing for PERF_FLAG_FD_CLOEXEC flag allowing
perf to run on older kernel:
* use "missing feature" pattern in evsel to disable
PERF_FLAG_FD_CLOEXEC in perf_evsel__open() if not
supported by kernel;
* add a dedicated function to probe for
PERF_FLAG_FD_CLOEXEC support in the current kernel.
This function is to be used by other functions
calling sys_perf_event_open() directly.
- remove PERF_FLAG_FD_CLOEXEC from PowerPC selftest
as it's not related to perf tool: it should be part
of a separate patch.
[1]
http://lkml.kernel.org/r/1389607770-4485-1-git-send-email-ydrone...@opteya.com
https://patchwork.kernel.org/patch/3474141/
[2]
http://lkml.kernel.org/r/1389463628-24869-1-git-send-email-ydrone...@opteya.com
https://patchwork.kernel.org/patch/3469571/
[3]
http://lkml.kernel.org/r/1389005485-12778-1-git-send-email-ydrone...@opteya.com
https://patchwork.kernel.org/patch/3437111/
Cc: Peter Zijlstra
Cc: Arnaldo Carvalho de Melo
Cc: Jiri Olsa
Cc: Andi Kleen
Cc: Ingo Molnar
Signed-off-by: Yann Droneaud
---
tools/perf/Makefile.perf | 1 +
tools/perf/bench/mem-memcpy.c | 4 ++-
tools/perf/bench/mem-memset.c | 4 ++-
tools/perf/builtin-sched.c| 4 ++-
tools/perf/tests/bp_signal.c | 4 ++-
tools/perf/tests/bp_signal_overflow.c | 4 ++-
tools/perf/tests/rdpmc.c | 4 ++-
tools/perf/util/cloexec.c | 56 +++
tools/perf/util/cloexec.h | 6
tools/perf/util/evsel.c | 12 ++--
tools/perf/util/record.c | 9 --
11 files changed, 96 insertions(+), 12 deletions(-)
create mode 100644 tools/perf/util/cloexec.c
create mode 100644 tools/perf/util/cloexec.h
diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index 7257e7e9e38a..6aeefd97c9db 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -373,6 +373,7 @@ LIB_OBJS += $(OUTPUT)util/stat.o
LIB_OBJS += $(OUTPUT)util/record.o
LIB_OBJS += $(OUTPUT)util/srcline.o
LIB_OBJS += $(OUTPUT)util/data.o
+LIB_OBJS += $(OUTPUT)util/cloexec.o
LIB_OBJS += $(OUTPUT)ui/setup.o
LIB_OBJS += $(OUTPUT)ui/helpline.o
diff --git a/tools/perf/bench/mem-memcpy.c b/tools/perf/bench/mem-memcpy.c
index 5ce71d3b72cf..bf5a21b848a9 100644
--- a/tools/perf/bench/mem-memcpy.c
+++ b/tools/perf/bench/mem-memcpy.c
@@ -10,6 +10,7 @@
#include "../util/util.h"
#include "../util/parse-options.h"
#include "../util/header.h"
+#include "../util/cloexec.h"
#include "bench.h"
#include "mem-memcpy-arch.h"
@@ -83,7 +84,8 @@ static struct perf_event_attr cycle_attr = {
static void init_cycle(void)
{
- cycle_fd = sys_perf_event_open(&cycle_attr, getpid(), -1, -1, 0);
+ cycle_fd = sys_perf_event_open(&cycle_attr, getpid(), -1, -1,
+ perf_event_open_cloexec_flag());
if (cycle_fd < 0 && errno == ENOSYS)
die("No CONFIG_PERF_EVENTS=y kernel support configured?\n");
diff --git a/tools/perf/bench/mem-memset.c b/tools/perf/bench/mem-memset.c
index 9af79d2b18e5..260747ea1e0e 100644
--- a/tools/perf/bench/mem-memset.c
+++ b/tools/perf/bench/mem-memset.c
@@ -10,6 +10,7 @@
#include "../util/util.h"
#include "../util/parse-options.h"
#include "../util/header.h"
+#include "../util/cloexec.h"
#include "bench.h"
#include "mem-memset-arch.h"
@@ -83,7 +84,8 @@ static struct perf_event_attr cycle_attr = {
static void init_cycle(void)
{
- cycle_fd = sys_perf_event_open(&cycle_attr, getpid(), -1, -1, 0);
+ cycle_fd = sys_perf_event_open(&cycle_attr, getpid(), -1, -1,
+ perf_event_open_cloexec_flag());
if (cycle_fd < 0 && errno == ENOSYS)
die("No CONFIG_PERF_EVENTS=y kernel support configured?\n");
diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
index 6a76a07b6789..54017bdec88c 10064
[PATCH 1/2] random: remove unused len argument in randomize_range() function
randomize_range() accepts a third parameter as the size of the object
to randomly fit in the range [PAGE_ALIGN(start), PAGE_ALIGN(end)].
The functions returns an address in the range [PAGE_ALIGN(start),
PAGE_ALIGN(end - len)].
But no kernel code is currently using the third parameter:
it's hardcoded to 0 in each functions calling randomize_range().
So len argument is useless in the current codebase and can be safely
removed: this patch removes the len argument from randomize_range()
and fixes code using randomize_range() with len set to 0.
Link: http://lkml.kernel.org/r/cover.1390770607.git.ydrone...@opteya.com
Cc: Theodore Ts'o
Signed-off-by: Yann Droneaud
---
arch/arm/kernel/process.c | 2 +-
arch/arm64/kernel/process.c | 2 +-
arch/tile/mm/mmap.c | 2 +-
arch/unicore32/kernel/process.c | 2 +-
arch/x86/kernel/process.c | 2 +-
arch/x86/kernel/sys_x86_64.c| 2 +-
drivers/char/random.c | 17 ++---
include/linux/random.h | 2 +-
8 files changed, 13 insertions(+), 18 deletions(-)
diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
index 92f7b15dd221..a13d456cc8d1 100644
--- a/arch/arm/kernel/process.c
+++ b/arch/arm/kernel/process.c
@@ -428,7 +428,7 @@ unsigned long get_wchan(struct task_struct *p)
unsigned long arch_randomize_brk(struct mm_struct *mm)
{
unsigned long range_end = mm->brk + 0x0200;
- return randomize_range(mm->brk, range_end, 0) ? : mm->brk;
+ return randomize_range(mm->brk, range_end) ? : mm->brk;
}
#ifdef CONFIG_MMU
diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
index 1e5a17811648..855c49be0050 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -340,7 +340,7 @@ unsigned long arch_align_stack(unsigned long sp)
static unsigned long randomize_base(unsigned long base)
{
unsigned long range_end = base + (STACK_RND_MASK << PAGE_SHIFT) + 1;
- return randomize_range(base, range_end, 0) ? : base;
+ return randomize_range(base, range_end) ? : base;
}
unsigned long arch_randomize_brk(struct mm_struct *mm)
diff --git a/arch/tile/mm/mmap.c b/arch/tile/mm/mmap.c
index 851a94e6ae58..bc29e8ce0d27 100644
--- a/arch/tile/mm/mmap.c
+++ b/arch/tile/mm/mmap.c
@@ -89,5 +89,5 @@ void arch_pick_mmap_layout(struct mm_struct *mm)
unsigned long arch_randomize_brk(struct mm_struct *mm)
{
unsigned long range_end = mm->brk + 0x0200;
- return randomize_range(mm->brk, range_end, 0) ? : mm->brk;
+ return randomize_range(mm->brk, range_end) ? : mm->brk;
}
diff --git a/arch/unicore32/kernel/process.c b/arch/unicore32/kernel/process.c
index 778ebba80827..79a4423c48ba 100644
--- a/arch/unicore32/kernel/process.c
+++ b/arch/unicore32/kernel/process.c
@@ -302,7 +302,7 @@ unsigned long get_wchan(struct task_struct *p)
unsigned long arch_randomize_brk(struct mm_struct *mm)
{
unsigned long range_end = mm->brk + 0x0200;
- return randomize_range(mm->brk, range_end, 0) ? : mm->brk;
+ return randomize_range(mm->brk, range_end) ? : mm->brk;
}
/*
diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index 3fb8d95ab8b5..2db44a7147d1 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -466,6 +466,6 @@ unsigned long arch_align_stack(unsigned long sp)
unsigned long arch_randomize_brk(struct mm_struct *mm)
{
unsigned long range_end = mm->brk + 0x0200;
- return randomize_range(mm->brk, range_end, 0) ? : mm->brk;
+ return randomize_range(mm->brk, range_end) ? : mm->brk;
}
diff --git a/arch/x86/kernel/sys_x86_64.c b/arch/x86/kernel/sys_x86_64.c
index 30277e27431a..5cd395f21a25 100644
--- a/arch/x86/kernel/sys_x86_64.c
+++ b/arch/x86/kernel/sys_x86_64.c
@@ -96,7 +96,7 @@ static void find_start_end(unsigned long flags, unsigned long
*begin,
*begin = 0x4000;
*end = 0x8000;
if (current->flags & PF_RANDOMIZE) {
- new_begin = randomize_range(*begin, *begin +
0x0200, 0);
+ new_begin = randomize_range(*begin, *begin +
0x0200);
if (new_begin)
*begin = new_begin;
}
diff --git a/drivers/char/random.c b/drivers/char/random.c
index d07575c99a5f..115b5a5381fb 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -1634,20 +1634,15 @@ unsigned int get_random_int(void)
EXPORT_SYMBOL(get_random_int);
/*
- * randomize_range() returns a start address such that
+ * randomize_range() returns a page aligned random address
+ * in range [PAGE_ALIGN(start), PAGE_ALIGN(end)].
*
- *[.. .]
- * start end
- *
- * a with size "len" starting at the return value is inside in the
- * area defined by [start, end], but is otherwise randomized.
[PATCHv4] driver core/platform: don't leak memory allocated for dma_mask
dma_mask; /* 728 8 */
+char name[0]; /* 736 0 */
-/* size: 736, cachelines: 12, members: 2 */
-/* padding: 7 */
+/* size: 736, cachelines: 12, members: 3 */
/* last cacheline: 32 bytes */
};
textdata bss dec hex filename
- 7100 960 4881081fac obj-x86_64/drivers/base/platform.o
+ 7020 960 4880281f5c obj-x86_64/drivers/base/platform.o
struct device {
struct device *parent; /* 0 8 */
struct device_private *p;/* 8 8 */
@@ -350,9 +350,9 @@ struct platform_device {
struct platform_object {
struct platform_device pdev; /* 0 712 */
/* --- cacheline 11 boundary (704 bytes) was 8 bytes ago --- */
-char name[1]; /* 712 1 */
+u64dma_mask; /* 712 8 */
+char name[0]; /* 720 0 */
-/* size: 720, cachelines: 12, members: 2 */
-/* padding: 7 */
+/* size: 720, cachelines: 12, members: 3 */
/* last cacheline: 16 bytes */
};
Changes from v3 [1]:
- fixed commit message so that git am doesn't fail.
Changes from v2 [2]:
- move 'dma_mask' to platform_object so that it's always
allocated and won't leak on release; remove all previously
added support functions.
- use C99 flexible array member for 'name' to remove padding
at the end of platform_object.
Changes from v1 [3]:
- remove unneeded kfree() from error path
- add reference to author/commit adding allocation of dmamask
Changes from v0 [4]:
- small rewrite to squeeze the patch to a bare minimal
[1]
http://lkml.kernel.org/r/1390771138-28348-1-git-send-email-ydrone...@opteya.com
https://patchwork.kernel.org/patch/3540081/
[2]
http://lkml.kernel.org/r/1389683909-17495-1-git-send-email-ydrone...@opteya.com
https://patchwork.kernel.org/patch/3484411/
[3]
http://lkml.kernel.org/r/1389649085-7365-1-git-send-email-ydrone...@opteya.com
https://patchwork.kernel.org/patch/3480961/
[4]
http://lkml.kernel.org/r/1386886207-2735-1-git-send-email-ydrone...@opteya.com
Cc: Uwe Kleine-König
Cc: Dmitry Torokhov
Cc: Greg Kroah-Hartman
Signed-off-by: Yann Droneaud
---
Hi,
I've fixed the commit message to move the diff on pahole
outside of the scope of git am.
Regards.
drivers/base/platform.c | 17 +++--
1 file changed, 7 insertions(+), 10 deletions(-)
diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index bc78848dd59a..1f6d1e832e03 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -157,7 +157,8 @@ EXPORT_SYMBOL_GPL(platform_add_devices);
struct platform_object {
struct platform_device pdev;
- char name[1];
+ u64 dma_mask;
+ char name[];
};
/**
@@ -193,16 +194,20 @@ static void platform_device_release(struct device *dev)
*
* Create a platform device object which can have other objects attached
* to it, and which will have attached objects freed when it is released.
+ *
+ * The associated struct device will be set up so that its dma_mask field
+ * is a valid pointer to an u64. This pointer must not be free'd manually.
*/
struct platform_device *platform_device_alloc(const char *name, int id)
{
struct platform_object *pa;
- pa = kzalloc(sizeof(struct platform_object) + strlen(name), GFP_KERNEL);
+ pa = kzalloc(sizeof(*pa) + strlen(name) + 1, GFP_KERNEL);
if (pa) {
strcpy(pa->name, name);
pa->pdev.name = pa->name;
pa->pdev.id = id;
+ pa->pdev.dev.dma_mask = &pa->dma_mask;
device_initialize(&pa->pdev.dev);
pa->pdev.dev.release = platform_device_release;
arch_setup_pdev_archdata(&pa->pdev);
@@ -436,16 +441,9 @@ struct platform_device *platform_device_register_full(
if (pdevinfo->dma_mask) {
/*
-* This memory isn't freed when the device is put,
-* I don't have a nice idea for that though. Conceptually
* dma_mask in struct device should not be a pointer.
* See http://thread.gmane.org/gmane.linux.kernel.pci/9081
*/
- pdev->dev.dma_mask =
- kmalloc(sizeof(*pdev->dev.dma_mask), GFP_KERNEL);
- if (!pdev->dev.dma_mask)
- goto err;
-
*pdev->dev.dma_mask = pdevinfo->dma_mask;
pdev->dev.coherent_dma_mask = pdevinfo->dma_mask;
}
@@ -464,7 +462,6 @@ struct platform_device *platform_device_register_full(
cscope: issue with symlinks in tools/testing/selftests/powerpc/copyloops/
Hi, I'm using cscope to browse kernel sources, but I'm facing warnings from the tool since following commit: http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=22d651dcef536c75f75537290bf3da5038e68b6b commit 22d651dcef536c75f75537290bf3da5038e68b6b Author: Michael Ellerman Date: Tue Jan 21 15:22:17 2014 +1100 selftests/powerpc: Import Anton's memcpy / copy_tofrom_user tests Turn Anton's memcpy / copy_tofrom_user test into something that can live in tools/testing/selftests. It requires one turd in arch/powerpc/lib/memcpy_64.S, but it's pretty harmless IMHO. We are sailing very close to the wind with the feature macros. We define them to nothing, which currently means we get a few extra nops and include the unaligned calls. Signed-off-by: Anton Blanchard Signed-off-by: Michael Ellerman Signed-off-by: Benjamin Herrenschmidt cscope reports error when generating the cross-reference database: $ make ALLSOURCE_ARCHS=all O=./obj-cscope/ cscope GEN cscope cscope: cannot find file /home/ydroneaud/src/linux/tools/testing/selftests/powerpc/copyloops/copyuser_power7.S cscope: cannot find file /home/ydroneaud/src/linux/tools/testing/selftests/powerpc/copyloops/memcpy_64.S cscope: cannot find file /home/ydroneaud/src/linux/tools/testing/selftests/powerpc/copyloops/memcpy_power7.S cscope: cannot find file /home/ydroneaud/src/linux/tools/testing/selftests/powerpc/copyloops/copyuser_64.S And when calling cscope from ./obj-cscope/ directory, it reports errors too. Hopefully it doesn't stop it from working, so I'm still able to use cscope to browse kernel sources. It's a rather uncommon side effect of having (for the first time ?) sources files as symlinks: looking for symlinks in the kernel sources returns only: $ find . -type l ./arch/mips/boot/dts/include/dt-bindings ./arch/microblaze/boot/dts/system.dts ./arch/powerpc/boot/dts/include/dt-bindings ./arch/metag/boot/dts/include/dt-bindings ./arch/arm/boot/dts/include/dt-bindings ./tools/testing/selftests/powerpc/copyloops/copyuser_power7.S ./tools/testing/selftests/powerpc/copyloops/memcpy_64.S ./tools/testing/selftests/powerpc/copyloops/memcpy_power7.S ./tools/testing/selftests/powerpc/copyloops/copyuser_64.S ./obj-cscope/source ./Documentation/DocBook/vidioc-g-sliced-vbi-cap.xml ./Documentation/DocBook/vidioc-decoder-cmd.xml ... ./Documentation/DocBook/media-func-ioctl.xml ./Documentation/DocBook/vidioc-enumoutput.xml So one can wonder if having symlinked sources files is an expected supported feature for kbuild and all the various kernel tools/infrastructure ? Regarding cscope specifically, it does not support symlink, and it's the expected behavior according to the bug reports I was able to find: #214 cscope ignores symlinks to files http://sourceforge.net/p/cscope/bugs/214/ #229 -I options doesn't handle symbolic link http://sourceforge.net/p/cscope/bugs/229/ #247 cscope: cannot find file http://sourceforge.net/p/cscope/bugs/247/ #252 cscope: cannot find file *** http://sourceforge.net/p/cscope/bugs/252/ #261 Regression - version 15.7a does not follow symbolic links http://sourceforge.net/p/cscope/bugs/261/ Regards. -- Yann Droneaud OPTEYA -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/2] perf tools: remove unused test-volatile-register-var.c
Since commit 01287e2cb7ad, test-volatile-register-var.c
is no more built as part of the automatic feature check.
This patch remove the unneeded file.
Cc: Ingo Molnar
Cc: Arnaldo Carvalho de Melo
Cc: Peter Zijlstra
Cc: David Ahern
Cc: Jiri Olsa
Cc: Namhyung Kim
Signed-off-by: Yann Droneaud
---
tools/perf/config/feature-checks/test-volatile-register-var.c | 6 --
1 file changed, 6 deletions(-)
delete mode 100644
tools/perf/config/feature-checks/test-volatile-register-var.c
diff --git a/tools/perf/config/feature-checks/test-volatile-register-var.c
b/tools/perf/config/feature-checks/test-volatile-register-var.c
deleted file mode 100644
index c9f398d87868..
--- a/tools/perf/config/feature-checks/test-volatile-register-var.c
+++ /dev/null
@@ -1,6 +0,0 @@
-#include
-
-int main(void)
-{
- return puts("hi");
-}
--
1.8.4.2
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/2] perf tools: add .gitignore for files generated for feature check
As part of system features auto-detection, some dependency files are generated and some programs are built. These files should not be considered by git, so this patch adds a dedicated .gitignore in tools/perf/config/ sub-directory to ignore the files. Signed-off-by: Yann Droneaud --- tools/perf/config/feature-checks/.gitignore | 28 1 file changed, 28 insertions(+) create mode 100644 tools/perf/config/feature-checks/.gitignore diff --git a/tools/perf/config/feature-checks/.gitignore b/tools/perf/config/feature-checks/.gitignore new file mode 100644 index ..53b84b102c78 --- /dev/null +++ b/tools/perf/config/feature-checks/.gitignore @@ -0,0 +1,28 @@ +*.d +test-all +test-backtrace +test-bionic +test-cplus-demangle +test-dwarf +test-fortify-source +test-glibc +test-gtk2 +test-gtk2-infobar +test-hello +test-libaudit +test-libbfd +test-libelf +test-libelf-getphdrnum +test-libelf-mmap +test-liberty +test-liberty-z +test-libnuma +test-libperl +test-libpython +test-libpython-version +test-libslang +test-libunwind +test-libunwind-debug-frame +test-on-exit +test-stackprotector-all +test-timerfd -- 1.8.4.2 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 0/2] perf: little build clean up
Hi, Please find 2 little patches to remove an unused file and to ignore files generated as part of the feature auto-detection. Regards. Yann Droneaud (2): perf tools: remove unused test-volatile-register-var.c perf tools: add .gitignore for files generated for feature check tools/perf/config/feature-checks/.gitignore| 28 ++ .../feature-checks/test-volatile-register-var.c| 6 - 2 files changed, 28 insertions(+), 6 deletions(-) create mode 100644 tools/perf/config/feature-checks/.gitignore delete mode 100644 tools/perf/config/feature-checks/test-volatile-register-var.c -- 1.8.4.2 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
