* [RFC PATCH 03/18] kthread: Add kthread_stop_current()
From: Petr Mladek @ 2015-06-05 15:01 UTC (permalink / raw)
To: Andrew Morton, Oleg Nesterov, Tejun Heo, Ingo Molnar,
Peter Zijlstra
Cc: linux-nfs, Borislav Petkov, Jiri Kosina, Richard Weinberger,
Trond Myklebust, linux-kernel, Steven Rostedt, Michal Hocko,
Chris Mason, Petr Mladek, linux-mtd, linux-api, Linus Torvalds,
live-patching, Thomas Gleixner, Paul E. McKenney, David Woodhouse,
Anna Schumaker
In-Reply-To: <1433516477-5153-1-git-send-email-pmladek@suse.cz>
For example, jffs2_gcd_mtd kthread can be stopped by SIGKILL.
The signal is handled inside the main function.
We would like to convert such kthreads to the iterant API
and use proper signal handlers.
The new functions will allow to pass the information
between the signal handler and the main kthread functions.
kthread_stop_current() allows to quit the kthread correctly.
It means to leave the main cycle on a safe point and call
clean up actions via post() function.
Signed-off-by: Petr Mladek <pmladek@suse.cz>
---
include/linux/kthread.h | 1 +
kernel/kthread.c | 13 +++++++++++++
2 files changed, 14 insertions(+)
diff --git a/include/linux/kthread.h b/include/linux/kthread.h
index 06fe9ad192dd..100c1e006729 100644
--- a/include/linux/kthread.h
+++ b/include/linux/kthread.h
@@ -86,6 +86,7 @@ kthread_iterant_create_on_cpu(struct kthread_iterant *kti,
})
void kthread_bind(struct task_struct *k, unsigned int cpu);
+void kthread_stop_current(void);
int kthread_stop(struct task_struct *k);
bool kthread_should_stop(void);
bool kthread_should_park(void);
diff --git a/kernel/kthread.c b/kernel/kthread.c
index 4b2698bcc622..688bb4cfd807 100644
--- a/kernel/kthread.c
+++ b/kernel/kthread.c
@@ -70,6 +70,19 @@ static struct kthread *to_live_kthread(struct task_struct *k)
}
/**
+ * kthread_stop_current - make the current kthread to terminate a safe way
+ *
+ * This function sets KTHREAD_SHOULD_STOP flags. It makes kthread to break
+ * the main loop and do some clean up actions before the main kthread
+ * function finishes. It is the standard behavior for SIGTERM signal.
+ */
+void kthread_stop_current(void)
+{
+ set_bit(KTHREAD_SHOULD_STOP, &to_kthread(current)->flags);
+}
+EXPORT_SYMBOL(kthread_stop_current);
+
+/**
* kthread_should_stop - should this kthread return now?
*
* When someone calls kthread_stop() on your kthread, it will be woken
--
1.8.5.6
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related
* [RFC PATCH 02/18] kthread: Add API for iterant kthreads
From: Petr Mladek @ 2015-06-05 15:01 UTC (permalink / raw)
To: Andrew Morton, Oleg Nesterov, Tejun Heo, Ingo Molnar,
Peter Zijlstra
Cc: Richard Weinberger, Steven Rostedt, David Woodhouse,
linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Trond Myklebust,
Anna Schumaker, linux-nfs-u79uwXL29TY76Z2rM5mHXA, Chris Mason,
Paul E. McKenney, Thomas Gleixner, Linus Torvalds, Jiri Kosina,
Borislav Petkov, Michal Hocko,
live-patching-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Petr Mladek
In-Reply-To: <1433516477-5153-1-git-send-email-pmladek-AlSwsSmVLrQ@public.gmane.org>
Kthreads are implemented as an infinite loop. They include check points
for termination, freezer, parking, and even signal handling.
In many cases there are some operations done before and after
the main cycle.
We need to touch all kthreads everytime we want to add or
modify the behavior of such checkpoints. It is not easy because
there are several hundreds of kthreads and each of them is
implemented slightly different way.
This anarchy brings potentially broken or non-standard behavior.
For example, few kthreads associated strange behavior to signals.
This patch defines new API that will help to standardize
kthreads and move all checkpoints to a single location.
The initial implementation allows to define functions that are
called before, in, and after the main cycle. It defines the common
checkpoint for freezing and ktherad stopping. The support for
parking and signal handling will be added later.
It will be sufficient for many kthreads. And it is ready for more
features that will be needed for the rest. For example, some kthreads
might want to call special functions after a return from the fridge.
Why new API?
First, we do not want to add yet another API that would need
to be supported. The aim is to _replace_ the current API,
split the monolithic threadfn, and define a standard structure
of the main cycle and standardize the related check points.
Well, the old API would need to stay around for some time until
all kthreads are converted.
Second, there are two more existing alternatives. They fulfill
the needs, will be used for some conversions, but they are not
well usable in all cases. Let's talk more about them.
Workqueue
Workqueues are quite popular and many kthreads have already been
converted into them.
Work queues allow to split the function into even more pieces and
reach the common check point more often. It is especially useful
when a kthread handles more tasks and is waken when some work
is needed. Then we could queue the appropriate work instead
of waking the whole kthread and checking what exactly needs
to be done.
But there are many kthreads that need to cycle many times
until some work is finished, e.g. khugepaged, virtio_balloon,
jffs2_garbage_collect_thread. They would need to queue
the work repeatedly or between more work items. It would be
a strange semantic. Note that we need to queue the work
repeatedly when it needs to be called in a cycle.
Work queues allow to share the same kthread between more users.
It helps to reduce the number of running kthreads. It is especially
useful if you would need a kthread for each CPU.
But this might also be a disadvantage. Just look into the output
of the command "ps" and see the many [kworker*] processes. One
might see this a black hole. If a kworker makes the system busy,
it is less obvious what the problem is in compare with the old
"simple" and dedicated kthreads.
Yes, we could add some debugging tools for work queues but
it would be another non-standard thing that developers and
system administrators would need to understand.
Another thing is that work queues have their own scheduler. If we
move even more tasks there it might need even more love. Anyway,
the extra scheduler adds another level of complexity when
debugging problems.
kthread_worker
kthread_worker is similar to workqueues in many ways. You need to
+ define work functions
+ define and initialize work structs
+ queue work items (structs pointing to the functions and data)
We could repeat here the paragraphs about splitting the work
and sharing the kthread between more users.
The kthread_worker implementation is much simpler than
the one for workqueues. It is more similar to a simple
kthread. Especially, it uses the system scheduler.
But it is still more complex that the simple kthread.
One interesting thing is that kthread_workers add internal work
items into the queue. They typically use a completion. An example
is the flush work. It is a nice trick but you need to be careful.
For example, if you would want to terminate the kthread, you might
want remove some work item from the queue, especially if you need
to break a work item that is called in a cycle (queues itself).
The question is what to do with the internal tasks. If you keep
them, they might wake up sleepers when the work was not really
completed. If you remove them, the counter part might sleep
forever.
Conclusion
The aim of the new API is to provide a simple and straightforward API
to create dedicated kthreads. It will split the current monolithic
function into few well defined pieces. This might look as a small
complication. But it will also move the typical checks into a common
place. This will reduce the complexity of the kthreads and will
allow to do the checks properly and maintain them more easily.
Signed-off-by: Petr Mladek <pmladek-AlSwsSmVLrQ@public.gmane.org>
---
include/linux/kthread.h | 48 ++++++++++++++++++++++++++++++
kernel/kthread.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 126 insertions(+)
diff --git a/include/linux/kthread.h b/include/linux/kthread.h
index 13d55206ccf6..06fe9ad192dd 100644
--- a/include/linux/kthread.h
+++ b/include/linux/kthread.h
@@ -4,6 +4,9 @@
#include <linux/err.h>
#include <linux/sched.h>
+/*
+ * Old API that defines kthreads using a single function.
+ */
__printf(4, 5)
struct task_struct *kthread_create_on_node(int (*threadfn)(void *data),
void *data,
@@ -37,6 +40,51 @@ struct task_struct *kthread_create_on_cpu(int (*threadfn)(void *data),
__k; \
})
+/*
+ * New API that allows to create kthreads with a clear semantic. It defines
+ * functions that are called inside the kthread. The functions must return
+ * in a consistent state so that the kthread can get frozen, parked,
+ * and even signals processed between the calls. It means that all locks
+ * must be released and non-interruptible work finished.
+ */
+
+/**
+ * struct kthread_iterant - structure describing the function of the kthread
+ * @data: pointer to a data passed to the functions.
+ * @init: function called when the kthread is created.
+ * @func: function called in the main cycle until the kthread is terminated.
+ * @destroy: function called when the kthread is being terminated.
+ */
+struct kthread_iterant {
+ void *data;
+ void (*init)(void *data);
+ void (*func)(void *data);
+ void (*destroy)(void *data);
+};
+
+__printf(3, 4)
+struct task_struct *
+kthread_iterant_create_on_node(struct kthread_iterant *kti,
+ int node,
+ const char namefmt[], ...);
+
+#define kthread_iterant_create(kti, namefmt, arg...) \
+ kthread_iterant_create_on_node(kti, -1, namefmt, ##arg)
+
+struct task_struct *
+kthread_iterant_create_on_cpu(struct kthread_iterant *kti,
+ unsigned int cpu,
+ const char *namefmt);
+
+#define kthread_iterant_run(kti, namefmt, ...) \
+({ \
+ struct task_struct *__k \
+ = kthread_iterant_create(kti, namefmt, ## __VA_ARGS__); \
+ if (!IS_ERR(__k)) \
+ wake_up_process(__k); \
+ __k; \
+})
+
void kthread_bind(struct task_struct *k, unsigned int cpu);
int kthread_stop(struct task_struct *k);
bool kthread_should_stop(void);
diff --git a/kernel/kthread.c b/kernel/kthread.c
index fca7cd124512..4b2698bcc622 100644
--- a/kernel/kthread.c
+++ b/kernel/kthread.c
@@ -393,6 +393,84 @@ struct task_struct *kthread_create_on_cpu(int (*threadfn)(void *data),
return p;
}
+/**
+ * kthread_iterant_fn - kthread function to process an iterant kthread
+ * @kti_ptr: pointer to an initialized struct kthread_iterant.
+ *
+ * This function defines the work flow of iterant kthreads. It calls
+ * the given callbacks. Also it defines and implements a common
+ * check point for freezing and stopping.
+ */
+static int kthread_iterant_fn(void *kti_ptr)
+{
+ struct kthread_iterant *kti = kti_ptr;
+ void *data = kti->data;
+
+ if (kti->init)
+ kti->init(data);
+
+ do {
+ if (kti->func)
+ kti->func(data);
+
+ /* this check is safe also for non-freezable kthreads */
+ } while (!kthread_freezable_should_stop(NULL));
+
+ if (kti->destroy)
+ kti->destroy(data);
+
+ return 0;
+}
+
+/**
+ * kthread_iterant_create_on_node - create an iterant kthread.
+ * @kti: structure describing the thread function.
+ * @node: memory node number.
+ * @namefmt: printf-style name for the thread.
+ *
+ * Description: This helper function creates and names an iterant kernel
+ * thread. The thread will be stopped: use wake_up_process() to start
+ * it. See also kthread_iterant_run().
+ *
+ * It behaves the same as kthread_create_on_node(). The only difference
+ * is that the function is described using struct kthread_iterant.
+ */
+struct task_struct *
+kthread_iterant_create_on_node(struct kthread_iterant *kti,
+ int node,
+ const char namefmt[], ...)
+{
+ struct task_struct *task;
+ va_list args;
+
+ va_start(args, namefmt);
+ task = __kthread_create_on_node(kthread_iterant_fn, kti, node,
+ namefmt, args);
+ va_end(args);
+
+ return task;
+}
+EXPORT_SYMBOL(kthread_iterant_create_on_node);
+
+/**
+ * kthread_iterant_create_on_cpu - Create a cpu bound iterant kthread
+ * @kti: structure describing the function.
+ * @cpu: The cpu on which the thread should be bound,
+ * @namefmt: printf-style name for the thread. Format is restricted
+ * to "name.*%u". Code fills in cpu number.
+ *
+ * Description: This helper function creates and names an iterant kernel
+ * thread. The thread will be woken and put into park mode.
+ */
+struct task_struct *
+kthread_iterant_create_on_cpu(struct kthread_iterant *kti,
+ unsigned int cpu,
+ const char *namefmt)
+{
+ return kthread_create_on_cpu(kthread_iterant_fn, kti,
+ cpu, namefmt);
+}
+
static void __kthread_unpark(struct task_struct *k, struct kthread *kthread)
{
clear_bit(KTHREAD_SHOULD_PARK, &kthread->flags);
--
1.8.5.6
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [RFC PATCH 01/18] kthread: Allow to call __kthread_create_on_node() with va_list args
From: Petr Mladek @ 2015-06-05 15:01 UTC (permalink / raw)
To: Andrew Morton, Oleg Nesterov, Tejun Heo, Ingo Molnar,
Peter Zijlstra
Cc: Richard Weinberger, Steven Rostedt, David Woodhouse, linux-mtd,
Trond Myklebust, Anna Schumaker, linux-nfs, Chris Mason,
Paul E. McKenney, Thomas Gleixner, Linus Torvalds, Jiri Kosina,
Borislav Petkov, Michal Hocko, live-patching, linux-api,
linux-kernel, Petr Mladek
In-Reply-To: <1433516477-5153-1-git-send-email-pmladek@suse.cz>
kthread_create_on_node() implements a bunch of logic to create
the kthread. It is already called by kthread_create_on_cpu().
We are going to add a new API that will allow to standardize kthreads
and define safe points for termination, freezing, parking, and even
signal handling. It will want to call kthread_create_on_node()
with va_list args.
This patch does only a refactoring and does not modify the existing
behavior.
Signed-off-by: Petr Mladek <pmladek@suse.cz>
---
kernel/kthread.c | 71 +++++++++++++++++++++++++++++++++-----------------------
1 file changed, 42 insertions(+), 29 deletions(-)
diff --git a/kernel/kthread.c b/kernel/kthread.c
index 10e489c448fe..fca7cd124512 100644
--- a/kernel/kthread.c
+++ b/kernel/kthread.c
@@ -242,32 +242,10 @@ static void create_kthread(struct kthread_create_info *create)
}
}
-/**
- * kthread_create_on_node - create a kthread.
- * @threadfn: the function to run until signal_pending(current).
- * @data: data ptr for @threadfn.
- * @node: memory node number.
- * @namefmt: printf-style name for the thread.
- *
- * Description: This helper function creates and names a kernel
- * thread. The thread will be stopped: use wake_up_process() to start
- * it. See also kthread_run().
- *
- * If thread is going to be bound on a particular cpu, give its node
- * in @node, to get NUMA affinity for kthread stack, or else give -1.
- * When woken, the thread will run @threadfn() with @data as its
- * argument. @threadfn() can either call do_exit() directly if it is a
- * standalone thread for which no one will call kthread_stop(), or
- * return when 'kthread_should_stop()' is true (which means
- * kthread_stop() has been called). The return value should be zero
- * or a negative error number; it will be passed to kthread_stop().
- *
- * Returns a task_struct or ERR_PTR(-ENOMEM) or ERR_PTR(-EINTR).
- */
-struct task_struct *kthread_create_on_node(int (*threadfn)(void *data),
- void *data, int node,
- const char namefmt[],
- ...)
+static struct task_struct *__kthread_create_on_node(int (*threadfn)(void *data),
+ void *data, int node,
+ const char namefmt[],
+ va_list args)
{
DECLARE_COMPLETION_ONSTACK(done);
struct task_struct *task;
@@ -308,11 +286,8 @@ struct task_struct *kthread_create_on_node(int (*threadfn)(void *data),
task = create->result;
if (!IS_ERR(task)) {
static const struct sched_param param = { .sched_priority = 0 };
- va_list args;
- va_start(args, namefmt);
vsnprintf(task->comm, sizeof(task->comm), namefmt, args);
- va_end(args);
/*
* root may have changed our (kthreadd's) priority or CPU mask.
* The kernel thread should not inherit these properties.
@@ -323,6 +298,44 @@ struct task_struct *kthread_create_on_node(int (*threadfn)(void *data),
kfree(create);
return task;
}
+
+
+/**
+ * kthread_create_on_node - create a kthread.
+ * @threadfn: the function to run until signal_pending(current).
+ * @data: data ptr for @threadfn.
+ * @node: memory node number.
+ * @namefmt: printf-style name for the thread.
+ *
+ * Description: This helper function creates and names a kernel
+ * thread. The thread will be stopped: use wake_up_process() to start
+ * it. See also kthread_run().
+ *
+ * If thread is going to be bound on a particular cpu, give its node
+ * in @node, to get NUMA affinity for kthread stack, or else give -1.
+ * When woken, the thread will run @threadfn() with @data as its
+ * argument. @threadfn() can either call do_exit() directly if it is a
+ * standalone thread for which no one will call kthread_stop(), or
+ * return when 'kthread_should_stop()' is true (which means
+ * kthread_stop() has been called). The return value should be zero
+ * or a negative error number; it will be passed to kthread_stop().
+ *
+ * Returns a task_struct or ERR_PTR(-ENOMEM) or ERR_PTR(-EINTR).
+ */
+struct task_struct *kthread_create_on_node(int (*threadfn)(void *data),
+ void *data, int node,
+ const char namefmt[],
+ ...)
+{
+ struct task_struct *task;
+ va_list args;
+
+ va_start(args, namefmt);
+ task = __kthread_create_on_node(threadfn, data, node, namefmt, args);
+ va_end(args);
+
+ return task;
+}
EXPORT_SYMBOL(kthread_create_on_node);
static void __kthread_bind(struct task_struct *p, unsigned int cpu, long state)
--
1.8.5.6
^ permalink raw reply related
* [RFC PATCH 00/18] kthreads/signal: Safer kthread API and signal handling
From: Petr Mladek @ 2015-06-05 15:00 UTC (permalink / raw)
To: Andrew Morton, Oleg Nesterov, Tejun Heo, Ingo Molnar,
Peter Zijlstra
Cc: Richard Weinberger, Steven Rostedt, David Woodhouse,
linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Trond Myklebust,
Anna Schumaker, linux-nfs-u79uwXL29TY76Z2rM5mHXA, Chris Mason,
Paul E. McKenney, Thomas Gleixner, Linus Torvalds, Jiri Kosina,
Borislav Petkov, Michal Hocko,
live-patching-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Petr Mladek
Kthreads are implemented as an infinite loop. They include check points
for termination, freezer, parking, and even signal handling.
We need to touch all kthreads every time we want to add or
modify the behavior of such checkpoints. It is not easy because
there are several hundreds of kthreads and each of them is
implemented in a slightly different way.
This anarchy brings potentially broken or non-standard behavior.
For example, few kthreads already handle signals a strange way.
This patchset is a _proof-of-concept_ how to improve the situation.
The goal is:
+ enforce cleaner and better maintainable kthreads implementation
using a new API
+ standardize signal handling in kthreads
+ hopefully solve some existing problems, e.g. with suspend
Why new API?
First, I do not want to add yet another API that would need
to be supported. The aim is to _replace_ the current API.
Well, the old API would need to stay around for some time until
all kthreads are converted.
Second, there are two more existing alternatives. They fulfill
the needs and can be used for some conversions. But IMHO, they
are not well usable in all cases. Let's talk more about them.
Workqueue
Workqueues are quite popular and many kthreads have already been
converted into them.
Work queues allow to split the function into even more pieces and
reach the common check point more often. It is especially useful
when a kthread handles more tasks and is woken when some work
is needed. Then we could queue the appropriate work instead
of waking the whole kthread and checking what exactly needs
to be done.
But there are many kthreads that need to cycle many times
until some work is finished, e.g. khugepaged, virtio_balloon,
jffs2_garbage_collect_thread. They would need to queue the
work item repeatedly from the same work item or between
more work items. It would be a strange semantic.
Work queues allow to share the same kthread between more users.
It helps to reduce the number of running kthreads. It is especially
useful if you would need a kthread for each CPU.
But this might also be a disadvantage. Just look into the output
of the command "ps" and see the many [kworker*] processes. One
might see this a black hole. If a kworker makes the system busy,
it is less obvious what the problem is in compare with the old
"simple" and dedicated kthreads.
Yes, we could add some debugging tools for work queues but
it would be another non-standard thing that developers and
system administrators would need to understand.
Another thing is that work queues have their own scheduler. If we
move even more tasks there it might need even more love. Anyway,
the extra scheduler adds another level of complexity when
debugging problems.
kthread_worker
kthread_worker is similar to workqueues in many ways. You need to
+ define work functions
+ define and initialize work structs
+ queue work items (structs pointing to the functions and data)
We could repeat the paragraphs about splitting the work
and sharing the kthread between more users here.
Well, the kthread_worker implementation is much simpler than
the one for workqueues. It is more similar to a simple
kthread. Especially, it uses the system scheduler.
But it is still more complex that the simple kthread.
One interesting thing is that kthread_workers add internal work
items into the queue. They typically use a completion. An example
is the flush work. see flush_kthread_work(). It is a nice trick
but you need to be careful. For example, if you would want to
terminate the kthread, you might want to remove some work item
from the queue, especially if you need to break a work item that
is called in a cycle (queues itself). The question is what to do
with the internal tasks. If you keep them, they might wake up
sleepers when the work was not really completed. If you remove
them, the counter part might sleep forever.
Conclusion
I think that we still want some rather simple API for kthreads
but it need to be more enforcing that the current simple one.
Content
This patchset is split the following way:
+ 2nd patch: defines basic structure of a new kthread API that
allows to get most of the checks into a single place
+ 6th patch: proposal of signal handling in kthreads
+ 7th patch: makes kthreads using the new API freezable by default
+ 9th, 16th patches: proposal how to maintain sleeping between
kthread iterations on a single place
+ 10th, 11th, 12th, 17th, 18th patches: show how the new API
could be used in some kthreads and hopefully clean them
a bit
+ the other patches add some helper functions or do some
related clean up
The patchset touches many areas: kthreads, scheduler, signal handling,
freezer, parking, many subsystems and drivers are using kthreads. This
is why I added so many people into CC.
The patch set can be applied against current Linus' tree for 4.1.0-rc6.
Petr Mladek (18):
kthread: Allow to call __kthread_create_on_node() with va_list args
kthread: Add API for iterant kthreads
kthread: Add kthread_stop_current()
signal: Rename kernel_sigaction() to kthread_sigaction() and clean it
up
freezer/scheduler: Add freezable_cond_resched()
signal/kthread: Initial implementation of kthread signal handling
kthread: Make iterant kthreads freezable by default
kthread: Allow to get struct kthread_iterant from task_struct
kthread: Make it easier to correctly sleep in iterant kthreads
jffs2: Remove forward definition of jffs2_garbage_collect_thread()
jffs2: Convert jffs2_gcd_mtd kthread into the iterant API
lockd: Convert the central lockd service to kthread_iterant API
ring_buffer: Use iterant kthreads API in the ring buffer benchmark
ring_buffer: Allow to cleanly freeze the ring buffer benchmark
kthreads
ring_buffer: Allow to exit the ring buffer benchmark immediately
kthread: Support interruptible sleep with a timeout by iterant
kthreads
ring_buffer: Use the new API for a sleep with a timeout in the
benchmark
jffs2: Use the new API for a sleep with a timeout
fs/jffs2/background.c | 178 ++++++++++------------
fs/lockd/svc.c | 80 +++++-----
include/linux/freezer.h | 8 +
include/linux/kthread.h | 67 ++++++++
include/linux/signal.h | 24 ++-
include/linux/sunrpc/svc.h | 2 +
kernel/kmod.c | 2 +-
kernel/kthread.c | 286 +++++++++++++++++++++++++++++++----
kernel/signal.c | 84 +++++++++-
kernel/trace/ring_buffer_benchmark.c | 110 +++++++-------
10 files changed, 611 insertions(+), 230 deletions(-)
--
1.8.5.6
^ permalink raw reply
* [PATCH V2 1/1] iio: Add iio_mod_light_uva, iio_mod_light_uvb, and iio_mod_light_uvc.
From: Kevin Tsai @ 2015-06-05 1:47 UTC (permalink / raw)
To: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald, Irina Tirdea, Daniel Baluta, Haneen Mohammed,
Darshana Padmadas, Roberta Dobrescu, Vlad Dogaru, Reyad Attiyat,
Octavian Purdila, Martin Fuzzey, Srinivas Pandruvada, Kevin Tsai
Cc: linux-iio, linux-kernel, linux-api
Add Ultraviolet(UV) support:
UVA: 315 ~ 400 nm
UVB: 280 ~ 315 nm
UVC: 100 ~ 280 nm
/sys/bus/iio/devices/iio:deviceX/in_intensity_uva_input
/sys/bus/iio/devices/iio:deviceX/in_intensity_uvb_input
/sys/bus/iio/devices/iio:deviceX/in_intensity_uvc_input
The SI unit is "uW/mm^2".
/sys/bus/iio/devices/iio:deviceX/in_intensity_uva_min_wavelength
/sys/bus/iio/devices/iio:deviceX/in_intensity_uva_max_wavelength
/sys/bus/iio/devices/iio:deviceX/in_intensity_uvb_min_wavelength
/sys/bus/iio/devices/iio:deviceX/in_intensity_uvb_max_wavelength
/sys/bus/iio/devices/iio:deviceX/in_intensity_uvc_min_wavelength
/sys/bus/iio/devices/iio:deviceX/in_intensity_uvc_max_wavelength
The SI unit is "nm".
Signed-off-by: Kevin Tsai <ktsai@capellamicro.com>
---
Documentation/ABI/testing/sysfs-bus-iio | 23 +++++++++++++++++++++++
drivers/iio/industrialio-core.c | 3 +++
include/uapi/linux/iio/types.h | 3 +++
tools/iio/iio_event_monitor.c | 6 ++++++
4 files changed, 35 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
index bbed111..2960142 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio
+++ b/Documentation/ABI/testing/sysfs-bus-iio
@@ -1463,3 +1463,26 @@ Description:
measurements and return the average value as output data. Each
value resulted from <type>[_name]_oversampling_ratio measurements
is considered as one sample for <type>[_name]_sampling_frequency.
+
+What: /sys/.../iio:deviceX/in_intensity_uva[_input|_raw]
+What: /sys/.../iio:deviceX/in_intensity_uvb[_input|_raw]
+What: /sys/.../iio:deviceX/in_intensity_uvc[_input|_raw]
+KernelVersion: 4.1.1
+Description:
+ uva is the ultraviolet wavelength from 315nm to 400nm.
+ uvb is the ultraviolet wavelength from 280nm to 315nm.
+ uvc is the ultraviolet wavelength from 100nm to 280nm.
+ The SI unit is "uW/mm^2". Otherwise it should include _raw.
+
+What: /sys/.../iio:deviceX/in_intensity_uva_min_wavelength
+What: /sys/.../iio:deviceX/in_intensity_uva_max_wavelength
+What: /sys/.../iio:deviceX/in_intensity_uvb_min_wavelength
+What: /sys/.../iio:deviceX/in_intensity_uvb_max_wavelength
+What: /sys/.../iio:deviceX/in_intensity_uvc_min_wavelength
+What: /sys/.../iio:deviceX/in_intensity_uvc_max_wavelength
+KernelVersion: 4.1.1
+Description:
+ Sensor may not cover the whole uva/uvb/uvc channel. Add the
+ min/max wavelength properties to descript sensor range.
+ The SI unit is "nm".
+
diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index 3524b0d..ecbdd1e 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -89,6 +89,9 @@ static const char * const iio_modifier_names[] = {
[IIO_MOD_LIGHT_RED] = "red",
[IIO_MOD_LIGHT_GREEN] = "green",
[IIO_MOD_LIGHT_BLUE] = "blue",
+ [IIO_MOD_LIGHT_UVA] = "uva",
+ [IIO_MOD_LIGHT_UVB] = "uvb",
+ [IIO_MOD_LIGHT_UVC] = "uvc",
[IIO_MOD_QUATERNION] = "quaternion",
[IIO_MOD_TEMP_AMBIENT] = "ambient",
[IIO_MOD_TEMP_OBJECT] = "object",
diff --git a/include/uapi/linux/iio/types.h b/include/uapi/linux/iio/types.h
index 2f8b117..5c99090 100644
--- a/include/uapi/linux/iio/types.h
+++ b/include/uapi/linux/iio/types.h
@@ -72,6 +72,9 @@ enum iio_modifier {
IIO_MOD_ROOT_SUM_SQUARED_X_Y_Z,
IIO_MOD_I,
IIO_MOD_Q,
+ IIO_MOD_LIGHT_UVA,
+ IIO_MOD_LIGHT_UVB,
+ IIO_MOD_LIGHT_UVC,
};
enum iio_event_type {
diff --git a/tools/iio/iio_event_monitor.c b/tools/iio/iio_event_monitor.c
index 427c271..46dd27f 100644
--- a/tools/iio/iio_event_monitor.c
+++ b/tools/iio/iio_event_monitor.c
@@ -88,6 +88,9 @@ static const char * const iio_modifier_names[] = {
[IIO_MOD_LIGHT_RED] = "red",
[IIO_MOD_LIGHT_GREEN] = "green",
[IIO_MOD_LIGHT_BLUE] = "blue",
+ [IIO_MOD_LIGHT_UVA] = "uva",
+ [IIO_MOD_LIGHT_UVB] = "uvb",
+ [IIO_MOD_LIGHT_UVC] = "uvc",
[IIO_MOD_QUATERNION] = "quaternion",
[IIO_MOD_TEMP_AMBIENT] = "ambient",
[IIO_MOD_TEMP_OBJECT] = "object",
@@ -156,6 +159,9 @@ static bool event_is_known(struct iio_event_data *event)
case IIO_MOD_LIGHT_RED:
case IIO_MOD_LIGHT_GREEN:
case IIO_MOD_LIGHT_BLUE:
+ case IIO_MOD_LIGHT_UVA:
+ case IIO_MOD_LIGHT_UVB:
+ case IIO_MOD_LIGHT_UVC:
case IIO_MOD_QUATERNION:
case IIO_MOD_TEMP_AMBIENT:
case IIO_MOD_TEMP_OBJECT:
--
1.8.3.1
^ permalink raw reply related
* Re: [CFT][PATCH 11/10] mnt: Avoid unnecessary regressions in fs_fully_visible
From: Andy Lutomirski @ 2015-06-05 0:46 UTC (permalink / raw)
To: Eric W. Biederman
Cc: Kenton Varda, Serge Hallyn, Seth Forshee, Linux API,
Linux Containers, Greg Kroah-Hartman, Michael Kerrisk-manpages,
Richard Weinberger, Linux FS Devel, Tejun Heo
In-Reply-To: <87eglseboh.fsf_-_-JOvCrm2gF+uungPnsOpG7nhyD016LWXt@public.gmane.org>
On Wed, Jun 3, 2015 at 2:15 PM, Eric W. Biederman <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org> wrote:
>
> Not allowing programs to clear nosuid, nodev, and noexec on new mounts
> of sysfs or proc will cause lxc and libvirt-lxc to fail to start (a
> regression). There are no device nodes or executables on sysfs or
> proc today which means clearing these flags is harmless today.
>
> Instead of failing the fresh mounts of sysfs and proc emit a warning
> when these flags are improprely cleared. We only reach this point
> because lxc and libvirt-lxc clear flags they mount flags had not
> intended to.
>
> In a couple of kernel releases when lxc and libvirt-lxc have been
> fixed we can start failing fresh mounts proc and sysfs that clear
> nosuid, nodev and noexec. Userspace clearly means to enforce those
> attributes and historically they have avoided bugs.
At the very least, I think this should be folded in so that the ABI
doesn't break in the middle of the series.
--Andy
^ permalink raw reply
* Re: [PATCH v3 2/2] capabilities: Add a securebit to disable PR_CAP_AMBIENT_RAISE
From: Serge E. Hallyn @ 2015-06-04 22:03 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Serge Hallyn, Andrew Morton, James Morris, Jarkko Sakkinen,
Ted Ts'o, Andrew G. Morgan, Linux API, Mimi Zohar,
Michael Kerrisk, Austin S Hemmelgarn, linux-security-module,
Aaron Jones, Serge Hallyn, LKML, Markku Savela, Kees Cook,
Jonathan Corbet, Christoph Lameter, Andy Lutomirski
In-Reply-To: <8b3a8b2fa031e043483f2f444f0b51aed0681e85.1432770087.git.luto@kernel.org>
On Wed, May 27, 2015 at 04:47:59PM -0700, Andy Lutomirski wrote:
> Per Andrew Morgan's request, add a securebit to allow admins to
> disable PR_CAP_AMBIENT_RAISE. This securebit will prevent processes
> from adding capabilities to their ambient set.
>
> For simplicity, this disables PR_CAP_AMBIENT_RAISE entirely rather
> than just disabling setting previously cleared bits.
>
> Acked-By: Andrew G. Morgan <morgan@kernel.org>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Christoph Lameter <cl@linux.com>
> Cc: Serge Hallyn <serge.hallyn@canonical.com>
Acked-by: Serge Hallyn <serge.hallyn@canonical.com>
> Cc: Andy Lutomirski <luto@amacapital.net>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: Aaron Jones <aaronmdjones@gmail.com>
> CC: Ted Ts'o <tytso@mit.edu>
> Cc: linux-security-module@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-api@vger.kernel.org
> Cc: akpm@linuxfoundation.org
> Cc: Andrew G. Morgan <morgan@kernel.org>
> Cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
> Cc: Austin S Hemmelgarn <ahferroin7@gmail.com>
> Cc: Markku Savela <msa@moth.iki.fi>
> Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> Cc: Michael Kerrisk <mtk.manpages@gmail.com>
> Signed-off-by: Andy Lutomirski <luto@kernel.org>
> ---
> include/uapi/linux/securebits.h | 11 ++++++++++-
> security/commoncap.c | 3 ++-
> 2 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/include/uapi/linux/securebits.h b/include/uapi/linux/securebits.h
> index 985aac9e6bf8..35ac35cef217 100644
> --- a/include/uapi/linux/securebits.h
> +++ b/include/uapi/linux/securebits.h
> @@ -43,9 +43,18 @@
> #define SECBIT_KEEP_CAPS (issecure_mask(SECURE_KEEP_CAPS))
> #define SECBIT_KEEP_CAPS_LOCKED (issecure_mask(SECURE_KEEP_CAPS_LOCKED))
>
> +/* When set, a process cannot add new capabilities to its ambient set. */
> +#define SECURE_NO_CAP_AMBIENT_RAISE 6
> +#define SECURE_NO_CAP_AMBIENT_RAISE_LOCKED 7 /* make bit-6 immutable */
> +
> +#define SECBIT_NO_CAP_AMBIENT_RAISE (issecure_mask(SECURE_NO_CAP_AMBIENT_RAISE))
> +#define SECBIT_NO_CAP_AMBIENT_RAISE_LOCKED \
> + (issecure_mask(SECURE_NO_CAP_AMBIENT_RAISE_LOCKED))
> +
> #define SECURE_ALL_BITS (issecure_mask(SECURE_NOROOT) | \
> issecure_mask(SECURE_NO_SETUID_FIXUP) | \
> - issecure_mask(SECURE_KEEP_CAPS))
> + issecure_mask(SECURE_KEEP_CAPS) | \
> + issecure_mask(SECURE_NO_CAP_AMBIENT_RAISE))
> #define SECURE_ALL_LOCKS (SECURE_ALL_BITS << 1)
>
> #endif /* _UAPI_LINUX_SECUREBITS_H */
> diff --git a/security/commoncap.c b/security/commoncap.c
> index 835a7584f7ea..22b7b91c5eae 100644
> --- a/security/commoncap.c
> +++ b/security/commoncap.c
> @@ -987,7 +987,8 @@ int cap_task_prctl(int option, unsigned long arg2, unsigned long arg3,
> if (arg2 == PR_CAP_AMBIENT_RAISE &&
> (!cap_raised(current_cred()->cap_permitted, arg3) ||
> !cap_raised(current_cred()->cap_inheritable,
> - arg3)))
> + arg3) ||
> + issecure(SECURE_NO_CAP_AMBIENT_RAISE)))
> return -EPERM;
>
> new = prepare_creds();
> --
> 2.1.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply
* Re: [PATCH v2] seccomp: add ptrace options for suspend/resume
From: Tycho Andersen @ 2015-06-04 21:05 UTC (permalink / raw)
To: Oleg Nesterov
Cc: linux-kernel, linux-api, Kees Cook, Andy Lutomirski, Will Drewry,
Roland McGrath, Pavel Emelyanov, Serge E. Hallyn
In-Reply-To: <20150604183149.GA560@redhat.com>
Hi Oleg,
On Thu, Jun 04, 2015 at 08:31:49PM +0200, Oleg Nesterov wrote:
> On 06/03, Tycho Andersen wrote:
> >
> > @@ -556,6 +557,11 @@ static int ptrace_setoptions(struct task_struct *child, unsigned long data)
> > if (data & ~(unsigned long)PTRACE_O_MASK)
> > return -EINVAL;
> >
> > +#ifdef CONFIG_CHECKPOINT_RESTORE
> > + if (data & PTRACE_O_SUSPEND_SECCOMP && !may_suspend_seccomp())
> > + return -EPERM;
> > +#endif
> > +
>
> Well. This -EPERM doesn't look consistent...
>
> if config_enabled(CONFIG_CHECKPOINT_RESTORE) == F, we return success
> but PTRACE_O_SUSPEND_SECCOMP has no effect because of another ifdef in
> seccomp.
>
> OTOH, if CONFIG_SECCOMP=n, this option has no effect too but we return
> -EPERM even.
Yes, something like:
if (unlikely(data & PTRACE_O_SUSPEND_SECCOMP)) {
if (!config_enabled(CONFIG_CHECKPOINT_RESTORE) ||
!config_enabled(CONFIG_SECCOMP))
return -EINVAL;
if (!may_suspend_seccomp())
return -EPERM;
}
I guess.
> Also. Suppose that the tracer sets SUSPEND_SECCOMP and then drops
> CAP_SYS_ADMIN. After that it can't set or clear other ptrace options.
Is this a case we're concerned about? I think this should be ok (i.e.
"don't do that" :).
> So if we really want the security checks (I still think we do not ;)
> then we should probably check "flags & SUSPEND_SECCOMP" as well.
Good point.
> > +#ifdef CONFIG_CHECKPOINT_RESTORE
> > +bool may_suspend_seccomp(void)
> > +{
> > + if (!capable(CAP_SYS_ADMIN))
> > + return false;
> > +
> > + if (current->seccomp.mode != SECCOMP_MODE_DISABLED)
> > + return false;
>
> Heh. OK, I won't argue with the new check too ;)
Actually now that I think about it I agree with you, these checks
don't seem necessary. Even inside a user namespace, if you can ptrace
a process you can make it do whatever you want irrespective of
seccomp, as long as it has the necessary capabilities. Once the
seccomp checks are run after ptrace, they'll be enforced so you
couldn't have it call whatever you want in the first place.
Still, perhaps I'm missing something...
Tycho
^ permalink raw reply
* Re: [PATCH v3 0/2] capabilities: Ambient capability patchset
From: Andy Lutomirski @ 2015-06-04 19:33 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Serge Hallyn, Andrew Morton, James Morris, Jarkko Sakkinen,
Ted Ts'o, Andrew G. Morgan, Linux API, Mimi Zohar,
Michael Kerrisk, Austin S Hemmelgarn, linux-security-module,
Aaron Jones, Serge Hallyn, LKML, Markku Savela, Kees Cook,
Jonathan Corbet
In-Reply-To: <cover.1432770087.git.luto-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
On Wed, May 27, 2015 at 4:47 PM, Andy Lutomirski <luto-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> This adds ambient capabilities. See the individual patch changelogs
> for details.
>
> Preliminary userspace code is here:
>
> https://git.kernel.org/cgit/linux/kernel/git/luto/util-linux-playground.git/commit/?h=cap_ambient&id=7f5afbd175d2
>
> James or akpm, I think this is ready for 4.2. Can one of you queue it up?
akpm / James: Ping? Do I need more acks?
--Andy
>
> Thanks,
> Andy
>
> Changes from v2:
> - Improve the patch 1 changelog a bit.
> - Add acks.
> - Add comment clarifying the pE' rule.
> - s/except/expect
>
> Changes from v1:
> - Lots of cleanups to the ambient cap code.
> - The securebit is new.
>
> Andy Lutomirski (2):
> capabilities: Ambient capabilities
> capabilities: Add a securebit to disable PR_CAP_AMBIENT_RAISE
>
> fs/proc/array.c | 5 ++-
> include/linux/cred.h | 8 ++++
> include/uapi/linux/prctl.h | 6 +++
> include/uapi/linux/securebits.h | 11 ++++-
> kernel/user_namespace.c | 1 +
> security/commoncap.c | 92 ++++++++++++++++++++++++++++++++++++-----
> security/keys/process_keys.c | 1 +
> 7 files changed, 112 insertions(+), 12 deletions(-)
>
> --
> 2.1.0
>
--
Andy Lutomirski
AMA Capital Management, LLC
^ permalink raw reply
* Re: [PATCH] tools, add tools/testing/selftests/timers/.gitignore
From: Shuah Khan @ 2015-06-04 19:28 UTC (permalink / raw)
To: Prarit Bhargava, linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: John Stultz, Thomas Gleixner, linux-api-u79uwXL29TY76Z2rM5mHXA,
Shuah Khan
In-Reply-To: <1433440392-31274-1-git-send-email-prarit-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
On 06/04/2015 11:53 AM, Prarit Bhargava wrote:
> After building the tests in the timers directory each executable shows up
> in 'git-status' as a new file. This patch adds a .gitignore file to the
> directory.
>
> Cc: John Stultz <john.stultz-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Cc: Thomas Gleixner <tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
> Cc: Shuah Khan <shuahkh-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
> Cc: linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Signed-off-by: Prarit Bhargava <prarit-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
> tools/testing/selftests/timers/.gitignore | 3 +++
> 1 file changed, 3 insertions(+)
> create mode 100644 tools/testing/selftests/timers/.gitignore
>
> diff --git a/tools/testing/selftests/timers/.gitignore b/tools/testing/selftests/timers/.gitignore
> new file mode 100644
> index 0000000..afe9743
> --- /dev/null
> +++ b/tools/testing/selftests/timers/.gitignore
> @@ -0,0 +1,3 @@
> +*
> +!*.c
> +!.gitignore
>
Zhang Zhen sent in a patch to fix this and it is queued for 4.2
in linux-kselftest next.
thanks,
-- Shuah
--
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America (Silicon Valley)
shuahkh-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org | (970) 217-8978
^ permalink raw reply
* Re: [Intel-gfx] [RFC PATCH 00/11] drm/i915: Expose OA metrics via perf PMU
From: Robert Bragg @ 2015-06-04 18:53 UTC (permalink / raw)
To: Peter Zijlstra
Cc: intel-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Daniel Vetter,
Jani Nikula, David Airlie, Paul Mackerras, Ingo Molnar,
Arnaldo Carvalho de Melo, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20150527153914.GC3644-ndre7Fmf5hadTX5a5knrm8zTDFooKrT+cvkQGrU6aU0@public.gmane.org>
On Wed, May 27, 2015 at 4:39 PM, <> wrote:
> On Thu, May 21, 2015 at 12:17:48AM +0100, Robert Bragg wrote:
>> >
>> > So for me the 'natural' way to represent this in perf would be through
>> > event groups. Create a perf event for every single event -- yes this is
>> > 53 events.
>>
>> So when I was first looking at this work I had considered the
>> possibility of separate events, and these are some of the things that
>> in the end made me forward the hardware's raw report data via a single
>> event instead...
>>
>> There are 100s of possible B counters depending on the MUX
>> configuration + fixed-function logic in addition to the A counters.
>
> There's only 8 B counters, what there is 100s of is configurations, and
> that's no different from any other PMU. There's 100s of events to select
> on the CPU side as well - yet only 4 (general purpose) counters (8 if
> you disable HT on recent machines) per CPU.
To clarify one thing here; In the PRM you'll see a reference to
'Reserved' slots in the report layouts and these effectively
correspond to a further 8 configurable counters. These further
counters get referred to as 'C' counters for 'custom' and similar to
the B counters they are defined based on the MUX configuration except
they skip the boolean logic pipeline.
Thanks for the comparison with the CPU.
My main thought here is to beware of considering these configurable
counters as 'general purpose' if that might also imply orthogonality.
All our configurable counters build on top of a common/shared MUX
configuration so I'd tend to think of them more like a 'general
purpose counter set'.
>
> Of course, you could create more than 8 events at any one time, and then
> perf will round-robin the configuration for you.
This hardware really isn't designed to allow round-robin
reconfiguration. If we change the metric set then we will loose the
aggregate value of our B counters. Besides writing the large MUX
config + boolean logic state we would also need to save the current
aggregated counter values only maintained by the OA unit so they could
be restored later as well as any internal state that's simply not
accessible to us. If we reconfigure to try and round-robin between
sets of counters each reconfiguration will trash state that we have no
way of restoring. The counters have to no longer be in use if there's
no going back once we reconfigure.
>
>> A
>> choice would need to be made about whether to expose events for the
>> configurable counters that aren't inherently associated with any
>> semantics, or instead defining events for counters with specific
>> semantics (with 100s of possible counters to define). The later would
>> seem more complex for userspace and the kernel if they both now have
>> to understand the constraints on what counters can be used together.
>
> Again, no different from existing PMUs. The most 'interesting' part of
> any PMU driver is event scheduling.
>
> The explicit design of perf was to put event scheduling _in_ the kernel
> and not allow userspace direct access to the hardware (as previous PMU
> models did).
I'm thinking your 'event scheduling' here refers to the kernel being
able to handle multiple concurrent users of the same pmu, but then we
may be talking cross purposes...
I was describing how (if we exposed events for counters with well
defined semantics - as opposed to just 16 events for the B/C counters)
the driver would need to do some kind of matching based on the events
added to a group to deduce which MUX configuration should be used and
saying that in this case userspace would need to be very aware of
which events belong to a specific MUX configuration.
I don't think we have the hardware flexibility to support scheduling
in terms of multiple concurrent users, unless they happen to want
exactly the same configuration.
I also don't think we have a use case for accessing gpu metrics where
concurrent access is really important. It's possible to think of cases
where it might be nice, except they are unlikely to involve the same
configuration so given the current OA unit design I haven't had plans
to try and support concurrent access and haven't had any requests for
it so far.
>
>> I
>> guess with either approach we would also need to have some form of
>> dedicated group leader event accepting attributes for configuring the
>> state that affects the group as a whole, such as the counter
>> configuration (3D vs GPGPU vs media etc).
>
> That depends a bit on how flexible you want to switch between these
> modes; with the cost being in the 100s of MMIO register writes, it might
> just not make sense to make this too dynamic.
>
> An option might be to select the mode through the PMU driver's sysfs
> files. Another option might be to expose the B counters 3 times and have
> each set be mutually exclusive. That is, as soon as you've created a 3D
> event, you can no longer create GPGPU/Media events.
Hmm, interesting idea to have global/pmu state be controlled via sysfs...
I suppose the security model doesn't seem as clear, with sysfs access
gated by file permissions while we want to gate some control based on
whether you have access to a gpu context. The risk of multiple users
looking to open a group of events and first setting a global mode via
sysfs seems like it could also silently misconfigure your events with
the wrong mode. Maybe it could become immutable once one event is
opened and you could double check afterwards.
Besides the mode/MUX config choice, other state that is shared by the
group includes:
- single context vs system wide profiling
- the timer exponent
Using sysfs I think we'd have to be careful to not expose
configuration options that relate to the security policy if there's a
risk of a non-privileged process affecting it.
>
>> I'm not sure where we would
>> handle the context-id + drm file descriptor attributes for initiating
>> single context profiling but guess we'd need to authenticate each
>> individual event open.
>
> Event open is a slow path, so that should not be a problem, right?
Yeah I don't think I'd be concerned about duplicating the checks
per-event from a performance pov.
I'm more concerned about differentiating security checks that relate
to the final configuration as a whole vs individual events. In this
case we use a drm file descriptor + context handle to allow profiling
a single gpu context that the current process has access to, but the
choice of profiling a single context or across all contexts affects
the final hardware configuration too which relates to the group. Even
if a process has the privileges to additionally open an event for
system wide metrics, this choice is part of the OA unit configuration
and incompatible with single-context filtering. For reference, on
Haswell the reports don't include a context identifier which makes it
difficult to try and emulate a single-context filtering event with a
system wide hardware configuration.
I can see that we'd be able to cross-check event compatibility for
each addition to the group but I also can't help but see this as an
example of how tightly coupled the configuration of these counters
are.
>
>> It's not clear if we'd configure the report
>> layout via the group leader, or try to automatically choose the most
>> compact format based on the group members. I'm not sure how pmus
>> currently handle the opening of enabled events on an enabled group but
>
> With the PERF_FORMAT_GROUP layout changing in-flight. I would recommend
> not doing that -- decoding the output will be 'interesting' but not
> impossible.
Right, I wouldn't want to have to handle such corner cases so I think
I'd want to be able to report an error back to userspace attempting to
extend a group that's ever been activated.
>
>> I think there would need to be limitations in our case that new
>> members can't result in a reconfigure of the counters if that might
>> loose the current counter values known to userspace.
>
> I'm not entirely sure what you mean here.
This is referring to the issue of us not being able to explicitly save
and restore the state of the OA unit to be able to allow reconfiguring
the counters while they are in use.
The counter values for B/C counters are only maintained by the OA unit
and in the case of B counters which supports e.g. referencing delayed
values, there is state held in the unit that we have no way to
explicitly access and save.
>
>> From a user's pov, there's no real freedom to mix and match which
>> counters are configured together, and there's only some limited
>> ability to ignore some of the currently selected counters by not
>> including them in reports.
>
> It is 'impossible' to create a group that is not programmable. That is,
> the pmu::event_init() call _should_ verify that the addition of the
> event to the group (as given by event->group_leader) is valid.
>
>> Something to understand here is that we have to work with sets of
>> pre-defined MUX + fixed-function logic configurations that have been
>> validated to give useful metrics for specific use cases, such as
>> benchmarking 3D rendering, GPGPU or media workloads.
>
> This is fine; as stated above. Since these are limited pieces of
> 'firmware' which are expensive to load, you don't have to do a fully
> dynamic solution here.
>
>> As it is currently the kernel doesn't need to know anything about the
>> semantics of individual counters being selected, so it's currently
>> convenient that we can aim to maintain all the counter meta data we
>> have in userspace according to the changing needs of tools or drivers
>> (e.g. names, descriptions, units, max values, normalization
>> equations), de-coupled from the kernel, instead of splitting it
>> between the kernel and userspace.
>
> And that fully violates the design premise of perf. The kernel should be
> in control of resources, not userspace.
This isn't referring to programmable resources though; it's only
talking about where the higher-level meta data about these counters is
maintained, including names and descriptions detailing the specific
counter semantics as well as a description of the
equations/expressions that should be used to normalize these counters
to be useful to users.
This seems very comparable to other PMUs that expose _RAW, hardware
specific data via samples that rely on a userspace library like
libpfm4 to understand.
I see different tools have slightly different needs and may want to
normalize some counters differently. This data is still evolving over
time, and overall feel it's going to be more practical/maintainable
for us to try and keep this semantic data consolidated to userspace.
For reference; what I'm really referring to here is an established XML
description of OA counters maintained and validated by engineers
closely involved with the hardware. I think it makes sense for us to
factor into this trade-off the benefit we get from being able to
easily leverage this data but considering the relative lack of
stability of this data makes me prefer to limit ourselves to just
putting the MUX and boolean logic configurations in the kernel.
>
> If we'd have put userspace in charge, we could now not profile the same
> task by two (or more) different observers. But with kernel side counter
> management that is no problem at all.
Sorry, I think my comment must have come across wrong because I wasn't
talking about a choice that affects who's responsible for configuring
the counters or supporting multiple observers or not.
I was talking about a trade-off for where we maintain the higher level
information about the counters that can be exposed, especially the
code for normalizing individual counters.
>
>> A benefit of being able to change the report size is to reduce memory
>> bandwidth usage that can skew measurements. It's possible to request
>> the gpu to write out periodic snapshots at a very high frequency (we
>> can program a period as low as 160 nanoseconds) and higher frequencies
>> can start to expose some interesting details about how the gpu is
>> utilized - though with notable observer effects too. How careful we
>> are to not waste bandwidth is expected to determine what sampling
>> resolutions we can achieve before significantly impacting what we are
>> measuring.
>>
>> Splitting the counters up looked like it could increase the bandwidth
>> we use quite a bit. The main difference comes from requiring 64bit
>> values instead of the 32bit values in our raw reports. This can be
>> offset partly since there are quite a few 'reserved'/redundant A
>> counters that don't need forwarding. As an example in the most extreme
>> case, instead of an 8 byte perf_event_header + 4byte raw_size + 256
>> byte reports + 4 byte padding every 160ns ~= 1.5GB/s, we might have 33
>> A counters (ignoring redundant ones) + 16 configurable counters = 400
>
> In that PDF there's only 8 configurable 'B' counters.
Sorry that it's another example of being incomplete. The current
document only says enough about B counters to show how to access the
gpu clock that is required to normalise many of the A counters. There
are also the 8 C counters I mentioned earlier derived from the MUX
configuration but without the boolean logic.
>
>> byte struct read_format (using PERF_FORMAT_GROUP) + 8 byte
>> perf_event_header every 160ns ~= 2.4GB/s. On the other hand though we
>> could choose to forward only 2 or 3 counters of interest at these high
>> frequencies which isn't possible currently.
>
> Right; although you'd have to spend some cpu cycles on the format shift.
> Which might make it moot again. That said; I would really prefer we
> start out with trying to make the generic format stuff work before
> trying to come up with special case hacks.
>
>> > Use the MMIO reads for the regular read() interface, and use a hrtimer
>> > placing MI_REPORT_PERF_COUNT commands, with a counter select mask
>> > covering the all events in the current group, for sampling.
>>
>> Unfortunately due to the mmio limitations and the need to relate
>> counters I can't imagine many use cases for directly accessing the
>> counters individually via the read() interface.
>
> Fair enough.
>
>> MI_REPORT_PERF_COUNT commands are really only intended for collecting
>> reports in sync with a command stream. We are experimenting currently
>> with an extension of my PMU driver that emits MI_REPORT_PERF_COUNT
>> commands automatically around the batches of commands submitted by
>> userspace so we can do a better job of filtering metrics across many
>> gpu contexts, but for now the expectation is that the kernel shouldn't
>> be emitting MI_REPORT_PERF_COUNT commands. We emit
>> MI_REPORT_PERF_COUNT commands within Mesa for example to implement the
>> GL_INTEL_performance_query extension, at the start and end of a query
>> around a sequence of commands that the application is interested in
>> measuring.
>
> I will try and read up on the GL_INTEL_performance_query thing.
You can see my code for this here, in case that's helpful:
https://github.com/rib/mesa/tree/wip/rib/oa-hsw-4.0.0
You might also be interested in my more recent 'codegen' branch too
(work in progress though):
https://github.com/rib/mesa/tree/wip/rib/oa-hsw-codegen
Here you can see an example of the meta data I mentioned earlier, e.g.
describing the equations for normalizing the counters for one metrics
set:
https://github.com/rib/mesa/blob/wip/rib/oa-hsw-codegen/src/mesa/drivers/dri/i965/brw_oa_hsw.xml
This set corresponds to the "3D" metric set enabled in the i915_oa pmu
driver.
More example data can also be seen in my gputop tool, with 'render
basic', 'compute basic', 'compute extended', 'memory reads', 'memory
writes' and 'sampler balance' metric sets:
https://github.com/rib/gputop/blob/641d7644df64a871f36f3c283cfa18a2f1530813/gputop/oa-hsw.xml
>
>> > You can use the perf_event_attr::config to select the counter (A0-A44,
>> > B0-B7) and use perf_event_attr::config1 (low and high dword) for the
>> > corresponding CEC registers.
>> >
>>
>> Hopefully covered above, but since the fixed-function state is so
>> dependent on the MUX configuration I think it currently makes sense to
>> treat the MUX plus logic state (including the CEC state) a tightly
>> coupled unit.
>
> Oh wait, the A counters are also affected by the MUX programming!
> That wasn't clear to me before this.
Er actually you were right before, the raw A counters aren't directly
affected by the MUX configuration. In this paragraph 'fixed-function
state' was referring to the boolean logic state for B counters in case
that was unclear and the CEC ('custom event counter') registers are a
part of that boolean logic state.
That said though, the raw A counters aren't really /useful/ without
the MUX programming because most of them are so closely related to the
gpu clock (which we access via a B or C counter) that we have to
program the MUX before we can normalize the A counters to be
meaningful to users.
>
> Same difference though; you could still do either the sysfs aided flips
> or the mutually exclusive counter types to deal with this; all still
> assuming reprogramming all that is 'expensive'.
>
>> The Flexible EU Counters for Broadwell+ could be more amenable to this
>> kind of independent configuration, as I don't believe they are
>> dependant on the MUX configuration.
>
> That sounds good :-)
>
>> One idea that's come up a lot though is having the possibility of
>> being able to configure an event with a full MUX + fixed-function
>> state description.
>
> Right; seeing how there's only a very limited number of these MUX
> programs validated (3D, GPGPU, Media, any others?) this should be
> doable. And as mentioned before, you could disable the other types once
> you create one in order to limit the reprogramming thing.
Here I meant being able to support userspace supplying a full MUX
configuration and boolean logic configuration (i.e. large arrays of
data), for userspace tools being able to experiment with new
configurations during the early stages of building and validating
these configurations - so mostly interesting to the engineers
responsible for defining and testing these configurations in the first
place since it means they can share experimental configurations with
tools with the caveat that you'd likely need root privileges to use
the interface. It's appealing to have a low barrier for enabling
people to test new configurations without the need for users to build
a custom kernel.
I'm not worried about supporting this in the short term, but it's
still a usecase that could be helpful to support eventually, that I
want to keep in mind at least.
About the number of different configurations, I've been using 3D,
gpgpu and media as summary examples, but there are quite a few
configurations really...
On Haswell these are names of some of the sets I'm aware of...
Render Metrics Basic Gen7.5
Compute Metrics Basic Gen7.5
Compute Metrics Extended Gen7.5
Render Metrics Slice Balance Gen7.5
Memory Reads Distribution Gen7.5
Memory Writes Distribution Gen7.5
Metric set SamplerBalance
Memory Reads on Write Port Distribution Gen7.5
Stencil PMA Hold Metrics Gen7.5
Media Memory Reads Distribution Gen7.5
Media Memory Writes Distribution Gen7.5
Media VME Pipe Gen7.5
For Broadwell we have more; for example...
Render Metrics Basic Gen8
Compute Metrics Basic Gen8
Render Metrics for 3D Pipeline Profile
Memory Reads Distribution Gen8
Memory Writes Distribution Gen8
Compute Metrics Extended Gen8
Compute Metrics L3 Cache Gen8
Data Port Reads Coalescing Gen8
Data Port Writes Coalescing Gen8
Metric set HDCAndSF
Metric set L3_1
Metric set L3_2
Metric set L3_3
Metric set L3_4
Metric set RasterizerAndPixelBackend
Metric set Sampler_1
Metric set Sampler_2
Metric set TDL_1
Metric set TDL_2
Stencil PMA Hold Metrics Gen8
Media Memory Reads Distribution Gen8
Media Memory Writes Distribution Gen8
Media VME Pipe Gen8
HDC URB Coalescing Metrics Gen8
I'm not sure that it will end up making sense to publish all of these,
depending on what kind of testing these have been through to validate
that they give helpful data, and on the other hand these are evolving
and there may be more over time, but hopefully it at least gives a
ballpark idea of the number of configurations that could be
interesting to enable between Haswell and Broadwell.
>
>> >
>> > This does not require random per driver ABI extentions for
>> > perf_event_attr, nor your custom output format.
>> >
>> > Am I missing something obvious here?
>>
>> Definitely nothing 'obvious' since the current documentation is
>> notably incomplete a.t.m, but I don't think we were on the same page
>> about how the hardware works and our use cases.
>>
>> Hopefully some of my above comments help clarify some details.
>
> Yes, thanks!
>
Ok, I think there are some clarifications on the nature of our OA
hardware that we're drilling into here that might also help explain
more of the trade-offs that were considered regarding forwarding raw
OA hardware reports.
I wonder if it could be good for us to take a little bit of a step
back here just to re-consider the implications of enabling a pmu that
really doesn't relate to the cpu or tasks running on cpus - since I
think this is the first example of that, and it's also had a bearing
on how we currently forward samples via perf.
I think the basic implications to consider are that for our use cases
we wouldn't expect to be requesting cpu centric info in samples, such
as _IP, _CALLCHAIN, _CPU, _BRANCH_STACK and_REGS/STACK_USER and that
we wouldn't open events for specific pids/tasks.
I don't think we'd expect to use existing userspace tools with this
pmu, such as perf, and instead:
- We're accessing perf from within OpenGL for implementing a
performance query extension that applications/games/tools can then use
to get metrics for a single gpu context owned by OpenGL.
- We're creating new tools that are geared for viewing gpu metrics,
including gputop and grafips
I have a feeling that Ingo and yourself may currently be thinking of
the OA unit like an uncore pmu with the idea that tools like perf
should Just Work™ with OA counters, while I don't think that's going
to be the case.
Even so, I still feel that we benefit from being able to leverage the
perf infrastructure to support our use cases and even though perf is
currently quite cpu centric I've been happy that it's turned out to be
a good fit for exposing device metrics too and ignoring/bypassing some
of cpu specific bits for our needs has been straight forward.
I don't bring this up as a tangent to the question of having one
raw-report event vs multiple single counter events, but rather to
highlight that our gpu focused use cases might be a bit different to
what you're used to and I hope our tooling and OpenGL work can inform
this discussion too; representing the practical ends that we're most
eager to support.
Regards,
- Robert
^ permalink raw reply
* Re: [PATCH v2] seccomp: add ptrace options for suspend/resume
From: Oleg Nesterov @ 2015-06-04 18:31 UTC (permalink / raw)
To: Tycho Andersen
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA, Kees Cook, Andy Lutomirski,
Will Drewry, Roland McGrath, Pavel Emelyanov, Serge E. Hallyn
In-Reply-To: <1433369396-13360-1-git-send-email-tycho.andersen-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
On 06/03, Tycho Andersen wrote:
>
> @@ -556,6 +557,11 @@ static int ptrace_setoptions(struct task_struct *child, unsigned long data)
> if (data & ~(unsigned long)PTRACE_O_MASK)
> return -EINVAL;
>
> +#ifdef CONFIG_CHECKPOINT_RESTORE
> + if (data & PTRACE_O_SUSPEND_SECCOMP && !may_suspend_seccomp())
> + return -EPERM;
> +#endif
> +
Well. This -EPERM doesn't look consistent...
if config_enabled(CONFIG_CHECKPOINT_RESTORE) == F, we return success
but PTRACE_O_SUSPEND_SECCOMP has no effect because of another ifdef in
seccomp.
OTOH, if CONFIG_SECCOMP=n, this option has no effect too but we return
-EPERM even.
Also. Suppose that the tracer sets SUSPEND_SECCOMP and then drops
CAP_SYS_ADMIN. After that it can't set or clear other ptrace options.
So if we really want the security checks (I still think we do not ;)
then we should probably check "flags & SUSPEND_SECCOMP" as well.
> +#ifdef CONFIG_CHECKPOINT_RESTORE
> +bool may_suspend_seccomp(void)
> +{
> + if (!capable(CAP_SYS_ADMIN))
> + return false;
> +
> + if (current->seccomp.mode != SECCOMP_MODE_DISABLED)
> + return false;
Heh. OK, I won't argue with the new check too ;)
Oleg.
^ permalink raw reply
* Re: [PATCH v2] seccomp: add ptrace options for suspend/resume
From: Kees Cook @ 2015-06-04 18:12 UTC (permalink / raw)
To: Tycho Andersen
Cc: LKML, Linux API, Andy Lutomirski, Will Drewry, Roland McGrath,
Oleg Nesterov, Pavel Emelyanov, Serge E. Hallyn
In-Reply-To: <20150604171501.GI3160@smitten>
On Thu, Jun 4, 2015 at 10:15 AM, Tycho Andersen
<tycho.andersen-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org> wrote:
> On Thu, Jun 04, 2015 at 09:44:36AM -0700, Kees Cook wrote:
>> On Wed, Jun 3, 2015 at 3:09 PM, Tycho Andersen
>> <tycho.andersen-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org> wrote:
>> > This patch is the first step in enabling checkpoint/restore of processes
>> > with seccomp enabled.
>> >
>> > One of the things CRIU does while dumping tasks is inject code into them
>> > via ptrace to collect information that is only available to the process
>> > itself. However, if we are in a seccomp mode where these processes are
>> > prohibited from making these syscalls, then what CRIU does kills the task.
>> >
>> > This patch adds a new ptrace option, PTRACE_O_SUSPEND_SECCOMP, that enables
>> > a task from the init user namespace which has CAP_SYS_ADMIN and no seccomp
>> > filters to disable (and re-enable) seccomp filters for another task so that
>> > they can be successfully dumped (and restored). We restrict the set of
>> > processes that can disable seccomp through ptrace because although today
>> > ptrace can be used to bypass seccomp, there is some discussion of closing
>> > this loophole in the future and we would like this patch to not depend on
>> > that behavior and be future proofed for when it is removed.
>> >
>> > Note that seccomp can be suspended before any filters are actually
>> > installed; this behavior is useful on criu restore, so that we can suspend
>> > seccomp, restore the filters, unmap our restore code from the restored
>> > process' address space, and then resume the task by detaching and have the
>> > filters resumed as well.
>> >
>> > v2 changes:
>> >
>> > * require that the tracer have no seccomp filters installed
>> > * drop TIF_NOTSC manipulation from the patch
>> > * change from ptrace command to a ptrace option and use this ptrace option
>> > as the flag to check. This means that as soon as the tracer
>> > detaches/dies, seccomp is re-enabled and as a corrollary that one can not
>> > disable seccomp across PTRACE_ATTACHs.
>>
>> This feature gives me the creeps, but I think it's okay.
>
> :D
>
>> Could it be
>> further restricted so that the process doing the suspension is already
>> ptracing the target?
>
> As far as I understand it you do have to PTRACE_{ATTACH,SEIZE} to the
> target before setting options in general. Is that not what you mean
> here?
Ah, true, yes. Okay, ignore me. I was thinking about the mechanism for
setting the flag wrong. :)
-Kees
>
> The rest of the changes sound good, I'll make those and resend.
>
>>
>> Thanks for working on this!
>
> Thanks for the review.
>
> Tycho
--
Kees Cook
Chrome OS Security
^ permalink raw reply
* Re: [PATCH v2] seccomp: add ptrace options for suspend/resume
From: Kees Cook @ 2015-06-04 18:10 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Tycho Andersen, LKML, Linux API, Andy Lutomirski, Will Drewry,
Roland McGrath, Pavel Emelyanov, Serge E. Hallyn
In-Reply-To: <20150604180303.GA32421-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
On Thu, Jun 4, 2015 at 11:03 AM, Oleg Nesterov <oleg-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> On 06/04, Kees Cook wrote:
>>
>> On Wed, Jun 3, 2015 at 3:09 PM, Tycho Andersen
>> > @@ -556,6 +557,11 @@ static int ptrace_setoptions(struct task_struct *child, unsigned long data)
>> > if (data & ~(unsigned long)PTRACE_O_MASK)
>> > return -EINVAL;
>> >
>> > +#ifdef CONFIG_CHECKPOINT_RESTORE
>> > + if (data & PTRACE_O_SUSPEND_SECCOMP && !may_suspend_seccomp())
>> > + return -EPERM;
>> > +#endif
>>
>> I'd like to avoid seeing any #ifdefs added to the .c files. Using a
>> static inline for may_suspend_seccomp() should cause this statement to
>> be eliminated by the compiler.
>
> Agreed, me too, but see below.
>
>> > @@ -590,6 +590,11 @@ void secure_computing_strict(int this_syscall)
>> > {
>> > int mode = current->seccomp.mode;
>> >
>> > +#ifdef CONFIG_CHECKPOINT_RESTORE
>> > + if (unlikely(current->ptrace & PT_SUSPEND_SECCOMP))
>> > + return;
>> > +#endif
>>
>> Could PT_SUSPEND_SECCOMP be defined to "0" with not
>> CONFIG_CHECKPOINT_RESTORE? Then this wouldn't need ifdefs, and should
>> be similarly eliminated by the compiler.
>
> Yes, but this way we add another ugly ifdef into .h, and if you read
> this code it is not clear that this check should be eliminated by gcc.
>
> I'd suggest
>
> if (config_enabled(CONFIG_CHECKPOINT_RESTORE) &&
> unlikely(current->ptrace & PT_SUSPEND_SECCOMP))
> return;
Ah! Yes, that makes things nicer.
-Kees
--
Kees Cook
Chrome OS Security
^ permalink raw reply
* Re: [PATCH v2] seccomp: add ptrace options for suspend/resume
From: Oleg Nesterov @ 2015-06-04 18:03 UTC (permalink / raw)
To: Kees Cook
Cc: Tycho Andersen, LKML, Linux API, Andy Lutomirski, Will Drewry,
Roland McGrath, Pavel Emelyanov, Serge E. Hallyn
In-Reply-To: <CAGXu5jLBm6ZBptsDTrNV6OrFSwoZ__k3867-Ji+6oYuTQ8ncWQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On 06/04, Kees Cook wrote:
>
> On Wed, Jun 3, 2015 at 3:09 PM, Tycho Andersen
> > @@ -556,6 +557,11 @@ static int ptrace_setoptions(struct task_struct *child, unsigned long data)
> > if (data & ~(unsigned long)PTRACE_O_MASK)
> > return -EINVAL;
> >
> > +#ifdef CONFIG_CHECKPOINT_RESTORE
> > + if (data & PTRACE_O_SUSPEND_SECCOMP && !may_suspend_seccomp())
> > + return -EPERM;
> > +#endif
>
> I'd like to avoid seeing any #ifdefs added to the .c files. Using a
> static inline for may_suspend_seccomp() should cause this statement to
> be eliminated by the compiler.
Agreed, me too, but see below.
> > @@ -590,6 +590,11 @@ void secure_computing_strict(int this_syscall)
> > {
> > int mode = current->seccomp.mode;
> >
> > +#ifdef CONFIG_CHECKPOINT_RESTORE
> > + if (unlikely(current->ptrace & PT_SUSPEND_SECCOMP))
> > + return;
> > +#endif
>
> Could PT_SUSPEND_SECCOMP be defined to "0" with not
> CONFIG_CHECKPOINT_RESTORE? Then this wouldn't need ifdefs, and should
> be similarly eliminated by the compiler.
Yes, but this way we add another ugly ifdef into .h, and if you read
this code it is not clear that this check should be eliminated by gcc.
I'd suggest
if (config_enabled(CONFIG_CHECKPOINT_RESTORE) &&
unlikely(current->ptrace & PT_SUSPEND_SECCOMP))
return;
Oleg.
^ permalink raw reply
* [PATCH] tools, add tools/testing/selftests/timers/.gitignore
From: Prarit Bhargava @ 2015-06-04 17:53 UTC (permalink / raw)
To: linux-kernel
Cc: Prarit Bhargava, John Stultz, Thomas Gleixner, Shuah Khan,
linux-api
After building the tests in the timers directory each executable shows up
in 'git-status' as a new file. This patch adds a .gitignore file to the
directory.
Cc: John Stultz <john.stultz@linaro.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Shuah Khan <shuahkh@osg.samsung.com>
Cc: linux-api@vger.kernel.org
Signed-off-by: Prarit Bhargava <prarit@redhat.com>
---
tools/testing/selftests/timers/.gitignore | 3 +++
1 file changed, 3 insertions(+)
create mode 100644 tools/testing/selftests/timers/.gitignore
diff --git a/tools/testing/selftests/timers/.gitignore b/tools/testing/selftests/timers/.gitignore
new file mode 100644
index 0000000..afe9743
--- /dev/null
+++ b/tools/testing/selftests/timers/.gitignore
@@ -0,0 +1,3 @@
+*
+!*.c
+!.gitignore
--
1.7.9.3
^ permalink raw reply related
* Re: [PATCH v2] seccomp: add ptrace options for suspend/resume
From: Tycho Andersen @ 2015-06-04 17:15 UTC (permalink / raw)
To: Kees Cook
Cc: LKML, Linux API, Andy Lutomirski, Will Drewry, Roland McGrath,
Oleg Nesterov, Pavel Emelyanov, Serge E. Hallyn
In-Reply-To: <CAGXu5jLBm6ZBptsDTrNV6OrFSwoZ__k3867-Ji+6oYuTQ8ncWQ@mail.gmail.com>
On Thu, Jun 04, 2015 at 09:44:36AM -0700, Kees Cook wrote:
> On Wed, Jun 3, 2015 at 3:09 PM, Tycho Andersen
> <tycho.andersen@canonical.com> wrote:
> > This patch is the first step in enabling checkpoint/restore of processes
> > with seccomp enabled.
> >
> > One of the things CRIU does while dumping tasks is inject code into them
> > via ptrace to collect information that is only available to the process
> > itself. However, if we are in a seccomp mode where these processes are
> > prohibited from making these syscalls, then what CRIU does kills the task.
> >
> > This patch adds a new ptrace option, PTRACE_O_SUSPEND_SECCOMP, that enables
> > a task from the init user namespace which has CAP_SYS_ADMIN and no seccomp
> > filters to disable (and re-enable) seccomp filters for another task so that
> > they can be successfully dumped (and restored). We restrict the set of
> > processes that can disable seccomp through ptrace because although today
> > ptrace can be used to bypass seccomp, there is some discussion of closing
> > this loophole in the future and we would like this patch to not depend on
> > that behavior and be future proofed for when it is removed.
> >
> > Note that seccomp can be suspended before any filters are actually
> > installed; this behavior is useful on criu restore, so that we can suspend
> > seccomp, restore the filters, unmap our restore code from the restored
> > process' address space, and then resume the task by detaching and have the
> > filters resumed as well.
> >
> > v2 changes:
> >
> > * require that the tracer have no seccomp filters installed
> > * drop TIF_NOTSC manipulation from the patch
> > * change from ptrace command to a ptrace option and use this ptrace option
> > as the flag to check. This means that as soon as the tracer
> > detaches/dies, seccomp is re-enabled and as a corrollary that one can not
> > disable seccomp across PTRACE_ATTACHs.
>
> This feature gives me the creeps, but I think it's okay.
:D
> Could it be
> further restricted so that the process doing the suspension is already
> ptracing the target?
As far as I understand it you do have to PTRACE_{ATTACH,SEIZE} to the
target before setting options in general. Is that not what you mean
here?
The rest of the changes sound good, I'll make those and resend.
>
> Thanks for working on this!
Thanks for the review.
Tycho
^ permalink raw reply
* Re: [PATCH v2] seccomp: add ptrace options for suspend/resume
From: Kees Cook @ 2015-06-04 16:44 UTC (permalink / raw)
To: Tycho Andersen
Cc: LKML, Linux API, Andy Lutomirski, Will Drewry, Roland McGrath,
Oleg Nesterov, Pavel Emelyanov, Serge E. Hallyn
In-Reply-To: <1433369396-13360-1-git-send-email-tycho.andersen@canonical.com>
On Wed, Jun 3, 2015 at 3:09 PM, Tycho Andersen
<tycho.andersen@canonical.com> wrote:
> This patch is the first step in enabling checkpoint/restore of processes
> with seccomp enabled.
>
> One of the things CRIU does while dumping tasks is inject code into them
> via ptrace to collect information that is only available to the process
> itself. However, if we are in a seccomp mode where these processes are
> prohibited from making these syscalls, then what CRIU does kills the task.
>
> This patch adds a new ptrace option, PTRACE_O_SUSPEND_SECCOMP, that enables
> a task from the init user namespace which has CAP_SYS_ADMIN and no seccomp
> filters to disable (and re-enable) seccomp filters for another task so that
> they can be successfully dumped (and restored). We restrict the set of
> processes that can disable seccomp through ptrace because although today
> ptrace can be used to bypass seccomp, there is some discussion of closing
> this loophole in the future and we would like this patch to not depend on
> that behavior and be future proofed for when it is removed.
>
> Note that seccomp can be suspended before any filters are actually
> installed; this behavior is useful on criu restore, so that we can suspend
> seccomp, restore the filters, unmap our restore code from the restored
> process' address space, and then resume the task by detaching and have the
> filters resumed as well.
>
> v2 changes:
>
> * require that the tracer have no seccomp filters installed
> * drop TIF_NOTSC manipulation from the patch
> * change from ptrace command to a ptrace option and use this ptrace option
> as the flag to check. This means that as soon as the tracer
> detaches/dies, seccomp is re-enabled and as a corrollary that one can not
> disable seccomp across PTRACE_ATTACHs.
This feature gives me the creeps, but I think it's okay. Could it be
further restricted so that the process doing the suspension is already
ptracing the target?
> Signed-off-by: Tycho Andersen <tycho.andersen@canonical.com>
> CC: Kees Cook <keescook@chromium.org>
> CC: Andy Lutomirski <luto@amacapital.net>
> CC: Will Drewry <wad@chromium.org>
> CC: Roland McGrath <roland@hack.frob.com>
> CC: Oleg Nesterov <oleg@redhat.com>
> CC: Pavel Emelyanov <xemul@parallels.com>
> CC: Serge E. Hallyn <serge.hallyn@ubuntu.com>
> ---
> include/linux/ptrace.h | 1 +
> include/linux/seccomp.h | 4 ++++
> include/uapi/linux/ptrace.h | 6 ++++--
> kernel/ptrace.c | 6 ++++++
> kernel/seccomp.c | 23 +++++++++++++++++++++++
> 5 files changed, 38 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/ptrace.h b/include/linux/ptrace.h
> index 987a73a..061265f 100644
> --- a/include/linux/ptrace.h
> +++ b/include/linux/ptrace.h
> @@ -34,6 +34,7 @@
> #define PT_TRACE_SECCOMP PT_EVENT_FLAG(PTRACE_EVENT_SECCOMP)
>
> #define PT_EXITKILL (PTRACE_O_EXITKILL << PT_OPT_FLAG_SHIFT)
> +#define PT_SUSPEND_SECCOMP (PTRACE_O_SUSPEND_SECCOMP << PT_OPT_FLAG_SHIFT)
>
> /* single stepping state bits (used on ARM and PA-RISC) */
> #define PT_SINGLESTEP_BIT 31
> diff --git a/include/linux/seccomp.h b/include/linux/seccomp.h
> index a19ddac..ae3ec52 100644
> --- a/include/linux/seccomp.h
> +++ b/include/linux/seccomp.h
> @@ -53,6 +53,10 @@ static inline int seccomp_mode(struct seccomp *s)
> return s->mode;
> }
>
> +#ifdef CONFIG_CHECKPOINT_RESTORE
> +extern bool may_suspend_seccomp(void);
> +#endif
#else
static inline bool may_suspend_seccomp(void) { return false; }
#endif
> +
> #else /* CONFIG_SECCOMP */
>
> #include <linux/errno.h>
> diff --git a/include/uapi/linux/ptrace.h b/include/uapi/linux/ptrace.h
> index cf1019e..a7a6979 100644
> --- a/include/uapi/linux/ptrace.h
> +++ b/include/uapi/linux/ptrace.h
> @@ -89,9 +89,11 @@ struct ptrace_peeksiginfo_args {
> #define PTRACE_O_TRACESECCOMP (1 << PTRACE_EVENT_SECCOMP)
>
> /* eventless options */
> -#define PTRACE_O_EXITKILL (1 << 20)
> +#define PTRACE_O_EXITKILL (1 << 20)
> +#define PTRACE_O_SUSPEND_SECCOMP (1 << 21)
>
> -#define PTRACE_O_MASK (0x000000ff | PTRACE_O_EXITKILL)
> +#define PTRACE_O_MASK (\
> + 0x000000ff | PTRACE_O_EXITKILL | PTRACE_O_SUSPEND_SECCOMP)
>
> #include <asm/ptrace.h>
>
> diff --git a/kernel/ptrace.c b/kernel/ptrace.c
> index c8e0e05..e3e68a2 100644
> --- a/kernel/ptrace.c
> +++ b/kernel/ptrace.c
> @@ -15,6 +15,7 @@
> #include <linux/highmem.h>
> #include <linux/pagemap.h>
> #include <linux/ptrace.h>
> +#include <linux/seccomp.h>
> #include <linux/security.h>
> #include <linux/signal.h>
> #include <linux/uio.h>
> @@ -556,6 +557,11 @@ static int ptrace_setoptions(struct task_struct *child, unsigned long data)
> if (data & ~(unsigned long)PTRACE_O_MASK)
> return -EINVAL;
>
> +#ifdef CONFIG_CHECKPOINT_RESTORE
> + if (data & PTRACE_O_SUSPEND_SECCOMP && !may_suspend_seccomp())
> + return -EPERM;
> +#endif
I'd like to avoid seeing any #ifdefs added to the .c files. Using a
static inline for may_suspend_seccomp() should cause this statement to
be eliminated by the compiler.
> +
> /* Avoid intermediate state when all opts are cleared */
> flags = child->ptrace;
> flags &= ~(PTRACE_O_MASK << PT_OPT_FLAG_SHIFT);
> diff --git a/kernel/seccomp.c b/kernel/seccomp.c
> index 980fd26..2a1bd35 100644
> --- a/kernel/seccomp.c
> +++ b/kernel/seccomp.c
> @@ -590,6 +590,11 @@ void secure_computing_strict(int this_syscall)
> {
> int mode = current->seccomp.mode;
>
> +#ifdef CONFIG_CHECKPOINT_RESTORE
> + if (unlikely(current->ptrace & PT_SUSPEND_SECCOMP))
> + return;
> +#endif
Could PT_SUSPEND_SECCOMP be defined to "0" with not
CONFIG_CHECKPOINT_RESTORE? Then this wouldn't need ifdefs, and should
be similarly eliminated by the compiler.
> +
> if (mode == 0)
> return;
> else if (mode == SECCOMP_MODE_STRICT)
> @@ -691,6 +696,11 @@ u32 seccomp_phase1(struct seccomp_data *sd)
> int this_syscall = sd ? sd->nr :
> syscall_get_nr(current, task_pt_regs(current));
>
> +#ifdef CONFIG_CHECKPOINT_RESTORE
> + if (unlikely(current->ptrace & PT_SUSPEND_SECCOMP))
> + return SECCOMP_PHASE1_OK;
> +#endif
> +
> switch (mode) {
> case SECCOMP_MODE_STRICT:
> __secure_computing_strict(this_syscall); /* may call do_exit */
> @@ -901,3 +911,16 @@ long prctl_set_seccomp(unsigned long seccomp_mode, char __user *filter)
> /* prctl interface doesn't have flags, so they are always zero. */
> return do_seccomp(op, 0, uargs);
> }
> +
> +#ifdef CONFIG_CHECKPOINT_RESTORE
> +bool may_suspend_seccomp(void)
> +{
> + if (!capable(CAP_SYS_ADMIN))
> + return false;
> +
> + if (current->seccomp.mode != SECCOMP_MODE_DISABLED)
> + return false;
> +
> + return true;
> +}
> +#endif /* CONFIG_CHECKPOINT_RESTORE */
> --
> 2.1.4
>
Thanks for working on this!
-Kees
--
Kees Cook
Chrome OS Security
^ permalink raw reply
* Re: [PATCH 09/98] via_drm.h: include linux/types.h instead of non-existing via_drmclient.h
From: Emil Velikov @ 2015-06-04 11:34 UTC (permalink / raw)
To: Mikko Rapeli; +Cc: linux-api, Linux-Kernel@Vger. Kernel. Org, ML dri-devel
In-Reply-To: <CACvgo52DtJNLGPT2j93kFUZcS_1fDdE8RQHWmHUrMQgbfx88dw@mail.gmail.com>
On 3 June 2015 at 18:16, Emil Velikov <emil.l.velikov@gmail.com> wrote:
> Hi Mikko,
>
> On 30 May 2015 at 16:38, Mikko Rapeli <mikko.rapeli@iki.fi> wrote:
>> Fixes compiler error:
>>
>> drm/via_drm.h:36:27: fatal error: via_drmclient.h: No such file or directory
>>
>> Signed-off-by: Mikko Rapeli <mikko.rapeli@iki.fi>
>> ---
>> include/uapi/drm/via_drm.h | 4 +---
>> 1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/include/uapi/drm/via_drm.h b/include/uapi/drm/via_drm.h
>> index 8b0533c..791531e 100644
>> --- a/include/uapi/drm/via_drm.h
>> +++ b/include/uapi/drm/via_drm.h
>> @@ -24,6 +24,7 @@
>> #ifndef _VIA_DRM_H_
>> #define _VIA_DRM_H_
>>
>> +#include <linux/types.h>
> As mentioned elsewhere one could avoid this, and just use drm.h to
> manage the approapriate types (uint32_t vs __u32 and so on).
>
>> #include <drm/drm.h>
>>
>> /* WARNING: These defines must be the same as what the Xserver uses.
>> @@ -33,9 +34,6 @@
>> #ifndef _VIA_DEFINES_
>> #define _VIA_DEFINES_
>>
>> -#ifndef __KERNEL__
>> -#include "via_drmclient.h"
>> -#endif
>>
> I fear that this one is a particular example of a nasty legacy from
> the UMS days. The file is available/provided in very old mesa versions
> and at the very same time mesa requires via_drm.h. So I would kindly
> ask that you:
>
> - Grab the libdrm userspace package, and apply a similar change.
> - Rebuild/install the above.
> - Fetch mesa 7.11, and try building the via dri module. Ideally
> things will continue to build, alternatively we might need to add
> another/additional guard for this include.
>
So the situation is "funnier" than expected:
- There are at least two users of via_drm.h (mesa and xf86-video-via)
with each providing different via_drmclient.h.
- Neither of the two projects includes the latter header, despite
that it uses the macros defined within.
- via_drm.h is included via multiple headers, so adding extra ifdef
guards sounds like a bad idea.
- While new version of the ddx can be released, a mesa one is
unlikely - 7.11.2 was released ~4 years ago.
- Even if we cover the above two project, other projects (how many,
where are they hosted, etc.) may need the same treatment.
With the above said I'd suspect that we're safer leaving the include
as is ? Yes, it is busted if one tries to use the standalone header,
jet (most/all?) official users rely on that behaviour :-\
Cheers
Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH v5 10/12] KVM: arm64: guest debug, HW assisted debug support
From: Christoffer Dall @ 2015-06-04 11:06 UTC (permalink / raw)
To: Alex Bennée
Cc: kvm-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
kvmarm-FPEHb7Xf0XXUo1n7N8X6UoWGPAHP3yOg, marc.zyngier-5wv7dgnIgG8,
peter.maydell-QSEj5FYQhm4dnm+yROfE0A, agraf-l3A5Bk7waGM,
drjones-H+wXaHxf7aLQT0dZR+AlfA, pbonzini-H+wXaHxf7aLQT0dZR+AlfA,
zhichao.huang-QSEj5FYQhm4dnm+yROfE0A,
jan.kiszka-kv7WeFo6aLtBDgjK7y7TUQ,
dahi-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
r65777-KZfg59tc24xl57MIdRCFDg, bp-l3A5Bk7waGM, Gleb Natapov,
Jonathan Corbet, Russell King, Catalin Marinas, Will Deacon,
Ingo Molnar, Peter Zijlstra, Lorenzo Pieralisi,
open list:DOCUMENTATION, open list, open list:ABI/API
In-Reply-To: <1432891828-4816-11-git-send-email-alex.bennee-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
On Fri, May 29, 2015 at 10:30:26AM +0100, Alex Bennée wrote:
> This adds support for userspace to control the HW debug registers for
> guest debug. In the debug ioctl we copy the IMPDEF defined number of
> registers into a new register set called host_debug_state. There is now
> a new vcpu parameter called debug_ptr which selects which register set
> is to copied into the real registers when world switch occurs.
>
> I've moved some helper functions into the hw_breakpoint.h header for
> re-use.
>
> As with single step we need to tweak the guest registers to enable the
> exceptions so we need to save and restore those bits.
>
> Two new capabilities have been added to the KVM_EXTENSION ioctl to allow
> userspace to query the number of hardware break and watch points
> available on the host hardware.
>
> Signed-off-by: Alex Bennée <alex.bennee-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>
> ---
> v2
> - switched to C setup
> - replace host debug registers directly into context
> - minor tweak to api docs
> - setup right register for debug
> - add FAR_EL2 to debug exit structure
> - add support for trapping debug register access
> v3
> - remove stray trace statement
> - fix spacing around operators (various)
> - clean-up usage of trap_debug
> - introduce debug_ptr, replace excessive memcpy stuff
> - don't use memcpy in ioctl, just assign
> - update cap ioctl documentation
> - reword a number comments
> - rename host_debug_state->external_debug_state
> v4
> - use the new u32/u64 split debug_ptr approach
> - fix some wording/comments
> v5
> - don't set MDSCR_EL1.KDE (not needed)
> ---
> Documentation/virtual/kvm/api.txt | 7 ++++++-
> arch/arm/kvm/arm.c | 7 +++++++
> arch/arm64/include/asm/hw_breakpoint.h | 12 +++++++++++
> arch/arm64/include/asm/kvm_host.h | 3 ++-
> arch/arm64/include/uapi/asm/kvm.h | 2 +-
> arch/arm64/kernel/hw_breakpoint.c | 12 -----------
> arch/arm64/kvm/debug.c | 37 +++++++++++++++++++++++++++++-----
> arch/arm64/kvm/handle_exit.c | 6 ++++++
> arch/arm64/kvm/reset.c | 12 +++++++++++
> include/uapi/linux/kvm.h | 2 ++
> 10 files changed, 80 insertions(+), 20 deletions(-)
>
> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
> index 33c8143..ada57df 100644
> --- a/Documentation/virtual/kvm/api.txt
> +++ b/Documentation/virtual/kvm/api.txt
> @@ -2668,7 +2668,7 @@ The top 16 bits of the control field are architecture specific control
> flags which can include the following:
>
> - KVM_GUESTDBG_USE_SW_BP: using software breakpoints [x86, arm64]
> - - KVM_GUESTDBG_USE_HW_BP: using hardware breakpoints [x86, s390]
> + - KVM_GUESTDBG_USE_HW_BP: using hardware breakpoints [x86, s390, arm64]
> - KVM_GUESTDBG_INJECT_DB: inject DB type exception [x86]
> - KVM_GUESTDBG_INJECT_BP: inject BP type exception [x86]
> - KVM_GUESTDBG_EXIT_PENDING: trigger an immediate guest exit [s390]
> @@ -2683,6 +2683,11 @@ updated to the correct (supplied) values.
> The second part of the structure is architecture specific and
> typically contains a set of debug registers.
>
> +For arm64 the number of debug registers is implementation defined and
> +can be determined by querying the KVM_CAP_GUEST_DEBUG_HW_BPS and
> +KVM_CAP_GUEST_DEBUG_HW_WPS capabilities which return a positive number
> +indicating the number of supported registers.
> +
> When debug events exit the main run loop with the reason
> KVM_EXIT_DEBUG with the kvm_debug_exit_arch part of the kvm_run
> structure containing architecture specific debug information.
> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
> index 0d17c7b..6df47c1 100644
> --- a/arch/arm/kvm/arm.c
> +++ b/arch/arm/kvm/arm.c
> @@ -307,6 +307,7 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
>
> #define KVM_GUESTDBG_VALID_MASK (KVM_GUESTDBG_ENABLE | \
> KVM_GUESTDBG_USE_SW_BP | \
> + KVM_GUESTDBG_USE_HW_BP | \
> KVM_GUESTDBG_SINGLESTEP)
>
> /**
> @@ -327,6 +328,12 @@ int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
>
> if (dbg->control & KVM_GUESTDBG_ENABLE) {
> vcpu->guest_debug = dbg->control;
> +
> + /* Hardware assisted Break and Watch points */
> + if (vcpu->guest_debug & KVM_GUESTDBG_USE_HW_BP) {
does KVM_GUESTDBG_USE_HW_BP cover watch points as well or why is this
mentionend in the comment?
I asked this once before already...
> + vcpu->arch.external_debug_state = dbg->arch;
> + }
> +
> } else {
> /* If not enabled clear all flags */
> vcpu->guest_debug = 0;
> diff --git a/arch/arm64/include/asm/hw_breakpoint.h b/arch/arm64/include/asm/hw_breakpoint.h
> index 52b484b..c450552 100644
> --- a/arch/arm64/include/asm/hw_breakpoint.h
> +++ b/arch/arm64/include/asm/hw_breakpoint.h
> @@ -130,6 +130,18 @@ static inline void ptrace_hw_copy_thread(struct task_struct *task)
> }
> #endif
>
> +/* Determine number of BRP registers available. */
> +static inline int get_num_brps(void)
> +{
> + return ((read_cpuid(ID_AA64DFR0_EL1) >> 12) & 0xf) + 1;
> +}
> +
> +/* Determine number of WRP registers available. */
> +static inline int get_num_wrps(void)
> +{
> + return ((read_cpuid(ID_AA64DFR0_EL1) >> 20) & 0xf) + 1;
> +}
> +
> extern struct pmu perf_ops_bp;
>
> #endif /* __KERNEL__ */
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index e5040b6..498d4f7 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -113,12 +113,13 @@ struct kvm_vcpu_arch {
>
> /*
> * For debugging the guest we need to keep a set of debug
> - * registers which can override the guests own debug state
> + * registers which can override the guest's own debug state
rebase error?
> * while being used. These are set via the KVM_SET_GUEST_DEBUG
> * ioctl.
> */
> struct kvm_guest_debug_arch *debug_ptr;
> struct kvm_guest_debug_arch vcpu_debug_state;
> + struct kvm_guest_debug_arch external_debug_state;
>
> /* Pointer to host CPU context */
> kvm_cpu_context_t *host_cpu_context;
> diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
> index 43758e7..95168c2 100644
> --- a/arch/arm64/include/uapi/asm/kvm.h
> +++ b/arch/arm64/include/uapi/asm/kvm.h
> @@ -116,7 +116,7 @@ struct kvm_guest_debug_arch {
>
> struct kvm_debug_exit_arch {
> __u32 hsr;
> - __u64 far;
> + __u64 far; /* used for watchpoints */
and I noticed this one before as well...
> };
>
> /*
> diff --git a/arch/arm64/kernel/hw_breakpoint.c b/arch/arm64/kernel/hw_breakpoint.c
> index e7d934d..3a41bbf 100644
> --- a/arch/arm64/kernel/hw_breakpoint.c
> +++ b/arch/arm64/kernel/hw_breakpoint.c
> @@ -49,18 +49,6 @@ static DEFINE_PER_CPU(int, stepping_kernel_bp);
> static int core_num_brps;
> static int core_num_wrps;
>
> -/* Determine number of BRP registers available. */
> -static int get_num_brps(void)
> -{
> - return ((read_cpuid(ID_AA64DFR0_EL1) >> 12) & 0xf) + 1;
> -}
> -
> -/* Determine number of WRP registers available. */
> -static int get_num_wrps(void)
> -{
> - return ((read_cpuid(ID_AA64DFR0_EL1) >> 20) & 0xf) + 1;
> -}
> -
> int hw_breakpoint_slots(int type)
> {
> /*
> diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c
> index 10a6baa..3c0daae 100644
> --- a/arch/arm64/kvm/debug.c
> +++ b/arch/arm64/kvm/debug.c
> @@ -98,10 +98,6 @@ void kvm_arm_setup_debug(struct kvm_vcpu *vcpu)
> MDCR_EL2_TDRA |
> MDCR_EL2_TDOSA);
>
> - /* Trap on access to debug registers? */
> - if (trap_debug)
> - vcpu->arch.mdcr_el2 |= MDCR_EL2_TDA;
> -
> /* Is Guest debugging in effect? */
> if (vcpu->guest_debug) {
> vcpu->arch.mdcr_el2 |= MDCR_EL2_TDE;
> @@ -124,11 +120,42 @@ void kvm_arm_setup_debug(struct kvm_vcpu *vcpu)
> } else {
> vcpu_sys_reg(vcpu, MDSCR_EL1) &= ~DBG_MDSCR_SS;
> }
> +
> + /*
> + * HW Break/Watch points
> + *
> + * We simply switch the debug_ptr to point to our new
> + * external_debug_state which has been populated by the
> + * debug ioctl. The existing KVM_ARM64_DEBUG_DIRTY
> + * mechanism ensures the registers are updated on the
> + * world switch.
> + */
> + if (vcpu->guest_debug & KVM_GUESTDBG_USE_HW_BP) {
> + /* Enable breakpoints/watchpoints */
> + vcpu_sys_reg(vcpu, MDSCR_EL1) |= DBG_MDSCR_MDE;
> +
> + vcpu->arch.debug_ptr = &vcpu->arch.external_debug_state;
> + vcpu->arch.debug_flags |= KVM_ARM64_DEBUG_DIRTY;
> + trap_debug = true;
> + }
> }
> +
> + /* Trap debug register access */
> + if (trap_debug)
> + vcpu->arch.mdcr_el2 |= MDCR_EL2_TDA;
> }
>
> void kvm_arm_clear_debug(struct kvm_vcpu *vcpu)
> {
> - if (vcpu->guest_debug)
> + if (vcpu->guest_debug) {
> restore_guest_debug_regs(vcpu);
> +
> + /*
> + * If we were using HW debug we need to restore the
> + * debug_ptr to the guest debug state.
> + */
> + if (vcpu->guest_debug & KVM_GUESTDBG_USE_HW_BP)
> + vcpu->arch.debug_ptr = &vcpu->arch.vcpu_debug_state;
you also ignored my comment (or never answered my question) on moving
this to kvm_arm_setup_debug() last time around.... oh well.
Thanks,
-Christoffer
^ permalink raw reply
* Re: [CFT][PATCH 00/10] Making new mounts of proc and sysfs as safe as bind mounts (take 2)
From: Eric W. Biederman @ 2015-06-04 7:34 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Seth Forshee, Linux API, Linux Containers, Serge Hallyn,
Andy Lutomirski, Kenton Varda, Michael Kerrisk-manpages,
Richard Weinberger, Linux FS Devel, Tejun Heo
In-Reply-To: <87h9qo6la9.fsf-JOvCrm2gF+uungPnsOpG7nhyD016LWXt@public.gmane.org>
ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org (Eric W. Biederman) writes:
> Greg Kroah-Hartman <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org> writes:
>
>> On Wed, Jun 03, 2015 at 04:13:21PM -0500, Eric W. Biederman wrote:
>>> Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> writes:
>>>
>>> > One option would be to break the nosuid, nodev, and noexec parts into
>>> > their own patch and then avoid tagging that patch for -stable if at
>>> > all possible. It would be nice to avoid another -stable ABI break if
>>> > at all possible.
>>>
>>> So I don't think we actually have anything that could be called an ABI
>>> break in the whole mess, but it is definitely a behavioral change that
>>> is a regression for lxc and libvirt-lxc that prevents them from starting.
>>>
>>> nodev does not actually matter because of the implicit silliness that
>>> is being added right now.
>>>
>>> We do want those programs fixed and after those programs are fixed we
>>> can safely begin failing mount when those attributes are being cleared
>>> in a fresh mount.
>>>
>>> So it looks to me like the best thing to do is to print a warning
>>> whenever lxc or libvirt-lxc gets it wrong, which should ensure the
>>> authors are sufficiently pestered that in a kernel release or 3 we can
>>> begin enforcing those attributes. Especially as the discussion on the
>>> fix for those applications has already begun.
>>
>> "pestering" never works, look at some of the SCSI drivers for examples
>> of how a distro will just patch out the "warning this driver is using an
>> old api and needs to be fixed" messages.
>
>> You can't break stuff like this, people will get upset :(
>
> A) To the best of my knowledge there are two programs on the face of the
> planet where this matters. (lxc and libvirt-lxc)
>
> B) The code in those two programs is buggy. That is the code in those
> two programs does not do what the authors intended. That is fixing
> those programs is something that should be done regardless of what
> I do in the kernel. I have already reached out to the developers of
> those programs. The pestering in the kernel is a form of reminder,
> not the primary source of communication.
>
> C) These bugs really are security holes. Currently they do not appear
> exploitable (thank goodness) but they are security holes.
>
> Since they are not currently exploitable it does make sense
> to give people a little time to get their act together.
>
> The bugs are larger then the case that is being hit here,
> this is just where they are noticed.
>
> D) Letting people know that there is a problem as part of a larger
> effort has actually worked for me. Distro's have stopped enabling
> the sysctl system call.
>
> E) Given that I have not audited sysfs and proc closely in recent years
> I may actually be wrong. Those bugs may actually be exploitable.
> All it takes is chmod to be supported on one file that can be made
> executable. That bug has existed in the past and I don't doubt
> someone will overlook something and we will see the bug again in the
> future.
>
> So it is my best judgment that I disable the code that stops
> containers from starting and just making it a warning (for now).
> Then in a release or so I start failing these operations instead of
> warning.
>
> This is the most fair and reasonable I can see to be.
>
> The only other choice I can see is to say I don't care it is a security
> issue I am breaking your sloopy insecure code.
>
> Am I being too nice with these security bugs?
Thinking about it a little more. There is a possibility that sometime
in the future that someone will deliberately add a suid executable as a
file in proc or sysfs and have a good reason for doing so.
Some sysadmin or sandbox builder with special requirements may then
disable suid and exec on proc because in their sandbox (not linux in
general) having access to that executable is a bad thing. At which
we have an exploitable security issue if nosuid and noexec are not
enforced.
Or in other words I am not smarter than the bad guys. This is a
security issue. I can not ignore nosuid and noexec indefinitely.
I have to make those cases fail at some point. At that point
current unfixed versions of lxc and libvirt-lxc will break.
A warning is the nicest I can imagine being.
Eric
^ permalink raw reply
* Re: [CFT][PATCH 00/10] Making new mounts of proc and sysfs as safe as bind mounts (take 2)
From: Eric W. Biederman @ 2015-06-04 6:27 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Andy Lutomirski, Kenton Varda, Serge Hallyn, Seth Forshee,
Linux API, Linux Containers, Michael Kerrisk-manpages,
Richard Weinberger, Linux FS Devel, Tejun Heo
In-Reply-To: <20150604051958.GA21049@kroah.com>
Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> On Wed, Jun 03, 2015 at 04:13:21PM -0500, Eric W. Biederman wrote:
>> Andy Lutomirski <luto@amacapital.net> writes:
>>
>> > One option would be to break the nosuid, nodev, and noexec parts into
>> > their own patch and then avoid tagging that patch for -stable if at
>> > all possible. It would be nice to avoid another -stable ABI break if
>> > at all possible.
>>
>> So I don't think we actually have anything that could be called an ABI
>> break in the whole mess, but it is definitely a behavioral change that
>> is a regression for lxc and libvirt-lxc that prevents them from starting.
>>
>> nodev does not actually matter because of the implicit silliness that
>> is being added right now.
>>
>> We do want those programs fixed and after those programs are fixed we
>> can safely begin failing mount when those attributes are being cleared
>> in a fresh mount.
>>
>> So it looks to me like the best thing to do is to print a warning
>> whenever lxc or libvirt-lxc gets it wrong, which should ensure the
>> authors are sufficiently pestered that in a kernel release or 3 we can
>> begin enforcing those attributes. Especially as the discussion on the
>> fix for those applications has already begun.
>
> "pestering" never works, look at some of the SCSI drivers for examples
> of how a distro will just patch out the "warning this driver is using an
> old api and needs to be fixed" messages.
> You can't break stuff like this, people will get upset :(
A) To the best of my knowledge there are two programs on the face of the
planet where this matters. (lxc and libvirt-lxc)
B) The code in those two programs is buggy. That is the code in those
two programs does not do what the authors intended. That is fixing
those programs is something that should be done regardless of what
I do in the kernel. I have already reached out to the developers of
those programs. The pestering in the kernel is a form of reminder,
not the primary source of communication.
C) These bugs really are security holes. Currently they do not appear
exploitable (thank goodness) but they are security holes.
Since they are not currently exploitable it does make sense
to give people a little time to get their act together.
The bugs are larger then the case that is being hit here,
this is just where they are noticed.
D) Letting people know that there is a problem as part of a larger
effort has actually worked for me. Distro's have stopped enabling
the sysctl system call.
E) Given that I have not audited sysfs and proc closely in recent years
I may actually be wrong. Those bugs may actually be exploitable.
All it takes is chmod to be supported on one file that can be made
executable. That bug has existed in the past and I don't doubt
someone will overlook something and we will see the bug again in the
future.
So it is my best judgment that I disable the code that stops
containers from starting and just making it a warning (for now).
Then in a release or so I start failing these operations instead of
warning.
This is the most fair and reasonable I can see to be.
The only other choice I can see is to say I don't care it is a security
issue I am breaking your sloopy insecure code.
Am I being too nice with these security bugs?
Eric
^ permalink raw reply
* Re: [CFT][PATCH 11/10] mnt: Avoid unnecessary regressions in fs_fully_visible (take 2)
From: Greg Kroah-Hartman @ 2015-06-04 5:20 UTC (permalink / raw)
To: Eric W. Biederman
Cc: Andy Lutomirski, Kenton Varda, Serge Hallyn, Seth Forshee,
Linux API, Linux Containers, Michael Kerrisk-manpages,
Richard Weinberger, Linux FS Devel, Tejun Heo
In-Reply-To: <874mmodral.fsf_-_-JOvCrm2gF+uungPnsOpG7nhyD016LWXt@public.gmane.org>
On Wed, Jun 03, 2015 at 11:35:30PM -0500, Eric W. Biederman wrote:
>
> Not allowing programs to clear nosuid and noexec on new mounts of
> sysfs or proc will cause lxc and libvirt-lxc to fail to start (a
> regression). There are no executables files on sysfs or proc today
> which means clearing these flags is harmless today.
>
> Instead of failing the fresh mounts of sysfs and proc emit a warning
> when these flags are improprely cleared. We only reach this point
> because lxc and libvirt-lxc clear flags they mount flags had not
> intended to.
>
> In a couple of kernel releases when lxc and libvirt-lxc have been
> fixed we can start failing fresh mounts proc and sysfs that clear
> nosuid and noexec. Userspace clearly means to enforce those
> attributes and enforcing these attributes have historically avoided
> bugs in the setattr implementations of proc and sysfs.
>
> Cc: stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Signed-off-by: "Eric W. Biederman" <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
> ---
>
> Now with warning on problematic remounts as well.
> nodev is also ignored because it is not currently problematic.
>
> fs/namespace.c | 33 +++++++++++++++++++++++++++++++++
> include/linux/mount.h | 5 +++++
> 2 files changed, 38 insertions(+)
>
> diff --git a/fs/namespace.c b/fs/namespace.c
> index eccd925c6e82..3c3f8172c734 100644
> --- a/fs/namespace.c
> +++ b/fs/namespace.c
> @@ -2162,6 +2162,18 @@ static int do_remount(struct path *path, int flags, int mnt_flags,
> ((mnt->mnt.mnt_flags & MNT_ATIME_MASK) != (mnt_flags & MNT_ATIME_MASK))) {
> return -EPERM;
> }
> + if ((mnt->mnt.mnt_flags & MNT_WARN_NOSUID) &&
> + !(mnt_flags & MNT_NOSUID) && printk_ratelimit()) {
> + printk(KERN_INFO
> + "warning: process `%s' clears nosuid in remount of %s\n",
> + current->comm, sb->s_type->name);
> + }
> + if ((mnt->mnt.mnt_flags & MNT_WARN_NOEXEC) &&
> + !(mnt_flags & MNT_NOEXEC) && printk_ratelimit()) {
> + printk(KERN_INFO
> + "warning: process `%s' clears noexec in remount of %s\n",
> + current->comm, sb->s_type->name);
> + }
>
> err = security_sb_remount(sb, data);
> if (err)
> @@ -3201,12 +3213,14 @@ static bool fs_fully_visible(struct file_system_type *type, int *new_mnt_flags)
> if ((mnt->mnt.mnt_flags & MNT_LOCK_NODEV) &&
> !(new_flags & MNT_NODEV))
> continue;
> +#if 0 /* Avoid unnecessary regressions */
> if ((mnt->mnt.mnt_flags & MNT_LOCK_NOSUID) &&
> !(new_flags & MNT_NOSUID))
> continue;
> if ((mnt->mnt.mnt_flags & MNT_LOCK_NOEXEC) &&
> !(new_flags & MNT_NOEXEC))
> continue;
> +#endif
> if ((mnt->mnt.mnt_flags & MNT_LOCK_ATIME) &&
> ((mnt->mnt.mnt_flags & MNT_ATIME_MASK) != (new_flags & MNT_ATIME_MASK)))
> continue;
> @@ -3227,9 +3241,28 @@ static bool fs_fully_visible(struct file_system_type *type, int *new_mnt_flags)
> /* Preserve the locked attributes */
> *new_mnt_flags |= mnt->mnt.mnt_flags & (MNT_LOCK_READONLY | \
> MNT_LOCK_NODEV | \
> + /* Avoid unnecessary regressions \
> MNT_LOCK_NOSUID | \
> MNT_LOCK_NOEXEC | \
> + */ \
> MNT_LOCK_ATIME);
> + /* For now, warn about the "harmless" but invalid mnt flags */
> + if (mnt->mnt.mnt_flags & MNT_LOCK_NOSUID) {
> + *new_mnt_flags |= MNT_WARN_NOSUID;
> + if (!(new_flags & MNT_NOSUID) && printk_ratelimit()) {
> + printk(KERN_INFO
> + "warning: process `%s' clears nosuid in mount of %s\n",
> + current->comm, type->name);
> + }
> + }
> + if (mnt->mnt.mnt_flags & MNT_LOCK_NOEXEC) {
> + *new_mnt_flags |= MNT_WARN_NOEXEC;
> + if (!(new_flags & MNT_NOEXEC) && printk_ratelimit()) {
> + printk(KERN_INFO
> + "warning: process `%s' clears noexec in mount of %s\n",
> + current->comm, type->name);
> + }
> + }
Adding this to a stable kernel is not going to be ok, sorry. We can't
start being noisy in system logs for things that were working just fine.
greg k-h
^ permalink raw reply
* Re: [CFT][PATCH 00/10] Making new mounts of proc and sysfs as safe as bind mounts (take 2)
From: Greg Kroah-Hartman @ 2015-06-04 5:19 UTC (permalink / raw)
To: Eric W. Biederman
Cc: Seth Forshee, Linux API, Linux Containers, Serge Hallyn,
Andy Lutomirski, Kenton Varda, Michael Kerrisk-manpages,
Richard Weinberger, Linux FS Devel, Tejun Heo
In-Reply-To: <87k2vkebri.fsf-JOvCrm2gF+uungPnsOpG7nhyD016LWXt@public.gmane.org>
On Wed, Jun 03, 2015 at 04:13:21PM -0500, Eric W. Biederman wrote:
> Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> writes:
>
> > One option would be to break the nosuid, nodev, and noexec parts into
> > their own patch and then avoid tagging that patch for -stable if at
> > all possible. It would be nice to avoid another -stable ABI break if
> > at all possible.
>
> So I don't think we actually have anything that could be called an ABI
> break in the whole mess, but it is definitely a behavioral change that
> is a regression for lxc and libvirt-lxc that prevents them from starting.
>
> nodev does not actually matter because of the implicit silliness that
> is being added right now.
>
> We do want those programs fixed and after those programs are fixed we
> can safely begin failing mount when those attributes are being cleared
> in a fresh mount.
>
> So it looks to me like the best thing to do is to print a warning
> whenever lxc or libvirt-lxc gets it wrong, which should ensure the
> authors are sufficiently pestered that in a kernel release or 3 we can
> begin enforcing those attributes. Especially as the discussion on the
> fix for those applications has already begun.
"pestering" never works, look at some of the SCSI drivers for examples
of how a distro will just patch out the "warning this driver is using an
old api and needs to be fixed" messages.
You can't break stuff like this, people will get upset :(
greg k-h
^ permalink raw reply
* [CFT][PATCH 11/10] mnt: Avoid unnecessary regressions in fs_fully_visible (take 2)
From: Eric W. Biederman @ 2015-06-04 4:35 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Kenton Varda, Serge Hallyn, Seth Forshee, Linux API,
Linux Containers, Greg Kroah-Hartman, Michael Kerrisk-manpages,
Richard Weinberger, Linux FS Devel, Tejun Heo
In-Reply-To: <87eglseboh.fsf_-_-JOvCrm2gF+uungPnsOpG7nhyD016LWXt@public.gmane.org>
Not allowing programs to clear nosuid and noexec on new mounts of
sysfs or proc will cause lxc and libvirt-lxc to fail to start (a
regression). There are no executables files on sysfs or proc today
which means clearing these flags is harmless today.
Instead of failing the fresh mounts of sysfs and proc emit a warning
when these flags are improprely cleared. We only reach this point
because lxc and libvirt-lxc clear flags they mount flags had not
intended to.
In a couple of kernel releases when lxc and libvirt-lxc have been
fixed we can start failing fresh mounts proc and sysfs that clear
nosuid and noexec. Userspace clearly means to enforce those
attributes and enforcing these attributes have historically avoided
bugs in the setattr implementations of proc and sysfs.
Cc: stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Signed-off-by: "Eric W. Biederman" <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
---
Now with warning on problematic remounts as well.
nodev is also ignored because it is not currently problematic.
fs/namespace.c | 33 +++++++++++++++++++++++++++++++++
include/linux/mount.h | 5 +++++
2 files changed, 38 insertions(+)
diff --git a/fs/namespace.c b/fs/namespace.c
index eccd925c6e82..3c3f8172c734 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -2162,6 +2162,18 @@ static int do_remount(struct path *path, int flags, int mnt_flags,
((mnt->mnt.mnt_flags & MNT_ATIME_MASK) != (mnt_flags & MNT_ATIME_MASK))) {
return -EPERM;
}
+ if ((mnt->mnt.mnt_flags & MNT_WARN_NOSUID) &&
+ !(mnt_flags & MNT_NOSUID) && printk_ratelimit()) {
+ printk(KERN_INFO
+ "warning: process `%s' clears nosuid in remount of %s\n",
+ current->comm, sb->s_type->name);
+ }
+ if ((mnt->mnt.mnt_flags & MNT_WARN_NOEXEC) &&
+ !(mnt_flags & MNT_NOEXEC) && printk_ratelimit()) {
+ printk(KERN_INFO
+ "warning: process `%s' clears noexec in remount of %s\n",
+ current->comm, sb->s_type->name);
+ }
err = security_sb_remount(sb, data);
if (err)
@@ -3201,12 +3213,14 @@ static bool fs_fully_visible(struct file_system_type *type, int *new_mnt_flags)
if ((mnt->mnt.mnt_flags & MNT_LOCK_NODEV) &&
!(new_flags & MNT_NODEV))
continue;
+#if 0 /* Avoid unnecessary regressions */
if ((mnt->mnt.mnt_flags & MNT_LOCK_NOSUID) &&
!(new_flags & MNT_NOSUID))
continue;
if ((mnt->mnt.mnt_flags & MNT_LOCK_NOEXEC) &&
!(new_flags & MNT_NOEXEC))
continue;
+#endif
if ((mnt->mnt.mnt_flags & MNT_LOCK_ATIME) &&
((mnt->mnt.mnt_flags & MNT_ATIME_MASK) != (new_flags & MNT_ATIME_MASK)))
continue;
@@ -3227,9 +3241,28 @@ static bool fs_fully_visible(struct file_system_type *type, int *new_mnt_flags)
/* Preserve the locked attributes */
*new_mnt_flags |= mnt->mnt.mnt_flags & (MNT_LOCK_READONLY | \
MNT_LOCK_NODEV | \
+ /* Avoid unnecessary regressions \
MNT_LOCK_NOSUID | \
MNT_LOCK_NOEXEC | \
+ */ \
MNT_LOCK_ATIME);
+ /* For now, warn about the "harmless" but invalid mnt flags */
+ if (mnt->mnt.mnt_flags & MNT_LOCK_NOSUID) {
+ *new_mnt_flags |= MNT_WARN_NOSUID;
+ if (!(new_flags & MNT_NOSUID) && printk_ratelimit()) {
+ printk(KERN_INFO
+ "warning: process `%s' clears nosuid in mount of %s\n",
+ current->comm, type->name);
+ }
+ }
+ if (mnt->mnt.mnt_flags & MNT_LOCK_NOEXEC) {
+ *new_mnt_flags |= MNT_WARN_NOEXEC;
+ if (!(new_flags & MNT_NOEXEC) && printk_ratelimit()) {
+ printk(KERN_INFO
+ "warning: process `%s' clears noexec in mount of %s\n",
+ current->comm, type->name);
+ }
+ }
visible = true;
goto found;
next: ;
diff --git a/include/linux/mount.h b/include/linux/mount.h
index f822c3c11377..a9ac188413fd 100644
--- a/include/linux/mount.h
+++ b/include/linux/mount.h
@@ -52,6 +52,11 @@ struct mnt_namespace;
#define MNT_INTERNAL 0x4000
+/* These warning options should be removed in a few kernel releases
+ * once userspace has been fixed.
+ */
+#define MNT_WARN_NOSUID 0x010000
+#define MNT_WARN_NOEXEC 0x020000
#define MNT_LOCK_ATIME 0x040000
#define MNT_LOCK_NOEXEC 0x080000
#define MNT_LOCK_NOSUID 0x100000
--
2.2.1
^ permalink raw reply related
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