* [PATCH 08/32] aio: replace kiocb_set_cancel_fn with a cancel_kiocb file operation
From: Christoph Hellwig @ 2018-05-11 11:07 UTC (permalink / raw)
To: viro; +Cc: Avi Kivity, linux-aio, linux-fsdevel, netdev, linux-api,
linux-kernel
In-Reply-To: <20180511110803.10910-1-hch@lst.de>
The current kiocb_set_cancel_fn implementation assumes the kiocb is
embedded into an aio_kiocb, which is fundamentally unsafe as it might
have been submitted by non-aio callers. Instead add a cancel_kiocb
file operation that replaced the ki_cancel function pointer set by
kiocb_set_cancel_fn, and only adds iocbs to the active list when
the read/write_iter methods return -EIOCBQUEUED and the file has
a cancel_kiocb method.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/usb/gadget/function/f_fs.c | 10 ++--------
drivers/usb/gadget/legacy/inode.c | 5 ++---
fs/aio.c | 31 ++++++++++++------------------
fs/orangefs/orangefs-kernel.h | 1 -
include/linux/aio.h | 7 -------
include/linux/fs.h | 1 +
6 files changed, 17 insertions(+), 38 deletions(-)
diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c
index 0294e4f18873..531a7206407b 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -26,7 +26,6 @@
#include <linux/usb/composite.h>
#include <linux/usb/functionfs.h>
-#include <linux/aio.h>
#include <linux/mmu_context.h>
#include <linux/poll.h>
#include <linux/eventfd.h>
@@ -1072,7 +1071,7 @@ ffs_epfile_open(struct inode *inode, struct file *file)
return 0;
}
-static int ffs_aio_cancel(struct kiocb *kiocb)
+static int ffs_epfile_cancel_kiocb(struct kiocb *kiocb)
{
struct ffs_io_data *io_data = kiocb->private;
struct ffs_epfile *epfile = kiocb->ki_filp->private_data;
@@ -1115,9 +1114,6 @@ static ssize_t ffs_epfile_write_iter(struct kiocb *kiocb, struct iov_iter *from)
kiocb->private = p;
- if (p->aio)
- kiocb_set_cancel_fn(kiocb, ffs_aio_cancel);
-
res = ffs_epfile_io(kiocb->ki_filp, p);
if (res == -EIOCBQUEUED)
return res;
@@ -1160,9 +1156,6 @@ static ssize_t ffs_epfile_read_iter(struct kiocb *kiocb, struct iov_iter *to)
kiocb->private = p;
- if (p->aio)
- kiocb_set_cancel_fn(kiocb, ffs_aio_cancel);
-
res = ffs_epfile_io(kiocb->ki_filp, p);
if (res == -EIOCBQUEUED)
return res;
@@ -1274,6 +1267,7 @@ static const struct file_operations ffs_epfile_operations = {
.read_iter = ffs_epfile_read_iter,
.release = ffs_epfile_release,
.unlocked_ioctl = ffs_epfile_ioctl,
+ .cancel_kiocb = ffs_epfile_cancel_kiocb,
};
diff --git a/drivers/usb/gadget/legacy/inode.c b/drivers/usb/gadget/legacy/inode.c
index 37ca0e669bd8..02ea83c8861a 100644
--- a/drivers/usb/gadget/legacy/inode.c
+++ b/drivers/usb/gadget/legacy/inode.c
@@ -21,7 +21,6 @@
#include <linux/slab.h>
#include <linux/poll.h>
#include <linux/mmu_context.h>
-#include <linux/aio.h>
#include <linux/uio.h>
#include <linux/refcount.h>
#include <linux/delay.h>
@@ -435,7 +434,7 @@ struct kiocb_priv {
unsigned actual;
};
-static int ep_aio_cancel(struct kiocb *iocb)
+static int ep_cancel_kiocb(struct kiocb *iocb)
{
struct kiocb_priv *priv = iocb->private;
struct ep_data *epdata;
@@ -528,7 +527,6 @@ static ssize_t ep_aio(struct kiocb *iocb,
iocb->private = priv;
priv->iocb = iocb;
- kiocb_set_cancel_fn(iocb, ep_aio_cancel);
get_ep(epdata);
priv->epdata = epdata;
priv->actual = 0;
@@ -700,6 +698,7 @@ static const struct file_operations ep_io_operations = {
.unlocked_ioctl = ep_ioctl,
.read_iter = ep_read_iter,
.write_iter = ep_write_iter,
+ .cancel_kiocb = ep_cancel_kiocb,
};
/* ENDPOINT INITIALIZATION
diff --git a/fs/aio.c b/fs/aio.c
index 8991baa38d5d..be10dde20c8e 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -171,7 +171,6 @@ struct aio_kiocb {
};
struct kioctx *ki_ctx;
- kiocb_cancel_fn *ki_cancel;
struct iocb __user *ki_user_iocb; /* user's aiocb */
__u64 ki_user_data; /* user's data for completion */
@@ -545,22 +544,6 @@ static int aio_setup_ring(struct kioctx *ctx, unsigned int nr_events)
#define AIO_EVENTS_FIRST_PAGE ((PAGE_SIZE - sizeof(struct aio_ring)) / sizeof(struct io_event))
#define AIO_EVENTS_OFFSET (AIO_EVENTS_PER_PAGE - AIO_EVENTS_FIRST_PAGE)
-void kiocb_set_cancel_fn(struct kiocb *iocb, kiocb_cancel_fn *cancel)
-{
- struct aio_kiocb *req = container_of(iocb, struct aio_kiocb, rw);
- struct kioctx *ctx = req->ki_ctx;
- unsigned long flags;
-
- if (WARN_ON_ONCE(!list_empty(&req->ki_list)))
- return;
-
- spin_lock_irqsave(&ctx->ctx_lock, flags);
- list_add_tail(&req->ki_list, &ctx->active_reqs);
- req->ki_cancel = cancel;
- spin_unlock_irqrestore(&ctx->ctx_lock, flags);
-}
-EXPORT_SYMBOL(kiocb_set_cancel_fn);
-
/*
* free_ioctx() should be RCU delayed to synchronize against the RCU
* protected lookup_ioctx() and also needs process context to call
@@ -608,7 +591,7 @@ static void free_ioctx_users(struct percpu_ref *ref)
req = list_first_entry(&ctx->active_reqs,
struct aio_kiocb, ki_list);
list_del_init(&req->ki_list);
- req->ki_cancel(&req->rw);
+ req->rw.ki_filp->f_op->cancel_kiocb(&req->rw);
}
spin_unlock_irq(&ctx->ctx_lock);
@@ -1447,6 +1430,16 @@ static inline ssize_t aio_rw_ret(struct kiocb *req, ssize_t ret)
{
switch (ret) {
case -EIOCBQUEUED:
+ if (req->ki_filp->f_op->cancel_kiocb) {
+ struct aio_kiocb *iocb =
+ container_of(req, struct aio_kiocb, rw);
+ struct kioctx *ctx = iocb->ki_ctx;
+ unsigned long flags;
+
+ spin_lock_irqsave(&ctx->ctx_lock, flags);
+ list_add_tail(&iocb->ki_list, &ctx->active_reqs);
+ spin_unlock_irqrestore(&ctx->ctx_lock, flags);
+ }
return ret;
case -ERESTARTSYS:
case -ERESTARTNOINTR:
@@ -1824,7 +1817,7 @@ SYSCALL_DEFINE3(io_cancel, aio_context_t, ctx_id, struct iocb __user *, iocb,
kiocb = lookup_kiocb(ctx, iocb);
if (kiocb) {
list_del_init(&kiocb->ki_list);
- ret = kiocb->ki_cancel(&kiocb->rw);
+ ret = kiocb->rw.ki_filp->f_op->cancel_kiocb(&kiocb->rw);
}
spin_unlock_irq(&ctx->ctx_lock);
diff --git a/fs/orangefs/orangefs-kernel.h b/fs/orangefs/orangefs-kernel.h
index c29bb0ebc6bb..5cd598c59bdc 100644
--- a/fs/orangefs/orangefs-kernel.h
+++ b/fs/orangefs/orangefs-kernel.h
@@ -34,7 +34,6 @@
#include <linux/fs.h>
#include <linux/vmalloc.h>
-#include <linux/aio.h>
#include <linux/posix_acl.h>
#include <linux/posix_acl_xattr.h>
#include <linux/compat.h>
diff --git a/include/linux/aio.h b/include/linux/aio.h
index b83e68dd006f..42e3cc953005 100644
--- a/include/linux/aio.h
+++ b/include/linux/aio.h
@@ -4,20 +4,13 @@
#include <linux/aio_abi.h>
-struct kioctx;
-struct kiocb;
struct mm_struct;
-typedef int (kiocb_cancel_fn)(struct kiocb *);
-
/* prototypes */
#ifdef CONFIG_AIO
extern void exit_aio(struct mm_struct *mm);
-void kiocb_set_cancel_fn(struct kiocb *req, kiocb_cancel_fn *cancel);
#else
static inline void exit_aio(struct mm_struct *mm) { }
-static inline void kiocb_set_cancel_fn(struct kiocb *req,
- kiocb_cancel_fn *cancel) { }
#endif /* CONFIG_AIO */
/* for sysctl: */
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 4b6045ebb2f2..7fe64df9eb09 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1742,6 +1742,7 @@ struct file_operations {
u64);
ssize_t (*dedupe_file_range)(struct file *, u64, u64, struct file *,
u64);
+ int (*cancel_kiocb)(struct kiocb *);
} __randomize_layout;
struct inode_operations {
--
2.17.0
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply related
* [PATCH 07/32] aio: simplify cancellation
From: Christoph Hellwig @ 2018-05-11 11:07 UTC (permalink / raw)
To: viro; +Cc: Avi Kivity, linux-aio, linux-fsdevel, netdev, linux-api,
linux-kernel
In-Reply-To: <20180511110803.10910-1-hch@lst.de>
With the current aio code there is no need for the magic KIOCB_CANCELLED
value, as a cancelation just kicks the driver to queue the completion
ASAP, with all actual completion handling done in another thread. Given
that both the completion path and cancelation take the context lock there
is no need for magic cmpxchg loops either. If we remove iocbs from the
active list before calling ->ki_cancel we can also rely on the invariant
thay anything found on the list has a ->ki_cancel callback and can be
cancelled, further simplifing the code.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/aio.c | 49 ++++++-------------------------------------------
1 file changed, 6 insertions(+), 43 deletions(-)
diff --git a/fs/aio.c b/fs/aio.c
index 2e0bc04f7920..8991baa38d5d 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -164,19 +164,6 @@ struct fsync_iocb {
bool datasync;
};
-/*
- * We use ki_cancel == KIOCB_CANCELLED to indicate that a kiocb has been either
- * cancelled or completed (this makes a certain amount of sense because
- * successful cancellation - io_cancel() - does deliver the completion to
- * userspace).
- *
- * And since most things don't implement kiocb cancellation and we'd really like
- * kiocb completion to be lockless when possible, we use ki_cancel to
- * synchronize cancellation and completion - we only set it to KIOCB_CANCELLED
- * with xchg() or cmpxchg(), see batch_complete_aio() and kiocb_cancel().
- */
-#define KIOCB_CANCELLED ((void *) (~0ULL))
-
struct aio_kiocb {
union {
struct kiocb rw;
@@ -574,27 +561,6 @@ void kiocb_set_cancel_fn(struct kiocb *iocb, kiocb_cancel_fn *cancel)
}
EXPORT_SYMBOL(kiocb_set_cancel_fn);
-static int kiocb_cancel(struct aio_kiocb *kiocb)
-{
- kiocb_cancel_fn *old, *cancel;
-
- /*
- * Don't want to set kiocb->ki_cancel = KIOCB_CANCELLED unless it
- * actually has a cancel function, hence the cmpxchg()
- */
-
- cancel = READ_ONCE(kiocb->ki_cancel);
- do {
- if (!cancel || cancel == KIOCB_CANCELLED)
- return -EINVAL;
-
- old = cancel;
- cancel = cmpxchg(&kiocb->ki_cancel, old, KIOCB_CANCELLED);
- } while (cancel != old);
-
- return cancel(&kiocb->rw);
-}
-
/*
* free_ioctx() should be RCU delayed to synchronize against the RCU
* protected lookup_ioctx() and also needs process context to call
@@ -641,9 +607,8 @@ static void free_ioctx_users(struct percpu_ref *ref)
while (!list_empty(&ctx->active_reqs)) {
req = list_first_entry(&ctx->active_reqs,
struct aio_kiocb, ki_list);
-
list_del_init(&req->ki_list);
- kiocb_cancel(req);
+ req->ki_cancel(&req->rw);
}
spin_unlock_irq(&ctx->ctx_lock);
@@ -1843,8 +1808,8 @@ SYSCALL_DEFINE3(io_cancel, aio_context_t, ctx_id, struct iocb __user *, iocb,
{
struct kioctx *ctx;
struct aio_kiocb *kiocb;
+ int ret = -EINVAL;
u32 key;
- int ret;
if (unlikely(get_user(key, &iocb->aio_key)))
return -EFAULT;
@@ -1856,13 +1821,11 @@ SYSCALL_DEFINE3(io_cancel, aio_context_t, ctx_id, struct iocb __user *, iocb,
return -EINVAL;
spin_lock_irq(&ctx->ctx_lock);
-
kiocb = lookup_kiocb(ctx, iocb);
- if (kiocb)
- ret = kiocb_cancel(kiocb);
- else
- ret = -EINVAL;
-
+ if (kiocb) {
+ list_del_init(&kiocb->ki_list);
+ ret = kiocb->ki_cancel(&kiocb->rw);
+ }
spin_unlock_irq(&ctx->ctx_lock);
if (!ret) {
--
2.17.0
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply related
* [PATCH 06/32] aio: simplify KIOCB_KEY handling
From: Christoph Hellwig @ 2018-05-11 11:07 UTC (permalink / raw)
To: viro; +Cc: Avi Kivity, linux-aio, linux-fsdevel, netdev, linux-api,
linux-kernel
In-Reply-To: <20180511110803.10910-1-hch@lst.de>
No need to pass the key field to lookup_iocb to compare it with KIOCB_KEY,
as we can do that right after retrieving it from userspace. Also move the
KIOCB_KEY definition to aio.c as it is an internal value not used by any
other place in the kernel.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/aio.c | 14 +++++++-------
include/linux/aio.h | 2 --
2 files changed, 7 insertions(+), 9 deletions(-)
diff --git a/fs/aio.c b/fs/aio.c
index f3eae5d5771b..2e0bc04f7920 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -46,6 +46,8 @@
#include "internal.h"
+#define KIOCB_KEY 0
+
#define AIO_RING_MAGIC 0xa10a10a1
#define AIO_RING_COMPAT_FEATURES 1
#define AIO_RING_INCOMPAT_FEATURES 0
@@ -1812,15 +1814,12 @@ COMPAT_SYSCALL_DEFINE3(io_submit, compat_aio_context_t, ctx_id,
* Finds a given iocb for cancellation.
*/
static struct aio_kiocb *
-lookup_kiocb(struct kioctx *ctx, struct iocb __user *iocb, u32 key)
+lookup_kiocb(struct kioctx *ctx, struct iocb __user *iocb)
{
struct aio_kiocb *kiocb;
assert_spin_locked(&ctx->ctx_lock);
- if (key != KIOCB_KEY)
- return NULL;
-
/* TODO: use a hash or array, this sucks. */
list_for_each_entry(kiocb, &ctx->active_reqs, ki_list) {
if (kiocb->ki_user_iocb == iocb)
@@ -1847,9 +1846,10 @@ SYSCALL_DEFINE3(io_cancel, aio_context_t, ctx_id, struct iocb __user *, iocb,
u32 key;
int ret;
- ret = get_user(key, &iocb->aio_key);
- if (unlikely(ret))
+ if (unlikely(get_user(key, &iocb->aio_key)))
return -EFAULT;
+ if (unlikely(key != KIOCB_KEY))
+ return -EINVAL;
ctx = lookup_ioctx(ctx_id);
if (unlikely(!ctx))
@@ -1857,7 +1857,7 @@ SYSCALL_DEFINE3(io_cancel, aio_context_t, ctx_id, struct iocb __user *, iocb,
spin_lock_irq(&ctx->ctx_lock);
- kiocb = lookup_kiocb(ctx, iocb, key);
+ kiocb = lookup_kiocb(ctx, iocb);
if (kiocb)
ret = kiocb_cancel(kiocb);
else
diff --git a/include/linux/aio.h b/include/linux/aio.h
index 9d8aabecfe2d..b83e68dd006f 100644
--- a/include/linux/aio.h
+++ b/include/linux/aio.h
@@ -8,8 +8,6 @@ struct kioctx;
struct kiocb;
struct mm_struct;
-#define KIOCB_KEY 0
-
typedef int (kiocb_cancel_fn)(struct kiocb *);
/* prototypes */
--
2.17.0
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply related
* [PATCH 05/32] fs: introduce new ->get_poll_head and ->poll_mask methods
From: Christoph Hellwig @ 2018-05-11 11:07 UTC (permalink / raw)
To: viro; +Cc: Avi Kivity, linux-aio, linux-fsdevel, netdev, linux-api,
linux-kernel
In-Reply-To: <20180511110803.10910-1-hch@lst.de>
->get_poll_head returns the waitqueue that the poll operation is going
to sleep on. Note that this means we can only use a single waitqueue
for the poll, unlike some current drivers that use two waitqueues for
different events. But now that we have keyed wakeups and heavily use
those for poll there aren't that many good reason left to keep the
multiple waitqueues, and if there are any ->poll is still around, the
driver just won't support aio poll.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
---
Documentation/filesystems/Locking | 7 ++++++-
Documentation/filesystems/vfs.txt | 13 +++++++++++++
fs/select.c | 23 +++++++++++++++++++++++
include/linux/fs.h | 2 ++
include/linux/poll.h | 12 ++++++------
5 files changed, 50 insertions(+), 7 deletions(-)
diff --git a/Documentation/filesystems/Locking b/Documentation/filesystems/Locking
index 220bba28f72b..6d227f9d7bd9 100644
--- a/Documentation/filesystems/Locking
+++ b/Documentation/filesystems/Locking
@@ -440,6 +440,8 @@ prototypes:
ssize_t (*write_iter) (struct kiocb *, struct iov_iter *);
int (*iterate) (struct file *, struct dir_context *);
__poll_t (*poll) (struct file *, struct poll_table_struct *);
+ struct wait_queue_head * (*get_poll_head)(struct file *, __poll_t);
+ __poll_t (*poll_mask) (struct file *, __poll_t);
long (*unlocked_ioctl) (struct file *, unsigned int, unsigned long);
long (*compat_ioctl) (struct file *, unsigned int, unsigned long);
int (*mmap) (struct file *, struct vm_area_struct *);
@@ -470,7 +472,7 @@ prototypes:
};
locking rules:
- All may block.
+ All except for ->poll_mask may block.
->llseek() locking has moved from llseek to the individual llseek
implementations. If your fs is not using generic_file_llseek, you
@@ -498,6 +500,9 @@ in sys_read() and friends.
the lease within the individual filesystem to record the result of the
operation
+->poll_mask can be called with or without the waitqueue lock for the waitqueue
+returned from ->get_poll_head.
+
--------------------------- dquot_operations -------------------------------
prototypes:
int (*write_dquot) (struct dquot *);
diff --git a/Documentation/filesystems/vfs.txt b/Documentation/filesystems/vfs.txt
index f608180ad59d..829a7b7857a4 100644
--- a/Documentation/filesystems/vfs.txt
+++ b/Documentation/filesystems/vfs.txt
@@ -857,6 +857,8 @@ struct file_operations {
ssize_t (*write_iter) (struct kiocb *, struct iov_iter *);
int (*iterate) (struct file *, struct dir_context *);
__poll_t (*poll) (struct file *, struct poll_table_struct *);
+ struct wait_queue_head * (*get_poll_head)(struct file *, __poll_t);
+ __poll_t (*poll_mask) (struct file *, __poll_t);
long (*unlocked_ioctl) (struct file *, unsigned int, unsigned long);
long (*compat_ioctl) (struct file *, unsigned int, unsigned long);
int (*mmap) (struct file *, struct vm_area_struct *);
@@ -901,6 +903,17 @@ otherwise noted.
activity on this file and (optionally) go to sleep until there
is activity. Called by the select(2) and poll(2) system calls
+ get_poll_head: Returns the struct wait_queue_head that callers can
+ wait on. Callers need to check the returned events using ->poll_mask
+ once woken. Can return NULL to indicate polling is not supported,
+ or any error code using the ERR_PTR convention to indicate that a
+ grave error occured and ->poll_mask shall not be called.
+
+ poll_mask: return the mask of EPOLL* values describing the file descriptor
+ state. Called either before going to sleep on the waitqueue returned by
+ get_poll_head, or after it has been woken. If ->get_poll_head and
+ ->poll_mask are implemented ->poll does not need to be implement.
+
unlocked_ioctl: called by the ioctl(2) system call.
compat_ioctl: called by the ioctl(2) system call when 32 bit system calls
diff --git a/fs/select.c b/fs/select.c
index e30def680b2e..bc3cc0f98896 100644
--- a/fs/select.c
+++ b/fs/select.c
@@ -34,6 +34,29 @@
#include <linux/uaccess.h>
+__poll_t vfs_poll(struct file *file, struct poll_table_struct *pt)
+{
+ if (file->f_op->poll) {
+ return file->f_op->poll(file, pt);
+ } else if (file_has_poll_mask(file)) {
+ unsigned int events = poll_requested_events(pt);
+ struct wait_queue_head *head;
+
+ if (pt && pt->_qproc) {
+ head = file->f_op->get_poll_head(file, events);
+ if (!head)
+ return DEFAULT_POLLMASK;
+ if (IS_ERR(head))
+ return EPOLLERR;
+ pt->_qproc(file, head, pt);
+ }
+
+ return file->f_op->poll_mask(file, events);
+ } else {
+ return DEFAULT_POLLMASK;
+ }
+}
+EXPORT_SYMBOL_GPL(vfs_poll);
/*
* Estimate expected accuracy in ns from a timeval.
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 760d8da1b6c7..4b6045ebb2f2 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1711,6 +1711,8 @@ struct file_operations {
int (*iterate) (struct file *, struct dir_context *);
int (*iterate_shared) (struct file *, struct dir_context *);
__poll_t (*poll) (struct file *, struct poll_table_struct *);
+ struct wait_queue_head * (*get_poll_head)(struct file *, __poll_t);
+ __poll_t (*poll_mask) (struct file *, __poll_t);
long (*unlocked_ioctl) (struct file *, unsigned int, unsigned long);
long (*compat_ioctl) (struct file *, unsigned int, unsigned long);
int (*mmap) (struct file *, struct vm_area_struct *);
diff --git a/include/linux/poll.h b/include/linux/poll.h
index 7e0fdcf905d2..fdf86b4cbc71 100644
--- a/include/linux/poll.h
+++ b/include/linux/poll.h
@@ -74,18 +74,18 @@ static inline void init_poll_funcptr(poll_table *pt, poll_queue_proc qproc)
pt->_key = ~(__poll_t)0; /* all events enabled */
}
-static inline bool file_can_poll(struct file *file)
+static inline bool file_has_poll_mask(struct file *file)
{
- return file->f_op->poll;
+ return file->f_op->get_poll_head && file->f_op->poll_mask;
}
-static inline __poll_t vfs_poll(struct file *file, struct poll_table_struct *pt)
+static inline bool file_can_poll(struct file *file)
{
- if (unlikely(!file->f_op->poll))
- return DEFAULT_POLLMASK;
- return file->f_op->poll(file, pt);
+ return file->f_op->poll || file_has_poll_mask(file);
}
+__poll_t vfs_poll(struct file *file, struct poll_table_struct *pt);
+
struct poll_table_entry {
struct file *filp;
__poll_t key;
--
2.17.0
^ permalink raw reply related
* [PATCH 04/32] fs: add new vfs_poll and file_can_poll helpers
From: Christoph Hellwig @ 2018-05-11 11:07 UTC (permalink / raw)
To: viro; +Cc: Avi Kivity, linux-aio, linux-fsdevel, netdev, linux-api,
linux-kernel
In-Reply-To: <20180511110803.10910-1-hch@lst.de>
These abstract out calls to the poll method in preparation for changes
in how we poll.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
---
drivers/staging/comedi/drivers/serial2002.c | 4 ++--
drivers/vfio/virqfd.c | 2 +-
drivers/vhost/vhost.c | 2 +-
fs/eventpoll.c | 5 ++---
fs/select.c | 23 +++++++--------------
include/linux/poll.h | 12 +++++++++++
mm/memcontrol.c | 2 +-
net/9p/trans_fd.c | 18 ++++------------
virt/kvm/eventfd.c | 2 +-
9 files changed, 32 insertions(+), 38 deletions(-)
diff --git a/drivers/staging/comedi/drivers/serial2002.c b/drivers/staging/comedi/drivers/serial2002.c
index b3f3b4a201af..5471b2212a62 100644
--- a/drivers/staging/comedi/drivers/serial2002.c
+++ b/drivers/staging/comedi/drivers/serial2002.c
@@ -113,7 +113,7 @@ static void serial2002_tty_read_poll_wait(struct file *f, int timeout)
long elapsed;
__poll_t mask;
- mask = f->f_op->poll(f, &table.pt);
+ mask = vfs_poll(f, &table.pt);
if (mask & (EPOLLRDNORM | EPOLLRDBAND | EPOLLIN |
EPOLLHUP | EPOLLERR)) {
break;
@@ -136,7 +136,7 @@ static int serial2002_tty_read(struct file *f, int timeout)
result = -1;
if (!IS_ERR(f)) {
- if (f->f_op->poll) {
+ if (file_can_poll(f)) {
serial2002_tty_read_poll_wait(f, timeout);
if (kernel_read(f, &ch, 1, &pos) == 1)
diff --git a/drivers/vfio/virqfd.c b/drivers/vfio/virqfd.c
index 085700f1be10..2a1be859ee71 100644
--- a/drivers/vfio/virqfd.c
+++ b/drivers/vfio/virqfd.c
@@ -166,7 +166,7 @@ int vfio_virqfd_enable(void *opaque,
init_waitqueue_func_entry(&virqfd->wait, virqfd_wakeup);
init_poll_funcptr(&virqfd->pt, virqfd_ptable_queue_proc);
- events = irqfd.file->f_op->poll(irqfd.file, &virqfd->pt);
+ events = vfs_poll(irqfd.file, &virqfd->pt);
/*
* Check if there was an event already pending on the eventfd
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index f3bd8e941224..f6022881f147 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -208,7 +208,7 @@ int vhost_poll_start(struct vhost_poll *poll, struct file *file)
if (poll->wqh)
return 0;
- mask = file->f_op->poll(file, &poll->table);
+ mask = vfs_poll(file, &poll->table);
if (mask)
vhost_poll_wakeup(&poll->wait, 0, 0, poll_to_key(mask));
if (mask & EPOLLERR) {
diff --git a/fs/eventpoll.c b/fs/eventpoll.c
index 602ca4285b2e..67db22fe99c5 100644
--- a/fs/eventpoll.c
+++ b/fs/eventpoll.c
@@ -884,8 +884,7 @@ static __poll_t ep_item_poll(const struct epitem *epi, poll_table *pt,
pt->_key = epi->event.events;
if (!is_file_epoll(epi->ffd.file))
- return epi->ffd.file->f_op->poll(epi->ffd.file, pt) &
- epi->event.events;
+ return vfs_poll(epi->ffd.file, pt) & epi->event.events;
ep = epi->ffd.file->private_data;
poll_wait(epi->ffd.file, &ep->poll_wait, pt);
@@ -2025,7 +2024,7 @@ SYSCALL_DEFINE4(epoll_ctl, int, epfd, int, op, int, fd,
/* The target file descriptor must support poll */
error = -EPERM;
- if (!tf.file->f_op->poll)
+ if (!file_can_poll(tf.file))
goto error_tgt_fput;
/* Check if EPOLLWAKEUP is allowed */
diff --git a/fs/select.c b/fs/select.c
index 25da26253485..e30def680b2e 100644
--- a/fs/select.c
+++ b/fs/select.c
@@ -502,14 +502,10 @@ static int do_select(int n, fd_set_bits *fds, struct timespec64 *end_time)
continue;
f = fdget(i);
if (f.file) {
- const struct file_operations *f_op;
- f_op = f.file->f_op;
- mask = DEFAULT_POLLMASK;
- if (f_op->poll) {
- wait_key_set(wait, in, out,
- bit, busy_flag);
- mask = (*f_op->poll)(f.file, wait);
- }
+ wait_key_set(wait, in, out, bit,
+ busy_flag);
+ mask = vfs_poll(f.file, wait);
+
fdput(f);
if ((mask & POLLIN_SET) && (in & bit)) {
res_in |= bit;
@@ -825,13 +821,10 @@ static inline __poll_t do_pollfd(struct pollfd *pollfd, poll_table *pwait,
/* userland u16 ->events contains POLL... bitmap */
filter = demangle_poll(pollfd->events) | EPOLLERR | EPOLLHUP;
- mask = DEFAULT_POLLMASK;
- if (f.file->f_op->poll) {
- pwait->_key = filter | busy_flag;
- mask = f.file->f_op->poll(f.file, pwait);
- if (mask & busy_flag)
- *can_busy_poll = true;
- }
+ pwait->_key = filter | busy_flag;
+ mask = vfs_poll(f.file, pwait);
+ if (mask & busy_flag)
+ *can_busy_poll = true;
mask &= filter; /* Mask out unneeded events. */
fdput(f);
diff --git a/include/linux/poll.h b/include/linux/poll.h
index a3576da63377..7e0fdcf905d2 100644
--- a/include/linux/poll.h
+++ b/include/linux/poll.h
@@ -74,6 +74,18 @@ static inline void init_poll_funcptr(poll_table *pt, poll_queue_proc qproc)
pt->_key = ~(__poll_t)0; /* all events enabled */
}
+static inline bool file_can_poll(struct file *file)
+{
+ return file->f_op->poll;
+}
+
+static inline __poll_t vfs_poll(struct file *file, struct poll_table_struct *pt)
+{
+ if (unlikely(!file->f_op->poll))
+ return DEFAULT_POLLMASK;
+ return file->f_op->poll(file, pt);
+}
+
struct poll_table_entry {
struct file *filp;
__poll_t key;
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 2bd3df3d101a..1695f38630f1 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -3849,7 +3849,7 @@ static ssize_t memcg_write_event_control(struct kernfs_open_file *of,
if (ret)
goto out_put_css;
- efile.file->f_op->poll(efile.file, &event->pt);
+ vfs_poll(efile.file, &event->pt);
spin_lock(&memcg->event_list_lock);
list_add(&event->list, &memcg->event_list);
diff --git a/net/9p/trans_fd.c b/net/9p/trans_fd.c
index 0cfba919d167..3811775692d0 100644
--- a/net/9p/trans_fd.c
+++ b/net/9p/trans_fd.c
@@ -231,7 +231,7 @@ static void p9_conn_cancel(struct p9_conn *m, int err)
static __poll_t
p9_fd_poll(struct p9_client *client, struct poll_table_struct *pt, int *err)
{
- __poll_t ret, n;
+ __poll_t ret;
struct p9_trans_fd *ts = NULL;
if (client && client->status == Connected)
@@ -243,19 +243,9 @@ p9_fd_poll(struct p9_client *client, struct poll_table_struct *pt, int *err)
return EPOLLERR;
}
- if (!ts->rd->f_op->poll)
- ret = DEFAULT_POLLMASK;
- else
- ret = ts->rd->f_op->poll(ts->rd, pt);
-
- if (ts->rd != ts->wr) {
- if (!ts->wr->f_op->poll)
- n = DEFAULT_POLLMASK;
- else
- n = ts->wr->f_op->poll(ts->wr, pt);
- ret = (ret & ~EPOLLOUT) | (n & ~EPOLLIN);
- }
-
+ ret = vfs_poll(ts->rd, pt);
+ if (ts->rd != ts->wr)
+ ret = (ret & ~EPOLLOUT) | (vfs_poll(ts->wr, pt) & ~EPOLLIN);
return ret;
}
diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
index 6e865e8b5b10..90d30fbe95ae 100644
--- a/virt/kvm/eventfd.c
+++ b/virt/kvm/eventfd.c
@@ -397,7 +397,7 @@ kvm_irqfd_assign(struct kvm *kvm, struct kvm_irqfd *args)
* Check if there was an event already pending on the eventfd
* before we registered, and trigger it as if we didn't miss it.
*/
- events = f.file->f_op->poll(f.file, &irqfd->pt);
+ events = vfs_poll(f.file, &irqfd->pt);
if (events & EPOLLIN)
schedule_work(&irqfd->inject);
--
2.17.0
^ permalink raw reply related
* [PATCH 03/32] fs: update documentation to mention __poll_t and match the code
From: Christoph Hellwig @ 2018-05-11 11:07 UTC (permalink / raw)
To: viro; +Cc: Avi Kivity, linux-aio, linux-fsdevel, netdev, linux-api,
linux-kernel
In-Reply-To: <20180511110803.10910-1-hch@lst.de>
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
Documentation/filesystems/Locking | 2 +-
Documentation/filesystems/vfs.txt | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/Documentation/filesystems/Locking b/Documentation/filesystems/Locking
index 75d2d57e2c44..220bba28f72b 100644
--- a/Documentation/filesystems/Locking
+++ b/Documentation/filesystems/Locking
@@ -439,7 +439,7 @@ prototypes:
ssize_t (*read_iter) (struct kiocb *, struct iov_iter *);
ssize_t (*write_iter) (struct kiocb *, struct iov_iter *);
int (*iterate) (struct file *, struct dir_context *);
- unsigned int (*poll) (struct file *, struct poll_table_struct *);
+ __poll_t (*poll) (struct file *, struct poll_table_struct *);
long (*unlocked_ioctl) (struct file *, unsigned int, unsigned long);
long (*compat_ioctl) (struct file *, unsigned int, unsigned long);
int (*mmap) (struct file *, struct vm_area_struct *);
diff --git a/Documentation/filesystems/vfs.txt b/Documentation/filesystems/vfs.txt
index 5fd325df59e2..f608180ad59d 100644
--- a/Documentation/filesystems/vfs.txt
+++ b/Documentation/filesystems/vfs.txt
@@ -856,7 +856,7 @@ struct file_operations {
ssize_t (*read_iter) (struct kiocb *, struct iov_iter *);
ssize_t (*write_iter) (struct kiocb *, struct iov_iter *);
int (*iterate) (struct file *, struct dir_context *);
- unsigned int (*poll) (struct file *, struct poll_table_struct *);
+ __poll_t (*poll) (struct file *, struct poll_table_struct *);
long (*unlocked_ioctl) (struct file *, unsigned int, unsigned long);
long (*compat_ioctl) (struct file *, unsigned int, unsigned long);
int (*mmap) (struct file *, struct vm_area_struct *);
--
2.17.0
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply related
* [PATCH 02/32] fs: cleanup do_pollfd
From: Christoph Hellwig @ 2018-05-11 11:07 UTC (permalink / raw)
To: viro; +Cc: Avi Kivity, linux-aio, linux-fsdevel, netdev, linux-api,
linux-kernel
In-Reply-To: <20180511110803.10910-1-hch@lst.de>
Use straightline code with failure handling gotos instead of a lot
of nested conditionals.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
---
fs/select.c | 48 +++++++++++++++++++++++-------------------------
1 file changed, 23 insertions(+), 25 deletions(-)
diff --git a/fs/select.c b/fs/select.c
index a87f396f0313..25da26253485 100644
--- a/fs/select.c
+++ b/fs/select.c
@@ -812,34 +812,32 @@ static inline __poll_t do_pollfd(struct pollfd *pollfd, poll_table *pwait,
bool *can_busy_poll,
__poll_t busy_flag)
{
- __poll_t mask;
- int fd;
-
- mask = 0;
- fd = pollfd->fd;
- if (fd >= 0) {
- struct fd f = fdget(fd);
- mask = EPOLLNVAL;
- if (f.file) {
- /* userland u16 ->events contains POLL... bitmap */
- __poll_t filter = demangle_poll(pollfd->events) |
- EPOLLERR | EPOLLHUP;
- mask = DEFAULT_POLLMASK;
- if (f.file->f_op->poll) {
- pwait->_key = filter;
- pwait->_key |= busy_flag;
- mask = f.file->f_op->poll(f.file, pwait);
- if (mask & busy_flag)
- *can_busy_poll = true;
- }
- /* Mask out unneeded events. */
- mask &= filter;
- fdput(f);
- }
+ int fd = pollfd->fd;
+ __poll_t mask = 0, filter;
+ struct fd f;
+
+ if (fd < 0)
+ goto out;
+ mask = EPOLLNVAL;
+ f = fdget(fd);
+ if (!f.file)
+ goto out;
+
+ /* userland u16 ->events contains POLL... bitmap */
+ filter = demangle_poll(pollfd->events) | EPOLLERR | EPOLLHUP;
+ mask = DEFAULT_POLLMASK;
+ if (f.file->f_op->poll) {
+ pwait->_key = filter | busy_flag;
+ mask = f.file->f_op->poll(f.file, pwait);
+ if (mask & busy_flag)
+ *can_busy_poll = true;
}
+ mask &= filter; /* Mask out unneeded events. */
+ fdput(f);
+
+out:
/* ... and so does ->revents */
pollfd->revents = mangle_poll(mask);
-
return mask;
}
--
2.17.0
^ permalink raw reply related
* [PATCH 01/32] fs: unexport poll_schedule_timeout
From: Christoph Hellwig @ 2018-05-11 11:07 UTC (permalink / raw)
To: viro; +Cc: Avi Kivity, linux-aio, linux-fsdevel, netdev, linux-api,
linux-kernel
In-Reply-To: <20180511110803.10910-1-hch@lst.de>
No users outside of select.c.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
---
fs/select.c | 3 +--
include/linux/poll.h | 2 --
2 files changed, 1 insertion(+), 4 deletions(-)
diff --git a/fs/select.c b/fs/select.c
index ba879c51288f..a87f396f0313 100644
--- a/fs/select.c
+++ b/fs/select.c
@@ -233,7 +233,7 @@ static void __pollwait(struct file *filp, wait_queue_head_t *wait_address,
add_wait_queue(wait_address, &entry->wait);
}
-int poll_schedule_timeout(struct poll_wqueues *pwq, int state,
+static int poll_schedule_timeout(struct poll_wqueues *pwq, int state,
ktime_t *expires, unsigned long slack)
{
int rc = -EINTR;
@@ -258,7 +258,6 @@ int poll_schedule_timeout(struct poll_wqueues *pwq, int state,
return rc;
}
-EXPORT_SYMBOL(poll_schedule_timeout);
/**
* poll_select_set_timeout - helper function to setup the timeout value
diff --git a/include/linux/poll.h b/include/linux/poll.h
index f45ebd017eaa..a3576da63377 100644
--- a/include/linux/poll.h
+++ b/include/linux/poll.h
@@ -96,8 +96,6 @@ struct poll_wqueues {
extern void poll_initwait(struct poll_wqueues *pwq);
extern void poll_freewait(struct poll_wqueues *pwq);
-extern int poll_schedule_timeout(struct poll_wqueues *pwq, int state,
- ktime_t *expires, unsigned long slack);
extern u64 select_estimate_accuracy(struct timespec64 *tv);
#define MAX_INT64_SECONDS (((s64)(~((u64)0)>>1)/HZ)-1)
--
2.17.0
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply related
* aio poll and a new in-kernel poll API V10
From: Christoph Hellwig @ 2018-05-11 11:07 UTC (permalink / raw)
To: viro; +Cc: Avi Kivity, linux-aio, linux-fsdevel, netdev, linux-api,
linux-kernel
Hi all,
this series adds support for the IOCB_CMD_POLL operation to poll for the
readyness of file descriptors using the aio subsystem. The API is based
on patches that existed in RHAS2.1 and RHEL3, which means it already is
supported by libaio. To implement the poll support efficiently new
methods to poll are introduced in struct file_operations: get_poll_head
and poll_mask. The first one returns a wait_queue_head to wait on
(lifetime is bound by the file), and the second does a non-blocking
check for the POLL* events. This allows aio poll to work without
any additional context switches, unlike epoll.
This series sits on top of the aio-fsync series that also includes
support for io_pgetevents.
The changes were sponsored by Scylladb, and improve performance
of the seastar framework up to 10%, while also removing the need
for a privileged SCHED_FIFO epoll listener thread.
git://git.infradead.org/users/hch/vfs.git aio-poll.10
Gitweb:
http://git.infradead.org/users/hch/vfs.git/shortlog/refs/heads/aio-poll.10
Libaio changes:
https://pagure.io/libaio.git io-poll
Seastar changes (not updated for the new io_pgetevens ABI yet):
https://github.com/avikivity/seastar/commits/aio
Changes since v9:
- add to the delayed_cancel_reqs earlier to avoid a race
- get rid of POLL_TO_PTR magic
Changes since v8:
- make delayed cancellation conditional again
- add a cancel_kiocb file operation to split delayed vs normal cancel
Changes since v7:
- make delayed cancellation safe and unconditional
Changes since v6:
- reworked cancellation
Changes since v5:
- small changelog updates
- rebased on top of the aio-fsync changes
Changes since v4:
- rebased ontop of Linux 4.16-rc4
Changes since v3:
- remove the pre-sleep ->poll_mask call in vfs_poll,
allow ->get_poll_head to return POLL* values.
Changes since v2:
- removed a double initialization
- new vfs_get_poll_head helper
- document that ->get_poll_head can return NULL
- call ->poll_mask before sleeping
- various ACKs
- add conversion of random to ->poll_mask
- add conversion of af_alg to ->poll_mask
- lacking ->poll_mask support now returns -EINVAL for IOCB_CMD_POLL
- reshuffled the series so that prep patches and everything not
requiring the new in-kernel poll API is in the beginning
Changes since v1:
- handle the NULL ->poll case in vfs_poll
- dropped the file argument to the ->poll_mask socket operation
- replace the ->pre_poll socket operation with ->get_poll_head as
in the file operations
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* [PATCH v5 7/7] proc: add option to mount only a pids subset
From: Alexey Gladkov @ 2018-05-11 9:37 UTC (permalink / raw)
To: Kees Cook, Andy Lutomirski, Andrew Morton, linux-fsdevel,
linux-kernel, kernel-hardening, linux-security-module, linux-api
Cc: Greg Kroah-Hartman, Alexander Viro, Akinobu Mita, Oleg Nesterov,
Jeff Layton, Ingo Molnar, Alexey Dobriyan, Eric W. Biederman,
Linus Torvalds, aniel Micay, Jonathan Corbet, bfields,
Stephen Rothwell, solar, Dmitry V. Levin, Djalal Harouni
This allows to hide all files and directories in the procfs that are not
related to tasks.
Signed-off-by: Alexey Gladkov <gladkov.alexey@gmail.com>
---
fs/proc/generic.c | 20 ++++++++++++++++++++
fs/proc/inode.c | 7 +++++++
fs/proc/root.c | 12 ++++++++++--
include/linux/proc_fs.h | 21 +++++++++++++++++++++
4 files changed, 58 insertions(+), 2 deletions(-)
diff --git a/fs/proc/generic.c b/fs/proc/generic.c
index 2078e70..af1295c 100644
--- a/fs/proc/generic.c
+++ b/fs/proc/generic.c
@@ -266,6 +266,11 @@ struct dentry *proc_lookup_de(struct inode *dir, struct dentry *dentry,
struct dentry *proc_lookup(struct inode *dir, struct dentry *dentry,
unsigned int flags)
{
+ struct proc_fs_info *fs_info = proc_sb(dir->i_sb);
+
+ if (proc_fs_pidonly(fs_info))
+ return ERR_PTR(-ENOENT);
+
return proc_lookup_de(dir, dentry, PDE(dir));
}
@@ -322,10 +327,24 @@ int proc_readdir_de(struct file *file, struct dir_context *ctx,
int proc_readdir(struct file *file, struct dir_context *ctx)
{
struct inode *inode = file_inode(file);
+ struct proc_fs_info *fs_info = proc_sb(inode->i_sb);
+
+ if (proc_fs_pidonly(fs_info))
+ return 1;
return proc_readdir_de(file, ctx, PDE(inode));
}
+static int proc_dir_open(struct inode *inode, struct file *file)
+{
+ struct proc_fs_info *fs_info = proc_sb(inode->i_sb);
+
+ if (proc_fs_pidonly(fs_info))
+ return -ENOENT;
+
+ return 0;
+}
+
/*
* These are the generic /proc directory operations. They
* use the in-memory "struct proc_dir_entry" tree to parse
@@ -335,6 +354,7 @@ static const struct file_operations proc_dir_operations = {
.llseek = generic_file_llseek,
.read = generic_read_dir,
.iterate_shared = proc_readdir,
+ .open = proc_dir_open,
};
/*
diff --git a/fs/proc/inode.c b/fs/proc/inode.c
index 5e62598..9b7d9cb 100644
--- a/fs/proc/inode.c
+++ b/fs/proc/inode.c
@@ -125,6 +125,9 @@ static int proc_show_options(struct seq_file *seq, struct dentry *root)
if (limit_pids > PROC_LIMIT_PIDS_OFF)
seq_printf(seq, ",limit_pids=%u", limit_pids);
+ if (proc_fs_pidonly(fs_info))
+ seq_printf(seq, ",pidonly");
+
return 0;
}
@@ -338,12 +341,16 @@ proc_reg_get_unmapped_area(struct file *file, unsigned long orig_addr,
static int proc_reg_open(struct inode *inode, struct file *file)
{
+ struct proc_fs_info *fs_info = proc_sb(inode->i_sb);
struct proc_dir_entry *pde = PDE(inode);
int rv = 0;
int (*open)(struct inode *, struct file *);
int (*release)(struct inode *, struct file *);
struct pde_opener *pdeo;
+ if (proc_fs_pidonly(fs_info))
+ return -ENOENT;
+
/*
* Ensure that
* 1) PDE's ->release hook will be called no matter what
diff --git a/fs/proc/root.c b/fs/proc/root.c
index c72d22c..daa318e 100644
--- a/fs/proc/root.c
+++ b/fs/proc/root.c
@@ -28,17 +28,18 @@
#include "internal.h"
enum {
- Opt_gid, Opt_hidepid, Opt_limit_pids, Opt_err,
+ Opt_gid, Opt_hidepid, Opt_limit_pids, Opt_pidonly, Opt_err,
};
static const match_table_t tokens = {
{Opt_hidepid, "hidepid=%u"},
{Opt_gid, "gid=%u"},
{Opt_limit_pids, "limit_pids=%u"},
+ {Opt_pidonly, "pidonly"},
{Opt_err, NULL},
};
-/* We only parse 'limit_pids' option here */
+/* We only parse 'limit_pids' and 'pidonly' option here */
int proc_parse_early_options(char *options, struct proc_fs_info *fs_info)
{
char *p, *opts, *orig;
@@ -74,6 +75,11 @@ int proc_parse_early_options(char *options, struct proc_fs_info *fs_info)
proc_fs_set_newinstance(fs_info, true);
pr_info("proc: mounting a new procfs instance ");
break;
+ case Opt_pidonly:
+ proc_fs_set_pidonly(fs_info, PROC_PIDONLY_ON);
+ proc_fs_set_newinstance(fs_info, true);
+ pr_info("proc: mounting a new procfs instance ");
+ break;
case Opt_gid:
case Opt_hidepid:
break;
@@ -126,6 +132,7 @@ int proc_parse_options(char *options, struct proc_fs_info *fs_info)
}
proc_fs_set_hide_pid(fs_info, option);
break;
+ case Opt_pidonly:
case Opt_limit_pids:
break;
default:
@@ -198,6 +205,7 @@ static struct dentry *proc_mount(struct file_system_type *fs_type,
/* Set it as early as possible */
proc_fs_set_newinstance(fs_info, false);
proc_fs_set_limit_pids(fs_info, PROC_LIMIT_PIDS_OFF);
+ proc_fs_set_pidonly(fs_info, PROC_PIDONLY_OFF);
if (flags & SB_KERNMOUNT) {
ns = data;
diff --git a/include/linux/proc_fs.h b/include/linux/proc_fs.h
index 70e8b10..77137c1 100644
--- a/include/linux/proc_fs.h
+++ b/include/linux/proc_fs.h
@@ -18,6 +18,11 @@ enum { /* definitions for proc mount option limit_pids */
PROC_LIMIT_PIDS_PTRACE = 1, /* Limit pids to only ptracable pids */
};
+enum {
+ PROC_PIDONLY_OFF = 0,
+ PROC_PIDONLY_ON = 1,
+};
+
struct proc_fs_info {
struct super_block *sb;
struct pid_namespace *pid_ns;
@@ -26,6 +31,7 @@ struct proc_fs_info {
struct dentry *proc_thread_self; /* For /proc/thread-self/ */
bool newinstance; /* Private flag for new separated instances */
int limit_pids:1;
+ int pidonly:1;
};
#ifdef CONFIG_PROC_FS
@@ -60,6 +66,16 @@ static inline int proc_fs_set_limit_pids(struct proc_fs_info *fs_info, int value
return 0;
}
+static inline int proc_fs_set_pidonly(struct proc_fs_info *fs_info, int value)
+{
+ if (value != PROC_PIDONLY_ON && value != PROC_PIDONLY_OFF)
+ return -EINVAL;
+
+ fs_info->pidonly = value;
+
+ return 0;
+}
+
static inline int proc_fs_hide_pid(struct proc_fs_info *fs_info)
{
return fs_info->pid_ns->hide_pid;
@@ -80,6 +96,11 @@ static inline int proc_fs_limit_pids(struct proc_fs_info *fs_info)
return fs_info->limit_pids;
}
+static inline int proc_fs_pidonly(struct proc_fs_info *fs_info)
+{
+ return fs_info->pidonly;
+}
+
extern void proc_root_init(void);
extern void proc_flush_task(struct task_struct *);
--
2.10.5
^ permalink raw reply related
* [PATCH v5 6/7] proc: flush task dcache entries from all procfs instances
From: Alexey Gladkov @ 2018-05-11 9:36 UTC (permalink / raw)
To: Kees Cook, Andy Lutomirski, Andrew Morton, linux-fsdevel,
linux-kernel, kernel-hardening, linux-security-module, linux-api
Cc: Greg Kroah-Hartman, Alexander Viro, Akinobu Mita, Oleg Nesterov,
Jeff Layton, Ingo Molnar, Alexey Dobriyan, Eric W. Biederman,
Linus Torvalds, aniel Micay, Jonathan Corbet, bfields,
Stephen Rothwell, solar, Dmitry V. Levin, Djalal Harouni
From: Djalal Harouni <tixxdz@gmail.com>
This allows to flush dcache entries of a task on multiple procfs mounts
per pid namespace.
Cc: Kees Cook <keescook@chromium.org>
Cc: Andy Lutomirski <luto@kernel.org>
Signed-off-by: Djalal Harouni <tixxdz@gmail.com>
---
fs/proc/base.c | 27 ++++++++++++++++++++++-----
fs/proc/inode.c | 9 ++++++++-
fs/proc/root.c | 10 ++++++++++
include/linux/pid_namespace.h | 42 ++++++++++++++++++++++++++++++++++++++++++
include/linux/proc_fs.h | 2 ++
5 files changed, 84 insertions(+), 6 deletions(-)
diff --git a/fs/proc/base.c b/fs/proc/base.c
index 31baeef..987d702 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -3068,7 +3068,8 @@ static const struct inode_operations proc_tgid_base_inode_operations = {
.permission = proc_pid_permission,
};
-static void proc_flush_task_mnt(struct vfsmount *mnt, pid_t pid, pid_t tgid)
+static void proc_flush_task_mnt_root(struct dentry *mnt_root,
+ pid_t pid, pid_t tgid)
{
struct dentry *dentry, *leader, *dir;
char buf[10 + 1];
@@ -3077,7 +3078,7 @@ static void proc_flush_task_mnt(struct vfsmount *mnt, pid_t pid, pid_t tgid)
name.name = buf;
name.len = snprintf(buf, sizeof(buf), "%u", pid);
/* no ->d_hash() rejects on procfs */
- dentry = d_hash_and_lookup(mnt->mnt_root, &name);
+ dentry = d_hash_and_lookup(mnt_root, &name);
if (dentry) {
d_invalidate(dentry);
dput(dentry);
@@ -3088,7 +3089,7 @@ static void proc_flush_task_mnt(struct vfsmount *mnt, pid_t pid, pid_t tgid)
name.name = buf;
name.len = snprintf(buf, sizeof(buf), "%u", tgid);
- leader = d_hash_and_lookup(mnt->mnt_root, &name);
+ leader = d_hash_and_lookup(mnt_root, &name);
if (!leader)
goto out;
@@ -3143,14 +3144,30 @@ void proc_flush_task(struct task_struct *task)
int i;
struct pid *pid, *tgid;
struct upid *upid;
+ struct proc_fs_info *fs_info_entry;
+ struct pid_namespace *pid_ns;
+ struct dentry *mnt_root;
pid = task_pid(task);
tgid = task_tgid(task);
for (i = 0; i <= pid->level; i++) {
upid = &pid->numbers[i];
- proc_flush_task_mnt(upid->ns->proc_mnt, upid->nr,
- tgid->numbers[i].nr);
+ pid_ns = upid->ns;
+
+ pidns_proc_lock_shared(pid_ns);
+ list_for_each_entry(fs_info_entry, &pid_ns->procfs_mounts,
+ pidns_entry) {
+ if (proc_fs_newinstance(fs_info_entry)) {
+ mnt_root = fs_info_entry->sb->s_root;
+ proc_flush_task_mnt_root(mnt_root, upid->nr,
+ tgid->numbers[i].nr);
+ }
+ }
+ pidns_proc_unlock_shared(pid_ns);
+
+ mnt_root = pid_ns->proc_mnt->mnt_root;
+ proc_flush_task_mnt_root(mnt_root, upid->nr, tgid->numbers[i].nr);
}
}
diff --git a/fs/proc/inode.c b/fs/proc/inode.c
index 985df4b..5e62598 100644
--- a/fs/proc/inode.c
+++ b/fs/proc/inode.c
@@ -498,10 +498,17 @@ struct inode *proc_get_inode(struct super_block *sb, struct proc_dir_entry *de)
int proc_fill_super(struct super_block *s, void *data, int silent)
{
struct proc_fs_info *fs_info = proc_sb(s);
+ struct pid_namespace *ns = get_pid_ns(fs_info->pid_ns);
struct inode *root_inode;
int ret;
- get_pid_ns(fs_info->pid_ns);
+ fs_info->sb = s;
+
+ if (proc_fs_newinstance(fs_info)) {
+ pidns_proc_lock(ns);
+ list_add_tail(&fs_info->pidns_entry, &ns->procfs_mounts);
+ pidns_proc_unlock(ns);
+ }
if (!proc_parse_options(data, fs_info))
return -EINVAL;
diff --git a/fs/proc/root.c b/fs/proc/root.c
index c5ce241..c72d22c 100644
--- a/fs/proc/root.c
+++ b/fs/proc/root.c
@@ -249,6 +249,13 @@ static void proc_kill_sb(struct super_block *sb)
dput(fs_info->proc_self);
if (fs_info->proc_thread_self)
dput(fs_info->proc_thread_self);
+
+ if (proc_fs_newinstance(fs_info)) {
+ pidns_proc_lock(ns);
+ list_del(&fs_info->pidns_entry);
+ pidns_proc_unlock(ns);
+ }
+
kill_anon_super(sb);
put_pid_ns(ns);
kfree(fs_info);
@@ -357,6 +364,9 @@ int pid_ns_prepare_proc(struct pid_namespace *ns)
return PTR_ERR(mnt);
ns->proc_mnt = mnt;
+ init_rwsem(&ns->rw_procfs_mnts);
+ INIT_LIST_HEAD(&ns->procfs_mounts);
+
return 0;
}
diff --git a/include/linux/pid_namespace.h b/include/linux/pid_namespace.h
index f91a8bf..767fc96 100644
--- a/include/linux/pid_namespace.h
+++ b/include/linux/pid_namespace.h
@@ -32,6 +32,8 @@ struct pid_namespace {
struct pid_namespace *parent;
#ifdef CONFIG_PROC_FS
struct vfsmount *proc_mnt; /* Internal proc mounted during each new pidns */
+ struct rw_semaphore rw_procfs_mnts;
+ struct list_head procfs_mounts; /* list of separated procfs mounts */
#endif
#ifdef CONFIG_BSD_PROCESS_ACCT
struct fs_pin *bacct;
@@ -98,4 +100,44 @@ extern struct pid_namespace *task_active_pid_ns(struct task_struct *tsk);
void pidhash_init(void);
void pid_idr_init(void);
+#ifdef CONFIG_PROC_FS
+static inline void pidns_proc_lock(struct pid_namespace *pid_ns)
+{
+ down_write(&pid_ns->rw_procfs_mnts);
+}
+
+static inline void pidns_proc_unlock(struct pid_namespace *pid_ns)
+{
+ up_write(&pid_ns->rw_procfs_mnts);
+}
+
+static inline void pidns_proc_lock_shared(struct pid_namespace *pid_ns)
+{
+ down_read(&pid_ns->rw_procfs_mnts);
+}
+
+static inline void pidns_proc_unlock_shared(struct pid_namespace *pid_ns)
+{
+ up_read(&pid_ns->rw_procfs_mnts);
+}
+#else /* !CONFIG_PROC_FS */
+
+static inline void pidns_proc_lock(struct pid_namespace *pid_ns)
+{
+}
+
+static inline void pidns_proc_unlock(struct pid_namespace *pid_ns)
+{
+}
+
+static inline void pidns_proc_lock_shared(struct pid_namespace *pid_ns)
+{
+}
+
+static inline void pidns_proc_unlock_shared(struct pid_namespace *pid_ns)
+{
+}
+
+#endif /* CONFIG_PROC_FS */
+
#endif /* _LINUX_PID_NS_H */
diff --git a/include/linux/proc_fs.h b/include/linux/proc_fs.h
index 2d16d0e..70e8b10 100644
--- a/include/linux/proc_fs.h
+++ b/include/linux/proc_fs.h
@@ -19,7 +19,9 @@ enum { /* definitions for proc mount option limit_pids */
};
struct proc_fs_info {
+ struct super_block *sb;
struct pid_namespace *pid_ns;
+ struct list_head pidns_entry; /* Node in procfs_mounts of a pidns */
struct dentry *proc_self; /* For /proc/self */
struct dentry *proc_thread_self; /* For /proc/thread-self/ */
bool newinstance; /* Private flag for new separated instances */
--
2.10.5
^ permalink raw reply related
* [PATCH v5 5/7] proc: instantiate only pids that we can ptrace on 'limit_pids=1' mount option
From: Alexey Gladkov @ 2018-05-11 9:36 UTC (permalink / raw)
To: Kees Cook, Andy Lutomirski, Andrew Morton, linux-fsdevel,
linux-kernel, kernel-hardening, linux-security-module, linux-api
Cc: Greg Kroah-Hartman, Alexander Viro, Akinobu Mita, Oleg Nesterov,
Jeff Layton, Ingo Molnar, Alexey Dobriyan, Eric W. Biederman,
Linus Torvalds, aniel Micay, Jonathan Corbet, bfields,
Stephen Rothwell, solar, Dmitry V. Levin, Djalal Harouni
From: Djalal Harouni <tixxdz@gmail.com>
If "limit_pids=1" mount option is set then do not instantiate pids that
we can not ptrace. "limit_pids=1" means that procfs should only contain
pids that the caller can ptrace.
Cc: Kees Cook <keescook@chromium.org>
Cc: Andy Lutomirski <luto@kernel.org>
Signed-off-by: Djalal Harouni <tixxdz@gmail.com>
---
fs/proc/base.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/fs/proc/base.c b/fs/proc/base.c
index 6f084344..31baeef 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -3187,6 +3187,7 @@ struct dentry *proc_pid_lookup(struct inode *dir, struct dentry * dentry, unsign
unsigned tgid;
struct proc_fs_info *fs_info = proc_sb(dir->i_sb);
struct pid_namespace *ns = fs_info->pid_ns;
+ int limit_pids = proc_fs_limit_pids(fs_info);
tgid = name_to_int(&dentry->d_name);
if (tgid == ~0U)
@@ -3200,7 +3201,15 @@ struct dentry *proc_pid_lookup(struct inode *dir, struct dentry * dentry, unsign
if (!task)
goto out;
+ /* Limit procfs to only ptracable tasks */
+ if (limit_pids == PROC_LIMIT_PIDS_PTRACE) {
+ cond_resched();
+ if (!has_pid_permissions(fs_info, task, HIDEPID_NO_ACCESS))
+ goto out_put_task;
+ }
+
result = proc_pid_instantiate(dir, dentry, task, NULL);
+out_put_task:
put_task_struct(task);
out:
return ERR_PTR(result);
--
2.10.5
^ permalink raw reply related
* [PATCH v5 4/7] proc: support mounting private procfs instances inside same pid namespace
From: Alexey Gladkov @ 2018-05-11 9:35 UTC (permalink / raw)
To: Kees Cook, Andy Lutomirski, Andrew Morton, linux-fsdevel,
linux-kernel, kernel-hardening, linux-security-module, linux-api
Cc: Greg Kroah-Hartman, Alexander Viro, Akinobu Mita, Oleg Nesterov,
Jeff Layton, Ingo Molnar, Alexey Dobriyan, Eric W. Biederman,
Linus Torvalds, aniel Micay, Jonathan Corbet, bfields,
Stephen Rothwell, solar, Dmitry V. Levin, Djalal Harouni
From: Djalal Harouni <tixxdz@gmail.com>
This patch allows to have multiple private procfs instances inside the
same pid namespace. Lot of other areas in the kernel and filesystems
have been updated to be able to support private instances, devpts is one
major example. The aim here is lightweight sandboxes, and to allow that we
have to modernize procfs internals.
1) The main aim of this work is to have on embedded systems one
supervisor for apps. Right now we have some lightweight sandbox support,
however if we create pid namespacess we have to manages all the
processes inside too, where our goal is to be able to run a bunch of
apps each one inside its own mount namespace without being able to
notice each other. We only want to use mount namespaces, and we want
procfs to behave more like a real mount point.
2) Linux Security Modules have multiple ptrace paths inside some
subsystems, however inside procfs, the implementation does not guarantee
that the ptrace() check which triggers the security_ptrace_check() hook
will always run. We have the 'hidepid' mount option that can be used to
force the ptrace_may_access() check inside has_pid_permissions() to run.
The problem is that 'hidepid' is per pid namespace and not attached to
the mount point, any remount or modification of 'hidepid' will propagate
to all other procfs mounts.
This also does not allow to support Yama LSM easily in desktop and user
sessions. Yama ptrace scope which restricts ptrace and some other
syscalls to be allowed only on inferiors, can be updated to have a
per-task context, where the context will be inherited during fork(),
clone() and preserved across execve(). If we support multiple private
procfs instances, then we may force the ptrace_may_access() on
/proc/<pids>/ to always run inside that new procfs instances. This will
allow to specifiy on user sessions if we should populate procfs with
pids that the user can ptrace or not.
By using Yama ptrace scope, some restricted users will only be able to see
inferiors inside /proc, they won't even be able to see their other
processes. Some software like Chromium, Firefox's crash handler, Wine
and others are already using Yama to restrict which processes can be
ptracable. With this change this will give the possibility to restrict
/proc/<pids>/ but more importantly this will give desktop users a
generic and usuable way to specifiy which users should see all processes
and which users can not.
Side notes:
* This covers the lack of seccomp where it is not able to parse
arguments, it is easy to install a seccomp filter on direct syscalls
that operate on pids, however /proc/<pid>/ is a Linux ABI using
filesystem syscalls. With this change LSMs should be able to analyze
open/read/write/close...
3) This will modernize procfs and align it with all other filesystems
and subsystems that have been updated recently to be able to work in a
flexible way. This is the same as devpts where each mount now is a distinct
filesystem such that ptys and their indicies allocated in one mount are
independent from ptys and their indicies in all other mounts.
We have to align procfs and modernize it to have a per mount context
where at least the mount option do not propagate to all other mounts,
then maybe we can continue to implement new features. One example is to
require CAP_SYS_ADMIN in the init user namespace on some /proc/* which are
not pids and which are are not virtualized by design, or CAP_NET_ADMIN
inside userns on the net bits that are virtualized, etc.
These mount options won't propagate to previous mounts, and the system
will continue to be usable.
Ths patch introduces the new 'limit_pids' mount option as it was also
suggesed by Andy Lutomirski [1]. When this option is passed we
automatically create a private procfs instance. This is not the default
behaviour since we do not want to break userspace and we do not want to
provide different devices IDs by default, please see [1] for why.
* If 'limit_pids=0' this will create a private procfs instance without
any restrictions.
* If 'limit_pids=1' this will create a private procfs instance where
processes will only be able to see pids that they can ptrace inside
/proc/
This allows to have a stable LSM path later inside has_pid_permissions()
to make sure that processes are not using filesystem syscall on
/proc/pid to inspect or write to pid where other mechanisms are supposed
to prevent this. This allows to align filesystem syscalls that go
through procfs on pids with the other syscalls that can be blocked by
seccomp filters.
Later Yama LSM can be updated to check that processes are able only
able to see their children inside /proc/.
[1] https://lkml.org/lkml/2017/3/31/324
Cc: Kees Cook <keescook@chromium.org>
Suggested-by: Andy Lutomirski <luto@kernel.org>
Signed-off-by: Djalal Harouni <tixxdz@gmail.com>
---
fs/proc/base.c | 21 ++++++++----
fs/proc/inode.c | 4 +++
fs/proc/root.c | 88 ++++++++++++++++++++++++++++++++++++++++++++++---
include/linux/proc_fs.h | 51 ++++++++++++++++++++++++++++
4 files changed, 153 insertions(+), 11 deletions(-)
diff --git a/fs/proc/base.c b/fs/proc/base.c
index 57fc895..6f084344 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -688,13 +688,22 @@ static bool has_pid_permissions(struct proc_fs_info *fs_info,
struct task_struct *task,
int hide_pid_min)
{
- int hide_pid = proc_fs_hide_pid(fs_info);
- kgid_t gid = proc_fs_pid_gid(fs_info);
+ int limit_pids = proc_fs_limit_pids(fs_info);
- if (hide_pid < hide_pid_min)
- return true;
- if (in_group_p(gid))
- return true;
+ /*
+ * If 'limit_pids' mount is set force a ptrace check,
+ * we indicate that we are using a filesystem syscall
+ * by passing PTRACE_MODE_READ_FSCREDS
+ */
+ if (limit_pids == PROC_LIMIT_PIDS_OFF) {
+ int hide_pid = proc_fs_hide_pid(fs_info);
+ kgid_t gid = proc_fs_pid_gid(fs_info);
+
+ if (hide_pid < hide_pid_min)
+ return true;
+ if (in_group_p(gid))
+ return true;
+ }
return ptrace_may_access(task, PTRACE_MODE_READ_FSCREDS);
}
diff --git a/fs/proc/inode.c b/fs/proc/inode.c
index 9585727..985df4b 100644
--- a/fs/proc/inode.c
+++ b/fs/proc/inode.c
@@ -115,12 +115,16 @@ static int proc_show_options(struct seq_file *seq, struct dentry *root)
struct super_block *sb = root->d_sb;
struct proc_fs_info *fs_info = proc_sb(sb);
struct pid_namespace *pid = fs_info->pid_ns;
+ int limit_pids = proc_fs_limit_pids(fs_info);
if (!gid_eq(pid->pid_gid, GLOBAL_ROOT_GID))
seq_printf(seq, ",gid=%u", from_kgid_munged(&init_user_ns, pid->pid_gid));
if (pid->hide_pid != HIDEPID_OFF)
seq_printf(seq, ",hidepid=%u", pid->hide_pid);
+ if (limit_pids > PROC_LIMIT_PIDS_OFF)
+ seq_printf(seq, ",limit_pids=%u", limit_pids);
+
return 0;
}
diff --git a/fs/proc/root.c b/fs/proc/root.c
index bdea220..c5ce241 100644
--- a/fs/proc/root.c
+++ b/fs/proc/root.c
@@ -28,15 +28,66 @@
#include "internal.h"
enum {
- Opt_gid, Opt_hidepid, Opt_err,
+ Opt_gid, Opt_hidepid, Opt_limit_pids, Opt_err,
};
static const match_table_t tokens = {
{Opt_hidepid, "hidepid=%u"},
{Opt_gid, "gid=%u"},
+ {Opt_limit_pids, "limit_pids=%u"},
{Opt_err, NULL},
};
+/* We only parse 'limit_pids' option here */
+int proc_parse_early_options(char *options, struct proc_fs_info *fs_info)
+{
+ char *p, *opts, *orig;
+ substring_t args[MAX_OPT_ARGS];
+ int option, ret;
+
+ if (!options)
+ return 0;
+
+ opts = kstrdup(options, GFP_KERNEL);
+ if (!opts)
+ return -ENOMEM;
+
+ orig = opts;
+
+ while ((p = strsep(&opts, ",")) != NULL) {
+ int token;
+
+ if (!*p)
+ continue;
+
+ token = match_token(p, tokens, args);
+ switch (token) {
+ case Opt_limit_pids:
+ if (match_int(&args[0], &option))
+ return -EINVAL;
+ ret = proc_fs_set_limit_pids(fs_info, option);
+ if (ret < 0) {
+ pr_err("proc: faild to parse mount option "
+ "\"%s\" \n", p);
+ return ret;
+ }
+ proc_fs_set_newinstance(fs_info, true);
+ pr_info("proc: mounting a new procfs instance ");
+ break;
+ case Opt_gid:
+ case Opt_hidepid:
+ break;
+ default:
+ pr_err("proc: unrecognized mount option \"%s\" "
+ "or missing value\n", p);
+ return -EINVAL;
+ }
+ }
+
+ kfree(orig);
+ return 0;
+}
+
int proc_parse_options(char *options, struct proc_fs_info *fs_info)
{
char *p;
@@ -75,6 +126,8 @@ int proc_parse_options(char *options, struct proc_fs_info *fs_info)
}
proc_fs_set_hide_pid(fs_info, option);
break;
+ case Opt_limit_pids:
+ break;
default:
pr_err("proc: unrecognized mount option \"%s\" "
"or missing value\n", p);
@@ -87,18 +140,34 @@ int proc_parse_options(char *options, struct proc_fs_info *fs_info)
int proc_remount(struct super_block *sb, int *flags, char *data)
{
+ int error;
struct proc_fs_info *fs_info = proc_sb(sb);
sync_filesystem(sb);
+
+ /*
+ * If this is a new instance, then parse again the proc mount
+ * options.
+ */
+ if (proc_fs_newinstance(fs_info)) {
+ error = proc_parse_early_options(data, fs_info);
+ if (error < 0)
+ return error;
+ }
+
return !proc_parse_options(data, fs_info);
}
-static int proc_test_super(struct super_block *s, void *data)
+static int proc_test_super(struct super_block *sb, void *data)
{
struct proc_fs_info *p = data;
- struct proc_fs_info *fs_info = proc_sb(s);
+ struct proc_fs_info *fs_info = proc_sb(sb);
- return p->pid_ns == fs_info->pid_ns;
+ if (!proc_fs_newinstance(p) && !proc_fs_newinstance(fs_info) &&
+ p->pid_ns == fs_info->pid_ns)
+ return 1;
+
+ return 0;
}
static int proc_set_super(struct super_block *sb, void *data)
@@ -110,7 +179,7 @@ static int proc_set_super(struct super_block *sb, void *data)
static struct dentry *proc_mount(struct file_system_type *fs_type,
int flags, const char *dev_name, void *data)
{
- int error;
+ int error = 0;
struct super_block *sb;
struct pid_namespace *ns;
struct proc_fs_info *fs_info;
@@ -126,10 +195,19 @@ static struct dentry *proc_mount(struct file_system_type *fs_type,
if (!fs_info)
return ERR_PTR(-ENOMEM);
+ /* Set it as early as possible */
+ proc_fs_set_newinstance(fs_info, false);
+ proc_fs_set_limit_pids(fs_info, PROC_LIMIT_PIDS_OFF);
+
if (flags & SB_KERNMOUNT) {
ns = data;
data = NULL;
} else {
+ /* Parse early mount options if not a MS_KERNMOUNT */
+ error = proc_parse_early_options(data, fs_info);
+ if (error < 0)
+ goto error_fs_info;
+
ns = task_active_pid_ns(current);
}
diff --git a/include/linux/proc_fs.h b/include/linux/proc_fs.h
index a7fc6a1..2d16d0e 100644
--- a/include/linux/proc_fs.h
+++ b/include/linux/proc_fs.h
@@ -13,10 +13,17 @@
struct proc_dir_entry;
struct pid_namespace;
+enum { /* definitions for proc mount option limit_pids */
+ PROC_LIMIT_PIDS_OFF = 0, /* Limit pids is off */
+ PROC_LIMIT_PIDS_PTRACE = 1, /* Limit pids to only ptracable pids */
+};
+
struct proc_fs_info {
struct pid_namespace *pid_ns;
struct dentry *proc_self; /* For /proc/self */
struct dentry *proc_thread_self; /* For /proc/thread-self/ */
+ bool newinstance; /* Private flag for new separated instances */
+ int limit_pids:1;
};
#ifdef CONFIG_PROC_FS
@@ -36,6 +43,21 @@ static inline void proc_fs_set_pid_gid(struct proc_fs_info *fs_info, kgid_t gid)
fs_info->pid_ns->pid_gid = gid;
}
+static inline void proc_fs_set_newinstance(struct proc_fs_info *fs_info, bool value)
+{
+ fs_info->newinstance = value;
+}
+
+static inline int proc_fs_set_limit_pids(struct proc_fs_info *fs_info, int value)
+{
+ if (value < PROC_LIMIT_PIDS_OFF || value > PROC_LIMIT_PIDS_PTRACE)
+ return -EINVAL;
+
+ fs_info->limit_pids = value;
+
+ return 0;
+}
+
static inline int proc_fs_hide_pid(struct proc_fs_info *fs_info)
{
return fs_info->pid_ns->hide_pid;
@@ -46,6 +68,16 @@ static inline kgid_t proc_fs_pid_gid(struct proc_fs_info *fs_info)
return fs_info->pid_ns->pid_gid;
}
+static inline bool proc_fs_newinstance(struct proc_fs_info *fs_info)
+{
+ return fs_info->newinstance;
+}
+
+static inline int proc_fs_limit_pids(struct proc_fs_info *fs_info)
+{
+ return fs_info->limit_pids;
+}
+
extern void proc_root_init(void);
extern void proc_flush_task(struct task_struct *);
@@ -90,6 +122,15 @@ static inline void proc_fs_set_pid_gid(struct proc_info_fs *fs_info, kgid_t gid)
{
}
+static inline void proc_fs_set_newinstance(struct proc_fs_info *fs_info, bool value)
+{
+}
+
+static inline int proc_fs_set_limit_pids(struct proc_fs_info *fs_info, int value)
+{
+ return 0;
+}
+
static inline int proc_fs_hide_pid(struct proc_fs_info *fs_info)
{
return 0;
@@ -100,6 +141,16 @@ extern kgid_t proc_fs_pid_gid(struct proc_fs_info *fs_info)
return GLOBAL_ROOT_GID;
}
+static inline bool proc_fs_newinstance(struct proc_fs_info *fs_info)
+{
+ return false;
+}
+
+static inline int proc_fs_limit_pids(struct proc_fs_info *fs_info)
+{
+ return 0;
+}
+
extern inline struct proc_fs_info *proc_sb(struct super_block *sb) { return NULL;}
static inline struct proc_dir_entry *proc_symlink(const char *name,
struct proc_dir_entry *parent,const char *dest) { return NULL;}
--
2.10.5
^ permalink raw reply related
* [PATCH v5 3/7] proc: add helpers to set and get proc hidepid and gid mount options
From: Alexey Gladkov @ 2018-05-11 9:35 UTC (permalink / raw)
To: Kees Cook, Andy Lutomirski, Andrew Morton, linux-fsdevel,
linux-kernel, kernel-hardening, linux-security-module, linux-api
Cc: Greg Kroah-Hartman, Alexander Viro, Akinobu Mita, Oleg Nesterov,
Jeff Layton, Ingo Molnar, Alexey Dobriyan, Eric W. Biederman,
Linus Torvalds, aniel Micay, Jonathan Corbet, bfields,
Stephen Rothwell, solar, Dmitry V. Levin, Djalal Harouni
From: Djalal Harouni <tixxdz@gmail.com>
This is a cleaning patch to add helpers to set and get proc mount
options instead of directly using them. This make it easy to track
what's happening and easy to update in future.
Cc: Kees Cook <keescook@chromium.org>
Cc: Andy Lutomirski <luto@kernel.org>
Signed-off-by: Djalal Harouni <tixxdz@gmail.com>
---
fs/proc/base.c | 16 +++++++++-------
fs/proc/inode.c | 5 +++--
fs/proc/internal.h | 2 +-
fs/proc/root.c | 15 ++++++++++-----
include/linux/proc_fs.h | 44 ++++++++++++++++++++++++++++++++++++++++++--
5 files changed, 65 insertions(+), 17 deletions(-)
diff --git a/fs/proc/base.c b/fs/proc/base.c
index e0c2afc..57fc895 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -684,13 +684,16 @@ int proc_setattr(struct dentry *dentry, struct iattr *attr)
* May current process learn task's sched/cmdline info (for hide_pid_min=1)
* or euid/egid (for hide_pid_min=2)?
*/
-static bool has_pid_permissions(struct pid_namespace *pid,
+static bool has_pid_permissions(struct proc_fs_info *fs_info,
struct task_struct *task,
int hide_pid_min)
{
- if (pid->hide_pid < hide_pid_min)
+ int hide_pid = proc_fs_hide_pid(fs_info);
+ kgid_t gid = proc_fs_pid_gid(fs_info);
+
+ if (hide_pid < hide_pid_min)
return true;
- if (in_group_p(pid->pid_gid))
+ if (in_group_p(gid))
return true;
return ptrace_may_access(task, PTRACE_MODE_READ_FSCREDS);
}
@@ -706,7 +709,7 @@ static int proc_pid_permission(struct inode *inode, int mask)
task = get_proc_task(inode);
if (!task)
return -ESRCH;
- has_perms = has_pid_permissions(pid, task, HIDEPID_NO_ACCESS);
+ has_perms = has_pid_permissions(fs_info, task, HIDEPID_NO_ACCESS);
put_task_struct(task);
if (!has_perms) {
@@ -1786,7 +1789,6 @@ int pid_getattr(const struct path *path, struct kstat *stat,
struct task_struct *task;
struct inode *inode = d_inode(path->dentry);
struct proc_fs_info *fs_info = proc_sb(inode->i_sb);
- struct pid_namespace *pid = fs_info->pid_ns;
generic_fillattr(inode, stat);
@@ -1795,7 +1797,7 @@ int pid_getattr(const struct path *path, struct kstat *stat,
stat->gid = GLOBAL_ROOT_GID;
task = pid_task(proc_pid(inode), PIDTYPE_PID);
if (task) {
- if (!has_pid_permissions(pid, task, HIDEPID_INVISIBLE)) {
+ if (!has_pid_permissions(fs_info, task, HIDEPID_INVISIBLE)) {
rcu_read_unlock();
/*
* This doesn't prevent learning whether PID exists,
@@ -3272,7 +3274,7 @@ int proc_pid_readdir(struct file *file, struct dir_context *ctx)
int len;
cond_resched();
- if (!has_pid_permissions(ns, iter.task, HIDEPID_INVISIBLE))
+ if (!has_pid_permissions(fs_info, iter.task, HIDEPID_INVISIBLE))
continue;
len = snprintf(name, sizeof(name), "%u", iter.tgid);
diff --git a/fs/proc/inode.c b/fs/proc/inode.c
index e34b89a..9585727 100644
--- a/fs/proc/inode.c
+++ b/fs/proc/inode.c
@@ -494,11 +494,12 @@ struct inode *proc_get_inode(struct super_block *sb, struct proc_dir_entry *de)
int proc_fill_super(struct super_block *s, void *data, int silent)
{
struct proc_fs_info *fs_info = proc_sb(s);
- struct pid_namespace *ns = get_pid_ns(fs_info->pid_ns);
struct inode *root_inode;
int ret;
- if (!proc_parse_options(data, ns))
+ get_pid_ns(fs_info->pid_ns);
+
+ if (!proc_parse_options(data, fs_info))
return -EINVAL;
/* User space would break if executables or devices appear on proc */
diff --git a/fs/proc/internal.h b/fs/proc/internal.h
index 0f1692e..facc2d9 100644
--- a/fs/proc/internal.h
+++ b/fs/proc/internal.h
@@ -253,7 +253,7 @@ static inline void proc_tty_init(void) {}
* root.c
*/
extern struct proc_dir_entry proc_root;
-extern int proc_parse_options(char *options, struct pid_namespace *pid);
+extern int proc_parse_options(char *options, struct proc_fs_info *fs_info);
extern void proc_self_init(void);
extern int proc_remount(struct super_block *, int *, char *);
diff --git a/fs/proc/root.c b/fs/proc/root.c
index 184c42b..bdea220 100644
--- a/fs/proc/root.c
+++ b/fs/proc/root.c
@@ -37,11 +37,12 @@ static const match_table_t tokens = {
{Opt_err, NULL},
};
-int proc_parse_options(char *options, struct pid_namespace *pid)
+int proc_parse_options(char *options, struct proc_fs_info *fs_info)
{
char *p;
substring_t args[MAX_OPT_ARGS];
int option;
+ kgid_t gid;
if (!options)
return 1;
@@ -57,7 +58,12 @@ int proc_parse_options(char *options, struct pid_namespace *pid)
case Opt_gid:
if (match_int(&args[0], &option))
return 0;
- pid->pid_gid = make_kgid(current_user_ns(), option);
+ gid = make_kgid(current_user_ns(), option);
+ if (!gid_valid(gid)) {
+ pr_err("proc: invalid gid mount option.\n");
+ return 0;
+ }
+ proc_fs_set_pid_gid(fs_info, gid);
break;
case Opt_hidepid:
if (match_int(&args[0], &option))
@@ -67,7 +73,7 @@ int proc_parse_options(char *options, struct pid_namespace *pid)
pr_err("proc: hidepid value must be between 0 and 2.\n");
return 0;
}
- pid->hide_pid = option;
+ proc_fs_set_hide_pid(fs_info, option);
break;
default:
pr_err("proc: unrecognized mount option \"%s\" "
@@ -82,10 +88,9 @@ int proc_parse_options(char *options, struct pid_namespace *pid)
int proc_remount(struct super_block *sb, int *flags, char *data)
{
struct proc_fs_info *fs_info = proc_sb(sb);
- struct pid_namespace *pid = fs_info->pid_ns;
sync_filesystem(sb);
- return !proc_parse_options(data, pid);
+ return !proc_parse_options(data, fs_info);
}
static int proc_test_super(struct super_block *s, void *data)
diff --git a/include/linux/proc_fs.h b/include/linux/proc_fs.h
index 5e461f6..a7fc6a1 100644
--- a/include/linux/proc_fs.h
+++ b/include/linux/proc_fs.h
@@ -8,6 +8,10 @@
#include <linux/types.h>
#include <linux/fs.h>
#include <linux/refcount.h>
+#include <linux/pid_namespace.h>
+
+struct proc_dir_entry;
+struct pid_namespace;
struct proc_fs_info {
struct pid_namespace *pid_ns;
@@ -15,8 +19,6 @@ struct proc_fs_info {
struct dentry *proc_thread_self; /* For /proc/thread-self/ */
};
-struct proc_dir_entry;
-
#ifdef CONFIG_PROC_FS
static inline struct proc_fs_info *proc_sb(struct super_block *sb)
@@ -24,6 +26,26 @@ static inline struct proc_fs_info *proc_sb(struct super_block *sb)
return sb->s_fs_info;
}
+static inline void proc_fs_set_hide_pid(struct proc_fs_info *fs_info, int hide_pid)
+{
+ fs_info->pid_ns->hide_pid = hide_pid;
+}
+
+static inline void proc_fs_set_pid_gid(struct proc_fs_info *fs_info, kgid_t gid)
+{
+ fs_info->pid_ns->pid_gid = gid;
+}
+
+static inline int proc_fs_hide_pid(struct proc_fs_info *fs_info)
+{
+ return fs_info->pid_ns->hide_pid;
+}
+
+static inline kgid_t proc_fs_pid_gid(struct proc_fs_info *fs_info)
+{
+ return fs_info->pid_ns->pid_gid;
+}
+
extern void proc_root_init(void);
extern void proc_flush_task(struct task_struct *);
@@ -60,6 +82,24 @@ static inline void proc_flush_task(struct task_struct *task)
{
}
+static inline void proc_fs_set_hide_pid(struct proc_fs_info *fs_info, int hide_pid)
+{
+}
+
+static inline void proc_fs_set_pid_gid(struct proc_info_fs *fs_info, kgid_t gid)
+{
+}
+
+static inline int proc_fs_hide_pid(struct proc_fs_info *fs_info)
+{
+ return 0;
+}
+
+extern kgid_t proc_fs_pid_gid(struct proc_fs_info *fs_info)
+{
+ return GLOBAL_ROOT_GID;
+}
+
extern inline struct proc_fs_info *proc_sb(struct super_block *sb) { return NULL;}
static inline struct proc_dir_entry *proc_symlink(const char *name,
struct proc_dir_entry *parent,const char *dest) { return NULL;}
--
2.10.5
^ permalink raw reply related
* [PATCH v5 2/7] proc: move /proc/{self|thread-self} dentries to proc_fs_info
From: Alexey Gladkov @ 2018-05-11 9:35 UTC (permalink / raw)
To: Kees Cook, Andy Lutomirski, Andrew Morton, linux-fsdevel,
linux-kernel, kernel-hardening, linux-security-module, linux-api
Cc: Greg Kroah-Hartman, Alexander Viro, Akinobu Mita, Oleg Nesterov,
Jeff Layton, Ingo Molnar, Alexey Dobriyan, Eric W. Biederman,
Linus Torvalds, aniel Micay, Jonathan Corbet, bfields,
Stephen Rothwell, solar, Dmitry V. Levin, Djalal Harouni
From: Djalal Harouni <tixxdz@gmail.com>
This is a preparation patch that moves /proc/{self|thread-self} dentries
to be stored inside procfs fs_info struct instead of making them per pid
namespace. Since we want to support multiple procfs instances we need to
make sure that these dentries are also per-superblock instead of
per-pidns, unmounting a private procfs won't clash with other procfs
mounts.
Cc: Kees Cook <keescook@chromium.org>
Cc: Andy Lutomirski <luto@kernel.org>
Signed-off-by: Djalal Harouni <tixxdz@gmail.com>
---
fs/proc/base.c | 4 ++--
fs/proc/root.c | 8 ++++----
fs/proc/self.c | 3 +--
fs/proc/thread_self.c | 5 ++---
include/linux/pid_namespace.h | 4 +---
include/linux/proc_fs.h | 2 ++
6 files changed, 12 insertions(+), 14 deletions(-)
diff --git a/fs/proc/base.c b/fs/proc/base.c
index 99a0f24..e0c2afc 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -3252,13 +3252,13 @@ int proc_pid_readdir(struct file *file, struct dir_context *ctx)
return 0;
if (pos == TGID_OFFSET - 2) {
- struct inode *inode = d_inode(ns->proc_self);
+ struct inode *inode = d_inode(fs_info->proc_self);
if (!dir_emit(ctx, "self", 4, inode->i_ino, DT_LNK))
return 0;
ctx->pos = pos = pos + 1;
}
if (pos == TGID_OFFSET - 1) {
- struct inode *inode = d_inode(ns->proc_thread_self);
+ struct inode *inode = d_inode(fs_info->proc_thread_self);
if (!dir_emit(ctx, "thread-self", 11, inode->i_ino, DT_LNK))
return 0;
ctx->pos = pos = pos + 1;
diff --git a/fs/proc/root.c b/fs/proc/root.c
index 5d4c454..184c42b 100644
--- a/fs/proc/root.c
+++ b/fs/proc/root.c
@@ -162,10 +162,10 @@ static void proc_kill_sb(struct super_block *sb)
struct proc_fs_info *fs_info = proc_sb(sb);
struct pid_namespace *ns = (struct pid_namespace *)fs_info->pid_ns;
- if (ns->proc_self)
- dput(ns->proc_self);
- if (ns->proc_thread_self)
- dput(ns->proc_thread_self);
+ if (fs_info->proc_self)
+ dput(fs_info->proc_self);
+ if (fs_info->proc_thread_self)
+ dput(fs_info->proc_thread_self);
kill_anon_super(sb);
put_pid_ns(ns);
kfree(fs_info);
diff --git a/fs/proc/self.c b/fs/proc/self.c
index 3f2a88f..9ad3c55 100644
--- a/fs/proc/self.c
+++ b/fs/proc/self.c
@@ -38,7 +38,6 @@ int proc_setup_self(struct super_block *s)
{
struct inode *root_inode = d_inode(s->s_root);
struct proc_fs_info *fs_info = proc_sb(s);
- struct pid_namespace *ns = fs_info->pid_ns;
struct dentry *self;
inode_lock(root_inode);
@@ -65,7 +64,7 @@ int proc_setup_self(struct super_block *s)
pr_err("proc_fill_super: can't allocate /proc/self\n");
return PTR_ERR(self);
}
- ns->proc_self = self;
+ fs_info->proc_self = self;
return 0;
}
diff --git a/fs/proc/thread_self.c b/fs/proc/thread_self.c
index e80dd79..8db3503 100644
--- a/fs/proc/thread_self.c
+++ b/fs/proc/thread_self.c
@@ -37,7 +37,6 @@ static unsigned thread_self_inum __ro_after_init;
int proc_setup_thread_self(struct super_block *s)
{
struct proc_fs_info *fs_info = proc_sb(s);
- struct pid_namespace *ns = fs_info->pid_ns;
struct inode *root_inode = d_inode(s->s_root);
struct dentry *thread_self;
@@ -62,10 +61,10 @@ int proc_setup_thread_self(struct super_block *s)
}
inode_unlock(root_inode);
if (IS_ERR(thread_self)) {
- pr_err("proc_fill_super: can't allocate /proc/thread_self\n");
+ pr_err("proc_fill_super: can't allocate /proc/thread-self\n");
return PTR_ERR(thread_self);
}
- ns->proc_thread_self = thread_self;
+ fs_info->proc_thread_self = thread_self;
return 0;
}
diff --git a/include/linux/pid_namespace.h b/include/linux/pid_namespace.h
index 49538b1..f91a8bf 100644
--- a/include/linux/pid_namespace.h
+++ b/include/linux/pid_namespace.h
@@ -31,9 +31,7 @@ struct pid_namespace {
unsigned int level;
struct pid_namespace *parent;
#ifdef CONFIG_PROC_FS
- struct vfsmount *proc_mnt;
- struct dentry *proc_self;
- struct dentry *proc_thread_self;
+ struct vfsmount *proc_mnt; /* Internal proc mounted during each new pidns */
#endif
#ifdef CONFIG_BSD_PROCESS_ACCT
struct fs_pin *bacct;
diff --git a/include/linux/proc_fs.h b/include/linux/proc_fs.h
index 7c8cf3c..5e461f6 100644
--- a/include/linux/proc_fs.h
+++ b/include/linux/proc_fs.h
@@ -11,6 +11,8 @@
struct proc_fs_info {
struct pid_namespace *pid_ns;
+ struct dentry *proc_self; /* For /proc/self */
+ struct dentry *proc_thread_self; /* For /proc/thread-self/ */
};
struct proc_dir_entry;
--
2.10.5
^ permalink raw reply related
* [PATCH v5 1/7] proc: add proc_fs_info struct to store proc information
From: Alexey Gladkov @ 2018-05-11 9:34 UTC (permalink / raw)
To: Kees Cook, Andy Lutomirski, Andrew Morton, linux-fsdevel,
linux-kernel, kernel-hardening, linux-security-module, linux-api
Cc: Greg Kroah-Hartman, Alexander Viro, Akinobu Mita, Oleg Nesterov,
Jeff Layton, Ingo Molnar, Alexey Dobriyan, Eric W. Biederman,
Linus Torvalds, aniel Micay, Jonathan Corbet, bfields,
Stephen Rothwell, solar, Dmitry V. Levin, Djalal Harouni
From: Djalal Harouni <tixxdz@gmail.com>
This is a preparation patch that adds proc_fs_info to be able to store
different procfs options and informations. Right now some mount options
are stored inside the pid namespace which makes it hard to change or
modernize procfs without affecting pid namespaces. Plus we do want to
treat proc as more of a real mount point and filesystem. procfs is part
of Linux API where it offers some features using filesystem syscalls and
in order to support some features where we are able to have multiple
instances of procfs, each one with its mount options inside the same pid
namespace, we have to separate these procfs instances.
This is the same feature that was also added to other Linux interfaces
like devpts in order to support containers, sandboxes, and to have
multiple instances of devpts filesystem [1].
[1] http://lxr.free-electrons.com/source/Documentation/filesystems/devpts.txt?v=3.14
Cc: Kees Cook <keescook@chromium.org>
Suggested-by: Andy Lutomirski <luto@kernel.org>
Signed-off-by: Djalal Harouni <tixxdz@gmail.com>
Signed-off-by: Alexey Gladkov <gladkov.alexey@gmail.com>
---
fs/locks.c | 6 +++--
fs/proc/base.c | 34 +++++++++++++++-----------
fs/proc/inode.c | 8 +++---
fs/proc/root.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++---
fs/proc/self.c | 8 +++---
fs/proc/thread_self.c | 6 +++--
fs/proc_namespace.c | 14 +++++------
include/linux/proc_fs.h | 11 +++++++++
8 files changed, 117 insertions(+), 35 deletions(-)
diff --git a/fs/locks.c b/fs/locks.c
index 62bbe8b..6c73288 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -2622,7 +2622,8 @@ static void lock_get_status(struct seq_file *f, struct file_lock *fl,
{
struct inode *inode = NULL;
unsigned int fl_pid;
- struct pid_namespace *proc_pidns = file_inode(f->file)->i_sb->s_fs_info;
+ struct proc_fs_info *fs_info = proc_sb(file_inode(f->file)->i_sb);
+ struct pid_namespace *proc_pidns = fs_info->pid_ns;
fl_pid = locks_translate_pid(fl, proc_pidns);
/*
@@ -2702,7 +2703,8 @@ static int locks_show(struct seq_file *f, void *v)
{
struct locks_iterator *iter = f->private;
struct file_lock *fl, *bfl;
- struct pid_namespace *proc_pidns = file_inode(f->file)->i_sb->s_fs_info;
+ struct proc_fs_info *fs_info = proc_sb(file_inode(f->file)->i_sb);
+ struct pid_namespace *proc_pidns = fs_info->pid_ns;
fl = hlist_entry(v, struct file_lock, fl_link);
diff --git a/fs/proc/base.c b/fs/proc/base.c
index 1b2ede6..99a0f24 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -698,7 +698,8 @@ static bool has_pid_permissions(struct pid_namespace *pid,
static int proc_pid_permission(struct inode *inode, int mask)
{
- struct pid_namespace *pid = inode->i_sb->s_fs_info;
+ struct proc_fs_info *fs_info = proc_sb(inode->i_sb);
+ struct pid_namespace *pid = fs_info->pid_ns;
struct task_struct *task;
bool has_perms;
@@ -733,12 +734,12 @@ static const struct inode_operations proc_def_inode_operations = {
static int proc_single_show(struct seq_file *m, void *v)
{
struct inode *inode = m->private;
- struct pid_namespace *ns;
struct pid *pid;
struct task_struct *task;
int ret;
- ns = inode->i_sb->s_fs_info;
+ struct proc_fs_info *fs_info = proc_sb(inode->i_sb);
+ struct pid_namespace *ns = fs_info->pid_ns;
pid = proc_pid(inode);
task = get_pid_task(pid, PIDTYPE_PID);
if (!task)
@@ -1410,7 +1411,8 @@ static const struct file_operations proc_fail_nth_operations = {
static int sched_show(struct seq_file *m, void *v)
{
struct inode *inode = m->private;
- struct pid_namespace *ns = inode->i_sb->s_fs_info;
+ struct proc_fs_info *fs_info = proc_sb(inode->i_sb);
+ struct pid_namespace *ns = fs_info->pid_ns;
struct task_struct *p;
p = get_proc_task(inode);
@@ -1781,9 +1783,10 @@ struct inode *proc_pid_make_inode(struct super_block * sb,
int pid_getattr(const struct path *path, struct kstat *stat,
u32 request_mask, unsigned int query_flags)
{
- struct inode *inode = d_inode(path->dentry);
struct task_struct *task;
- struct pid_namespace *pid = path->dentry->d_sb->s_fs_info;
+ struct inode *inode = d_inode(path->dentry);
+ struct proc_fs_info *fs_info = proc_sb(inode->i_sb);
+ struct pid_namespace *pid = fs_info->pid_ns;
generic_fillattr(inode, stat);
@@ -2330,6 +2333,8 @@ static const struct seq_operations proc_timers_seq_ops = {
static int proc_timers_open(struct inode *inode, struct file *file)
{
struct timers_private *tp;
+ struct proc_fs_info *fs_info = proc_sb(inode->i_sb);
+ struct pid_namespace *ns = fs_info->pid_ns;
tp = __seq_open_private(file, &proc_timers_seq_ops,
sizeof(struct timers_private));
@@ -2337,7 +2342,7 @@ static int proc_timers_open(struct inode *inode, struct file *file)
return -ENOMEM;
tp->pid = proc_pid(inode);
- tp->ns = inode->i_sb->s_fs_info;
+ tp->ns = ns;
return 0;
}
@@ -3169,13 +3174,13 @@ struct dentry *proc_pid_lookup(struct inode *dir, struct dentry * dentry, unsign
int result = -ENOENT;
struct task_struct *task;
unsigned tgid;
- struct pid_namespace *ns;
+ struct proc_fs_info *fs_info = proc_sb(dir->i_sb);
+ struct pid_namespace *ns = fs_info->pid_ns;
tgid = name_to_int(&dentry->d_name);
if (tgid == ~0U)
goto out;
- ns = dentry->d_sb->s_fs_info;
rcu_read_lock();
task = find_task_by_pid_ns(tgid, ns);
if (task)
@@ -3239,7 +3244,8 @@ static struct tgid_iter next_tgid(struct pid_namespace *ns, struct tgid_iter ite
int proc_pid_readdir(struct file *file, struct dir_context *ctx)
{
struct tgid_iter iter;
- struct pid_namespace *ns = file_inode(file)->i_sb->s_fs_info;
+ struct proc_fs_info *fs_info = proc_sb(file_inode(file)->i_sb);
+ struct pid_namespace *ns = fs_info->pid_ns;
loff_t pos = ctx->pos;
if (pos >= PID_MAX_LIMIT + TGID_OFFSET)
@@ -3465,7 +3471,8 @@ static struct dentry *proc_task_lookup(struct inode *dir, struct dentry * dentry
struct task_struct *task;
struct task_struct *leader = get_proc_task(dir);
unsigned tid;
- struct pid_namespace *ns;
+ struct proc_fs_info *fs_info = proc_sb(dentry->d_sb);
+ struct pid_namespace *ns = fs_info->pid_ns;
if (!leader)
goto out_no_task;
@@ -3474,7 +3481,6 @@ static struct dentry *proc_task_lookup(struct inode *dir, struct dentry * dentry
if (tid == ~0U)
goto out;
- ns = dentry->d_sb->s_fs_info;
rcu_read_lock();
task = find_task_by_pid_ns(tid, ns);
if (task)
@@ -3576,7 +3582,8 @@ static int proc_task_readdir(struct file *file, struct dir_context *ctx)
{
struct inode *inode = file_inode(file);
struct task_struct *task;
- struct pid_namespace *ns;
+ struct proc_fs_info *fs_info = proc_sb(inode->i_sb);
+ struct pid_namespace *ns = fs_info->pid_ns;
int tid;
if (proc_inode_is_dead(inode))
@@ -3588,7 +3595,6 @@ static int proc_task_readdir(struct file *file, struct dir_context *ctx)
/* f_version caches the tgid value that the last readdir call couldn't
* return. lseek aka telldir automagically resets f_version to 0.
*/
- ns = inode->i_sb->s_fs_info;
tid = (int)file->f_version;
file->f_version = 0;
for (task = first_tid(proc_pid(inode), tid, ctx->pos - 2, ns);
diff --git a/fs/proc/inode.c b/fs/proc/inode.c
index 2cf3b74..e34b89a 100644
--- a/fs/proc/inode.c
+++ b/fs/proc/inode.c
@@ -113,7 +113,8 @@ void __init proc_init_kmemcache(void)
static int proc_show_options(struct seq_file *seq, struct dentry *root)
{
struct super_block *sb = root->d_sb;
- struct pid_namespace *pid = sb->s_fs_info;
+ struct proc_fs_info *fs_info = proc_sb(sb);
+ struct pid_namespace *pid = fs_info->pid_ns;
if (!gid_eq(pid->pid_gid, GLOBAL_ROOT_GID))
seq_printf(seq, ",gid=%u", from_kgid_munged(&init_user_ns, pid->pid_gid));
@@ -492,7 +493,8 @@ struct inode *proc_get_inode(struct super_block *sb, struct proc_dir_entry *de)
int proc_fill_super(struct super_block *s, void *data, int silent)
{
- struct pid_namespace *ns = get_pid_ns(s->s_fs_info);
+ struct proc_fs_info *fs_info = proc_sb(s);
+ struct pid_namespace *ns = get_pid_ns(fs_info->pid_ns);
struct inode *root_inode;
int ret;
@@ -514,7 +516,7 @@ int proc_fill_super(struct super_block *s, void *data, int silent)
* top of it
*/
s->s_stack_depth = FILESYSTEM_MAX_STACK_DEPTH;
-
+
pde_get(&proc_root);
root_inode = proc_get_inode(s, &proc_root);
if (!root_inode) {
diff --git a/fs/proc/root.c b/fs/proc/root.c
index 61b7340..5d4c454 100644
--- a/fs/proc/root.c
+++ b/fs/proc/root.c
@@ -16,6 +16,7 @@
#include <linux/init.h>
#include <linux/sched.h>
#include <linux/sched/stat.h>
+#include <linux/slab.h>
#include <linux/module.h>
#include <linux/bitops.h>
#include <linux/user_namespace.h>
@@ -80,16 +81,45 @@ int proc_parse_options(char *options, struct pid_namespace *pid)
int proc_remount(struct super_block *sb, int *flags, char *data)
{
- struct pid_namespace *pid = sb->s_fs_info;
+ struct proc_fs_info *fs_info = proc_sb(sb);
+ struct pid_namespace *pid = fs_info->pid_ns;
sync_filesystem(sb);
return !proc_parse_options(data, pid);
}
+static int proc_test_super(struct super_block *s, void *data)
+{
+ struct proc_fs_info *p = data;
+ struct proc_fs_info *fs_info = proc_sb(s);
+
+ return p->pid_ns == fs_info->pid_ns;
+}
+
+static int proc_set_super(struct super_block *sb, void *data)
+{
+ sb->s_fs_info = data;
+ return set_anon_super(sb, NULL);
+}
+
static struct dentry *proc_mount(struct file_system_type *fs_type,
int flags, const char *dev_name, void *data)
{
+ int error;
+ struct super_block *sb;
struct pid_namespace *ns;
+ struct proc_fs_info *fs_info;
+
+ /*
+ * Don't allow mounting unless the caller has CAP_SYS_ADMIN over
+ * the namespace.
+ */
+ if (!(flags & MS_KERNMOUNT) && !ns_capable(current_user_ns(), CAP_SYS_ADMIN))
+ return ERR_PTR(-EPERM);
+
+ fs_info = kzalloc(sizeof(*fs_info), GFP_NOFS);
+ if (!fs_info)
+ return ERR_PTR(-ENOMEM);
if (flags & SB_KERNMOUNT) {
ns = data;
@@ -98,20 +128,47 @@ static struct dentry *proc_mount(struct file_system_type *fs_type,
ns = task_active_pid_ns(current);
}
- return mount_ns(fs_type, flags, data, ns, ns->user_ns, proc_fill_super);
+ fs_info->pid_ns = ns;
+
+ sb = sget_userns(fs_type, proc_test_super, proc_set_super, flags,
+ ns->user_ns, fs_info);
+ if (IS_ERR(sb)) {
+ error = PTR_ERR(sb);
+ goto error_fs_info;
+ }
+
+ if (sb->s_root) {
+ kfree(fs_info);
+ } else {
+ error = proc_fill_super(sb, data, flags & MS_SILENT ? 1 : 0);
+ if (error) {
+ deactivate_locked_super(sb);
+ goto error;
+ }
+
+ sb->s_flags |= MS_ACTIVE;
+ }
+
+ return dget(sb->s_root);
+
+error_fs_info:
+ kfree(fs_info);
+error:
+ return ERR_PTR(error);
}
static void proc_kill_sb(struct super_block *sb)
{
- struct pid_namespace *ns;
+ struct proc_fs_info *fs_info = proc_sb(sb);
+ struct pid_namespace *ns = (struct pid_namespace *)fs_info->pid_ns;
- ns = (struct pid_namespace *)sb->s_fs_info;
if (ns->proc_self)
dput(ns->proc_self);
if (ns->proc_thread_self)
dput(ns->proc_thread_self);
kill_anon_super(sb);
put_pid_ns(ns);
+ kfree(fs_info);
}
static struct file_system_type proc_fs_type = {
diff --git a/fs/proc/self.c b/fs/proc/self.c
index 4d7d061..3f2a88f 100644
--- a/fs/proc/self.c
+++ b/fs/proc/self.c
@@ -12,7 +12,8 @@ static const char *proc_self_get_link(struct dentry *dentry,
struct inode *inode,
struct delayed_call *done)
{
- struct pid_namespace *ns = inode->i_sb->s_fs_info;
+ struct proc_fs_info *fs_info = proc_sb(inode->i_sb);
+ struct pid_namespace *ns = fs_info->pid_ns;
pid_t tgid = task_tgid_nr_ns(current, ns);
char *name;
@@ -36,9 +37,10 @@ static unsigned self_inum __ro_after_init;
int proc_setup_self(struct super_block *s)
{
struct inode *root_inode = d_inode(s->s_root);
- struct pid_namespace *ns = s->s_fs_info;
+ struct proc_fs_info *fs_info = proc_sb(s);
+ struct pid_namespace *ns = fs_info->pid_ns;
struct dentry *self;
-
+
inode_lock(root_inode);
self = d_alloc_name(s->s_root, "self");
if (self) {
diff --git a/fs/proc/thread_self.c b/fs/proc/thread_self.c
index 9d2efac..e80dd79 100644
--- a/fs/proc/thread_self.c
+++ b/fs/proc/thread_self.c
@@ -12,7 +12,8 @@ static const char *proc_thread_self_get_link(struct dentry *dentry,
struct inode *inode,
struct delayed_call *done)
{
- struct pid_namespace *ns = inode->i_sb->s_fs_info;
+ struct proc_fs_info *fs_info = proc_sb(inode->i_sb);
+ struct pid_namespace *ns = fs_info->pid_ns;
pid_t tgid = task_tgid_nr_ns(current, ns);
pid_t pid = task_pid_nr_ns(current, ns);
char *name;
@@ -35,8 +36,9 @@ static unsigned thread_self_inum __ro_after_init;
int proc_setup_thread_self(struct super_block *s)
{
+ struct proc_fs_info *fs_info = proc_sb(s);
+ struct pid_namespace *ns = fs_info->pid_ns;
struct inode *root_inode = d_inode(s->s_root);
- struct pid_namespace *ns = s->s_fs_info;
struct dentry *thread_self;
inode_lock(root_inode);
diff --git a/fs/proc_namespace.c b/fs/proc_namespace.c
index e16fb8f..f500331 100644
--- a/fs/proc_namespace.c
+++ b/fs/proc_namespace.c
@@ -37,23 +37,23 @@ static __poll_t mounts_poll(struct file *file, poll_table *wait)
return res;
}
-struct proc_fs_info {
+struct proc_fs_opts {
int flag;
const char *str;
};
static int show_sb_opts(struct seq_file *m, struct super_block *sb)
{
- static const struct proc_fs_info fs_info[] = {
+ static const struct proc_fs_opts fs_opts[] = {
{ SB_SYNCHRONOUS, ",sync" },
{ SB_DIRSYNC, ",dirsync" },
{ SB_MANDLOCK, ",mand" },
{ SB_LAZYTIME, ",lazytime" },
{ 0, NULL }
};
- const struct proc_fs_info *fs_infop;
+ const struct proc_fs_opts *fs_infop;
- for (fs_infop = fs_info; fs_infop->flag; fs_infop++) {
+ for (fs_infop = fs_opts; fs_infop->flag; fs_infop++) {
if (sb->s_flags & fs_infop->flag)
seq_puts(m, fs_infop->str);
}
@@ -63,7 +63,7 @@ static int show_sb_opts(struct seq_file *m, struct super_block *sb)
static void show_mnt_opts(struct seq_file *m, struct vfsmount *mnt)
{
- static const struct proc_fs_info mnt_info[] = {
+ static const struct proc_fs_opts mnt_opts[] = {
{ MNT_NOSUID, ",nosuid" },
{ MNT_NODEV, ",nodev" },
{ MNT_NOEXEC, ",noexec" },
@@ -72,9 +72,9 @@ static void show_mnt_opts(struct seq_file *m, struct vfsmount *mnt)
{ MNT_RELATIME, ",relatime" },
{ 0, NULL }
};
- const struct proc_fs_info *fs_infop;
+ const struct proc_fs_opts *fs_infop;
- for (fs_infop = mnt_info; fs_infop->flag; fs_infop++) {
+ for (fs_infop = mnt_opts; fs_infop->flag; fs_infop++) {
if (mnt->mnt_flags & fs_infop->flag)
seq_puts(m, fs_infop->str);
}
diff --git a/include/linux/proc_fs.h b/include/linux/proc_fs.h
index 928ef9e..7c8cf3c 100644
--- a/include/linux/proc_fs.h
+++ b/include/linux/proc_fs.h
@@ -7,11 +7,21 @@
#include <linux/types.h>
#include <linux/fs.h>
+#include <linux/refcount.h>
+
+struct proc_fs_info {
+ struct pid_namespace *pid_ns;
+};
struct proc_dir_entry;
#ifdef CONFIG_PROC_FS
+static inline struct proc_fs_info *proc_sb(struct super_block *sb)
+{
+ return sb->s_fs_info;
+}
+
extern void proc_root_init(void);
extern void proc_flush_task(struct task_struct *);
@@ -48,6 +58,7 @@ static inline void proc_flush_task(struct task_struct *task)
{
}
+extern inline struct proc_fs_info *proc_sb(struct super_block *sb) { return NULL;}
static inline struct proc_dir_entry *proc_symlink(const char *name,
struct proc_dir_entry *parent,const char *dest) { return NULL;}
static inline struct proc_dir_entry *proc_mkdir(const char *name,
--
2.10.5
^ permalink raw reply related
* [PATCH v5 0/7] proc: modernize proc to support multiple private instances
From: Alexey Gladkov @ 2018-05-11 9:32 UTC (permalink / raw)
To: Kees Cook, Andy Lutomirski, Andrew Morton, linux-fsdevel,
linux-kernel, kernel-hardening, linux-security-module, linux-api
Cc: Greg Kroah-Hartman, Alexander Viro, Akinobu Mita, Oleg Nesterov,
Jeff Layton, Ingo Molnar, Alexey Dobriyan, Eric W. Biederman,
Linus Torvalds, aniel Micay, Jonathan Corbet, bfields,
Stephen Rothwell, solar, Dmitry V. Levin, Djalal Harouni
Greetings!
Preface:
--------
This is RFC v5 to modernize procfs and make it able to support multiple
private instances per the same pid namespace.
RFC v1 is here:
https://lkml.org/lkml/2017/3/30/670
RFC v2 is here:
https://lkml.org/lkml/2017/4/25/282
This RFC v5 can be applied on top of v4.17-rc4-45-g2f4474d
This RFC was tested on Ubuntu/Debian/Altlinux/Fedora.
Procfs modernization:
---------------------
Historically procfs was always tied to pid namespaces, during pid
namespace creation we internally create a procfs mount for it. However,
this has the effect that all new procfs mounts are just a mirror of the
internal one, any change, any mount option update, any new future
introduction will propagate to all other procfs mounts that are in the
same pid namespace.
This may have solved several use cases in that time. However today we
face new requirements, and making procfs able to support new private
instances inside same pid namespace seems a major point. If we want to
to introduce new features and security mechanisms we have to make sure
first that we do not break existing usecases. Supporting private procfs
instances will allow to support new features and behaviour without
propagating it to all other procfs mounts.
Today procfs is more of a burden especially to some Embedded, IoT,
sandbox, container use cases. In user space we are over-mounting null
or inaccessible files on top to hide files and information. If we want
to hide pids we have to create PID namespaces otherwise mount options
propagate to all other proc mounts, changing a mount option value in one
mount will propagate to all other proc mounts. If we want to introduce
new features, then they will propagate to all other mounts too, resulting
either maybe new useful functionality or maybe breaking stuff. We have
also to note that userspace should not workaround procfs, the kernel
should just provide a sane simple interface.
In this regard several developers and maintainers pointed out that
there are problems with procfs and it has to be modernized:
"Here's another one: split up and modernize /proc." by Andy Lutomirski [1]
Discussion about kernel pointer leaks:
"And yes, as Kees and Daniel mentioned, it's definitely not just dmesg.
In fact, the primary things tend to be /proc and /sys, not dmesg
itself." By Linus Torvalds [2]
Lot of other areas in the kernel and filesystems have been updated to be
able to support private instances, devpts is one major example [3]. The aim
here is to modernize procfs without breaking userspace, or without affecting
the shared procfs mount. Later new features will apply on the private
instances, and after more testing, months, maybe it can be made the default
especially for IoT.
We want the possibility to do:
$ mount -t proc -onewinstance,newfeature none /proc
newfeature: we are planning new features later for procfs, for now in
this RFC we only introduce "pids=all|ptraceable" mount option.
This allows to absorbe changes, make improvments without breaking use
cases.
Which will be used for:
1) Embedded systems and IoT: usually we have one supervisor for
apps, we have some lightweight sandbox support, however if we create
pid namespaces we have to manage all the processes inside too,
where our goal is to be able to run a bunch of apps each one inside
its own mount namespace, maybe use network namespaces for vlans
setups, but right now we only want mount namespaces, without all the
other complexity. We want procfs to behave more like a real file system,
and block access to inodes that belong to other users. The 'hidepid=' will
not work since it is a shared mount option.
2) Containers, sandboxes and Private instances of file systems - devpts case
Historically, lot of file systems inside Linux kernel view when instantiated
were just a mirror of an already created and mounted filesystem. This was the
case of devpts filesystem, it seems at that time the requirements were to
optimize things and reuse the same memory, etc. This design used to work but not
anymore with today’s containers, IoT, hostile environments and all the privacy
challenges that Linux faces.
In that regards, devpts was updated so that each new mounts is a total
independent file system by the following patches:
“devpts: Make each mount of devpts an independent filesystem” by
Eric W. Biederman [3] [4]
3) Linux Security Modules have multiple ptrace paths inside some
subsystems, however inside procfs, the implementation does not guarantee
that the ptrace() check which triggers the security_ptrace_check() hook
will always run. We have the 'hidepid' mount option that can be used to
force the ptrace_may_access() check inside has_pid_permissions() to run.
The problem is that 'hidepid' is per pid namespace and not attached to
the mount point, any remount or modification of 'hidepid' will propagate
to all other procfs mounts.
This also does not allow to support Yama LSM easily in desktop and user
sessions. Yama ptrace scope which restricts ptrace and some other
syscalls to be allowed only on inferiors, can be updated to have a
per-task context, where the context will be inherited during fork(),
clone() and preserved across execve(). If we support multiple private
procfs instances, then we may force the ptrace_may_access() on
/proc/<pids>/ to always run inside that new procfs instances. This will
allow to specifiy on user sessions if we should populate procfs with
pids that the user can ptrace or not.
By using Yama ptrace scope, some restricted users will only be able to see
inferiors inside /proc, they won't even be able to see their other
processes. Some software like Chromium, Firefox's crash handler, Wine
and others are already using Yama to restrict which processes can be
ptracable. With this change this will give the possibility to restrict
/proc/<pids>/ but more importantly this will give desktop users a
generic and usuable way to specifiy which users should see all processes
and which user can not.
Side notes:
* This covers the lack of seccomp where it is not able to parse
arguments, it is easy to install a seccomp filter on direct syscalls
that operate on pids, however /proc/<pid>/ is a Linux ABI using
filesystem syscalls. With this change all LSMs should be able to analyze
open/read/write/close... on /proc/<pid>/
4) This will allow to implement new features either in kernel or
userspace without having to worry about procfs.
In containers, sandboxes, etc we have workarounds to hide some /proc
inodes, this should be supported natively without doing extra complex
work, the kernel should be able to support sane options that work with
today and future Linux use cases.
Introduced changes:
-------------------
This series adds few new mount options:
* 'newinstance' mount option, it was also suggesed by Andy Lutomirski [5].
When this option is passed we automatically create a private procfs instance.
This is not the default behaviour since we do not want to break userspace
and we do not want to provide different devices IDs by default when
stat()ing inodes, I am not sure about all the use cases there [6].
* 'pidonly' that allows to hide non-pid inodes from procfs. It can be used
in containers and sandboxes, as these are already trying to hide and block
access to procfs inodes anyway.
* 'pids' mount option, as discussed with Andy Lutomirski.
If 'pids=' is passed without 'newinstance' then it has no effect.
If 'newinstance,pids=all' then processes will be show inside the proc
mount.
If 'newinstance,pids=ptraceable' then only ptraceable processes will be
shown.
This allows to support lightweight sandboxes in Embedded Linux, also
solves the case for LSM where now with this mount option, we make sure
that they have a ptrace path in procfs.
Use cases of 'newinstance' mount option:
* We create a private procfs instance that it is disconnected from the
shared or other procfs instances.
* "hidepid" instead of chaning all other mirrored procfs mounts, now
it will work only on the new private instance.
* "gid" instead of chaning all other mirrored procfs mounts, now it will
work only on the new private instance.
* "pids=ptraceable" mount option which will take precendence over
"hidepid" will only work when 'newinstance' is set. Otherwise it is ignored.
This should allow later after real testing to have a smooth transition
to a procfs with default private instances.
How to test:
$ sudo mount -t proc -onewinstance,pids=ptraceable none /test
Note for userspace that should be documented:
If you are over mounting /proc, then make sure you are in a new mount
namespace where propagation to master is disconnected. This will avoid
to pin that new /proc mount.
References:
-----------
[1] https://lists.linuxfoundation.org/pipermail/ksummit-discuss/2017-January/004215.html
[2] http://www.openwall.com/lists/kernel-hardening/2017/10/05/5
[3] https://lwn.net/Articles/689539/
[4] http://lxr.free-electrons.com/source/Documentation/filesystems/devpts.txt?v=3.14
[5] https://lkml.org/lkml/2017/5/2/407
[6] https://lkml.org/lkml/2017/5/3/357
# Changes since RFC v5:
*) Fixed a bug that caused a problem with the Fedora boot.
*) The 'pidonly' option is visible among the mount options.
# Changes since RFC v2:
*) Renamed mount options to 'newinstance' and 'pids='
Suggested-by: Andy Lutomirski <luto@kernel.org>
*) Fixed order of commit, Suggested-by: Andy Lutomirski <luto@kernel.org>
*) Many bug fixes.
# Changes since RFC v1:
*) Removed 'unshared' mount option and replaced it with 'limit_pids'
which is attached to the current procfs mount.
Suggested-by Andy Lutomirski <luto@kernel.org>
*) Do not fill dcache with pid entries that we can not ptrace.
*) Many bug fixes.
Alexey Gladkov (1):
proc: add option to mount only a pids subset
Djalal Harouni (6):
proc: add proc_fs_info struct to store proc information
proc: move /proc/{self|thread-self} dentries to proc_fs_info
proc: add helpers to set and get proc hidepid and gid mount options
proc: support mounting private procfs instances inside same pid
namespace
proc: instantiate only pids that we can ptrace on 'limit_pids=1' mount
option
proc: flush task dcache entries from all procfs instances
fs/locks.c | 6 +-
fs/proc/base.c | 101 ++++++++++++++++-------
fs/proc/generic.c | 20 +++++
fs/proc/inode.c | 29 ++++++-
fs/proc/internal.h | 2 +-
fs/proc/root.c | 184 +++++++++++++++++++++++++++++++++++++++---
fs/proc/self.c | 9 ++-
fs/proc/thread_self.c | 9 ++-
fs/proc_namespace.c | 14 ++--
include/linux/pid_namespace.h | 46 ++++++++++-
include/linux/proc_fs.h | 127 +++++++++++++++++++++++++++++
11 files changed, 480 insertions(+), 67 deletions(-)
--
2.10.5
^ permalink raw reply
* Re: [RFC PATCH] Add /proc/<pid>/numa_vamaps for numa node information
From: Michal Hocko @ 2018-05-11 6:39 UTC (permalink / raw)
To: Prakash Sangappa
Cc: Christopher Lameter, linux-kernel, linux-mm, linux-api, akpm,
kirill.shutemov, n-horiguchi, drepper, rientjes
In-Reply-To: <ce2c36aa-8c03-63d6-e1ce-031197f45a5d@oracle.com>
On Thu 10-05-18 09:00:24, Prakash Sangappa wrote:
>
>
> On 5/10/18 12:42 AM, Michal Hocko wrote:
> > On Fri 04-05-18 09:18:11, Prakash Sangappa wrote:
> > >
> > > On 5/4/18 4:12 AM, Michal Hocko wrote:
> > > > On Thu 03-05-18 15:39:49, prakash.sangappa wrote:
> > > > > On 05/03/2018 11:03 AM, Christopher Lameter wrote:
> > > > > > On Tue, 1 May 2018, Prakash Sangappa wrote:
> > > > > >
> > > > > > > For analysis purpose it is useful to have numa node information
> > > > > > > corresponding mapped address ranges of the process. Currently
> > > > > > > /proc/<pid>/numa_maps provides list of numa nodes from where pages are
> > > > > > > allocated per VMA of the process. This is not useful if an user needs to
> > > > > > > determine which numa node the mapped pages are allocated from for a
> > > > > > > particular address range. It would have helped if the numa node information
> > > > > > > presented in /proc/<pid>/numa_maps was broken down by VA ranges showing the
> > > > > > > exact numa node from where the pages have been allocated.
> > > > > > Cant you write a small script that scans the information in numa_maps and
> > > > > > then displays the total pages per NUMA node and then a list of which
> > > > > > ranges have how many pages on a particular node?
> > > > > Don't think we can determine which numa node a given user process
> > > > > address range has pages from, based on the existing 'numa_maps' file.
> > > > yes we have. See move_pages...
> > > Sure using move_pages, not based on just 'numa_maps'.
> > >
> > > > > > > reading this file will not be restricted(i.e requiring CAP_SYS_ADMIN).
> > > > > > So a prime motivator here is security restricted access to numa_maps?
> > > > > No it is the opposite. A regular user should be able to determine
> > > > > numa node information.
> > > > Well, that breaks the layout randomization, doesn't it?
> > > Exposing numa node information itself should not break randomization right?
> > I thought you planned to expose address ranges for each numa node as
> > well. /me confused.
>
> Yes, are you suggesting this information should not be available to a
> regular
> user?
absolutely. We do check ptrace_may_access(task, PTRACE_MODE_READ_REALCREDS)
to make sure the access is authorised.
>
> Is it not possible to get that same information using the move_pages() api
> as a regular
> user, although one / set of pages at a time?
No, see PTRACE_MODE_READ_REALCREDS check in move_pages.
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* Re: [RFC] vfs: skip extra attributes check on removal for symlinks
From: Al Viro @ 2018-05-11 2:42 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: linux-fsdevel, linux-api, sandeen, darrick.wong, dhowells, tytso,
fliu, jack, jeffm, nborisov, jake.norris, mtk.manpages, linux-xfs,
linux-kernel
In-Reply-To: <20180510230551.GE27853@wotan.suse.de>
On Thu, May 10, 2018 at 11:05:51PM +0000, Luis R. Rodriguez wrote:
> On Thu, May 10, 2018 at 09:48:07PM +0100, Al Viro wrote:
> > On Thu, Apr 26, 2018 at 04:46:39PM -0700, Luis R. Rodriguez wrote:
> >
> > > Since we cannot set these attributes we should special-case the
> > > immutable/append on delete for symlinks, this would be consistent with
> > > what we *do* allow on Linux for all filesystems.
> >
> > Er... So why not simply sanity-check it in places that set it on
> > inodes?
>
> The patch is not about sanity-checks on setters though as *that* is in place
> already. Its about the case where the filesystem gets corrupted and the VFS
> *still* does process these attributes for symlinks and still prevents
> deletion because of these attributes.
... and this corrupted fs ends up setting those flags on in-core inodes.
Which is where we ought to block that. Seriously, let's make sure that
->i_flags manipulations are done by inode_set_flags() (e.g. btrfs open-codes
that, apparently) and let's make _that_ check and reject those.
^ permalink raw reply
* Re: [RFC v2 2/4] xfs: add verifier check for symlink with append/immutable flags
From: Darrick J. Wong @ 2018-05-11 2:20 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: viro, tytso, adilger.kernel, clm, jbacik, dsterba, sandeen,
dhowells, fliu, jack, jeffm, nborisov, jake.norris, mtk.manpages,
linux-api, linux-fsdevel, linux-xfs, linux-ext4, linux-btrfs,
linux-kernel
In-Reply-To: <20180510231359.16899-3-mcgrof@kernel.org>
On Thu, May 10, 2018 at 04:13:57PM -0700, Luis R. Rodriguez wrote:
> The Linux VFS does not allow a way to set append/immuttable attributes
> to symlinks, this is just not possible. If this is detected we can
> correct this with xfs_repair, so inform the user.
>
> Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
> ---
> fs/xfs/libxfs/xfs_symlink_remote.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/fs/xfs/libxfs/xfs_symlink_remote.c b/fs/xfs/libxfs/xfs_symlink_remote.c
> index 5ef5f354587e..42dd81ede3d6 100644
> --- a/fs/xfs/libxfs/xfs_symlink_remote.c
> +++ b/fs/xfs/libxfs/xfs_symlink_remote.c
> @@ -242,5 +242,10 @@ xfs_symlink_shortform_verify(
> /* We /did/ null-terminate the buffer, right? */
> if (*endp != 0)
> return __this_address;
> +
> + /* Immutable and append flags are not allowed on symlinks */
> + if (ip->i_d.di_flags & (XFS_DIFLAG_APPEND | XFS_DIFLAG_IMMUTABLE))
> + return __this_address;
This belongs in xfs_dinode_verify so that it checks all symlinks, not
just the one shortform ones.
--D
> +
> return NULL;
> }
> --
> 2.17.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [RFC v2 4/4] btrfs: verify symlinks with append/immutable flags
From: Luis R. Rodriguez @ 2018-05-10 23:13 UTC (permalink / raw)
To: viro, darrick.wong, tytso, adilger.kernel, clm, jbacik, dsterba
Cc: sandeen, dhowells, fliu, jack, jeffm, nborisov, jake.norris,
mtk.manpages, linux-api, linux-fsdevel, linux-xfs, linux-ext4,
linux-btrfs, linux-kernel, Luis R. Rodriguez
In-Reply-To: <20180510231359.16899-1-mcgrof@kernel.org>
The Linux VFS does not allow a way to set append/immuttable
attributes to symlinks, this is just not possible. If this is
detected inform the user as the filesystem must be corrupted.
Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
fs/btrfs/inode.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index c4bdb597b323..d9c786be408c 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -3933,6 +3933,15 @@ static int btrfs_read_locked_inode(struct inode *inode)
inode->i_op = &btrfs_dir_inode_operations;
break;
case S_IFLNK:
+ /* VFS does not allow setting these so must be corruption */
+ if (IS_APPEND(inode) || IS_IMMUTABLE(inode)) {
+ ret = -EUCLEAN;
+ btrfs_crit(fs_info,
+ "corrupt symlink with append/immutable ino=%llu root=%llu\n",
+ btrfs_ino(BTRFS_I(inode)),
+ root->root_key.objectid);
+ goto make_bad;
+ }
inode->i_op = &btrfs_symlink_inode_operations;
inode_nohighmem(inode);
inode->i_mapping->a_ops = &btrfs_symlink_aops;
--
2.17.0
^ permalink raw reply related
* [RFC v2 3/4] ext4: add verifier check for symlink with append/immutable flags
From: Luis R. Rodriguez @ 2018-05-10 23:13 UTC (permalink / raw)
To: viro, darrick.wong, tytso, adilger.kernel, clm, jbacik, dsterba
Cc: sandeen, dhowells, fliu, jack, jeffm, nborisov, jake.norris,
mtk.manpages, linux-api, linux-fsdevel, linux-xfs, linux-ext4,
linux-btrfs, linux-kernel, Luis R. Rodriguez
In-Reply-To: <20180510231359.16899-1-mcgrof@kernel.org>
The Linux VFS does not allow a way to set append/immuttable
attributes to symlinks, this is just not possible. If this is
detected inform the user as the filesystem must be corrupted.
Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
fs/ext4/inode.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 37a2f7a2b66a..6acf0dd6b6e6 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -4947,6 +4947,13 @@ struct inode *ext4_iget(struct super_block *sb, unsigned long ino)
inode->i_op = &ext4_dir_inode_operations;
inode->i_fop = &ext4_dir_operations;
} else if (S_ISLNK(inode->i_mode)) {
+ /* VFS does not allow setting these so must be corruption */
+ if (IS_APPEND(inode) || IS_IMMUTABLE(inode)) {
+ EXT4_ERROR_INODE(inode,
+ "immutable or append flags not allowed on symlinks");
+ ret = -EFSCORRUPTED;
+ goto bad_inode;
+ }
if (ext4_encrypted_inode(inode)) {
inode->i_op = &ext4_encrypted_symlink_inode_operations;
ext4_set_aops(inode);
--
2.17.0
^ permalink raw reply related
* [RFC v2 2/4] xfs: add verifier check for symlink with append/immutable flags
From: Luis R. Rodriguez @ 2018-05-10 23:13 UTC (permalink / raw)
To: viro, darrick.wong, tytso, adilger.kernel, clm, jbacik, dsterba
Cc: sandeen, dhowells, fliu, jack, jeffm, nborisov, jake.norris,
mtk.manpages, linux-api, linux-fsdevel, linux-xfs, linux-ext4,
linux-btrfs, linux-kernel, Luis R. Rodriguez
In-Reply-To: <20180510231359.16899-1-mcgrof@kernel.org>
The Linux VFS does not allow a way to set append/immuttable attributes
to symlinks, this is just not possible. If this is detected we can
correct this with xfs_repair, so inform the user.
Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
fs/xfs/libxfs/xfs_symlink_remote.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/fs/xfs/libxfs/xfs_symlink_remote.c b/fs/xfs/libxfs/xfs_symlink_remote.c
index 5ef5f354587e..42dd81ede3d6 100644
--- a/fs/xfs/libxfs/xfs_symlink_remote.c
+++ b/fs/xfs/libxfs/xfs_symlink_remote.c
@@ -242,5 +242,10 @@ xfs_symlink_shortform_verify(
/* We /did/ null-terminate the buffer, right? */
if (*endp != 0)
return __this_address;
+
+ /* Immutable and append flags are not allowed on symlinks */
+ if (ip->i_d.di_flags & (XFS_DIFLAG_APPEND | XFS_DIFLAG_IMMUTABLE))
+ return __this_address;
+
return NULL;
}
--
2.17.0
^ permalink raw reply related
* [RFC v2 1/4] vfs: skip extra attributes check on removal for symlinks
From: Luis R. Rodriguez @ 2018-05-10 23:13 UTC (permalink / raw)
To: viro, darrick.wong, tytso, adilger.kernel, clm, jbacik, dsterba
Cc: sandeen, dhowells, fliu, jack, jeffm, nborisov, jake.norris,
mtk.manpages, linux-api, linux-fsdevel, linux-xfs, linux-ext4,
linux-btrfs, linux-kernel, Luis R. Rodriguez
In-Reply-To: <20180510231359.16899-1-mcgrof@kernel.org>
Linux filesystems cannot set extra file attributes (stx_attributes as per
statx(2)) on a symbolic link. To set extra file attributes you issue
ioctl(2) with FS_IOC_SETFLAGS, *all* ioctl(2) calls on a symbolic link
yield EBADF.
This is because ioctl(2) tries to obtain struct fd from the symbolic link
file descriptor passed using fdget(), fdget() in turn always returns no
file set when a file descriptor is open with O_PATH. As per symlink(2)
O_PATH and O_NOFOLLOW must *always* be used when you want to get the file
descriptor of a symbolic link, and this holds true for Linux, as such extra
file attributes cannot possibly be set on symbolic links on Linux.
Filesystems repair utilities should be updated to detect this as
corruption and correct this, however, the VFS *does* respect these
extra attributes on symlinks for removal.
Since we cannot set these attributes we should special-case the
immutable/append on delete for symlinks, this would be consistent with
what we *do* allow on Linux for all filesystems. Since this is a clear
sign to the VFS the filesystem must be corrupted filesystems can
implement a verifier to catch this earlier. A generic warning issued for
filesystems which don't implement these verifiers, and the VFS also lets
users delete these pesky symlinks as otherwise users cannot get rid of
them.
The userspace utility chattr(1) cannot set these attributes on symlinks
*and* other special files as well:
# chattr -a symlink
chattr: Operation not supported while reading flags on b
The reason for this is different though. Refer to commit 023d111e92195
("chattr.1.in: Document the compression attribute flags E, X, and ...")
merged on e2fsprogs v1.28 since August 2002. This commit prevented
issuing the ioctl() for symlink *and* special files in consideration for
a buggy DRM driver where issuing lsattr on their special files crashed
the system. For details refer to Debian bug 152029 [0].
You can craft your own tool to query the extra file attributes with
the new shiny statx(2) tool, statx(2) will list the attributes if
they were set for instance on a corrupt filesystem. However statx(2)
is only used for *querying* -- not for setting the attributes.
If you implement issuing your own ioctl() for FS_IOC_FSGETXATTR or
FS_IOC_FSSETXATTR on special files (block, char, fifo) it will fail
returning -1 and errno is set to ENOTTY (Inappropriate ioctl for
device). The reason for this is different than for symlinks.
For special files this fails on vfs_ioctl() when the filesystem
f_op callbacks are not set for these special files:
long vfs_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
{
int error = -ENOTTY;
if (!filp->f_op->unlocked_ioctl)
goto out;
error = filp->f_op->unlocked_ioctl(filp, cmd, arg);
if (error == -ENOIOCTLCMD)
error = -ENOTTY;
out:
return error;
}
The same applies to PF_LOCAL named sockets. Since this varies by
filesystem for special files, only make a special rule to respect
the immutable and append attribute on symlinks.
[0] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=152029
Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
fs/namei.c | 24 ++++++++++++++++++++++--
1 file changed, 22 insertions(+), 2 deletions(-)
diff --git a/fs/namei.c b/fs/namei.c
index e861b409c241..23ebc14805dc 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -2760,6 +2760,26 @@ int __check_sticky(struct inode *dir, struct inode *inode)
}
EXPORT_SYMBOL(__check_sticky);
+/* Process extra file attributes only when they make sense */
+static bool may_delete_stx_attributes(struct inode *inode)
+{
+ /*
+ * The VFS does not allow setting append/immutable on symlinks.
+ *
+ * Filesystems can implement their own verifier which would avoid this
+ * generic splat, this generic splat is desirable if the respective
+ * filesystem repair utility won't implement a fix for this, otherwise
+ * users end up with a nagging dangling file which is impossible to
+ * fix in userspace.
+ */
+ if (S_ISLNK(inode->i_mode)) {
+ WARN_ONCE((IS_APPEND(inode) || IS_IMMUTABLE(inode)),
+ "Immutable or append flag set on symlink. VFS does not allow this, must be a filesystem corruption. Allowing deletion though");
+ } else if (IS_APPEND(inode) || IS_IMMUTABLE(inode))
+ return false;
+ return true;
+}
+
/*
* Check whether we can remove a link victim from directory dir, check
* whether the type of victim is right.
@@ -2798,8 +2818,8 @@ static int may_delete(struct inode *dir, struct dentry *victim, bool isdir)
if (IS_APPEND(dir))
return -EPERM;
- if (check_sticky(dir, inode) || IS_APPEND(inode) ||
- IS_IMMUTABLE(inode) || IS_SWAPFILE(inode) || HAS_UNMAPPED_ID(inode))
+ if (check_sticky(dir, inode) || !may_delete_stx_attributes(inode) ||
+ IS_SWAPFILE(inode) || HAS_UNMAPPED_ID(inode))
return -EPERM;
if (isdir) {
if (!d_is_dir(victim))
--
2.17.0
^ permalink raw reply related
* [RFC v2 0/4] vfs: detect symlink corruption with attributes
From: Luis R. Rodriguez @ 2018-05-10 23:13 UTC (permalink / raw)
To: viro, darrick.wong, tytso, adilger.kernel, clm, jbacik, dsterba
Cc: sandeen, dhowells, fliu, jack, jeffm, nborisov, jake.norris,
mtk.manpages, linux-api, linux-fsdevel, linux-xfs, linux-ext4,
linux-btrfs, linux-kernel, Luis R. Rodriguez
Filesystems which detect symlinks with append/immutable should inform
users their filesystem is corrupted and their respective filesystem
checker tool should fix this.
In lieu of this though users may be stuck with pesky files or
directories which they cannot remove. We cannot expect all filesystems
to be updated to address this, so since the VFS does not let filesystems
set these attributes -- let the VFS enable users to remove symlink with
these attributes, but also provide a fallback warning, in case the
users's own filesystem does not catch it.
Sending again as RFC as this just goes compile tested so far, and it
is still unclear if this is the direction we want to go with this.
v2:
As per Darrick, even though the VFS should probably allow not so popular
filesystems to delete corrupt symlinks with append/immutable -- popular
filesystems should check for this themselves and inform the users of
corruption. These filesystems should have their respective filesystem
checker tools updated to correct this as well.
v1:
Sent out a single patch just to ignore the append/immutable attributes
set on symlinks.
Luis R. Rodriguez (4):
vfs: skip extra attributes check on removal for symlinks
xfs: add verifier check for symlink with append/immutable flags
ext4: add verifier check for symlink with append/immutable flags
btrfs: verify symlinks with append/immutable flags
fs/btrfs/inode.c | 9 +++++++++
fs/ext4/inode.c | 7 +++++++
fs/namei.c | 24 ++++++++++++++++++++++--
fs/xfs/libxfs/xfs_symlink_remote.c | 5 +++++
4 files changed, 43 insertions(+), 2 deletions(-)
--
2.17.0
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox