* [PATCH v4 35/35] sched: Enable context analysis for core.c and fair.c
From: Marco Elver @ 2025-11-20 15:10 UTC (permalink / raw)
To: elver, Peter Zijlstra, Boqun Feng, Ingo Molnar, Will Deacon
Cc: David S. Miller, Luc Van Oostenryck, Chris Li, Paul E. McKenney,
Alexander Potapenko, Arnd Bergmann, Bart Van Assche,
Christoph Hellwig, Dmitry Vyukov, Eric Dumazet,
Frederic Weisbecker, Greg Kroah-Hartman, Herbert Xu, Ian Rogers,
Jann Horn, Joel Fernandes, Johannes Berg, Jonathan Corbet,
Josh Triplett, Justin Stitt, Kees Cook, Kentaro Takeda,
Lukas Bulwahn, Mark Rutland, Mathieu Desnoyers, Miguel Ojeda,
Nathan Chancellor, Neeraj Upadhyay, Nick Desaulniers,
Steven Rostedt, Tetsuo Handa, Thomas Gleixner, Thomas Graf,
Uladzislau Rezki, Waiman Long, kasan-dev, linux-crypto, linux-doc,
linux-kbuild, linux-kernel, linux-mm, linux-security-module,
linux-sparse, linux-wireless, llvm, rcu, Ingo Molnar
In-Reply-To: <20251120151033.3840508-7-elver@google.com>
This demonstrates a larger conversion to use Clang's context
analysis. The benefit is additional static checking of locking rules,
along with better documentation.
Notably, kernel/sched contains sufficiently complex synchronization
patterns, and application to core.c & fair.c demonstrates that the
latest Clang version has become powerful enough to start applying this
to more complex subsystems (with some modest annotations and changes).
Signed-off-by: Marco Elver <elver@google.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@redhat.com>
---
v4:
* Rename capability -> context analysis.
v3:
* New patch.
---
include/linux/sched.h | 6 +-
include/linux/sched/signal.h | 4 +-
include/linux/sched/task.h | 5 +-
include/linux/sched/wake_q.h | 3 +
kernel/sched/Makefile | 3 +
kernel/sched/core.c | 89 ++++++++++++------
kernel/sched/fair.c | 9 +-
kernel/sched/sched.h | 110 +++++++++++++++--------
scripts/context-analysis-suppression.txt | 1 +
9 files changed, 162 insertions(+), 68 deletions(-)
diff --git a/include/linux/sched.h b/include/linux/sched.h
index b469878de25c..9088cbda6915 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -2108,9 +2108,9 @@ static inline int _cond_resched(void)
_cond_resched(); \
})
-extern int __cond_resched_lock(spinlock_t *lock);
-extern int __cond_resched_rwlock_read(rwlock_t *lock);
-extern int __cond_resched_rwlock_write(rwlock_t *lock);
+extern int __cond_resched_lock(spinlock_t *lock) __must_hold(lock);
+extern int __cond_resched_rwlock_read(rwlock_t *lock) __must_hold_shared(lock);
+extern int __cond_resched_rwlock_write(rwlock_t *lock) __must_hold(lock);
#define MIGHT_RESCHED_RCU_SHIFT 8
#define MIGHT_RESCHED_PREEMPT_MASK ((1U << MIGHT_RESCHED_RCU_SHIFT) - 1)
diff --git a/include/linux/sched/signal.h b/include/linux/sched/signal.h
index a63f65aa5bdd..a22248aebcf9 100644
--- a/include/linux/sched/signal.h
+++ b/include/linux/sched/signal.h
@@ -738,10 +738,12 @@ static inline int thread_group_empty(struct task_struct *p)
(thread_group_leader(p) && !thread_group_empty(p))
extern struct sighand_struct *lock_task_sighand(struct task_struct *task,
- unsigned long *flags);
+ unsigned long *flags)
+ __acquires(&task->sighand->siglock);
static inline void unlock_task_sighand(struct task_struct *task,
unsigned long *flags)
+ __releases(&task->sighand->siglock)
{
spin_unlock_irqrestore(&task->sighand->siglock, *flags);
}
diff --git a/include/linux/sched/task.h b/include/linux/sched/task.h
index 525aa2a632b2..2acf9e1c4b0b 100644
--- a/include/linux/sched/task.h
+++ b/include/linux/sched/task.h
@@ -214,15 +214,18 @@ static inline struct vm_struct *task_stack_vm_area(const struct task_struct *t)
* write_lock_irq(&tasklist_lock), neither inside nor outside.
*/
static inline void task_lock(struct task_struct *p)
+ __acquires(&p->alloc_lock)
{
spin_lock(&p->alloc_lock);
}
static inline void task_unlock(struct task_struct *p)
+ __releases(&p->alloc_lock)
{
spin_unlock(&p->alloc_lock);
}
-DEFINE_GUARD(task_lock, struct task_struct *, task_lock(_T), task_unlock(_T))
+DEFINE_LOCK_GUARD_1(task_lock, struct task_struct, task_lock(_T->lock), task_unlock(_T->lock))
+DECLARE_LOCK_GUARD_1_ATTRS(task_lock, __assumes_ctx_guard(_T->alloc_lock), /* */)
#endif /* _LINUX_SCHED_TASK_H */
diff --git a/include/linux/sched/wake_q.h b/include/linux/sched/wake_q.h
index 0f28b4623ad4..765bbc3d54be 100644
--- a/include/linux/sched/wake_q.h
+++ b/include/linux/sched/wake_q.h
@@ -66,6 +66,7 @@ extern void wake_up_q(struct wake_q_head *head);
/* Spin unlock helpers to unlock and call wake_up_q with preempt disabled */
static inline
void raw_spin_unlock_wake(raw_spinlock_t *lock, struct wake_q_head *wake_q)
+ __releases(lock)
{
guard(preempt)();
raw_spin_unlock(lock);
@@ -77,6 +78,7 @@ void raw_spin_unlock_wake(raw_spinlock_t *lock, struct wake_q_head *wake_q)
static inline
void raw_spin_unlock_irq_wake(raw_spinlock_t *lock, struct wake_q_head *wake_q)
+ __releases(lock)
{
guard(preempt)();
raw_spin_unlock_irq(lock);
@@ -89,6 +91,7 @@ void raw_spin_unlock_irq_wake(raw_spinlock_t *lock, struct wake_q_head *wake_q)
static inline
void raw_spin_unlock_irqrestore_wake(raw_spinlock_t *lock, unsigned long flags,
struct wake_q_head *wake_q)
+ __releases(lock)
{
guard(preempt)();
raw_spin_unlock_irqrestore(lock, flags);
diff --git a/kernel/sched/Makefile b/kernel/sched/Makefile
index 8ae86371ddcd..b1f1a367034f 100644
--- a/kernel/sched/Makefile
+++ b/kernel/sched/Makefile
@@ -1,5 +1,8 @@
# SPDX-License-Identifier: GPL-2.0
+CONTEXT_ANALYSIS_core.o := y
+CONTEXT_ANALYSIS_fair.o := y
+
# The compilers are complaining about unused variables inside an if(0) scope
# block. This is daft, shut them up.
ccflags-y += $(call cc-disable-warning, unused-but-set-variable)
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index f754a60de848..ebfd107d17af 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -395,6 +395,8 @@ static atomic_t sched_core_count;
static struct cpumask sched_core_mask;
static void sched_core_lock(int cpu, unsigned long *flags)
+ __context_unsafe(/* acquires multiple */)
+ __acquires(&runqueues.__lock) /* overapproximation */
{
const struct cpumask *smt_mask = cpu_smt_mask(cpu);
int t, i = 0;
@@ -405,6 +407,8 @@ static void sched_core_lock(int cpu, unsigned long *flags)
}
static void sched_core_unlock(int cpu, unsigned long *flags)
+ __context_unsafe(/* releases multiple */)
+ __releases(&runqueues.__lock) /* overapproximation */
{
const struct cpumask *smt_mask = cpu_smt_mask(cpu);
int t;
@@ -629,6 +633,7 @@ EXPORT_SYMBOL(__trace_set_current_state);
*/
void raw_spin_rq_lock_nested(struct rq *rq, int subclass)
+ __context_unsafe()
{
raw_spinlock_t *lock;
@@ -654,6 +659,7 @@ void raw_spin_rq_lock_nested(struct rq *rq, int subclass)
}
bool raw_spin_rq_trylock(struct rq *rq)
+ __context_unsafe()
{
raw_spinlock_t *lock;
bool ret;
@@ -695,15 +701,16 @@ void double_rq_lock(struct rq *rq1, struct rq *rq2)
raw_spin_rq_lock(rq1);
if (__rq_lockp(rq1) != __rq_lockp(rq2))
raw_spin_rq_lock_nested(rq2, SINGLE_DEPTH_NESTING);
+ else
+ __acquire_ctx_guard(__rq_lockp(rq2)); /* fake acquire */
double_rq_clock_clear_update(rq1, rq2);
}
/*
- * __task_rq_lock - lock the rq @p resides on.
+ * ___task_rq_lock - lock the rq @p resides on.
*/
-struct rq *__task_rq_lock(struct task_struct *p, struct rq_flags *rf)
- __acquires(rq->lock)
+struct rq *___task_rq_lock(struct task_struct *p, struct rq_flags *rf)
{
struct rq *rq;
@@ -726,9 +733,7 @@ struct rq *__task_rq_lock(struct task_struct *p, struct rq_flags *rf)
/*
* task_rq_lock - lock p->pi_lock and lock the rq @p resides on.
*/
-struct rq *task_rq_lock(struct task_struct *p, struct rq_flags *rf)
- __acquires(p->pi_lock)
- __acquires(rq->lock)
+struct rq *_task_rq_lock(struct task_struct *p, struct rq_flags *rf)
{
struct rq *rq;
@@ -2463,6 +2468,7 @@ static inline bool is_cpu_allowed(struct task_struct *p, int cpu)
*/
static struct rq *move_queued_task(struct rq *rq, struct rq_flags *rf,
struct task_struct *p, int new_cpu)
+ __must_hold(__rq_lockp(rq))
{
lockdep_assert_rq_held(rq);
@@ -2509,6 +2515,7 @@ struct set_affinity_pending {
*/
static struct rq *__migrate_task(struct rq *rq, struct rq_flags *rf,
struct task_struct *p, int dest_cpu)
+ __must_hold(__rq_lockp(rq))
{
/* Affinity changed (again). */
if (!is_cpu_allowed(p, dest_cpu))
@@ -2545,6 +2552,12 @@ static int migration_cpu_stop(void *data)
*/
flush_smp_call_function_queue();
+ /*
+ * We may change the underlying rq, but the locks held will
+ * appropriately be "transferred" when switching.
+ */
+ context_unsafe_alias(rq);
+
raw_spin_lock(&p->pi_lock);
rq_lock(rq, &rf);
@@ -2654,6 +2667,8 @@ int push_cpu_stop(void *arg)
if (!lowest_rq)
goto out_unlock;
+ lockdep_assert_rq_held(lowest_rq);
+
// XXX validate p is still the highest prio task
if (task_rq(p) == rq) {
move_queued_task_locked(rq, lowest_rq, p);
@@ -2899,8 +2914,7 @@ void release_user_cpus_ptr(struct task_struct *p)
*/
static int affine_move_task(struct rq *rq, struct task_struct *p, struct rq_flags *rf,
int dest_cpu, unsigned int flags)
- __releases(rq->lock)
- __releases(p->pi_lock)
+ __releases(__rq_lockp(rq), &p->pi_lock)
{
struct set_affinity_pending my_pending = { }, *pending = NULL;
bool stop_pending, complete = false;
@@ -3055,8 +3069,7 @@ static int __set_cpus_allowed_ptr_locked(struct task_struct *p,
struct affinity_context *ctx,
struct rq *rq,
struct rq_flags *rf)
- __releases(rq->lock)
- __releases(p->pi_lock)
+ __releases(__rq_lockp(rq), &p->pi_lock)
{
const struct cpumask *cpu_allowed_mask = task_cpu_possible_mask(p);
const struct cpumask *cpu_valid_mask = cpu_active_mask;
@@ -4348,29 +4361,30 @@ static bool __task_needs_rq_lock(struct task_struct *p)
*/
int task_call_func(struct task_struct *p, task_call_f func, void *arg)
{
- struct rq *rq = NULL;
struct rq_flags rf;
int ret;
raw_spin_lock_irqsave(&p->pi_lock, rf.flags);
- if (__task_needs_rq_lock(p))
- rq = __task_rq_lock(p, &rf);
+ if (__task_needs_rq_lock(p)) {
+ struct rq *rq = __task_rq_lock(p, &rf);
- /*
- * At this point the task is pinned; either:
- * - blocked and we're holding off wakeups (pi->lock)
- * - woken, and we're holding off enqueue (rq->lock)
- * - queued, and we're holding off schedule (rq->lock)
- * - running, and we're holding off de-schedule (rq->lock)
- *
- * The called function (@func) can use: task_curr(), p->on_rq and
- * p->__state to differentiate between these states.
- */
- ret = func(p, arg);
+ /*
+ * At this point the task is pinned; either:
+ * - blocked and we're holding off wakeups (pi->lock)
+ * - woken, and we're holding off enqueue (rq->lock)
+ * - queued, and we're holding off schedule (rq->lock)
+ * - running, and we're holding off de-schedule (rq->lock)
+ *
+ * The called function (@func) can use: task_curr(), p->on_rq and
+ * p->__state to differentiate between these states.
+ */
+ ret = func(p, arg);
- if (rq)
rq_unlock(rq, &rf);
+ } else {
+ ret = func(p, arg);
+ }
raw_spin_unlock_irqrestore(&p->pi_lock, rf.flags);
return ret;
@@ -5046,6 +5060,8 @@ void balance_callbacks(struct rq *rq, struct balance_callback *head)
static inline void
prepare_lock_switch(struct rq *rq, struct task_struct *next, struct rq_flags *rf)
+ __releases(__rq_lockp(rq))
+ __acquires(__rq_lockp(this_rq()))
{
/*
* Since the runqueue lock will be released by the next
@@ -5059,9 +5075,15 @@ prepare_lock_switch(struct rq *rq, struct task_struct *next, struct rq_flags *rf
/* this is a valid case when another task releases the spinlock */
rq_lockp(rq)->owner = next;
#endif
+ /*
+ * Model the rq reference switcheroo.
+ */
+ __release(__rq_lockp(rq));
+ __acquire(__rq_lockp(this_rq()));
}
static inline void finish_lock_switch(struct rq *rq)
+ __releases(__rq_lockp(rq))
{
/*
* If we are tracking spinlock dependencies then we have to
@@ -5117,6 +5139,7 @@ static inline void kmap_local_sched_in(void)
static inline void
prepare_task_switch(struct rq *rq, struct task_struct *prev,
struct task_struct *next)
+ __must_hold(__rq_lockp(rq))
{
kcov_prepare_switch(prev);
sched_info_switch(rq, prev, next);
@@ -5148,7 +5171,7 @@ prepare_task_switch(struct rq *rq, struct task_struct *prev,
* because prev may have moved to another CPU.
*/
static struct rq *finish_task_switch(struct task_struct *prev)
- __releases(rq->lock)
+ __releases(__rq_lockp(this_rq()))
{
struct rq *rq = this_rq();
struct mm_struct *mm = rq->prev_mm;
@@ -5236,7 +5259,7 @@ static struct rq *finish_task_switch(struct task_struct *prev)
* @prev: the thread we just switched away from.
*/
asmlinkage __visible void schedule_tail(struct task_struct *prev)
- __releases(rq->lock)
+ __releases(__rq_lockp(this_rq()))
{
/*
* New tasks start with FORK_PREEMPT_COUNT, see there and
@@ -5268,6 +5291,7 @@ asmlinkage __visible void schedule_tail(struct task_struct *prev)
static __always_inline struct rq *
context_switch(struct rq *rq, struct task_struct *prev,
struct task_struct *next, struct rq_flags *rf)
+ __releases(__rq_lockp(rq))
{
prepare_task_switch(rq, prev, next);
@@ -5948,6 +5972,7 @@ static void prev_balance(struct rq *rq, struct task_struct *prev,
*/
static inline struct task_struct *
__pick_next_task(struct rq *rq, struct task_struct *prev, struct rq_flags *rf)
+ __must_hold(__rq_lockp(rq))
{
const struct sched_class *class;
struct task_struct *p;
@@ -6040,6 +6065,7 @@ static void queue_core_balance(struct rq *rq);
static struct task_struct *
pick_next_task(struct rq *rq, struct task_struct *prev, struct rq_flags *rf)
+ __must_hold(__rq_lockp(rq))
{
struct task_struct *next, *p, *max = NULL;
const struct cpumask *smt_mask;
@@ -6339,6 +6365,7 @@ static bool steal_cookie_task(int cpu, struct sched_domain *sd)
}
static void sched_core_balance(struct rq *rq)
+ __must_hold(__rq_lockp(rq))
{
struct sched_domain *sd;
int cpu = cpu_of(rq);
@@ -6484,6 +6511,7 @@ static inline void sched_core_cpu_dying(unsigned int cpu) {}
static struct task_struct *
pick_next_task(struct rq *rq, struct task_struct *prev, struct rq_flags *rf)
+ __must_hold(__rq_lockp(rq))
{
return __pick_next_task(rq, prev, rf);
}
@@ -8141,6 +8169,12 @@ static int __balance_push_cpu_stop(void *arg)
struct rq_flags rf;
int cpu;
+ /*
+ * We may change the underlying rq, but the locks held will
+ * appropriately be "transferred" when switching.
+ */
+ context_unsafe_alias(rq);
+
raw_spin_lock_irq(&p->pi_lock);
rq_lock(rq, &rf);
@@ -8168,6 +8202,7 @@ static DEFINE_PER_CPU(struct cpu_stop_work, push_work);
* effective when the hotplug motion is down.
*/
static void balance_push(struct rq *rq)
+ __must_hold(__rq_lockp(rq))
{
struct task_struct *push_task = rq->curr;
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 5b752324270b..b8cebd1f1f4d 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -2876,6 +2876,7 @@ static int preferred_group_nid(struct task_struct *p, int nid)
}
static void task_numa_placement(struct task_struct *p)
+ __context_unsafe(/* conditional locking */)
{
int seq, nid, max_nid = NUMA_NO_NODE;
unsigned long max_faults = 0;
@@ -4803,7 +4804,8 @@ static inline unsigned long cfs_rq_load_avg(struct cfs_rq *cfs_rq)
return cfs_rq->avg.load_avg;
}
-static int sched_balance_newidle(struct rq *this_rq, struct rq_flags *rf);
+static int sched_balance_newidle(struct rq *this_rq, struct rq_flags *rf)
+ __must_hold(__rq_lockp(this_rq));
static inline unsigned long task_util(struct task_struct *p)
{
@@ -6229,6 +6231,7 @@ static bool distribute_cfs_runtime(struct cfs_bandwidth *cfs_b)
* used to track this state.
*/
static int do_sched_cfs_period_timer(struct cfs_bandwidth *cfs_b, int overrun, unsigned long flags)
+ __must_hold(&cfs_b->lock)
{
int throttled;
@@ -8714,6 +8717,7 @@ static void set_cpus_allowed_fair(struct task_struct *p, struct affinity_context
static int
balance_fair(struct rq *rq, struct task_struct *prev, struct rq_flags *rf)
+ __must_hold(__rq_lockp(rq))
{
if (sched_fair_runnable(rq))
return 1;
@@ -8867,6 +8871,7 @@ static void set_next_task_fair(struct rq *rq, struct task_struct *p, bool first)
struct task_struct *
pick_next_task_fair(struct rq *rq, struct task_struct *prev, struct rq_flags *rf)
+ __must_hold(__rq_lockp(rq))
{
struct sched_entity *se;
struct task_struct *p;
@@ -8953,6 +8958,7 @@ pick_next_task_fair(struct rq *rq, struct task_struct *prev, struct rq_flags *rf
}
static struct task_struct *__pick_next_task_fair(struct rq *rq, struct task_struct *prev)
+ __must_hold(__rq_lockp(rq))
{
return pick_next_task_fair(rq, prev, NULL);
}
@@ -12784,6 +12790,7 @@ static inline void nohz_newidle_balance(struct rq *this_rq) { }
* > 0 - success, new (fair) tasks present
*/
static int sched_balance_newidle(struct rq *this_rq, struct rq_flags *rf)
+ __must_hold(__rq_lockp(this_rq))
{
unsigned long next_balance = jiffies + HZ;
int this_cpu = this_rq->cpu;
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index adfb6e3409d7..2d4b9ba30d58 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -1350,8 +1350,13 @@ static inline bool is_migration_disabled(struct task_struct *p)
DECLARE_PER_CPU_SHARED_ALIGNED(struct rq, runqueues);
+static __always_inline struct rq *__this_rq(void)
+{
+ return this_cpu_ptr(&runqueues);
+}
+
#define cpu_rq(cpu) (&per_cpu(runqueues, (cpu)))
-#define this_rq() this_cpu_ptr(&runqueues)
+#define this_rq() __this_rq()
#define task_rq(p) cpu_rq(task_cpu(p))
#define cpu_curr(cpu) (cpu_rq(cpu)->curr)
#define raw_rq() raw_cpu_ptr(&runqueues)
@@ -1396,6 +1401,7 @@ static inline raw_spinlock_t *rq_lockp(struct rq *rq)
}
static inline raw_spinlock_t *__rq_lockp(struct rq *rq)
+ __returns_ctx_guard(rq_lockp(rq)) /* alias them */
{
if (rq->core_enabled)
return &rq->core->__lock;
@@ -1492,6 +1498,7 @@ static inline raw_spinlock_t *rq_lockp(struct rq *rq)
}
static inline raw_spinlock_t *__rq_lockp(struct rq *rq)
+ __returns_ctx_guard(rq_lockp(rq)) /* alias them */
{
return &rq->__lock;
}
@@ -1534,32 +1541,42 @@ static inline bool rt_group_sched_enabled(void)
#endif /* !CONFIG_RT_GROUP_SCHED */
static inline void lockdep_assert_rq_held(struct rq *rq)
+ __assumes_ctx_guard(__rq_lockp(rq))
{
lockdep_assert_held(__rq_lockp(rq));
}
-extern void raw_spin_rq_lock_nested(struct rq *rq, int subclass);
-extern bool raw_spin_rq_trylock(struct rq *rq);
-extern void raw_spin_rq_unlock(struct rq *rq);
+extern void raw_spin_rq_lock_nested(struct rq *rq, int subclass)
+ __acquires(__rq_lockp(rq));
+
+extern bool raw_spin_rq_trylock(struct rq *rq)
+ __cond_acquires(true, __rq_lockp(rq));
+
+extern void raw_spin_rq_unlock(struct rq *rq)
+ __releases(__rq_lockp(rq));
static inline void raw_spin_rq_lock(struct rq *rq)
+ __acquires(__rq_lockp(rq))
{
raw_spin_rq_lock_nested(rq, 0);
}
static inline void raw_spin_rq_lock_irq(struct rq *rq)
+ __acquires(__rq_lockp(rq))
{
local_irq_disable();
raw_spin_rq_lock(rq);
}
static inline void raw_spin_rq_unlock_irq(struct rq *rq)
+ __releases(__rq_lockp(rq))
{
raw_spin_rq_unlock(rq);
local_irq_enable();
}
static inline unsigned long _raw_spin_rq_lock_irqsave(struct rq *rq)
+ __acquires(__rq_lockp(rq))
{
unsigned long flags;
@@ -1570,6 +1587,7 @@ static inline unsigned long _raw_spin_rq_lock_irqsave(struct rq *rq)
}
static inline void raw_spin_rq_unlock_irqrestore(struct rq *rq, unsigned long flags)
+ __releases(__rq_lockp(rq))
{
raw_spin_rq_unlock(rq);
local_irq_restore(flags);
@@ -1818,17 +1836,15 @@ static inline void rq_repin_lock(struct rq *rq, struct rq_flags *rf)
rq->clock_update_flags |= rf->clock_update_flags;
}
-extern
-struct rq *__task_rq_lock(struct task_struct *p, struct rq_flags *rf)
- __acquires(rq->lock);
+#define __task_rq_lock(...) __acquire_ret(___task_rq_lock(__VA_ARGS__), __rq_lockp(__ret))
+extern struct rq *___task_rq_lock(struct task_struct *p, struct rq_flags *rf) __acquires_ret;
-extern
-struct rq *task_rq_lock(struct task_struct *p, struct rq_flags *rf)
- __acquires(p->pi_lock)
- __acquires(rq->lock);
+#define task_rq_lock(...) __acquire_ret(_task_rq_lock(__VA_ARGS__), __rq_lockp(__ret))
+extern struct rq *_task_rq_lock(struct task_struct *p, struct rq_flags *rf)
+ __acquires(&p->pi_lock) __acquires_ret;
static inline void __task_rq_unlock(struct rq *rq, struct rq_flags *rf)
- __releases(rq->lock)
+ __releases(__rq_lockp(rq))
{
rq_unpin_lock(rq, rf);
raw_spin_rq_unlock(rq);
@@ -1836,8 +1852,7 @@ static inline void __task_rq_unlock(struct rq *rq, struct rq_flags *rf)
static inline void
task_rq_unlock(struct rq *rq, struct task_struct *p, struct rq_flags *rf)
- __releases(rq->lock)
- __releases(p->pi_lock)
+ __releases(__rq_lockp(rq), &p->pi_lock)
{
rq_unpin_lock(rq, rf);
raw_spin_rq_unlock(rq);
@@ -1848,44 +1863,45 @@ DEFINE_LOCK_GUARD_1(task_rq_lock, struct task_struct,
_T->rq = task_rq_lock(_T->lock, &_T->rf),
task_rq_unlock(_T->rq, _T->lock, &_T->rf),
struct rq *rq; struct rq_flags rf)
+DECLARE_LOCK_GUARD_1_ATTRS(task_rq_lock, __assumes_ctx_guard(_T->pi_lock), /* */)
static inline void rq_lock_irqsave(struct rq *rq, struct rq_flags *rf)
- __acquires(rq->lock)
+ __acquires(__rq_lockp(rq))
{
raw_spin_rq_lock_irqsave(rq, rf->flags);
rq_pin_lock(rq, rf);
}
static inline void rq_lock_irq(struct rq *rq, struct rq_flags *rf)
- __acquires(rq->lock)
+ __acquires(__rq_lockp(rq))
{
raw_spin_rq_lock_irq(rq);
rq_pin_lock(rq, rf);
}
static inline void rq_lock(struct rq *rq, struct rq_flags *rf)
- __acquires(rq->lock)
+ __acquires(__rq_lockp(rq))
{
raw_spin_rq_lock(rq);
rq_pin_lock(rq, rf);
}
static inline void rq_unlock_irqrestore(struct rq *rq, struct rq_flags *rf)
- __releases(rq->lock)
+ __releases(__rq_lockp(rq))
{
rq_unpin_lock(rq, rf);
raw_spin_rq_unlock_irqrestore(rq, rf->flags);
}
static inline void rq_unlock_irq(struct rq *rq, struct rq_flags *rf)
- __releases(rq->lock)
+ __releases(__rq_lockp(rq))
{
rq_unpin_lock(rq, rf);
raw_spin_rq_unlock_irq(rq);
}
static inline void rq_unlock(struct rq *rq, struct rq_flags *rf)
- __releases(rq->lock)
+ __releases(__rq_lockp(rq))
{
rq_unpin_lock(rq, rf);
raw_spin_rq_unlock(rq);
@@ -1896,18 +1912,24 @@ DEFINE_LOCK_GUARD_1(rq_lock, struct rq,
rq_unlock(_T->lock, &_T->rf),
struct rq_flags rf)
+DECLARE_LOCK_GUARD_1_ATTRS(rq_lock, __assumes_ctx_guard(__rq_lockp(_T)), /* */);
+
DEFINE_LOCK_GUARD_1(rq_lock_irq, struct rq,
rq_lock_irq(_T->lock, &_T->rf),
rq_unlock_irq(_T->lock, &_T->rf),
struct rq_flags rf)
+DECLARE_LOCK_GUARD_1_ATTRS(rq_lock_irq, __assumes_ctx_guard(__rq_lockp(_T)), /* */);
+
DEFINE_LOCK_GUARD_1(rq_lock_irqsave, struct rq,
rq_lock_irqsave(_T->lock, &_T->rf),
rq_unlock_irqrestore(_T->lock, &_T->rf),
struct rq_flags rf)
-static inline struct rq *this_rq_lock_irq(struct rq_flags *rf)
- __acquires(rq->lock)
+DECLARE_LOCK_GUARD_1_ATTRS(rq_lock_irqsave, __assumes_ctx_guard(__rq_lockp(_T)), /* */);
+
+#define this_rq_lock_irq(...) __acquire_ret(_this_rq_lock_irq(__VA_ARGS__), __rq_lockp(__ret))
+static inline struct rq *_this_rq_lock_irq(struct rq_flags *rf) __acquires_ret
{
struct rq *rq;
@@ -2908,8 +2930,13 @@ static inline void double_rq_clock_clear_update(struct rq *rq1, struct rq *rq2)
#define DEFINE_LOCK_GUARD_2(name, type, _lock, _unlock, ...) \
__DEFINE_UNLOCK_GUARD(name, type, _unlock, type *lock2; __VA_ARGS__) \
static inline class_##name##_t class_##name##_constructor(type *lock, type *lock2) \
+ __no_context_analysis \
{ class_##name##_t _t = { .lock = lock, .lock2 = lock2 }, *_T = &_t; \
_lock; return _t; }
+#define DECLARE_LOCK_GUARD_2_ATTRS(_name, _lock, _unlock) \
+static inline class_##_name##_t class_##_name##_constructor(lock_##_name##_t *_T1, \
+ lock_##_name##_t *_T2) _lock; \
+static inline void class_##_name##_destructor(class_##_name##_t *_T) _unlock
static inline bool rq_order_less(struct rq *rq1, struct rq *rq2)
{
@@ -2937,7 +2964,8 @@ static inline bool rq_order_less(struct rq *rq1, struct rq *rq2)
return rq1->cpu < rq2->cpu;
}
-extern void double_rq_lock(struct rq *rq1, struct rq *rq2);
+extern void double_rq_lock(struct rq *rq1, struct rq *rq2)
+ __acquires(__rq_lockp(rq1), __rq_lockp(rq2));
#ifdef CONFIG_PREEMPTION
@@ -2950,9 +2978,8 @@ extern void double_rq_lock(struct rq *rq1, struct rq *rq2);
* also adds more overhead and therefore may reduce throughput.
*/
static inline int _double_lock_balance(struct rq *this_rq, struct rq *busiest)
- __releases(this_rq->lock)
- __acquires(busiest->lock)
- __acquires(this_rq->lock)
+ __must_hold(__rq_lockp(this_rq))
+ __acquires(__rq_lockp(busiest))
{
raw_spin_rq_unlock(this_rq);
double_rq_lock(this_rq, busiest);
@@ -2969,12 +2996,16 @@ static inline int _double_lock_balance(struct rq *this_rq, struct rq *busiest)
* regardless of entry order into the function.
*/
static inline int _double_lock_balance(struct rq *this_rq, struct rq *busiest)
- __releases(this_rq->lock)
- __acquires(busiest->lock)
- __acquires(this_rq->lock)
+ __must_hold(__rq_lockp(this_rq))
+ __acquires(__rq_lockp(busiest))
{
- if (__rq_lockp(this_rq) == __rq_lockp(busiest) ||
- likely(raw_spin_rq_trylock(busiest))) {
+ if (__rq_lockp(this_rq) == __rq_lockp(busiest)) {
+ __acquire(__rq_lockp(busiest)); /* already held */
+ double_rq_clock_clear_update(this_rq, busiest);
+ return 0;
+ }
+
+ if (likely(raw_spin_rq_trylock(busiest))) {
double_rq_clock_clear_update(this_rq, busiest);
return 0;
}
@@ -2997,6 +3028,8 @@ static inline int _double_lock_balance(struct rq *this_rq, struct rq *busiest)
* double_lock_balance - lock the busiest runqueue, this_rq is locked already.
*/
static inline int double_lock_balance(struct rq *this_rq, struct rq *busiest)
+ __must_hold(__rq_lockp(this_rq))
+ __acquires(__rq_lockp(busiest))
{
lockdep_assert_irqs_disabled();
@@ -3004,14 +3037,17 @@ static inline int double_lock_balance(struct rq *this_rq, struct rq *busiest)
}
static inline void double_unlock_balance(struct rq *this_rq, struct rq *busiest)
- __releases(busiest->lock)
+ __releases(__rq_lockp(busiest))
{
if (__rq_lockp(this_rq) != __rq_lockp(busiest))
raw_spin_rq_unlock(busiest);
+ else
+ __release(__rq_lockp(busiest)); /* fake release */
lock_set_subclass(&__rq_lockp(this_rq)->dep_map, 0, _RET_IP_);
}
static inline void double_lock(spinlock_t *l1, spinlock_t *l2)
+ __acquires(l1, l2)
{
if (l1 > l2)
swap(l1, l2);
@@ -3021,6 +3057,7 @@ static inline void double_lock(spinlock_t *l1, spinlock_t *l2)
}
static inline void double_lock_irq(spinlock_t *l1, spinlock_t *l2)
+ __acquires(l1, l2)
{
if (l1 > l2)
swap(l1, l2);
@@ -3030,6 +3067,7 @@ static inline void double_lock_irq(spinlock_t *l1, spinlock_t *l2)
}
static inline void double_raw_lock(raw_spinlock_t *l1, raw_spinlock_t *l2)
+ __acquires(l1, l2)
{
if (l1 > l2)
swap(l1, l2);
@@ -3039,6 +3077,7 @@ static inline void double_raw_lock(raw_spinlock_t *l1, raw_spinlock_t *l2)
}
static inline void double_raw_unlock(raw_spinlock_t *l1, raw_spinlock_t *l2)
+ __releases(l1, l2)
{
raw_spin_unlock(l1);
raw_spin_unlock(l2);
@@ -3048,6 +3087,8 @@ DEFINE_LOCK_GUARD_2(double_raw_spinlock, raw_spinlock_t,
double_raw_lock(_T->lock, _T->lock2),
double_raw_unlock(_T->lock, _T->lock2))
+DECLARE_LOCK_GUARD_2_ATTRS(double_raw_spinlock, __assumes_ctx_guard(_T1) __assumes_ctx_guard(_T2), /* */);
+
/*
* double_rq_unlock - safely unlock two runqueues
*
@@ -3055,13 +3096,12 @@ DEFINE_LOCK_GUARD_2(double_raw_spinlock, raw_spinlock_t,
* you need to do so manually after calling.
*/
static inline void double_rq_unlock(struct rq *rq1, struct rq *rq2)
- __releases(rq1->lock)
- __releases(rq2->lock)
+ __releases(__rq_lockp(rq1), __rq_lockp(rq2))
{
if (__rq_lockp(rq1) != __rq_lockp(rq2))
raw_spin_rq_unlock(rq2);
else
- __release(rq2->lock);
+ __release(__rq_lockp(rq2)); /* fake release */
raw_spin_rq_unlock(rq1);
}
diff --git a/scripts/context-analysis-suppression.txt b/scripts/context-analysis-suppression.txt
index df25c3d07a5b..fd8951d06706 100644
--- a/scripts/context-analysis-suppression.txt
+++ b/scripts/context-analysis-suppression.txt
@@ -26,6 +26,7 @@ src:*include/linux/refcount.h=emit
src:*include/linux/rhashtable.h=emit
src:*include/linux/rwlock*.h=emit
src:*include/linux/rwsem.h=emit
+src:*include/linux/sched*=emit
src:*include/linux/seqlock*.h=emit
src:*include/linux/spinlock*.h=emit
src:*include/linux/srcu*.h=emit
--
2.52.0.rc1.455.g30608eb744-goog
^ permalink raw reply related
* Re: [BUG] landlock: sleeping function called from invalid context in hook_sb_delete()
From: Mickaël Salaün @ 2025-11-20 14:48 UTC (permalink / raw)
To: Günther Noack
Cc: 许佳凯, linux-kernel, linux-security-module,
Günther Noack, Serge E. Hallyn
In-Reply-To: <20251120.c5c17c664315@gnoack.org>
Thanks for the report.
This was indeed a false positive. The fix (previously reported by
syzkaller) is merged in -next and soon in the master branche:
https://lore.kernel.org/r/20251105212025.807549-1-mjguzik@gmail.com
On Thu, Nov 20, 2025 at 09:48:43AM +0100, Günther Noack wrote:
> Hello!
>
> Thanks for the report!
>
> CC-ing Mickaël, who authored that code
>
> On Wed, Nov 12, 2025 at 10:35:17AM +0800, 许佳凯 wrote:
> > The call trace indicates that hook_sb_delete() holds s_inode_list_lock (a spinlock) while invoking operations that may eventually call iput(), which can sleep.
> > This violates the locking context expectations and triggers __might_sleep() warnings.
> > The issue seems to be related to how Landlock handles superblock cleanup during security_sb_delete().
> >
> >
> > I’m currently only reporting this issue to the community; the exact fix will likely need to be confirmed and implemented by the Landlock and filesystem maintainers.
>
> This looks like a false positive to me.
>
> There are three places where iput() is being called in hook_sb_delete,
> two of them are in places where it is *not* holding the
> s_inode_list_lock. The one that *is* holding the s_inode_list_lock
> has the following comment:
>
> /*
> * At this point, we own the ihold() reference that was
> * originally set up by get_inode_object() and the
> * __iget() reference that we just set in this loop
> * walk. Therefore the following call to iput() will
> * not sleep nor drop the inode because there is now at
> * least two references to it.
> */
>
> That seems to indicate that the sleepability concern was taken into
> consideration. iput() only sleeps if the refcount reaches zero, and
> if you can exclude that, it won't sleep.
>
> —Günther
>
> --
>
^ permalink raw reply
* Re: [Patch V1] ima: avoid duplicate policy rules insertions
From: Mimi Zohar @ 2025-11-20 15:39 UTC (permalink / raw)
To: Tahera Fahimi, roberto.sassu, dmitry.kasatkin, eric.snowberg,
paul, jmorris, serge, linux-integrity, linux-security-module,
linux-kernel, code
Cc: Lennart Poettering
In-Reply-To: <16c73b4e0761a1622770102d1d48982fe9ae86ac.camel@linux.ibm.com>
On Tue, 2025-11-11 at 06:40 -0500, Mimi Zohar wrote:
> [Cc'ing Lennart Poettering]
>
> On Thu, 2025-11-06 at 18:14 +0000, Tahera Fahimi wrote:
> > Prevent redundant IMA policy rules by checking for duplicates before insertion. This ensures that
> > rules are not re-added when userspace is restarted (using systemd-soft-reboot) without a full system
> > reboot. ima_rule_exists() detects duplicates in both temporary and active rule lists.
> >
> > Signed-off-by: Tahera Fahimi <taherafahimi@linux.microsoft.com>
>
> Sorry for the delay in responding ...
>
> Before trying to fix the "problem", let's try to understand it first. At least
> on my test system (-rc5), kexec is working as designed. On boot, systemd
> replaces the existing builtin IMA policy with a custom IMA policy. The arch
> specific policies are not affected, as they are persistent. On a soft reboot
> (kexec), the IMA custom policy is re-loaded as expected.
>
> To verify the above behavior, extend the IMA policy before the soft reboot.
> Notice after the soft reboot that the original custom IMA policy is loaded and
> not the extended IMA policy. Roberto, if there is a problem with this behavior,
> we'll discuss it independently of this proposed patch.
>
> The question is why are you seeing duplicate IMA policy rules? What is special
> about your environment?
I'm now able to reproduce what you're seeing by executing "systemctl soft-
reboot". After each soft-reboot the custom IMA policy rules are re-loaded and
appended to the existing IMA policy. As Roberto mentioned, there is no option
to replace the existing custom IMA policy rules with a new custom policy. We
might consider doing so in the future, but for now the onus for not re-loading
the custom IMA policy should not be on IMA, but on systemd.
Reading the same securityfs policy file used for loading the IMA policy will
return the current IMA policy. systemd could detect whether the existing IMA
policy contains the custom policy rules, before actually loading them. There's
no reason for adding this functionality in the kernel.
--
thanks,
Mimi
^ permalink raw reply
* Re: [PATCH RFC 0/4] landlock: add LANDLOCK_SCOPE_MEMFD_EXEC execution
From: Mickaël Salaün @ 2025-11-20 15:42 UTC (permalink / raw)
To: Abhinav Saxena
Cc: Günther Noack, Paul Moore, James Morris, Serge E. Hallyn,
Shuah Khan, Nathan Chancellor, Nick Desaulniers, Bill Wendling,
Justin Stitt, linux-security-module, linux-kernel,
linux-kselftest, llvm, Daniel Verkamp, Jeff Xu,
Thiébaud Weksteen, Stephen Smalley
In-Reply-To: <878qhy2lch.fsf@gmail.com>
On Sun, Sep 28, 2025 at 05:37:02PM -0600, Abhinav Saxena wrote:
> Thanks for the detailed reply Mickaël!
>
> Mickaël Salaün <mic@digikod.net> writes:
>
> > Thanks for this patch series Abhinav! The code looks good overall, but
> > we should clarify the design. Sorry for the delayed response, it is on
> > my radar now.
Please feel free to ping me after two weeks without answer, I might miss
some emails.
> >
> > CCing Jeff and Daniel
> >
> > On Sat, Jul 19, 2025 at 05:13:10AM -0600, Abhinav Saxena wrote:
> >> This patch series introduces LANDLOCK_SCOPE_MEMFD_EXEC, a new Landlock
> >> scoping mechanism that restricts execution of anonymous memory file
> >> descriptors (memfd) created via memfd_create(2). This addresses security
> >> gaps where processes can bypass W^X policies and execute arbitrary code
> >> through anonymous memory objects.
> >>
> >> Fixes: <https://github.com/landlock-lsm/linux/issues/37>
> >>
> >> SECURITY PROBLEM
> >> `=============='
> >>
> >> Current Landlock filesystem restrictions do not cover memfd objects,
> >> allowing processes to:
> >>
> >> 1. Read-to-execute bypass: Create writable memfd, inject code,
> >> then execute via mmap(PROT_EXEC) or direct execve()
> >> 2. Anonymous execution: Execute code without touching the filesystem via
> >> execve(“/proc/self/fd/N”) where N is a memfd descriptor
> >
> >> 3. Cross-domain access violations: Pass memfd between processes to
> >> bypass domain restrictions
> >
> > Landlock only restricts access at open time, which is a useful property.
> > This enables to create more restricted sandboxes but still get access to
> > outside resources via trusted processes. If the process passing the FDs
> > is not trusted, the sandboxed process could just ask to execute
> > arbitrary code outside the sandbox anyway.
> >
> > However, the Landlock scopes are designed to block IPC from within a
> > sandbox to outside the sandbox. We could have a new scope to forbid a
> > sandbox process to receive or inherit file descriptors, but that would
> > be a different and generic feature. For compatibility reasons, this
> > might not be easy to implement and I think there are more important
> > features to implement before that.
> >
> > Thinking more about it, restricting memfd should not be a “scoped” flag
> > because the semantic is not the same, but we should have a new ruleset
> > property instead, something like “ruleset.denied” with a related
> > LANDLOCK_DENY_EXECUTE_MEMFD flag. This flag will only have an impact on
> > newly created memfd from a sandboxed process with this restriction at
> > creation time. This could be implemented with hook_file_alloc_security()
> > by checking if the file is indeed a memfd and checking inode->i_mode for
> > executability bits (which would imply MFD_NOEXEC_SEAL).
> >
>
> Thanks for the clarification! So if I understood correctly we are
> proposing adding a `denied` field to the `landlock_ruleset_attr` struct
>
> struct landlock_ruleset_attr {
> __u64 handled_access_fs;
> __u64 handled_access_net;
> __u64 scoped;
> __u64 denied; /* New field */
> };
>
> which allows memfd_create() to be allowed by default unless
> LANDLOCK_DENY_EXECUTE_MEMFD bit is set.
Yes
> Also it seems Thiébaud
> Weksteen’s patch[1] will land, and maybe we can use
> security_inode_init_security_anon instead? What do you think?
Definitely, and this patch is now merged in -next.
>
> Apologies for my ignorance, do we have to wait till his patch has
> landed into Linus’s tree?
As long as you explain this dependency in the commit message and point
to the patch (as a comment, after a "---" line), we're good.
>
> >>
> >> These scenarios can occur in sandboxed environments where filesystem
> >> access is restricted but memfd creation remains possible.
> >>
> >> IMPLEMENTATION
> >> `============'
> >>
> >> The implementation adds hierarchical execution control through domain
> >> scoping:
> >>
> >> Core Components:
> >> - is_memfd_file(): Reliable memfd detection via “memfd:” dentry prefix
> >> - domain_is_scoped(): Cross-domain hierarchy checking (moved to domain.c)
> >> - LSM hooks: mmap_file, file_mprotect, bprm_creds_for_exec
> >> - Creation-time restrictions: hook_file_alloc_security
> >>
> >> Security Matrix:
> >> Execution decisions follow domain hierarchy rules preventing both
> >> same-domain bypass attempts and cross-domain access violations while
> >> preserving legitimate hierarchical access patterns.
> >>
> >> Domain Hierarchy with LANDLOCK_SCOPE_MEMFD_EXEC:
> >> `============================================='
> >>
> >> Root (no domain) - No restrictions
> >> |
> >> +– Domain A [SCOPE_MEMFD_EXEC] Layer 1
> >> | +– memfd_A (tagged with Domain A as creator)
> >> | |
> >> | +– Domain A1 (child) [NO SCOPE] Layer 2
> >> | | +– Inherits Layer 1 restrictions from parent
> >> | | +– memfd_A1 (can create, inherits restrictions)
> >> | | +– Domain A1a [SCOPE_MEMFD_EXEC] Layer 3
> >> | | +– memfd_A1a (tagged with Domain A1a)
> >> | |
> >> | +– Domain A2 (child) [SCOPE_MEMFD_EXEC] Layer 2
> >> | +– memfd_A2 (tagged with Domain A2 as creator)
> >> | +– CANNOT access memfd_A1 (different subtree)
> >> |
> >> +– Domain B [SCOPE_MEMFD_EXEC] Layer 1
> >> +– memfd_B (tagged with Domain B as creator)
> >> +– CANNOT access ANY memfd from Domain A subtree
> >>
> >> Execution Decision Matrix:
> >> `======================'
> >> Executor-> | A | A1 | A1a | A2 | B | Root
> >> Creator | | | | | |
> >> ————|—–|—-|—–|—-|—-|—–
> >> Domain A | X | X | X | X | X | Y
> >> Domain A1 | Y | X | X | X | X | Y
> >> Domain A1a | Y | Y | X | X | X | Y
> >> Domain A2 | Y | X | X | X | X | Y
> >> Domain B | X | X | X | X | X | Y
> >> Root | Y | Y | Y | Y | Y | Y
> >>
> >> Legend: Y = Execution allowed, X = Execution denied
> >
> > Because checks should not be related to scopes, this will be much
> > simpler.
> >
> >>
> >> Scenarios Covered:
> >> - Direct mmap(PROT_EXEC) on memfd files
> >> - Two-stage mmap(PROT_READ) + mprotect(PROT_EXEC) bypass attempts
> >> - execve("/proc/self/fd/N") anonymous execution
> >> - execveat() and fexecve() file descriptor execution
> >> - Cross-process memfd inheritance and IPC passing
> >>
> >> TESTING
> >> `====='
> >>
> >> All patches have been validated with:
> >> - scripts/checkpatch.pl –strict (clean)
> >> - Selftests covering same-domain restrictions, cross-domain
> >> hierarchy enforcement, and regular file isolation
> >> - KUnit tests for memfd detection edge cases
> >
> > Thanks for all these tests!
> >
> >>
> >> DISCLAIMER
> >> `========'
> >>
> >> My understanding of Landlock scoping semantics may be limited, but this
> >> implementation reflects my current understanding based on available
> >> documentation and code analysis. I welcome feedback and corrections
> >> regarding the scoping logic and domain hierarchy enforcement.
> >>
> >> Signed-off-by: Abhinav Saxena <xandfury@gmail.com>
> >> —
> >> Abhinav Saxena (4):
> >> landlock: add LANDLOCK_SCOPE_MEMFD_EXEC scope
> >> landlock: implement memfd detection
> >> landlock: add memfd exec LSM hooks and scoping
> >> selftests/landlock: add memfd execution tests
> >>
> >> include/uapi/linux/landlock.h | 5 +
> >> security/landlock/.kunitconfig | 1 +
> >> security/landlock/audit.c | 4 +
> >> security/landlock/audit.h | 1 +
> >> security/landlock/cred.c | 14 -
> >> security/landlock/domain.c | 67 ++++
> >> security/landlock/domain.h | 4 +
> >> security/landlock/fs.c | 405 ++++++++++++++++++++-
> >> security/landlock/limits.h | 2 +-
> >> security/landlock/task.c | 67 —-
> >> …/selftests/landlock/scoped_memfd_exec_test.c | 325 +++++++++++++++++
> >> 11 files changed, 812 insertions(+), 83 deletions(-)
> >> —
> >> base-commit: 5b74b2eff1eeefe43584e5b7b348c8cd3b723d38
> >> change-id: 20250716-memfd-exec-ac0d582018c3
> >>
> >> Best regards,
> >> –
> >> Abhinav Saxena <xandfury@gmail.com>
> >>
> >>
>
> Best,
> Abhinav
>
> [1] - <https://lore.kernel.org/all/20250918020434.1612137-1-tweek@google.com/>
^ permalink raw reply
* Re: [PATCH v18] exec: Fix dead-lock in de_thread with ptrace_attach
From: Eric W. Biederman @ 2025-11-20 15:15 UTC (permalink / raw)
To: Bernd Edlinger
Cc: Alexander Viro, Alexey Dobriyan, Oleg Nesterov, Kees Cook,
Andy Lutomirski, Will Drewry, Christian Brauner, Andrew Morton,
Michal Hocko, Serge Hallyn, James Morris, Randy Dunlap,
Suren Baghdasaryan, Yafang Shao, Helge Deller, Adrian Reber,
Thomas Gleixner, Jens Axboe, Alexei Starovoitov,
linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-kselftest, linux-mm, linux-security-module, tiozhang,
Luis Chamberlain, Paulo Alcantara (SUSE), Sergey Senozhatsky,
Frederic Weisbecker, YueHaibing, Paul Moore, Aleksa Sarai,
Stefan Roesch, Chao Yu, xu xin, Jeff Layton, Jan Kara,
David Hildenbrand, Dave Chinner, Shuah Khan, Elena Reshetova,
David Windsor, Mateusz Guzik, Ard Biesheuvel,
Joel Fernandes (Google), Matthew Wilcox (Oracle),
Hans Liljestrand, Penglei Jiang, Lorenzo Stoakes, Adrian Ratiu,
Ingo Molnar, Peter Zijlstra (Intel), Cyrill Gorcunov,
Eric Dumazet
In-Reply-To: <GV2PPF74270EBEEE807D016A79FE7A2F463E4D6A@GV2PPF74270EBEE.EURP195.PROD.OUTLOOK.COM>
Bernd Edlinger <bernd.edlinger@hotmail.de> writes:
> This introduces signal->exec_bprm, which is used to
> fix the case when at least one of the sibling threads
> is traced, and therefore the trace process may dead-lock
> in ptrace_attach, but de_thread will need to wait for the
> tracer to continue execution.
A small quibble it isn't a dead lock. It isn't even really a live lock,
as it is possible to SIGKILL our way out.
Thinking about this there is a really silly and simple way we can deal
with this situation for PTRACE_ATTACH. We can send SIGSTOP and wait for
the thread to stop before doing anything with cred_guard_mutex.
PTRACE_ATTACH already implies sending SIGSTOP so as long as we have
enough permissions to send SIGSTOP I don't see that being a problem.
The worst case I can see is that we get a case where we stop the
process, the permission check fails under cred_guard_mutex and
and ptrace attach has fails and has to send SIGCONT to undo it's
premature SIGSTOP. That might almost be visible, but it would still
be legitimate because we can still check that we have permission to
send SIGSTOP.
Eric
^ permalink raw reply
* Re: [PATCH v18] exec: Fix dead-lock in de_thread with ptrace_attach
From: Eric W. Biederman @ 2025-11-20 17:29 UTC (permalink / raw)
To: Bernd Edlinger
Cc: Alexander Viro, Alexey Dobriyan, Oleg Nesterov, Kees Cook,
Andy Lutomirski, Will Drewry, Christian Brauner, Andrew Morton,
Michal Hocko, Serge Hallyn, James Morris, Randy Dunlap,
Suren Baghdasaryan, Yafang Shao, Helge Deller, Adrian Reber,
Thomas Gleixner, Jens Axboe, Alexei Starovoitov,
linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-kselftest, linux-mm, linux-security-module, tiozhang,
Luis Chamberlain, Paulo Alcantara (SUSE), Sergey Senozhatsky,
Frederic Weisbecker, YueHaibing, Paul Moore, Aleksa Sarai,
Stefan Roesch, Chao Yu, xu xin, Jeff Layton, Jan Kara,
David Hildenbrand, Dave Chinner, Shuah Khan, Elena Reshetova,
David Windsor, Mateusz Guzik, Ard Biesheuvel,
Joel Fernandes (Google), Matthew Wilcox (Oracle),
Hans Liljestrand, Penglei Jiang, Lorenzo Stoakes, Adrian Ratiu,
Ingo Molnar, Peter Zijlstra (Intel), Cyrill Gorcunov,
Eric Dumazet
In-Reply-To: <87tsyozqdu.fsf@email.froward.int.ebiederm.org>
"Eric W. Biederman" <ebiederm@xmission.com> writes:
> Bernd Edlinger <bernd.edlinger@hotmail.de> writes:
>
>> This introduces signal->exec_bprm, which is used to
>> fix the case when at least one of the sibling threads
>> is traced, and therefore the trace process may dead-lock
>> in ptrace_attach, but de_thread will need to wait for the
>> tracer to continue execution.
>
> A small quibble it isn't a dead lock. It isn't even really a live lock,
> as it is possible to SIGKILL our way out.
>
> Thinking about this there is a really silly and simple way we can deal
> with this situation for PTRACE_ATTACH. We can send SIGSTOP and wait for
> the thread to stop before doing anything with cred_guard_mutex.
>
> PTRACE_ATTACH already implies sending SIGSTOP so as long as we have
> enough permissions to send SIGSTOP I don't see that being a problem.
>
> The worst case I can see is that we get a case where we stop the
> process, the permission check fails under cred_guard_mutex and
> and ptrace attach has fails and has to send SIGCONT to undo it's
> premature SIGSTOP. That might almost be visible, but it would still
> be legitimate because we can still check that we have permission to
> send SIGSTOP.
Bah no I am full of it.
The challenging behavior is in the semantics of the kernel operations.
We need to describe it as such please.
It is the same class of problem as a single threaded process calls exec
with a pipe attached to both stdin and stdout of the new process.
For the stdin and stdout we can say just use pull and nonblocking I/O.
The problem is that both PTRACE_ATTACH and PTRACE_SEIZE block over
the duration of exec, and if exec is waiting for a thread to exit,
and that thread is blocked in PTRACE_EVENT_EXIT waiting for that very
same tracer those processes will hang. Not deadlock.
I haven't seen anyone clearly describe the problem lately so I am
repeating it.
Just looking at the code I don't think there is any fundamental reason
to call commit_creds after de_thread. If we can change that we can sort
this out without any change in userspace semantics.
If we can't move commit_creds we have to either give
PTRACE_ATTACH/PTRACE_SEIZE a non-block mode, or break out of
PTRACE_EVENT_EXIT in de_thread.
I will post a proof of concept of moving commit_creds in just a minute.
Eric
^ permalink raw reply
* Re: [PATCH v4 02/35] compiler-context-analysis: Add infrastructure for Context Analysis with Clang
From: Linus Torvalds @ 2025-11-20 18:14 UTC (permalink / raw)
To: Marco Elver
Cc: Peter Zijlstra, Boqun Feng, Ingo Molnar, Will Deacon,
David S. Miller, Luc Van Oostenryck, Chris Li, Paul E. McKenney,
Alexander Potapenko, Arnd Bergmann, Bart Van Assche,
Christoph Hellwig, Dmitry Vyukov, Eric Dumazet,
Frederic Weisbecker, Greg Kroah-Hartman, Herbert Xu, Ian Rogers,
Jann Horn, Joel Fernandes, Johannes Berg, Jonathan Corbet,
Josh Triplett, Justin Stitt, Kees Cook, Kentaro Takeda,
Lukas Bulwahn, Mark Rutland, Mathieu Desnoyers, Miguel Ojeda,
Nathan Chancellor, Neeraj Upadhyay, Nick Desaulniers,
Steven Rostedt, Tetsuo Handa, Thomas Gleixner, Thomas Graf,
Uladzislau Rezki, Waiman Long, kasan-dev, linux-crypto, linux-doc,
linux-kbuild, linux-kernel, linux-mm, linux-security-module,
linux-sparse, linux-wireless, llvm, rcu
In-Reply-To: <20251120145835.3833031-4-elver@google.com>
On Thu, 20 Nov 2025 at 07:13, Marco Elver <elver@google.com> wrote:
>
> --- a/include/linux/compiler-context-analysis.h
> +++ b/include/linux/compiler-context-analysis.h
> @@ -6,27 +6,465 @@
> #ifndef _LINUX_COMPILER_CONTEXT_ANALYSIS_H
> #define _LINUX_COMPILER_CONTEXT_ANALYSIS_H
>
> +#if defined(WARN_CONTEXT_ANALYSIS)
Note the 400+ added lines to this header...
And then note how the header gets used:
> +++ b/scripts/Makefile.context-analysis
> @@ -0,0 +1,7 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +context-analysis-cflags := -DWARN_CONTEXT_ANALYSIS \
> + -fexperimental-late-parse-attributes -Wthread-safety \
> + -Wthread-safety-pointer -Wthread-safety-beta
> +
> +export CFLAGS_CONTEXT_ANALYSIS := $(context-analysis-cflags)
Please let's *not* do it this way, where the header contents basically
get enabled or not based on a compiler flag, but then everybody
includes this 400+ line file whether they need it or not.
Can we please just make the header file *itself* not have any
conditionals, and what happens is that the header file is included (or
not) using a pattern something like
-include $(srctree)/include/linux/$(context-analysis-header)
instead.
IOW, we'd have three different header files entirely: the "no context
analysis", the "sparse" and the "clang context analysis" header, and
instead of having a "-DWARN_CONTEXT_ANALYSIS" define, we'd just
include the appropriate header automatically.
We already use that "-include" pattern for <linux/kconfig.h> and
<linux/compiler-version.h>. It's probably what we should have done for
<linux/compiler.h> and friends too.
The reason I react to things like this is that I've actually seen just
the parsing of header files being a surprisingly big cost in build
times. People think that optimizations are expensive, and yes, some of
them really are, but when a lot of the code we parse is never actually
*used*, but just hangs out in header files that gets included by
everybody, the parsing overhead tends to be noticeable. There's a
reason why most C compilers end up integrating the C pre-processor: it
avoids parsing and tokenizing things multiple times.
The other reason is that I often use "git grep" for looking up
definitions of things, and when there are multiple definitions of the
same thing, I actually find it much more informative when they are in
two different files than when I see two different definitions (or
declarations) in the same file and then I have to go look at what the
#ifdef condition is. In contrast, when it's something where there are
per-architecture definitions, you *see* that, because the grep results
come from different header files.
I dunno. This is not a huge deal, but I do think that it would seem to
be much simpler and more straightforward to treat this as a kind of "N
different baseline header files" than as "include this one header file
in everything, and then we'll have #ifdef's for the configuration".
Particularly when that config is not even a global config, but a per-file one.
Hmm? Maybe there's some reason why this suggestion is very
inconvenient, but please at least consider it.
Linus
^ permalink raw reply
* Re: [PATCH v2] lsm: use unrcu_pointer() for current->cred in security_init()
From: Paul Moore @ 2025-11-20 21:27 UTC (permalink / raw)
To: Xiu Jianfeng; +Cc: linux-security-module
In-Reply-To: <f3631b3b-4461-4176-9178-27fa1dc66226@huawei.com>
On Thu, Nov 20, 2025 at 2:48 AM Xiu Jianfeng <xiujianfeng@huawei.com> wrote:
> On 11/19/2025 11:36 PM, Paul Moore wrote:
> > We need to directly allocate the cred's LSM state for the initial task
> > when we initialize the LSM framework. Unfortunately, this results in a
> > RCU related type mismatch, use the unrcu_pointer() macro to handle this
> > a bit more elegantly.
> >
> > The explicit type casting still remains as we need to work around the
> > constification of current->cred in this particular case.
> >
> > Signed-off-by: Paul Moore <paul@paul-moore.com>
>
> Reviewed-by: Xiu Jianfeng <xiujianfeng@huawei.com>
Thanks for the review, I just merged this into lsm/dev.
--
paul-moore.com
^ permalink raw reply
* [RFC][PATCH] exec: Move cred computation under exec_update_lock
From: Eric W. Biederman @ 2025-11-20 20:57 UTC (permalink / raw)
To: Bernd Edlinger
Cc: Alexander Viro, Alexey Dobriyan, Oleg Nesterov, Kees Cook,
Andy Lutomirski, Will Drewry, Christian Brauner, Andrew Morton,
Michal Hocko, Serge Hallyn, James Morris, Randy Dunlap,
Suren Baghdasaryan, Yafang Shao, Helge Deller, Adrian Reber,
Thomas Gleixner, Jens Axboe, Alexei Starovoitov,
linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-kselftest, linux-mm, linux-security-module, tiozhang,
Luis Chamberlain, Paulo Alcantara (SUSE), Sergey Senozhatsky,
Frederic Weisbecker, YueHaibing, Paul Moore, Aleksa Sarai,
Stefan Roesch, Chao Yu, xu xin, Jeff Layton, Jan Kara,
David Hildenbrand, Dave Chinner, Shuah Khan, Elena Reshetova,
David Windsor, Mateusz Guzik, Ard Biesheuvel,
Joel Fernandes (Google), Matthew Wilcox (Oracle),
Hans Liljestrand, Penglei Jiang, Lorenzo Stoakes, Adrian Ratiu,
Ingo Molnar, Peter Zijlstra (Intel), Cyrill Gorcunov,
Eric Dumazet
In-Reply-To: <87wm3ky5n9.fsf@email.froward.int.ebiederm.org>
Instead of computing the new cred before we pass the point of no
return compute the new cred just before we use it.
This allows the removal of fs_struct->in_exec and cred_guard_mutex.
I am not certain why we wanted to compute the cred for the new
executable so early. Perhaps I missed something but I did not see any
common errors being signaled. So I don't think we loose anything by
computing the new cred later.
We gain a lot.
We stop holding the cred_guard_mutex over places where the code sleeps
and waits for userspace. These places include the waiting for the
tracer in PTRACE_EVENT_EXIT, "put_user(0, tsk->clear_child_tid)" in
mm_release, and "get_user(futex_offset, ...") in exit_robust_mutex.
We can remove fs_struct->in_exec. The case where it was used simply
never comes up, when we compute the cred after de_thread completes.
We remove the possibility of a hang between a tracer calling
PTRACE_ATTACH/PTRACE_SIEZE and the kernel waiting for the tracer
in PTRACE_EVENT_EXIT.
---
Oleg, Kees, Bernd, Can you see anything I am missing?
The code compiles but I haven't test it yet.
I thought I was going to move commit_creds before de_thread, but that
would have taken commit_cred out of exec_update_lock (which introduces
races).
However I can't see any drawbacks of going the other direction.
fs/exec.c | 88 ++++++++++++++----------------------
fs/fs_struct.c | 1 -
fs/proc/base.c | 4 +-
include/linux/fs_struct.h | 1 -
include/linux/sched/signal.h | 6 ---
init/init_task.c | 1 -
kernel/cred.c | 2 +-
kernel/fork.c | 8 +---
kernel/ptrace.c | 4 +-
kernel/seccomp.c | 12 ++---
10 files changed, 45 insertions(+), 82 deletions(-)
diff --git a/fs/exec.c b/fs/exec.c
index 4298e7e08d5d..5ae96584dab0 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1090,6 +1090,9 @@ void __set_task_comm(struct task_struct *tsk, const char *buf, bool exec)
perf_event_comm(tsk, exec);
}
+static int prepare_bprm_creds(struct linux_binprm *bprm);
+static void check_unsafe_exec(struct linux_binprm *bprm);
+
/*
* Calling this is the point of no return. None of the failures will be
* seen by userspace since either the process is already taking a fatal
@@ -1101,10 +1104,6 @@ int begin_new_exec(struct linux_binprm * bprm)
struct task_struct *me = current;
int retval;
- /* Once we are committed compute the creds */
- retval = bprm_creds_from_file(bprm);
- if (retval)
- return retval;
/*
* This tracepoint marks the point before flushing the old exec where
@@ -1123,8 +1122,6 @@ int begin_new_exec(struct linux_binprm * bprm)
retval = de_thread(me);
if (retval)
goto out;
- /* see the comment in check_unsafe_exec() */
- current->fs->in_exec = 0;
/*
* Cancel any io_uring activity across execve
*/
@@ -1251,6 +1248,25 @@ int begin_new_exec(struct linux_binprm * bprm)
WRITE_ONCE(me->self_exec_id, me->self_exec_id + 1);
flush_signal_handlers(me, 0);
+ retval = prepare_bprm_creds(bprm);
+ if (retval)
+ goto out_unlock;
+
+ /*
+ * Check for unsafe execution states before exec_binprm(), which
+ * will call back into begin_new_exec(), into bprm_creds_from_file(),
+ * where setuid-ness is evaluated.
+ */
+ check_unsafe_exec(bprm);
+
+ /* Set the unchanging part of bprm->cred */
+ retval = security_bprm_creds_for_exec(bprm);
+
+ /* Once we are committed compute the creds */
+ retval = bprm_creds_from_file(bprm);
+ if (retval)
+ goto out_unlock;
+
retval = set_cred_ucounts(bprm->cred);
if (retval < 0)
goto out_unlock;
@@ -1272,9 +1288,9 @@ int begin_new_exec(struct linux_binprm * bprm)
if (get_dumpable(me->mm) != SUID_DUMP_USER)
perf_event_exit_task(me);
/*
- * cred_guard_mutex must be held at least to this point to prevent
+ * exec_update_lock must be held at least to this point to prevent
* ptrace_attach() from altering our determination of the task's
- * credentials; any time after this it may be unlocked.
+ * credentials.
*/
security_bprm_committed_creds(bprm);
@@ -1291,8 +1307,6 @@ int begin_new_exec(struct linux_binprm * bprm)
out_unlock:
up_write(&me->signal->exec_update_lock);
- if (!bprm->cred)
- mutex_unlock(&me->signal->cred_guard_mutex);
out:
return retval;
@@ -1336,7 +1350,6 @@ void setup_new_exec(struct linux_binprm * bprm)
*/
me->mm->task_size = TASK_SIZE;
up_write(&me->signal->exec_update_lock);
- mutex_unlock(&me->signal->cred_guard_mutex);
}
EXPORT_SYMBOL(setup_new_exec);
@@ -1351,21 +1364,15 @@ void finalize_exec(struct linux_binprm *bprm)
EXPORT_SYMBOL(finalize_exec);
/*
- * Prepare credentials and lock ->cred_guard_mutex.
- * setup_new_exec() commits the new creds and drops the lock.
- * Or, if exec fails before, free_bprm() should release ->cred
- * and unlock.
+ * Prepare credentials. begin_new_exec() commits the new creds.
+ * Or, if exec fails before, free_bprm() should release ->cred.
*/
static int prepare_bprm_creds(struct linux_binprm *bprm)
{
- if (mutex_lock_interruptible(¤t->signal->cred_guard_mutex))
- return -ERESTARTNOINTR;
-
bprm->cred = prepare_exec_creds();
if (likely(bprm->cred))
return 0;
- mutex_unlock(¤t->signal->cred_guard_mutex);
return -ENOMEM;
}
@@ -1386,9 +1393,7 @@ static void free_bprm(struct linux_binprm *bprm)
}
free_arg_pages(bprm);
if (bprm->cred) {
- /* in case exec fails before de_thread() succeeds */
- current->fs->in_exec = 0;
- mutex_unlock(¤t->signal->cred_guard_mutex);
+ /* in case exec fails before commit_creds succeeds */
abort_creds(bprm->cred);
}
do_close_execat(bprm->file);
@@ -1486,13 +1491,12 @@ EXPORT_SYMBOL(bprm_change_interp);
/*
* determine how safe it is to execute the proposed program
- * - the caller must hold ->cred_guard_mutex to protect against
+ * - the caller must hold ->exec_update_lock to protect against
* PTRACE_ATTACH or seccomp thread-sync
*/
static void check_unsafe_exec(struct linux_binprm *bprm)
{
- struct task_struct *p = current, *t;
- unsigned n_fs;
+ struct task_struct *p = current;
if (p->ptrace)
bprm->unsafe |= LSM_UNSAFE_PTRACE;
@@ -1509,25 +1513,9 @@ static void check_unsafe_exec(struct linux_binprm *bprm)
* suid exec because the differently privileged task
* will be able to manipulate the current directory, etc.
* It would be nice to force an unshare instead...
- *
- * Otherwise we set fs->in_exec = 1 to deny clone(CLONE_FS)
- * from another sub-thread until de_thread() succeeds, this
- * state is protected by cred_guard_mutex we hold.
*/
- n_fs = 1;
- read_seqlock_excl(&p->fs->seq);
- rcu_read_lock();
- for_other_threads(p, t) {
- if (t->fs == p->fs)
- n_fs++;
- }
- rcu_read_unlock();
-
- /* "users" and "in_exec" locked for copy_fs() */
- if (p->fs->users > n_fs)
+ if (p->fs->users > 1)
bprm->unsafe |= LSM_UNSAFE_SHARE;
- else
- p->fs->in_exec = 1;
read_sequnlock_excl(&p->fs->seq);
}
@@ -1731,25 +1719,15 @@ static int bprm_execve(struct linux_binprm *bprm)
{
int retval;
- retval = prepare_bprm_creds(bprm);
- if (retval)
- return retval;
+ if (bprm->is_check)
+ return 0;
- /*
- * Check for unsafe execution states before exec_binprm(), which
- * will call back into begin_new_exec(), into bprm_creds_from_file(),
- * where setuid-ness is evaluated.
- */
- check_unsafe_exec(bprm);
current->in_execve = 1;
sched_mm_cid_before_execve(current);
sched_exec();
- /* Set the unchanging part of bprm->cred */
- retval = security_bprm_creds_for_exec(bprm);
- if (retval || bprm->is_check)
- goto out;
+
retval = exec_binprm(bprm);
if (retval < 0)
diff --git a/fs/fs_struct.c b/fs/fs_struct.c
index 28be762ac1c6..945bc0916f65 100644
--- a/fs/fs_struct.c
+++ b/fs/fs_struct.c
@@ -109,7 +109,6 @@ struct fs_struct *copy_fs_struct(struct fs_struct *old)
/* We don't need to lock fs - think why ;-) */
if (fs) {
fs->users = 1;
- fs->in_exec = 0;
seqlock_init(&fs->seq);
fs->umask = old->umask;
diff --git a/fs/proc/base.c b/fs/proc/base.c
index 6299878e3d97..7041fb4d1689 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -2834,14 +2834,14 @@ static ssize_t proc_pid_attr_write(struct file * file, const char __user * buf,
}
/* Guard against adverse ptrace interaction */
- rv = mutex_lock_interruptible(¤t->signal->cred_guard_mutex);
+ rv = down_write_killable(¤t->signal->exec_update_lock);
if (rv < 0)
goto out_free;
rv = security_setprocattr(PROC_I(inode)->op.lsmid,
file->f_path.dentry->d_name.name, page,
count);
- mutex_unlock(¤t->signal->cred_guard_mutex);
+ up_write(¤t->signal->exec_update_lock);
out_free:
kfree(page);
out:
diff --git a/include/linux/fs_struct.h b/include/linux/fs_struct.h
index baf200ab5c77..29d0f7d57743 100644
--- a/include/linux/fs_struct.h
+++ b/include/linux/fs_struct.h
@@ -10,7 +10,6 @@ struct fs_struct {
int users;
seqlock_t seq;
int umask;
- int in_exec;
struct path root, pwd;
} __randomize_layout;
diff --git a/include/linux/sched/signal.h b/include/linux/sched/signal.h
index 7d6449982822..7e9259c8fb2b 100644
--- a/include/linux/sched/signal.h
+++ b/include/linux/sched/signal.h
@@ -241,12 +241,6 @@ struct signal_struct {
struct mm_struct *oom_mm; /* recorded mm when the thread group got
* killed by the oom killer */
- struct mutex cred_guard_mutex; /* guard against foreign influences on
- * credential calculations
- * (notably. ptrace)
- * Deprecated do not use in new code.
- * Use exec_update_lock instead.
- */
struct rw_semaphore exec_update_lock; /* Held while task_struct is
* being updated during exec,
* and may have inconsistent
diff --git a/init/init_task.c b/init/init_task.c
index a55e2189206f..4813bffe217e 100644
--- a/init/init_task.c
+++ b/init/init_task.c
@@ -30,7 +30,6 @@ static struct signal_struct init_signals = {
#ifdef CONFIG_CGROUPS
.cgroup_threadgroup_rwsem = __RWSEM_INITIALIZER(init_signals.cgroup_threadgroup_rwsem),
#endif
- .cred_guard_mutex = __MUTEX_INITIALIZER(init_signals.cred_guard_mutex),
.exec_update_lock = __RWSEM_INITIALIZER(init_signals.exec_update_lock),
#ifdef CONFIG_POSIX_TIMERS
.posix_timers = HLIST_HEAD_INIT,
diff --git a/kernel/cred.c b/kernel/cred.c
index dbf6b687dc5c..80e376ce005f 100644
--- a/kernel/cred.c
+++ b/kernel/cred.c
@@ -252,7 +252,7 @@ EXPORT_SYMBOL(prepare_creds);
/*
* Prepare credentials for current to perform an execve()
- * - The caller must hold ->cred_guard_mutex
+ * - The caller must hold ->exec_update_lock
*/
struct cred *prepare_exec_creds(void)
{
diff --git a/kernel/fork.c b/kernel/fork.c
index 3da0f08615a9..996c649b9a4c 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1555,11 +1555,6 @@ static int copy_fs(u64 clone_flags, struct task_struct *tsk)
if (clone_flags & CLONE_FS) {
/* tsk->fs is already what we want */
read_seqlock_excl(&fs->seq);
- /* "users" and "in_exec" locked for check_unsafe_exec() */
- if (fs->in_exec) {
- read_sequnlock_excl(&fs->seq);
- return -EAGAIN;
- }
fs->users++;
read_sequnlock_excl(&fs->seq);
return 0;
@@ -1699,7 +1694,6 @@ static int copy_signal(u64 clone_flags, struct task_struct *tsk)
sig->oom_score_adj = current->signal->oom_score_adj;
sig->oom_score_adj_min = current->signal->oom_score_adj_min;
- mutex_init(&sig->cred_guard_mutex);
init_rwsem(&sig->exec_update_lock);
return 0;
@@ -1710,7 +1704,7 @@ static void copy_seccomp(struct task_struct *p)
#ifdef CONFIG_SECCOMP
/*
* Must be called with sighand->lock held, which is common to
- * all threads in the group. Holding cred_guard_mutex is not
+ * all threads in the group. Holding exec_update_lock is not
* needed because this new task is not yet running and cannot
* be racing exec.
*/
diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index 75a84efad40f..8140d4bfc279 100644
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -444,8 +444,8 @@ static int ptrace_attach(struct task_struct *task, long request,
* SUID, SGID and LSM creds get determined differently
* under ptrace.
*/
- scoped_cond_guard (mutex_intr, return -ERESTARTNOINTR,
- &task->signal->cred_guard_mutex) {
+ scoped_cond_guard (rwsem_read_intr, return -ERESTARTNOINTR,
+ &task->signal->exec_update_lock) {
scoped_guard (task_lock, task) {
retval = __ptrace_may_access(task, PTRACE_MODE_ATTACH_REALCREDS);
diff --git a/kernel/seccomp.c b/kernel/seccomp.c
index 25f62867a16d..87de8d47d876 100644
--- a/kernel/seccomp.c
+++ b/kernel/seccomp.c
@@ -479,7 +479,7 @@ static int is_ancestor(struct seccomp_filter *parent,
/**
* seccomp_can_sync_threads: checks if all threads can be synchronized
*
- * Expects sighand and cred_guard_mutex locks to be held.
+ * Expects sighand and exec_update_lock locks to be held.
*
* Returns 0 on success, -ve on error, or the pid of a thread which was
* either not in the correct seccomp mode or did not have an ancestral
@@ -489,7 +489,7 @@ static inline pid_t seccomp_can_sync_threads(void)
{
struct task_struct *thread, *caller;
- BUG_ON(!mutex_is_locked(¤t->signal->cred_guard_mutex));
+ BUG_ON(!rwsem_is_locked(¤t->signal->exec_update_lock));
assert_spin_locked(¤t->sighand->siglock);
/* Validate all threads being eligible for synchronization. */
@@ -590,7 +590,7 @@ void seccomp_filter_release(struct task_struct *tsk)
*
* @flags: SECCOMP_FILTER_FLAG_* flags to set during sync.
*
- * Expects sighand and cred_guard_mutex locks to be held, and for
+ * Expects sighand and exec_update_lock locks to be held, and for
* seccomp_can_sync_threads() to have returned success already
* without dropping the locks.
*
@@ -599,7 +599,7 @@ static inline void seccomp_sync_threads(unsigned long flags)
{
struct task_struct *thread, *caller;
- BUG_ON(!mutex_is_locked(¤t->signal->cred_guard_mutex));
+ BUG_ON(!rwsem_is_locked(¤t->signal->exec_update_lock));
assert_spin_locked(¤t->sighand->siglock);
/*
@@ -2011,7 +2011,7 @@ static long seccomp_set_mode_filter(unsigned int flags,
* while another thread is in the middle of calling exec.
*/
if (flags & SECCOMP_FILTER_FLAG_TSYNC &&
- mutex_lock_killable(¤t->signal->cred_guard_mutex))
+ down_read_killable(¤t->signal->exec_update_lock))
goto out_put_fd;
spin_lock_irq(¤t->sighand->siglock);
@@ -2034,7 +2034,7 @@ static long seccomp_set_mode_filter(unsigned int flags,
out:
spin_unlock_irq(¤t->sighand->siglock);
if (flags & SECCOMP_FILTER_FLAG_TSYNC)
- mutex_unlock(¤t->signal->cred_guard_mutex);
+ up_read(¤t->signal->exec_update_lock);
out_put_fd:
if (flags & SECCOMP_FILTER_FLAG_NEW_LISTENER) {
if (ret) {
--
2.41.0
^ permalink raw reply related
* [PATCH v2 0/6] Implement LANDLOCK_ADD_RULE_NO_INHERIT
From: Justin Suess @ 2025-11-20 22:23 UTC (permalink / raw)
To: linux-security-module
Cc: Tingmao Wang, Günther Noack, Jan Kara, Abhinav Saxena,
Mickaël Salaün, Justin Suess
Hi,
This series introduces a new Landlock rule flag,
LANDLOCK_ADD_RULE_NO_INHERIT, which disables inheriting access rights
from parent objects within the same Landlock layer.
This is useful for policies where a parent directory requires broader
access than its children. For example, a sandbox may allow read-write
access to /home/user but prohibit writes to ~/.bashrc or ~/.ssh even
though they are nested inside the parent directory. Today, this is not
possible because access rights always propagate from parent to child
within a layer.
When a rule is added with LANDLOCK_ADD_RULE_NO_INHERIT:
* access rights on parent inodes are ignored for that inode and its
descendants; and
* operations that change the direct parent subtree of such an object
(rename, rmdir, link) are denied up to the mountpoint.
These parent-directory restrictions mitigate sandbox-restart attacks.
Without them, a sandboxed program could move a protected directory
before exit, causing the next sandbox instance to apply policies on the
wrong path. Denying such operations prevents these attacks.
This v2 series differs from v1 in the following ways:
1. Adds four new selftests for the new flag.
2. Adds a KUnit test for access layer propagation with flags.
2. Extends parent-directory protections (rename/unlink/link) to prevent
sandbox-restart attacks.
3. Rebased on v4 of Tingmao Wang's amazing "quiet flag" series.
For convenience, the original v1 is here:
https://lore.kernel.org/linux-security-module/20251105180019.1432367-1-utilityemal77@gmail.com/T/#t
And the quiet-flag v4 series is here:
https://lore.kernel.org/linux-security-module/cover.1763330228.git.m@maowtm.org/T/#t
A short usage example:
# LL_FS_RO="" LL_FS_RW="/" LL_FS_RO_NO_INHERIT="/a/b/c" landlock-sandboxer sh
# touch /a/b/c/fi # denied (does not inherit / RW)
# rmdir /a/b/c # denied due to parent-directory protections
# mv /a /bad # denied
# mkdir /a/good; touch /a/good/fi # allowed (unrelated to /a/b/c)
All kunit and selftests related to landlock, including the new ones, pass on my test system.
Feedback is very welcome. Thank you for your time and consideration.
Regards,
Justin Suess
Justin Suess (6):
landlock: Implement LANDLOCK_ADD_RULE_NO_INHERIT
landlock: Implement LANDLOCK_ADD_RULE_NO_INHERIT userspace api
samples/landlock: Add LANDLOCK_ADD_RULE_NO_INHERIT to
landlock-sandboxer
selftests/landlock: Implement selftests for
LANDLOCK_ADD_RULE_NO_INHERIT
landlock: Fix compilation error for kunit tests when CONFIG_AUDIT is
disabled.
landlock: Implement KUnit test for LANDLOCK_ADD_RULE_NO_INHERIT
include/uapi/linux/landlock.h | 9 +
samples/landlock/sandboxer.c | 39 +-
security/landlock/audit.h | 3 +-
security/landlock/fs.c | 433 ++++++++++++++++++++-
security/landlock/net.h | 2 +-
security/landlock/object.h | 2 +
security/landlock/ruleset.c | 171 +++++++-
security/landlock/ruleset.h | 39 ++
security/landlock/syscalls.c | 5 +-
tools/testing/selftests/landlock/fs_test.c | 222 ++++++++++-
10 files changed, 883 insertions(+), 42 deletions(-)
base-commit: d74278e895485ffe1c3708f7f2bc5c5b364aab0a
--
2.51.2
^ permalink raw reply
* [PATCH 1/6] landlock: Implement LANDLOCK_ADD_RULE_NO_INHERIT
From: Justin Suess @ 2025-11-20 22:23 UTC (permalink / raw)
To: linux-security-module
Cc: Tingmao Wang, Günther Noack, Jan Kara, Abhinav Saxena,
Mickaël Salaün, Justin Suess
In-Reply-To: <20251120222346.1157004-1-utilityemal77@gmail.com>
Implements a flag to prevent access grant inheritance within the filesystem hierarchy
for landlock rules.
If a landlock rule on an inode has this flag, any access grants on parent inodes will be
ignored. Moreover, operations that involve altering the direct parent tree of the subject with
LANDLOCK_ADD_RULE_NO_INHERIT will be denied up to the mountpoint.
For example, if /a/b/c/ = read only + LANDLOCK_ADD_RULE_NO_INHERIT and / = read write, writes to
files in /a/b/c will be denied. Moreover, moving /a to /bad, removing /a/b/c, or creating links to
/a will be prohibited.
The parent directory restrictions mitigate sandbox-restart attacks. For example, if a sandboxed program
is able to move a LANDLOCK_ADD_RULE_NO_INHERIT restricted directory, upon sandbox restart, the policy
applied naively on the same filenames would be invalid. Preventing these operations mitigates these attacks.
Signed-off-by: Justin Suess <utilityemal77@gmail.com>
---
security/landlock/fs.c | 431 ++++++++++++++++++++++++++++++++++--
security/landlock/object.h | 2 +
security/landlock/ruleset.c | 84 ++++++-
security/landlock/ruleset.h | 39 ++++
4 files changed, 539 insertions(+), 17 deletions(-)
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index 5fb2c9810e6b..ebeee080ea7a 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -14,6 +14,7 @@
#include <linux/atomic.h>
#include <linux/bitops.h>
#include <linux/bits.h>
+#include <linux/compiler.h>
#include <linux/compiler_types.h>
#include <linux/dcache.h>
#include <linux/err.h>
@@ -23,6 +24,7 @@
#include <linux/kernel.h>
#include <linux/limits.h>
#include <linux/list.h>
+#include <linux/lockdep.h>
#include <linux/lsm_audit.h>
#include <linux/lsm_hooks.h>
#include <linux/mount.h>
@@ -36,6 +38,7 @@
#include <linux/types.h>
#include <linux/wait_bit.h>
#include <linux/workqueue.h>
+#include <linux/xarray.h>
#include <uapi/linux/fiemap.h>
#include <uapi/linux/landlock.h>
@@ -317,21 +320,152 @@ static struct landlock_object *get_inode_object(struct inode *const inode)
LANDLOCK_ACCESS_FS_IOCTL_DEV)
/* clang-format on */
+static inline layer_mask_t landlock_layer_bit(
+ const struct landlock_layer *layer, u32 layer_index)
+{
+ u32 level = layer->level;
+
+ if (!level)
+ level = layer_index + 1;
+ return BIT_ULL(level - 1);
+}
+
+static const struct landlock_rule *find_rule(
+ const struct landlock_ruleset *const domain,
+ const struct dentry *const dentry);
+
+static layer_mask_t get_ruleset_cached_no_inherit_layers(
+ const struct landlock_ruleset *const ruleset,
+ struct landlock_object *object)
+{
+ struct landlock_no_inherit_desc *desc;
+
+ if (!ruleset || !object)
+ return 0;
+
+ /* xa_load doesn't accept const, but we're only reading */
+ desc = xa_load(&((struct landlock_ruleset *)ruleset)->no_inherit_desc,
+ (unsigned long)object);
+ if (!desc)
+ return 0;
+ return desc->desc_layers;
+}
+
+static layer_mask_t get_no_inherit_desc_layers_for_dentry(
+ const struct landlock_ruleset *const ruleset,
+ struct dentry *const dentry)
+{
+ const struct landlock_rule *rule;
+ struct landlock_object *object;
+ layer_mask_t layers = 0;
+ u32 layer_index;
+
+ if (!ruleset || !dentry || d_is_negative(dentry))
+ return 0;
+
+ rcu_read_lock();
+ object = rcu_dereference(landlock_inode(d_backing_inode(dentry))->object);
+ if (object)
+ layers |= get_ruleset_cached_no_inherit_layers(ruleset, object);
+ rcu_read_unlock();
+
+ rule = find_rule(ruleset, dentry);
+ if (!rule)
+ return layers;
+
+ for (layer_index = 0; layer_index < rule->num_layers; layer_index++) {
+ const struct landlock_layer *layer = &rule->layers[layer_index];
+
+ if (!layer->flags.no_inherit &&
+ !layer->flags.has_no_inherit_descendant)
+ continue;
+ layers |= landlock_layer_bit(layer, layer_index);
+ }
+
+ return layers;
+}
+
+static void mark_no_inherit_ancestors(struct landlock_ruleset *ruleset,
+ struct dentry *dentry,
+ layer_mask_t descendant_layers);
+
+static bool mask_no_inherit_descendant_layers(
+ const struct landlock_ruleset *const domain,
+ struct dentry *const dentry,
+ layer_mask_t child_layers,
+ const access_mask_t access_request,
+ layer_mask_t (*const layer_masks)[LANDLOCK_NUM_ACCESS_FS],
+ struct collected_rule_flags *const rule_flags)
+{
+ layer_mask_t descendant_layers;
+ const unsigned long access_req = access_request;
+ unsigned long access_bit;
+ bool changed = false;
+
+ if (!access_request || !layer_masks || !rule_flags || !dentry)
+ return false;
+ if (d_is_negative(dentry))
+ return false;
+
+ descendant_layers = get_no_inherit_desc_layers_for_dentry(domain, dentry);
+ {
+ layer_mask_t shared_layers = descendant_layers & child_layers;
+
+ if (shared_layers) {
+ rule_flags->no_inherit_masks |= shared_layers;
+ rule_flags->no_inherit_desc_masks |= shared_layers;
+ }
+ }
+ descendant_layers &= ~child_layers;
+ descendant_layers &= ~rule_flags->no_inherit_masks;
+ if (!descendant_layers)
+ return false;
+
+ for_each_set_bit(access_bit, &access_req,
+ ARRAY_SIZE(*layer_masks)) {
+ layer_mask_t *const layer_mask = &(*layer_masks)[access_bit];
+
+ if (*layer_mask & descendant_layers) {
+ *layer_mask &= ~descendant_layers;
+ changed = true;
+ }
+ }
+
+ if (!changed)
+ return false;
+
+ rule_flags->no_inherit_masks |= descendant_layers;
+ rule_flags->no_inherit_desc_masks |= descendant_layers;
+
+ return true;
+}
+
/*
* @path: Should have been checked by get_path_from_fd().
*/
int landlock_append_fs_rule(struct landlock_ruleset *const ruleset,
- const struct path *const path,
- access_mask_t access_rights, const int flags)
+ const struct path *const path,
+ access_mask_t access_rights, const int flags)
{
int err;
+ const bool is_dir = d_is_dir(path->dentry);
struct landlock_id id = {
.type = LANDLOCK_KEY_INODE,
};
/* Files only get access rights that make sense. */
- if (!d_is_dir(path->dentry) &&
- (access_rights | ACCESS_FILE) != ACCESS_FILE)
+ if (!is_dir) {
+ const access_mask_t file_rights = access_rights & ACCESS_FILE;
+
+ /*
+ * Reject purely directory-only access on files, but allow
+ * metadata-only quiet rules.
+ */
+ if (!file_rights && !(flags & LANDLOCK_ADD_RULE_QUIET))
+ return -EINVAL;
+ access_rights = file_rights;
+ }
+ if ((flags & LANDLOCK_ADD_RULE_NO_INHERIT) && !is_dir)
return -EINVAL;
if (WARN_ON_ONCE(ruleset->num_layers != 1))
return -EINVAL;
@@ -344,6 +478,39 @@ int landlock_append_fs_rule(struct landlock_ruleset *const ruleset,
return PTR_ERR(id.key.object);
mutex_lock(&ruleset->lock);
err = landlock_insert_rule(ruleset, id, access_rights, flags);
+ if (!err && (flags & LANDLOCK_ADD_RULE_NO_INHERIT)) {
+ const struct landlock_rule *rule;
+ layer_mask_t descendant_layers = 0;
+ u32 layer_index;
+
+ rule = find_rule(ruleset, path->dentry);
+ if (rule) {
+ for (layer_index = 0; layer_index < rule->num_layers; layer_index++) {
+ const struct landlock_layer *layer =
+ &rule->layers[layer_index];
+
+ if (layer->flags.no_inherit ||
+ layer->flags.has_no_inherit_descendant)
+ descendant_layers |=
+ landlock_layer_bit(layer, layer_index);
+ }
+ if (descendant_layers)
+ mark_no_inherit_ancestors(ruleset, path->dentry,
+ descendant_layers);
+ }
+ }
+
+ /*
+ * Register this object in the xarray even if it doesn't currently have
+ * descendant no-inherit layers, so collect_no_inherit_layers() can walk
+ * ancestors reliably when checking topology changes.
+ */
+ if (!err) {
+ const struct landlock_rule *rule = find_rule(ruleset, path->dentry);
+
+ if (rule)
+ landlock_set_no_inherit_desc_layers(ruleset, id.key.object, 0);
+ }
mutex_unlock(&ruleset->lock);
/*
* No need to check for an error because landlock_insert_rule()
@@ -382,6 +549,102 @@ find_rule(const struct landlock_ruleset *const domain,
return rule;
}
+
+static void mark_no_inherit_ancestors(struct landlock_ruleset *ruleset,
+ struct dentry *dentry,
+ layer_mask_t descendant_layers)
+{
+ struct dentry *cursor;
+ u32 layer_index;
+
+ if (!ruleset || !dentry || !descendant_layers)
+ return;
+
+ lockdep_assert_held(&ruleset->lock);
+
+ cursor = dget(dentry);
+ while (cursor) {
+ struct dentry *parent;
+
+ if (IS_ROOT(cursor)) {
+ dput(cursor);
+ break;
+ }
+
+ parent = dget_parent(cursor);
+ dput(cursor);
+ if (!parent)
+ break;
+
+ if (!d_is_negative(parent)) {
+ const struct landlock_rule *rule;
+ struct landlock_object *parent_object;
+
+ rule = find_rule(ruleset, parent);
+ if (rule) {
+ struct landlock_rule *mutable_rule =
+ (struct landlock_rule *)rule;
+
+ for (layer_index = 0;
+ layer_index < mutable_rule->num_layers;
+ layer_index++) {
+ struct landlock_layer *layer =
+ &mutable_rule->layers[layer_index];
+ layer_mask_t layer_bit =
+ landlock_layer_bit(layer, layer_index);
+
+ if (descendant_layers & layer_bit)
+ layer->flags.has_no_inherit_descendant = true;
+ }
+ }
+
+ parent_object = get_inode_object(d_backing_inode(parent));
+ if (!IS_ERR(parent_object)) {
+ landlock_set_no_inherit_desc_layers(ruleset, parent_object,
+ descendant_layers);
+ landlock_put_object(parent_object);
+ }
+ }
+
+ cursor = parent;
+ }
+}
+
+static layer_mask_t get_no_inherit_layers_for_target(
+ const struct landlock_ruleset *const ruleset,
+ struct dentry *const dentry,
+ const bool include_descendants)
+{
+ struct landlock_object *object;
+ const struct landlock_rule *rule;
+ layer_mask_t layers = 0;
+ u32 layer_index;
+
+ if (!ruleset || !dentry || d_is_negative(dentry))
+ return 0;
+
+ rcu_read_lock();
+ object = rcu_dereference(landlock_inode(d_backing_inode(dentry))->object);
+ if (object && include_descendants)
+ layers |= get_ruleset_cached_no_inherit_layers(ruleset, object);
+ rcu_read_unlock();
+
+ rule = find_rule(ruleset, dentry);
+ if (!rule)
+ return layers;
+
+ for (layer_index = 0; layer_index < rule->num_layers; layer_index++) {
+ const struct landlock_layer *layer = &rule->layers[layer_index];
+
+ if (layer->flags.no_inherit ||
+ (include_descendants &&
+ layer->flags.has_no_inherit_descendant))
+ layers |= landlock_layer_bit(layer, layer_index);
+ }
+
+ return layers;
+}
+
/*
* Allows access to pseudo filesystems that will never be mountable (e.g.
* sockfs, pipefs), but can still be reachable through
@@ -778,6 +1041,13 @@ static bool is_access_to_paths_allowed(
layer_mask_t(*layer_masks_child1)[LANDLOCK_NUM_ACCESS_FS] = NULL,
(*layer_masks_child2)[LANDLOCK_NUM_ACCESS_FS] = NULL;
struct collected_rule_flags _rule_flag_parent1_bkp, _rule_flag_parent2_bkp;
+ layer_mask_t child1_layers = 0;
+ layer_mask_t child2_layers = 0;
+
+ if (dentry_child1)
+ child1_layers = get_no_inherit_desc_layers_for_dentry(domain, dentry_child1);
+ if (dentry_child2)
+ child2_layers = get_no_inherit_desc_layers_for_dentry(domain, dentry_child2);
if (!access_request_parent1 && !access_request_parent2)
return true;
@@ -976,8 +1246,12 @@ static bool is_access_to_paths_allowed(
memcpy(&_rule_flag_parent2_bkp,
rule_flags_parent2,
sizeof(_rule_flag_parent2_bkp));
- is_dom_check_bkp = is_dom_check;
}
+ is_dom_check_bkp = is_dom_check;
+ child1_layers = get_no_inherit_desc_layers_for_dentry(
+ domain, walker_path.dentry);
+ if (layer_masks_parent2)
+ child2_layers = child1_layers;
/* Ignores hidden mount points. */
goto jump_up;
@@ -1001,15 +1275,48 @@ static bool is_access_to_paths_allowed(
break;
}
- /*
- * We reached a disconnected root directory from a bind mount, and
- * we need to reset the walk to the current mount root.
- */
- goto reset_to_mount_root;
- }
- parent_dentry = dget_parent(walker_path.dentry);
- dput(walker_path.dentry);
- walker_path.dentry = parent_dentry;
+ /*
+ * We reached a disconnected root directory from a bind mount, and
+ * we need to reset the walk to the current mount root.
+ */
+ goto reset_to_mount_root;
+ }
+ if (likely(!d_is_negative(walker_path.dentry))) {
+ child1_layers = get_no_inherit_desc_layers_for_dentry(
+ domain, walker_path.dentry);
+ if (layer_masks_parent2)
+ child2_layers = child1_layers;
+ } else {
+ child1_layers = 0;
+ if (layer_masks_parent2)
+ child2_layers = 0;
+ }
+ parent_dentry = dget_parent(walker_path.dentry);
+ dput(walker_path.dentry);
+ walker_path.dentry = parent_dentry;
+ /*
+ * Apply descendant no-inherit masking now that we've moved to the
+ * parent. This ensures the parent respects any no-inherit rules from
+ * the child we just left. Only applies to refer operations (rename/link).
+ */
+ if (unlikely(layer_masks_parent2)) {
+ if (mask_no_inherit_descendant_layers(
+ domain, walker_path.dentry, child1_layers,
+ access_masked_parent1,
+ layer_masks_parent1, rule_flags_parent1))
+ allowed_parent1 =
+ allowed_parent1 ||
+ is_layer_masks_allowed(layer_masks_parent1);
+
+ if (rule_flags_parent2 &&
+ mask_no_inherit_descendant_layers(
+ domain, walker_path.dentry, child2_layers,
+ access_masked_parent2,
+ layer_masks_parent2, rule_flags_parent2))
+ allowed_parent2 =
+ allowed_parent2 ||
+ is_layer_masks_allowed(layer_masks_parent2);
+ }
continue;
reset_to_mount_root:
@@ -1057,6 +1364,10 @@ static bool is_access_to_paths_allowed(
dput(walker_path.dentry);
walker_path.dentry = walker_path.mnt->mnt_root;
dget(walker_path.dentry);
+ child1_layers = get_no_inherit_desc_layers_for_dentry(
+ domain, walker_path.dentry);
+ if (layer_masks_parent2)
+ child2_layers = child1_layers;
}
path_put(&walker_path);
@@ -1083,7 +1394,7 @@ static bool is_access_to_paths_allowed(
}
static int current_check_access_path(const struct path *const path,
- access_mask_t access_request)
+ access_mask_t access_request)
{
const struct access_masks masks = {
.fs = access_request,
@@ -1102,7 +1413,7 @@ static int current_check_access_path(const struct path *const path,
LANDLOCK_KEY_INODE);
if (is_access_to_paths_allowed(subject->domain, path, access_request,
&layer_masks, &rule_flags, &request,
- NULL, 0, NULL, NULL, NULL, NULL))
+ NULL, 0, NULL, NULL, NULL, NULL))
return 0;
landlock_log_denial(subject, &request, rule_flags);
@@ -1233,6 +1544,65 @@ static bool collect_domain_accesses(
return ret;
}
+static layer_mask_t collect_no_inherit_layers(
+ const struct landlock_ruleset *domain, struct dentry *dentry)
+{
+ struct dentry *cursor, *parent;
+ layer_mask_t layers = 0;
+ bool include_descendants = true;
+
+ if (!domain || !dentry)
+ return 0;
+
+ cursor = dget(dentry);
+ if (!cursor)
+ return 0;
+
+ while (true) {
+ layers = get_no_inherit_layers_for_target(domain, cursor,
+ include_descendants);
+ if (layers || IS_ROOT(cursor) || d_is_negative(cursor))
+ break;
+
+ parent = dget_parent(cursor);
+ dput(cursor);
+ if (!parent)
+ return layers;
+ cursor = parent;
+ include_descendants = false;
+ }
+
+ dput(cursor);
+ return layers;
+}
+
+static int deny_no_inherit_topology_change(
+ const struct landlock_cred_security *subject,
+ struct dentry *dentry)
+{
+ layer_mask_t sealed_layers;
+ unsigned long layer_index;
+
+ if (!subject || !dentry || d_is_negative(dentry))
+ return 0;
+
+ sealed_layers = collect_no_inherit_layers(subject->domain, dentry);
+ if (!sealed_layers)
+ return 0;
+
+ layer_index = __ffs((unsigned long)sealed_layers);
+ landlock_log_denial(subject, &(struct landlock_request) {
+ .type = LANDLOCK_REQUEST_FS_CHANGE_TOPOLOGY,
+ .audit = {
+ .type = LSM_AUDIT_DATA_DENTRY,
+ .u.dentry = dentry,
+ },
+ .layer_plus_one = layer_index + 1,
+ }, no_rule_flags);
+
+ return -EACCES;
+}
+
/**
* current_check_refer_path - Check if a rename or link action is allowed
*
@@ -1319,6 +1689,16 @@ static int current_check_refer_path(struct dentry *const old_dentry,
access_request_parent2 =
get_mode_access(d_backing_inode(old_dentry)->i_mode);
if (removable) {
+ int err;
+
+ err = deny_no_inherit_topology_change(subject, old_dentry);
+ if (err)
+ return err;
+ if (exchange) {
+ err = deny_no_inherit_topology_change(subject, new_dentry);
+ if (err)
+ return err;
+ }
access_request_parent1 |= maybe_remove(old_dentry);
access_request_parent2 |= maybe_remove(new_dentry);
}
@@ -1711,12 +2091,31 @@ static int hook_path_symlink(const struct path *const dir,
static int hook_path_unlink(const struct path *const dir,
struct dentry *const dentry)
{
+ const struct landlock_cred_security *const subject =
+ landlock_get_applicable_subject(current_cred(), any_fs, NULL);
+ int err;
+
+ if (subject) {
+ err = deny_no_inherit_topology_change(subject, dentry);
+ if (err)
+ return err;
+ }
return current_check_access_path(dir, LANDLOCK_ACCESS_FS_REMOVE_FILE);
}
static int hook_path_rmdir(const struct path *const dir,
struct dentry *const dentry)
{
+ const struct landlock_cred_security *const subject =
+ landlock_get_applicable_subject(current_cred(), any_fs, NULL);
+ int err;
+
+ if (subject) {
+ err = deny_no_inherit_topology_change(subject, dentry);
+ if (err)
+ return err;
+ }
+
return current_check_access_path(dir, LANDLOCK_ACCESS_FS_REMOVE_DIR);
}
diff --git a/security/landlock/object.h b/security/landlock/object.h
index 5f28c35e8aa8..2520955825bb 100644
--- a/security/landlock/object.h
+++ b/security/landlock/object.h
@@ -13,6 +13,8 @@
#include <linux/refcount.h>
#include <linux/spinlock.h>
+#include "access.h"
+
struct landlock_object;
/**
diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
index fd633aee0599..63aa420ab593 100644
--- a/security/landlock/ruleset.c
+++ b/security/landlock/ruleset.c
@@ -48,6 +48,7 @@ static struct landlock_ruleset *create_ruleset(const u32 num_layers)
#endif /* IS_ENABLED(CONFIG_INET) */
new_ruleset->num_layers = num_layers;
+ xa_init(&new_ruleset->no_inherit_desc);
/*
* hierarchy = NULL
* num_rules = 0
@@ -108,6 +109,45 @@ static bool is_object_pointer(const enum landlock_key_type key_type)
}
}
+void landlock_set_no_inherit_desc_layers(struct landlock_ruleset *ruleset,
+ struct landlock_object *object,
+ layer_mask_t layers)
+{
+ struct landlock_no_inherit_desc *desc;
+ int err;
+
+ if (!ruleset || !object || !layers)
+ return;
+
+ desc = xa_load(&ruleset->no_inherit_desc, (unsigned long)object);
+ if (desc) {
+ desc->desc_layers |= layers;
+ return;
+ }
+
+ desc = kzalloc(sizeof(*desc), GFP_KERNEL_ACCOUNT);
+ if (!desc)
+ return;
+
+ desc->object = object;
+ desc->desc_layers = layers;
+ landlock_get_object(object);
+ err = xa_insert(&ruleset->no_inherit_desc, (unsigned long)object, desc,
+ GFP_KERNEL_ACCOUNT);
+ if (err) {
+ struct landlock_no_inherit_desc *existing;
+
+ if (err == -EBUSY) {
+ existing = xa_load(&ruleset->no_inherit_desc,
+ (unsigned long)object);
+ if (existing)
+ existing->desc_layers |= layers;
+ }
+ landlock_put_object(object);
+ kfree(desc);
+ }
+}
+
static struct landlock_rule *
create_rule(const struct landlock_id id,
const struct landlock_layer (*const layers)[], const u32 num_layers,
@@ -257,6 +297,10 @@ static int insert_rule(struct landlock_ruleset *const ruleset,
return -EINVAL;
this->layers[0].access |= (*layers)[0].access;
this->layers[0].flags.quiet |= (*layers)[0].flags.quiet;
+ this->layers[0].flags.no_inherit |=
+ (*layers)[0].flags.no_inherit;
+ this->layers[0].flags.has_no_inherit_descendant |=
+ (*layers)[0].flags.has_no_inherit_descendant;
return 0;
}
@@ -315,7 +359,10 @@ int landlock_insert_rule(struct landlock_ruleset *const ruleset,
.level = 0,
.flags = {
.quiet = !!(flags & LANDLOCK_ADD_RULE_QUIET),
- },
+ .no_inherit = !!(flags & LANDLOCK_ADD_RULE_NO_INHERIT),
+ .has_no_inherit_descendant =
+ !!(flags & LANDLOCK_ADD_RULE_NO_INHERIT),
+ }
} };
build_check_layer();
@@ -402,6 +449,15 @@ static int merge_ruleset(struct landlock_ruleset *const dst,
goto out_unlock;
#endif /* IS_ENABLED(CONFIG_INET) */
+ {
+ unsigned long index;
+ struct landlock_no_inherit_desc *desc;
+
+ xa_for_each(&src->no_inherit_desc, index, desc)
+ landlock_set_no_inherit_desc_layers(dst, desc->object,
+ desc->desc_layers);
+ }
+
out_unlock:
mutex_unlock(&src->lock);
mutex_unlock(&dst->lock);
@@ -465,6 +521,15 @@ static int inherit_ruleset(struct landlock_ruleset *const parent,
goto out_unlock;
#endif /* IS_ENABLED(CONFIG_INET) */
+ {
+ unsigned long index;
+ struct landlock_no_inherit_desc *desc;
+
+ xa_for_each(&parent->no_inherit_desc, index, desc)
+ landlock_set_no_inherit_desc_layers(child, desc->object,
+ desc->desc_layers);
+ }
+
if (WARN_ON_ONCE(child->num_layers <= parent->num_layers)) {
err = -EINVAL;
goto out_unlock;
@@ -489,6 +554,8 @@ static int inherit_ruleset(struct landlock_ruleset *const parent,
static void free_ruleset(struct landlock_ruleset *const ruleset)
{
struct landlock_rule *freeme, *next;
+ struct landlock_no_inherit_desc *desc;
+ unsigned long index;
might_sleep();
rbtree_postorder_for_each_entry_safe(freeme, next, &ruleset->root_inode,
@@ -501,6 +568,12 @@ static void free_ruleset(struct landlock_ruleset *const ruleset)
free_rule(freeme, LANDLOCK_KEY_NET_PORT);
#endif /* IS_ENABLED(CONFIG_INET) */
+ xa_for_each(&ruleset->no_inherit_desc, index, desc) {
+ landlock_put_object(desc->object);
+ kfree(desc);
+ }
+ xa_destroy(&ruleset->no_inherit_desc);
+
landlock_put_hierarchy(ruleset->hierarchy);
kfree(ruleset);
}
@@ -660,9 +733,18 @@ bool landlock_unmask_layers(const struct landlock_rule *const rule,
unsigned long access_bit;
bool is_empty;
+ /* Skip layers that already have no inherit flags. */
+ if (rule_flags &&
+ (rule_flags->no_inherit_masks & layer_bit))
+ continue;
+
/* Collect rule flags for each layer. */
if (rule_flags && layer->flags.quiet)
rule_flags->quiet_masks |= layer_bit;
+ if (rule_flags && layer->flags.no_inherit)
+ rule_flags->no_inherit_masks |= layer_bit;
+ if (rule_flags && layer->flags.has_no_inherit_descendant)
+ rule_flags->no_inherit_desc_masks |= layer_bit;
/*
* Records in @layer_masks which layer grants access to each requested
diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
index cd0434d8dc63..4e0484c1575d 100644
--- a/security/landlock/ruleset.h
+++ b/security/landlock/ruleset.h
@@ -15,6 +15,7 @@
#include <linux/rbtree.h>
#include <linux/refcount.h>
#include <linux/workqueue.h>
+#include <linux/xarray.h>
#include "access.h"
#include "limits.h"
@@ -40,6 +41,18 @@ struct landlock_layer {
* down the file hierarchy.
*/
bool quiet:1;
+ /**
+ * @no_inherit: When set, this layer's rule does not inherit
+ * allowed accesses from parent objects within the same layer.
+ * (currently only applies to filesystem objects)
+ */
+ bool no_inherit:1;
+ /**
+ * @has_no_inherit_descendant: Marks that a descendant rule within
+ * this layer carries the no-inherit flag and therefore seals
+ * topology changes on the path.
+ */
+ bool has_no_inherit_descendant:1;
} flags;
/**
* @access: Bitfield of allowed actions on the kernel object. They are
@@ -56,6 +69,15 @@ struct collected_rule_flags {
* @quiet_masks: Layers for which the quiet flag is effective.
*/
layer_mask_t quiet_masks;
+ /**
+ * @no_inherit_masks: Layers for which the no_inherit flag is effective.
+ */
+ layer_mask_t no_inherit_masks;
+ /**
+ * @no_inherit_desc_masks: Layers for which a descendant rule carries
+ * the no_inherit flag.
+ */
+ layer_mask_t no_inherit_desc_masks;
};
/**
@@ -136,6 +158,11 @@ struct landlock_rule {
struct landlock_layer layers[] __counted_by(num_layers);
};
+struct landlock_no_inherit_desc {
+ struct landlock_object *object;
+ layer_mask_t desc_layers;
+};
+
/**
* struct landlock_ruleset - Landlock ruleset
*
@@ -190,6 +217,12 @@ struct landlock_ruleset {
* @num_rules: Number of non-overlapping (i.e. not for
* the same object) rules in this ruleset.
*/
+ /**
+ * @no_inherit_desc: XArray containing objects
+ * with no_inherit descendants in this ruleset.
+ * This is used to quickly merge no_inherit flags,
+ */
+ struct xarray no_inherit_desc;
u32 num_rules;
/**
* @num_layers: Number of layers that are used in this
@@ -345,4 +378,10 @@ landlock_init_layer_masks(const struct landlock_ruleset *const domain,
layer_mask_t (*const layer_masks)[],
const enum landlock_key_type key_type);
+
+
+void landlock_set_no_inherit_desc_layers(struct landlock_ruleset *ruleset,
+ struct landlock_object *object,
+ layer_mask_t layers);
+
#endif /* _SECURITY_LANDLOCK_RULESET_H */
--
2.51.2
^ permalink raw reply related
* [PATCH 2/6] landlock: Implement LANDLOCK_ADD_RULE_NO_INHERIT userspace api
From: Justin Suess @ 2025-11-20 22:23 UTC (permalink / raw)
To: linux-security-module
Cc: Tingmao Wang, Günther Noack, Jan Kara, Abhinav Saxena,
Mickaël Salaün, Justin Suess
In-Reply-To: <20251120222346.1157004-1-utilityemal77@gmail.com>
Implements the syscall side flag handling and kernel api headers for the
LANDLOCK_ADD_RULE_NO_INHERIT flag.
Signed-off-by: Justin Suess <utilityemal77@gmail.com>
---
include/uapi/linux/landlock.h | 9 +++++++++
security/landlock/syscalls.c | 5 ++++-
2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
index 50f0806b7e33..d9daef551d96 100644
--- a/include/uapi/linux/landlock.h
+++ b/include/uapi/linux/landlock.h
@@ -127,10 +127,19 @@ struct landlock_ruleset_attr {
* allowed_access in the passed in rule_attr. When this flag is
* present, the caller is also allowed to pass in an empty
* allowed_access.
+ * %LANDLOCK_ADD_RULE_NO_INHERIT
+ * When this flag is set while adding a rule to a ruleset, the rule
+ * will not inherit allowed accesses from rules on parent objects
+ * within the same layer. (currently only applies to filesystem objects)
+ * By default, Landlock rules added to a ruleset inherit allowed accesses
+ * from parent objects, meaning that if a parent directory has been granted
+ * certain access rights, those rights will also apply to its child objects.
+ * This flag prevents such inheritance for the specific rule being added.
*/
/* clang-format off */
#define LANDLOCK_ADD_RULE_QUIET (1U << 0)
+#define LANDLOCK_ADD_RULE_NO_INHERIT (1U << 1)
/* clang-format on */
/**
diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c
index 93396bfc1500..200287a34895 100644
--- a/security/landlock/syscalls.c
+++ b/security/landlock/syscalls.c
@@ -463,7 +463,10 @@ SYSCALL_DEFINE4(landlock_add_rule, const int, ruleset_fd,
if (!is_initialized())
return -EOPNOTSUPP;
- if (flags && flags != LANDLOCK_ADD_RULE_QUIET)
+ if (flags && flags & ~(LANDLOCK_ADD_RULE_QUIET | LANDLOCK_ADD_RULE_NO_INHERIT))
+ return -EINVAL;
+ if ((flags & LANDLOCK_ADD_RULE_NO_INHERIT) &&
+ rule_type != LANDLOCK_RULE_PATH_BENEATH)
return -EINVAL;
/* Gets and checks the ruleset. */
--
2.51.2
^ permalink raw reply related
* [PATCH 3/6] samples/landlock: Add LANDLOCK_ADD_RULE_NO_INHERIT to landlock-sandboxer
From: Justin Suess @ 2025-11-20 22:23 UTC (permalink / raw)
To: linux-security-module
Cc: Tingmao Wang, Günther Noack, Jan Kara, Abhinav Saxena,
Mickaël Salaün, Justin Suess
In-Reply-To: <20251120222346.1157004-1-utilityemal77@gmail.com>
Adds support to landlock-sandboxer with environment variables LL_FS_RO_NO_INHERIT
and LL_FS_RW_NO_INHERIT. These create the same rulesets as their non-no inherit varients,
plus the LANDLOCK_ADD_RULE_NO_INHERIT flag.
Signed-off-by: Justin Suess <utilityemal77@gmail.com>
---
samples/landlock/sandboxer.c | 39 +++++++++++++++++++++++++++---------
1 file changed, 29 insertions(+), 10 deletions(-)
diff --git a/samples/landlock/sandboxer.c b/samples/landlock/sandboxer.c
index 2d8e3e94b77b..2b40b2df83b4 100644
--- a/samples/landlock/sandboxer.c
+++ b/samples/landlock/sandboxer.c
@@ -58,6 +58,8 @@ static inline int landlock_restrict_self(const int ruleset_fd,
#define ENV_FS_RO_NAME "LL_FS_RO"
#define ENV_FS_RW_NAME "LL_FS_RW"
+#define ENV_FS_RO_NO_INHERIT_NAME "LL_FS_RO_NO_INHERIT"
+#define ENV_FS_RW_NO_INHERIT_NAME "LL_FS_RW_NO_INHERIT"
#define ENV_FS_QUIET_NAME "LL_FS_QUIET"
#define ENV_FS_QUIET_ACCESS_NAME "LL_FS_QUIET_ACCESS"
#define ENV_TCP_BIND_NAME "LL_TCP_BIND"
@@ -121,7 +123,8 @@ static int parse_path(char *env_path, const char ***const path_list)
/* clang-format on */
static int populate_ruleset_fs(const char *const env_var, const int ruleset_fd,
- const __u64 allowed_access, bool quiet)
+ const __u64 allowed_access,
+ __u32 add_rule_flags, bool mandatory)
{
int num_paths, i, ret = 1;
char *env_path_name;
@@ -132,9 +135,13 @@ static int populate_ruleset_fs(const char *const env_var, const int ruleset_fd,
env_path_name = getenv(env_var);
if (!env_path_name) {
- /* Prevents users to forget a setting. */
- fprintf(stderr, "Missing environment variable %s\n", env_var);
- return 1;
+ if (mandatory) {
+ /* Prevents users to forget a setting. */
+ fprintf(stderr, "Missing environment variable %s\n",
+ env_var);
+ return 1;
+ }
+ return 0;
}
env_path_name = strdup(env_path_name);
unsetenv(env_var);
@@ -171,8 +178,7 @@ static int populate_ruleset_fs(const char *const env_var, const int ruleset_fd,
if (!S_ISDIR(statbuf.st_mode))
path_beneath.allowed_access &= ACCESS_FILE;
if (landlock_add_rule(ruleset_fd, LANDLOCK_RULE_PATH_BENEATH,
- &path_beneath,
- quiet ? LANDLOCK_ADD_RULE_QUIET : 0)) {
+ &path_beneath, add_rule_flags)) {
fprintf(stderr,
"Failed to update the ruleset with \"%s\": %s\n",
path_list[i], strerror(errno));
@@ -375,6 +381,8 @@ static const char help[] =
"Optional settings (when not set, their associated access check "
"is always allowed, which is different from an empty string which "
"means an empty list):\n"
+ "* " ENV_FS_RO_NO_INHERIT_NAME ": read-only paths without rule inheritance\n"
+ "* " ENV_FS_RW_NO_INHERIT_NAME ": read-write paths without rule inheritance\n"
"* " ENV_TCP_BIND_NAME ": ports allowed to bind (server)\n"
"* " ENV_TCP_CONNECT_NAME ": ports allowed to connect (client)\n"
"* " ENV_SCOPED_NAME ": actions denied on the outside of the landlock domain\n"
@@ -596,17 +604,28 @@ int main(const int argc, char *const argv[], char *const *const envp)
}
if (populate_ruleset_fs(ENV_FS_RO_NAME, ruleset_fd, access_fs_ro,
- false)) {
+ 0, true)) {
goto err_close_ruleset;
}
if (populate_ruleset_fs(ENV_FS_RW_NAME, ruleset_fd, access_fs_rw,
- false)) {
+ 0, true)) {
+ goto err_close_ruleset;
+ }
+ /* Optional no-inherit rules mirror the regular read-only/read-write sets. */
+ if (populate_ruleset_fs(ENV_FS_RO_NO_INHERIT_NAME, ruleset_fd,
+ access_fs_ro, LANDLOCK_ADD_RULE_NO_INHERIT,
+ false)) {
+ goto err_close_ruleset;
+ }
+ if (populate_ruleset_fs(ENV_FS_RW_NO_INHERIT_NAME, ruleset_fd,
+ access_fs_rw, LANDLOCK_ADD_RULE_NO_INHERIT,
+ false)) {
goto err_close_ruleset;
}
/* Don't require this env to be present. */
- if (quiet_supported && getenv(ENV_FS_QUIET_NAME)) {
+ if (quiet_supported) {
if (populate_ruleset_fs(ENV_FS_QUIET_NAME, ruleset_fd, 0,
- true)) {
+ LANDLOCK_ADD_RULE_QUIET, false)) {
goto err_close_ruleset;
}
}
--
2.51.2
^ permalink raw reply related
* [PATCH 4/6] selftests/landlock: Implement selftests for LANDLOCK_ADD_RULE_NO_INHERIT
From: Justin Suess @ 2025-11-20 22:23 UTC (permalink / raw)
To: linux-security-module
Cc: Tingmao Wang, Günther Noack, Jan Kara, Abhinav Saxena,
Mickaël Salaün, Justin Suess
In-Reply-To: <20251120222346.1157004-1-utilityemal77@gmail.com>
Implements 5 selftests for the flag, covering allowed and disallowed operations on parent
and child directories when this flag is set, as well as multi-layer configurations.
Signed-off-by: Justin Suess <utilityemal77@gmail.com>
---
tools/testing/selftests/landlock/fs_test.c | 222 +++++++++++++++++++--
1 file changed, 210 insertions(+), 12 deletions(-)
diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
index d4819ff44230..1cdded3f67e6 100644
--- a/tools/testing/selftests/landlock/fs_test.c
+++ b/tools/testing/selftests/landlock/fs_test.c
@@ -717,16 +717,12 @@ TEST_F_FORK(layout1, rule_with_unhandled_access)
}
static void add_path_beneath(struct __test_metadata *const _metadata,
- const int ruleset_fd, const __u64 allowed_access,
- const char *const path, bool quiet)
+ const int ruleset_fd, const __u64 allowed_access,
+ const char *const path, __u32 flags)
{
struct landlock_path_beneath_attr path_beneath = {
.allowed_access = allowed_access,
};
- __u32 flags = 0;
-
- if (quiet)
- flags |= LANDLOCK_ADD_RULE_QUIET;
path_beneath.parent_fd = open(path, O_PATH | O_CLOEXEC);
ASSERT_LE(0, path_beneath.parent_fd)
@@ -790,7 +786,7 @@ static int create_ruleset(struct __test_metadata *const _metadata,
continue;
add_path_beneath(_metadata, ruleset_fd, rules[i].access,
- rules[i].path, false);
+ rules[i].path, 0);
}
return ruleset_fd;
}
@@ -1368,7 +1364,7 @@ TEST_F_FORK(layout1, inherit_subset)
* ANDed with the previous ones.
*/
add_path_beneath(_metadata, ruleset_fd, LANDLOCK_ACCESS_FS_WRITE_FILE,
- dir_s1d2, false);
+ dir_s1d2, 0);
/*
* According to ruleset_fd, dir_s1d2 should now have the
* LANDLOCK_ACCESS_FS_READ_FILE and LANDLOCK_ACCESS_FS_WRITE_FILE
@@ -1400,7 +1396,7 @@ TEST_F_FORK(layout1, inherit_subset)
* Try to get more privileges by adding new access rights to the parent
* directory: dir_s1d1.
*/
- add_path_beneath(_metadata, ruleset_fd, ACCESS_RW, dir_s1d1, false);
+ add_path_beneath(_metadata, ruleset_fd, ACCESS_RW, dir_s1d1, 0);
enforce_ruleset(_metadata, ruleset_fd);
/* Same tests and results as above. */
@@ -1423,7 +1419,7 @@ TEST_F_FORK(layout1, inherit_subset)
* that there was no rule tied to it before.
*/
add_path_beneath(_metadata, ruleset_fd, LANDLOCK_ACCESS_FS_WRITE_FILE,
- dir_s1d3, false);
+ dir_s1d3, 0);
enforce_ruleset(_metadata, ruleset_fd);
ASSERT_EQ(0, close(ruleset_fd));
@@ -1476,7 +1472,7 @@ TEST_F_FORK(layout1, inherit_superset)
add_path_beneath(_metadata, ruleset_fd,
LANDLOCK_ACCESS_FS_READ_FILE |
LANDLOCK_ACCESS_FS_READ_DIR,
- dir_s1d2, false);
+ dir_s1d2, 0);
enforce_ruleset(_metadata, ruleset_fd);
ASSERT_EQ(0, close(ruleset_fd));
@@ -1488,6 +1484,39 @@ TEST_F_FORK(layout1, inherit_superset)
ASSERT_EQ(0, test_open(file1_s1d3, O_RDONLY));
}
+TEST_F_FORK(layout1, inherit_no_inherit_flag)
+{
+ struct landlock_ruleset_attr ruleset_attr = {
+ .handled_access_fs = ACCESS_RW,
+ };
+ int ruleset_fd;
+
+ ruleset_fd =
+ landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
+ ASSERT_LE(0, ruleset_fd);
+
+ add_path_beneath(_metadata, ruleset_fd, ACCESS_RW, dir_s1d1, 0);
+ add_path_beneath(_metadata, ruleset_fd, ACCESS_RO, dir_s1d2,
+ LANDLOCK_ADD_RULE_NO_INHERIT);
+
+ enforce_ruleset(_metadata, ruleset_fd);
+ ASSERT_EQ(0, close(ruleset_fd));
+
+ /* Parent directory still grants write access to its direct children. */
+ EXPECT_EQ(0, test_open(dir_s1d1, O_RDONLY | O_DIRECTORY));
+ EXPECT_EQ(0, test_open(file1_s1d1, O_WRONLY));
+
+ /* dir_s1d2 gets only its explicit read-only access rights. */
+ EXPECT_EQ(0, test_open(dir_s1d2, O_RDONLY | O_DIRECTORY));
+ EXPECT_EQ(0, test_open(file1_s1d2, O_RDONLY));
+ EXPECT_EQ(EACCES, test_open(file1_s1d2, O_WRONLY));
+
+ /* Descendants of dir_s1d2 inherit the reduced access mask. */
+ EXPECT_EQ(0, test_open(dir_s1d3, O_RDONLY | O_DIRECTORY));
+ EXPECT_EQ(0, test_open(file1_s1d3, O_RDONLY));
+ EXPECT_EQ(EACCES, test_open(file1_s1d3, O_WRONLY));
+}
+
TEST_F_FORK(layout0, max_layers)
{
int i, err;
@@ -4412,6 +4441,175 @@ TEST_F_FORK(layout1, named_unix_domain_socket_ioctl)
ASSERT_EQ(0, close(cli_fd));
}
+TEST_F_FORK(layout1, inherit_no_inherit_topology_dir)
+{
+ const struct rule rules[] = {
+ {
+ .path = TMP_DIR,
+ .access = ACCESS_RW,
+ },
+ {},
+ };
+ int ruleset_fd;
+
+ ruleset_fd = create_ruleset(_metadata, ACCESS_RW, rules);
+ ASSERT_LE(0, ruleset_fd);
+
+ /* Adds a no-inherit rule on a leaf directory. */
+ add_path_beneath(_metadata, ruleset_fd, ACCESS_RO, dir_s1d3,
+ LANDLOCK_ADD_RULE_NO_INHERIT);
+
+ enforce_ruleset(_metadata, ruleset_fd);
+ ASSERT_EQ(0, close(ruleset_fd));
+
+ /*
+ * Topology modifications of the rule path and its parents are denied.
+ */
+
+ /* Target directory s1d3 */
+ ASSERT_EQ(-1, rmdir(dir_s1d3));
+ ASSERT_EQ(EACCES, errno);
+ ASSERT_EQ(-1, rename(dir_s1d3, dir_s2d3));
+ ASSERT_EQ(EACCES, errno);
+
+ /* Parent directory s1d2 */
+ ASSERT_EQ(-1, rmdir(dir_s1d2));
+ ASSERT_EQ(EACCES, errno);
+ ASSERT_EQ(-1, rename(dir_s1d2, dir_s2d2));
+ ASSERT_EQ(EACCES, errno);
+
+ /* Grandparent directory s1d1 */
+ ASSERT_EQ(-1, rmdir(dir_s1d1));
+ ASSERT_EQ(EACCES, errno);
+ ASSERT_EQ(-1, rename(dir_s1d1, dir_s2d1));
+ ASSERT_EQ(EACCES, errno);
+
+ /*
+ * Sibling operations are allowed.
+ */
+ /* Sibling of s1d3 */
+ ASSERT_EQ(0, unlink(file1_s1d2));
+ /* Sibling of s1d2 */
+ ASSERT_EQ(0, unlink(file1_s1d1));
+
+ /*
+ * Content of the no-inherit directory is restricted by the rule (RO).
+ */
+ ASSERT_EQ(-1, unlink(file1_s1d3));
+ ASSERT_EQ(EACCES, errno);
+}
+
+TEST_F_FORK(layout1, inherit_no_inherit_topology_unrelated)
+{
+ const struct rule rules[] = {
+ {
+ .path = TMP_DIR,
+ .access = ACCESS_RW,
+ },
+ {},
+ };
+ static const char unrelated_dir[] = TMP_DIR "/s2d1/unrelated";
+ static const char unrelated_file[] = TMP_DIR "/s2d1/unrelated/f1";
+ int ruleset_fd;
+
+ ruleset_fd = create_ruleset(_metadata, ACCESS_RW, rules);
+ ASSERT_LE(0, ruleset_fd);
+
+ /* Adds a no-inherit rule on a leaf directory unrelated to s2. */
+ add_path_beneath(_metadata, ruleset_fd, ACCESS_RO, dir_s1d3,
+ LANDLOCK_ADD_RULE_NO_INHERIT);
+
+ enforce_ruleset(_metadata, ruleset_fd);
+ ASSERT_EQ(0, close(ruleset_fd));
+
+ /* Ensure we can still create and delete files outside the sealed branch. */
+ ASSERT_EQ(0, mkdir(unrelated_dir, 0700));
+ ASSERT_EQ(0, mknod(unrelated_file, S_IFREG | 0600, 0));
+ ASSERT_EQ(0, unlink(unrelated_file));
+ ASSERT_EQ(0, rmdir(unrelated_dir));
+
+ /* Existing siblings in s2 remain modifiable. */
+ ASSERT_EQ(0, unlink(file1_s2d1));
+ ASSERT_EQ(0, mknod(file1_s2d1, S_IFREG | 0700, 0));
+}
+
+TEST_F_FORK(layout1, inherit_no_inherit_topology_file)
+{
+ const struct rule rules[] = {
+ {
+ .path = TMP_DIR,
+ .access = ACCESS_RW,
+ },
+ {},
+ };
+ int ruleset_fd;
+ struct landlock_path_beneath_attr path_beneath = {
+ .allowed_access = ACCESS_RO,
+ };
+
+ ruleset_fd = create_ruleset(_metadata, ACCESS_RW, rules);
+ ASSERT_LE(0, ruleset_fd);
+
+ path_beneath.parent_fd = open(file1_s1d2, O_PATH | O_CLOEXEC);
+ ASSERT_LE(0, path_beneath.parent_fd);
+ ASSERT_EQ(-1, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_PATH_BENEATH,
+ &path_beneath,
+ LANDLOCK_ADD_RULE_NO_INHERIT));
+ ASSERT_EQ(EINVAL, errno);
+ ASSERT_EQ(0, close(path_beneath.parent_fd));
+ ASSERT_EQ(0, close(ruleset_fd));
+}
+
+TEST_F_FORK(layout1, inherit_no_inherit_layered)
+{
+ const struct rule layer1[] = {
+ {
+ .path = TMP_DIR,
+ .access = ACCESS_RW,
+ },
+ {},
+ };
+ int ruleset_fd;
+ static const char unrelated_dir[] = TMP_DIR "/s2d1/unrelated";
+ static const char unrelated_file[] = TMP_DIR "/s2d1/unrelated/f1";
+
+ /* Layer 1: RW on TMP_DIR */
+ ruleset_fd = create_ruleset(_metadata, ACCESS_RW, layer1);
+ ASSERT_LE(0, ruleset_fd);
+ enforce_ruleset(_metadata, ruleset_fd);
+ ASSERT_EQ(0, close(ruleset_fd));
+
+ /* Layer 2: Add no-inherit RO rule on s1d2 */
+ ruleset_fd = create_ruleset(_metadata, ACCESS_RW, layer1);
+ ASSERT_LE(0, ruleset_fd);
+ add_path_beneath(_metadata, ruleset_fd, ACCESS_RO, dir_s1d2,
+ LANDLOCK_ADD_RULE_NO_INHERIT);
+ enforce_ruleset(_metadata, ruleset_fd);
+ ASSERT_EQ(0, close(ruleset_fd));
+
+ /* Operations in unrelated areas should still work */
+ ASSERT_EQ(0, mkdir(unrelated_dir, 0700));
+ ASSERT_EQ(0, mknod(unrelated_file, S_IFREG | 0600, 0));
+ ASSERT_EQ(0, unlink(unrelated_file));
+ ASSERT_EQ(0, rmdir(unrelated_dir));
+
+ /* Creating in s1d1 should be allowed (parent still has RW) */
+ ASSERT_EQ(0, mknod(TMP_DIR "/s1d1/newfile", S_IFREG | 0600, 0));
+ ASSERT_EQ(0, unlink(TMP_DIR "/s1d1/newfile"));
+
+ /* Content of s1d2 should be read-only */
+ ASSERT_EQ(-1, unlink(file1_s1d2));
+ ASSERT_EQ(EACCES, errno);
+
+ /* Topology changes to s1d2 should be denied */
+ ASSERT_EQ(-1, rename(dir_s1d2, TMP_DIR "/s2d1/renamed"));
+ ASSERT_EQ(EACCES, errno);
+
+ /* Renaming s1d1 should also be denied (it's an ancestor) */
+ ASSERT_EQ(-1, rename(dir_s1d1, TMP_DIR "/s2d1/renamed"));
+ ASSERT_EQ(EACCES, errno);
+}
+
/* clang-format off */
FIXTURE(ioctl) {};
@@ -7647,7 +7845,7 @@ static int apply_a_layer(struct __test_metadata *const _metadata,
continue;
add_path_beneath(_metadata, rs_fd, r->access, r->path,
- r->quiet);
+ r->quiet ? LANDLOCK_ADD_RULE_QUIET : 0);
}
ASSERT_EQ(0, prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0));
--
2.51.2
^ permalink raw reply related
* [PATCH 5/6] landlock: Fix compilation error for kunit tests when CONFIG_AUDIT is disabled.
From: Justin Suess @ 2025-11-20 22:23 UTC (permalink / raw)
To: linux-security-module
Cc: Tingmao Wang, Günther Noack, Jan Kara, Abhinav Saxena,
Mickaël Salaün, Justin Suess
In-Reply-To: <20251120222346.1157004-1-utilityemal77@gmail.com>
This was necessary when fixing the no inherit patch in the implementation of kunit tests.
When compiled without the audit flag enabled, kunit tests would fail to compile because of
the missing quiet flag field. This fixes this issue.
Signed-off-by: Justin Suess <utilityemal77@gmail.com>
---
security/landlock/audit.h | 3 ++-
security/landlock/fs.c | 4 +++-
security/landlock/net.h | 2 +-
security/landlock/ruleset.c | 2 ++
4 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/security/landlock/audit.h b/security/landlock/audit.h
index 950365cd223d..1a5a9cd28cf4 100644
--- a/security/landlock/audit.h
+++ b/security/landlock/audit.h
@@ -69,7 +69,8 @@ landlock_log_drop_domain(const struct landlock_hierarchy *const hierarchy)
static inline void
landlock_log_denial(const struct landlock_cred_security *const subject,
- const struct landlock_request *const request)
+ const struct landlock_request *const request,
+ const struct collected_rule_flags rule_flags)
{
}
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index ebeee080ea7a..20a24f3988bc 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -2094,7 +2094,7 @@ static int hook_path_unlink(const struct path *const dir,
const struct landlock_cred_security *const subject =
landlock_get_applicable_subject(current_cred(), any_fs, NULL);
int err;
-
+
if (subject) {
err = deny_no_inherit_topology_change(subject, dentry);
if (err)
@@ -2155,6 +2155,7 @@ get_required_file_open_access(const struct file *const file)
static void build_check_file_security(void)
{
+#ifdef CONFIG_AUDIT
const struct landlock_file_security file_sec = {
.quiet_optional_accesses = ~0,
.fown_layer = ~0,
@@ -2171,6 +2172,7 @@ static void build_check_file_security(void)
__const_hweight64(_LANDLOCK_ACCESS_FS_OPTIONAL));
/* Makes sure all layers can be identified. */
BUILD_BUG_ON(file_sec.fown_layer < LANDLOCK_MAX_NUM_LAYERS - 1);
+#endif /* CONFIG_AUDIT */
}
static int hook_file_alloc_security(struct file *const file)
diff --git a/security/landlock/net.h b/security/landlock/net.h
index 799cedd5d0b7..72c47f4d6803 100644
--- a/security/landlock/net.h
+++ b/security/landlock/net.h
@@ -25,7 +25,7 @@ static inline void landlock_add_net_hooks(void)
static inline int
landlock_append_net_rule(struct landlock_ruleset *const ruleset, const u16 port,
- access_mask_t access_rights)
+ access_mask_t access_rights, const int flags)
{
return -EAFNOSUPPORT;
}
diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
index 63aa420ab593..5d190d6779da 100644
--- a/security/landlock/ruleset.c
+++ b/security/landlock/ruleset.c
@@ -660,7 +660,9 @@ landlock_merge_ruleset(struct landlock_ruleset *const parent,
if (err)
return ERR_PTR(err);
+#ifdef CONFIG_AUDIT
new_dom->hierarchy->quiet_masks = ruleset->quiet_masks;
+#endif /* CONFIG_AUDIT */
return no_free_ptr(new_dom);
}
--
2.51.2
^ permalink raw reply related
* [PATCH 6/6] landlock: Implement KUnit test for LANDLOCK_ADD_RULE_NO_INHERIT
From: Justin Suess @ 2025-11-20 22:23 UTC (permalink / raw)
To: linux-security-module
Cc: Tingmao Wang, Günther Noack, Jan Kara, Abhinav Saxena,
Mickaël Salaün, Justin Suess
In-Reply-To: <20251120222346.1157004-1-utilityemal77@gmail.com>
Add a unit test for rule_flag collection, ensuring that access masks are properly
propagated with the flags.
Signed-off-by: Justin Suess <utilityemal77@gmail.com>
---
security/landlock/ruleset.c | 85 +++++++++++++++++++++++++++++++++++++
1 file changed, 85 insertions(+)
diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
index 5d190d6779da..c407ec264207 100644
--- a/security/landlock/ruleset.c
+++ b/security/landlock/ruleset.c
@@ -22,6 +22,7 @@
#include <linux/spinlock.h>
#include <linux/workqueue.h>
#include <uapi/linux/landlock.h>
+#include <kunit/test.h>
#include "access.h"
#include "audit.h"
@@ -835,3 +836,87 @@ landlock_init_layer_masks(const struct landlock_ruleset *const domain,
}
return handled_accesses;
}
+
+#ifdef CONFIG_SECURITY_LANDLOCK_KUNIT_TEST
+
+static void test_unmask_layers_no_inherit(struct kunit *const test)
+{
+ struct landlock_rule *rule;
+ layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS];
+ struct collected_rule_flags rule_flags;
+ const access_mask_t access_request = BIT_ULL(0) | BIT_ULL(1);
+ const layer_mask_t layers_initialized = BIT_ULL(0) | BIT_ULL(1);
+ size_t i;
+
+ rule = kzalloc(struct_size(rule, layers, 2), GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, rule);
+
+ rule->num_layers = 2;
+
+ /* Layer 1: allows access 0, no_inherit */
+ rule->layers[0].level = 1;
+ rule->layers[0].access = BIT_ULL(0);
+ rule->layers[0].flags.no_inherit = 1;
+
+ /* Layer 2: allows access 1 */
+ rule->layers[1].level = 2;
+ rule->layers[1].access = BIT_ULL(1);
+
+ /* Case 1: No rule_flags provided (should behave normally) */
+ for (i = 0; i < ARRAY_SIZE(layer_masks); i++)
+ layer_masks[i] = layers_initialized;
+
+ landlock_unmask_layers(rule, access_request, &layer_masks,
+ ARRAY_SIZE(layer_masks), NULL);
+
+ /* Access 0 should be unmasked by layer 1 */
+ KUNIT_EXPECT_EQ(test, layer_masks[0], layers_initialized & ~BIT_ULL(0));
+ /* Access 1 should be unmasked by layer 2 */
+ KUNIT_EXPECT_EQ(test, layer_masks[1], layers_initialized & ~BIT_ULL(1));
+
+ /* Case 2: rule_flags provided, no existing no_inherit_masks */
+ for (i = 0; i < ARRAY_SIZE(layer_masks); i++)
+ layer_masks[i] = layers_initialized;
+ memset(&rule_flags, 0, sizeof(rule_flags));
+
+ landlock_unmask_layers(rule, access_request, &layer_masks,
+ ARRAY_SIZE(layer_masks), &rule_flags);
+
+ /* Access 0 should be unmasked by layer 1 */
+ KUNIT_EXPECT_EQ(test, layer_masks[0], layers_initialized & ~BIT_ULL(0));
+ /* Access 1 should be unmasked by layer 2 */
+ KUNIT_EXPECT_EQ(test, layer_masks[1], layers_initialized & ~BIT_ULL(1));
+
+ /* rule_flags should collect no_inherit from layer 1 */
+ KUNIT_EXPECT_EQ(test, rule_flags.no_inherit_masks, (layer_mask_t)BIT_ULL(0));
+
+ /* Case 3: rule_flags provided, layer 1 is masked by no_inherit_masks */
+ for (i = 0; i < ARRAY_SIZE(layer_masks); i++)
+ layer_masks[i] = layers_initialized;
+ memset(&rule_flags, 0, sizeof(rule_flags));
+ rule_flags.no_inherit_masks = BIT_ULL(0); /* Mask layer 1 */
+
+ landlock_unmask_layers(rule, access_request, &layer_masks,
+ ARRAY_SIZE(layer_masks), &rule_flags);
+
+ /* Access 0 should NOT be unmasked by layer 1 because it is skipped */
+ KUNIT_EXPECT_EQ(test, layer_masks[0], layers_initialized);
+ /* Access 1 should be unmasked by layer 2 */
+ KUNIT_EXPECT_EQ(test, layer_masks[1], layers_initialized & ~BIT_ULL(1));
+
+ kfree(rule);
+}
+
+static struct kunit_case ruleset_test_cases[] = {
+ KUNIT_CASE(test_unmask_layers_no_inherit),
+ {}
+};
+
+static struct kunit_suite ruleset_test_suite = {
+ .name = "landlock_ruleset",
+ .test_cases = ruleset_test_cases,
+};
+
+kunit_test_suite(ruleset_test_suite);
+
+#endif /* CONFIG_SECURITY_LANDLOCK_KUNIT_TEST */
--
2.51.2
^ permalink raw reply related
* Re: [RFC v1 0/1] Implement IMA Event Log Trimming
From: Mimi Zohar @ 2025-11-20 22:58 UTC (permalink / raw)
To: Anirudh Venkataramanan, linux-integrity
Cc: Roberto Sassu, Dmitry Kasatkin, Eric Snowberg, Paul Moore,
James Morris, Serge E . Hallyn, linux-security-module,
Steven Chen, Gregory Lumen, Lakshmi Ramasubramanian,
Sush Shringarputale
In-Reply-To: <20251119213359.39397-1-anirudhve@linux.microsoft.com>
On Wed, 2025-11-19 at 13:33 -0800, Anirudh Venkataramanan wrote:
>
>
> 2. The kernel uses the userspace supplied PCR values to trim the IMA
> measurements list at a specific point, and so these are referred to as
> "trim-to PCR values" in this context.
>
> Note that the kernel doesn't really understand what these userspace
> provided PCR values mean or what IMA event they correspond to, and so
> it does its own IMA event replay till either the replayed PCR values
> match with the userspace provided ones, or it runs out of events.
>
> If a match is found, the kernel can proceed with trimming the IMA
> measurements list. This is done in two steps, to keep locking context
> minimal.
>
> step 1: Find and return the list entry (as a count from head) of exact
> match. This does not lock the measurements list mutex, ensuring
> new events can be appended to the log.
>
> step 2: Lock the measurements list mutex and trim the measurements list
> at the previously identified list entry.
>
> If the trim is successful, the trim-to PCR values are saved as "starting
> PCR values". The next time userspace wants to replay the IMA event log,
> it will use the starting PCR values as the base for the IMA event log
> replay.
Depending on the IMA policy, the IMA measurement list may contain integrity
violations, such as "ToM/ToU" (Time of Measurement/Time of Use) or "open-
writer". Either the userspace supplied PCR values will not match any measurement
record or the PCR values will be "fixed" to match the well known violation hash
value extended into the TPM. Depending on how the userspace PCR values will
subsequently be used, saving the "fixed" PCR values could result in whitewashing
the integrity of the measurement list.
--
thanks,
Mimi
^ permalink raw reply
* Re: [RFC][PATCH] exec: Move cred computation under exec_update_lock
From: Eric W. Biederman @ 2025-11-20 23:50 UTC (permalink / raw)
To: Bernd Edlinger
Cc: Alexander Viro, Alexey Dobriyan, Oleg Nesterov, Kees Cook,
Andy Lutomirski, Will Drewry, Christian Brauner, Andrew Morton,
Michal Hocko, Serge Hallyn, James Morris, Randy Dunlap,
Suren Baghdasaryan, Yafang Shao, Helge Deller, Adrian Reber,
Thomas Gleixner, Jens Axboe, Alexei Starovoitov,
linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-kselftest, linux-mm, linux-security-module, tiozhang,
Luis Chamberlain, Paulo Alcantara (SUSE), Sergey Senozhatsky,
Frederic Weisbecker, YueHaibing, Paul Moore, Aleksa Sarai,
Stefan Roesch, Chao Yu, xu xin, Jeff Layton, Jan Kara,
David Hildenbrand, Dave Chinner, Shuah Khan, Elena Reshetova,
David Windsor, Mateusz Guzik, Ard Biesheuvel,
Joel Fernandes (Google), Matthew Wilcox (Oracle),
Hans Liljestrand, Penglei Jiang, Lorenzo Stoakes, Adrian Ratiu,
Ingo Molnar, Peter Zijlstra (Intel), Cyrill Gorcunov,
Eric Dumazet
In-Reply-To: <87h5uoxw06.fsf_-_@email.froward.int.ebiederm.org>
"Eric W. Biederman" <ebiederm@xmission.com> writes:
> Instead of computing the new cred before we pass the point of no
> return compute the new cred just before we use it.
>
> This allows the removal of fs_struct->in_exec and cred_guard_mutex.
>
> I am not certain why we wanted to compute the cred for the new
> executable so early. Perhaps I missed something but I did not see any
> common errors being signaled. So I don't think we loose anything by
> computing the new cred later.
I should add that the permission checks happen in open_exec,
everything that follows credential wise is just about representing in
struct cred the credentials the new executable will have.
So I am really at a loss why we have had this complicated way of
computing of computed the credentials all of these years full of
time of check to time of use problems.
Eric
^ permalink raw reply
* Re: [PATCH v4 02/35] compiler-context-analysis: Add infrastructure for Context Analysis with Clang
From: Marco Elver @ 2025-11-20 23:51 UTC (permalink / raw)
To: Linus Torvalds
Cc: Peter Zijlstra, Boqun Feng, Ingo Molnar, Will Deacon,
David S. Miller, Luc Van Oostenryck, Chris Li, Paul E. McKenney,
Alexander Potapenko, Arnd Bergmann, Bart Van Assche,
Christoph Hellwig, Dmitry Vyukov, Eric Dumazet,
Frederic Weisbecker, Greg Kroah-Hartman, Herbert Xu, Ian Rogers,
Jann Horn, Joel Fernandes, Johannes Berg, Jonathan Corbet,
Josh Triplett, Justin Stitt, Kees Cook, Kentaro Takeda,
Lukas Bulwahn, Mark Rutland, Mathieu Desnoyers, Miguel Ojeda,
Nathan Chancellor, Neeraj Upadhyay, Nick Desaulniers,
Steven Rostedt, Tetsuo Handa, Thomas Gleixner, Thomas Graf,
Uladzislau Rezki, Waiman Long, kasan-dev, linux-crypto, linux-doc,
linux-kbuild, linux-kernel, linux-mm, linux-security-module,
linux-sparse, linux-wireless, llvm, rcu
In-Reply-To: <CAHk-=whyKteNtcLON-gScv6tu8ssvKWdNw-k371ufDrjOv374g@mail.gmail.com>
On Thu, Nov 20, 2025 at 10:14AM -0800, Linus Torvalds wrote:
> On Thu, 20 Nov 2025 at 07:13, Marco Elver <elver@google.com> wrote:
[..]
> > +#if defined(WARN_CONTEXT_ANALYSIS)
>
> Note the 400+ added lines to this header...
>
[..]
> Please let's *not* do it this way, where the header contents basically
> get enabled or not based on a compiler flag, but then everybody
> includes this 400+ line file whether they need it or not.
Note, there are a good amount of kernel-doc comments in there, so we
have 125 real code lines.
% cloc include/linux/compiler-context-analysis.h
1 text file.
1 unique file.
0 files ignored.
github.com/AlDanial/cloc v 2.06 T=0.01 s (97.1 files/s, 41646.9 lines/s)
-------------------------------------------------------------------------------
Language files blank comment code
-------------------------------------------------------------------------------
C/C++ Header 1 37 267 125
-------------------------------------------------------------------------------
> Can we please just make the header file *itself* not have any
> conditionals, and what happens is that the header file is included (or
> not) using a pattern something like
>
> -include $(srctree)/include/linux/$(context-analysis-header)
>
> instead.
>
> IOW, we'd have three different header files entirely: the "no context
> analysis", the "sparse" and the "clang context analysis" header, and
> instead of having a "-DWARN_CONTEXT_ANALYSIS" define, we'd just
> include the appropriate header automatically.
>
> We already use that "-include" pattern for <linux/kconfig.h> and
> <linux/compiler-version.h>. It's probably what we should have done for
> <linux/compiler.h> and friends too.
>
> The reason I react to things like this is that I've actually seen just
> the parsing of header files being a surprisingly big cost in build
> times. People think that optimizations are expensive, and yes, some of
> them really are, but when a lot of the code we parse is never actually
> *used*, but just hangs out in header files that gets included by
> everybody, the parsing overhead tends to be noticeable. There's a
> reason why most C compilers end up integrating the C pre-processor: it
> avoids parsing and tokenizing things multiple times.
>
> The other reason is that I often use "git grep" for looking up
> definitions of things, and when there are multiple definitions of the
> same thing, I actually find it much more informative when they are in
> two different files than when I see two different definitions (or
> declarations) in the same file and then I have to go look at what the
> #ifdef condition is. In contrast, when it's something where there are
> per-architecture definitions, you *see* that, because the grep results
> come from different header files.
>
> I dunno. This is not a huge deal, but I do think that it would seem to
> be much simpler and more straightforward to treat this as a kind of "N
> different baseline header files" than as "include this one header file
> in everything, and then we'll have #ifdef's for the configuration".
>
> Particularly when that config is not even a global config, but a per-file one.
>
> Hmm? Maybe there's some reason why this suggestion is very
> inconvenient, but please at least consider it.
Fair points; I gave this a shot, as a patch on top so we can skip the
Sparse version.
Reduced version below:
-------------------------------------------------------------------------------
Language files blank comment code
-------------------------------------------------------------------------------
C/C++ Header 1 26 189 80
-------------------------------------------------------------------------------
My suspicion (or I'm doing it wrong): there really isn't all that much
we can conditionally -include, because we need at least the no-op stubs
everywhere regardless because of annotations provided by common headers
(spinlock, mutex, rcu, etc. etc.).
If we assume that in the common case we need the no-op macros
everywhere, thus every line in <linux/compiler-context-analysis.h> is
required in the common case with the below version, the below experiment
should be be close to what we can achieve.
However, it might still be worthwhile for the code organization aspect?
Thoughts?
Thanks,
-- Marco
------ >8 ------
From: Marco Elver <elver@google.com>
Date: Thu, 20 Nov 2025 22:37:52 +0100
Subject: [PATCH] compiler-context-analysis: Move Clang definitions to separate
header
In the interest of improving compile-times, it makes sense to move the
conditionally enabled definitions when the analysis is enabled to a
separate file and include it only with -include.
A very unscientific comparison, on a system with 72 CPUs; before:
125.67 wallclock secs = ( 5681.04 usr secs + 367.63 sys secs / 4815.83% CPU )
After:
125.61 wallclock secs = ( 5684.80 usr secs + 366.53 sys secs / 4817.95% CPU )
[ Work in progress - with this version, there is no measurable
difference in compile times. ]
Signed-off-by: Marco Elver <elver@google.com>
---
Documentation/dev-tools/context-analysis.rst | 10 +-
.../linux/compiler-context-analysis-clang.h | 144 ++++++++++++++++++
include/linux/compiler-context-analysis.h | 136 +----------------
scripts/Makefile.context-analysis | 3 +-
4 files changed, 153 insertions(+), 140 deletions(-)
create mode 100644 include/linux/compiler-context-analysis-clang.h
diff --git a/Documentation/dev-tools/context-analysis.rst b/Documentation/dev-tools/context-analysis.rst
index e53f089d0c52..71b9c5e57eb4 100644
--- a/Documentation/dev-tools/context-analysis.rst
+++ b/Documentation/dev-tools/context-analysis.rst
@@ -99,10 +99,7 @@ Keywords
~~~~~~~~
.. kernel-doc:: include/linux/compiler-context-analysis.h
- :identifiers: context_guard_struct
- token_context_guard token_context_guard_instance
- __guarded_by __pt_guarded_by
- __must_hold
+ :identifiers: __must_hold
__must_not_hold
__acquires
__cond_acquires
@@ -119,6 +116,11 @@ Keywords
__acquire_shared_ret
context_unsafe
__context_unsafe
+
+.. kernel-doc:: include/linux/compiler-context-analysis-clang.h
+ :identifiers: __guarded_by __pt_guarded_by
+ context_guard_struct
+ token_context_guard token_context_guard_instance
disable_context_analysis enable_context_analysis
.. note::
diff --git a/include/linux/compiler-context-analysis-clang.h b/include/linux/compiler-context-analysis-clang.h
new file mode 100644
index 000000000000..534a41a25596
--- /dev/null
+++ b/include/linux/compiler-context-analysis-clang.h
@@ -0,0 +1,144 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Macros and attributes for compiler-based static context analysis that map to
+ * Clang's "Thread Safety Analysis".
+ */
+
+#ifndef _LINUX_COMPILER_CONTEXT_ANALYSIS_CLANG_H
+#define _LINUX_COMPILER_CONTEXT_ANALYSIS_CLANG_H
+
+#ifndef WARN_CONTEXT_ANALYSIS
+#error "This header should not be included"
+#endif
+
+/*
+ * These attributes define new context guard (Clang: capability) types.
+ * Internal only.
+ */
+#define __ctx_guard_type(name) __attribute__((capability(#name)))
+#define __reentrant_ctx_guard __attribute__((reentrant_capability))
+#define __acquires_ctx_guard(...) __attribute__((acquire_capability(__VA_ARGS__)))
+#define __acquires_shared_ctx_guard(...) __attribute__((acquire_shared_capability(__VA_ARGS__)))
+#define __try_acquires_ctx_guard(ret, var) __attribute__((try_acquire_capability(ret, var)))
+#define __try_acquires_shared_ctx_guard(ret, var) __attribute__((try_acquire_shared_capability(ret, var)))
+#define __releases_ctx_guard(...) __attribute__((release_capability(__VA_ARGS__)))
+#define __releases_shared_ctx_guard(...) __attribute__((release_shared_capability(__VA_ARGS__)))
+#define __assumes_ctx_guard(...) __attribute__((assert_capability(__VA_ARGS__)))
+#define __assumes_shared_ctx_guard(...) __attribute__((assert_shared_capability(__VA_ARGS__)))
+#define __returns_ctx_guard(var) __attribute__((lock_returned(var)))
+
+/*
+ * The below are used to annotate code being checked. Internal only.
+ */
+#define __excludes_ctx_guard(...) __attribute__((locks_excluded(__VA_ARGS__)))
+#define __requires_ctx_guard(...) __attribute__((requires_capability(__VA_ARGS__)))
+#define __requires_shared_ctx_guard(...) __attribute__((requires_shared_capability(__VA_ARGS__)))
+
+/**
+ * __guarded_by - struct member and globals attribute, declares variable
+ * only accessible within active context
+ *
+ * Declares that the struct member or global variable is only accessible within
+ * the context entered by the given context guard. Read operations on the data
+ * require shared access, while write operations require exclusive access.
+ *
+ * .. code-block:: c
+ *
+ * struct some_state {
+ * spinlock_t lock;
+ * long counter __guarded_by(&lock);
+ * };
+ */
+#define __guarded_by(...) __attribute__((guarded_by(__VA_ARGS__)))
+
+/**
+ * __pt_guarded_by - struct member and globals attribute, declares pointed-to
+ * data only accessible within active context
+ *
+ * Declares that the data pointed to by the struct member pointer or global
+ * pointer is only accessible within the context entered by the given context
+ * guard. Read operations on the data require shared access, while write
+ * operations require exclusive access.
+ *
+ * .. code-block:: c
+ *
+ * struct some_state {
+ * spinlock_t lock;
+ * long *counter __pt_guarded_by(&lock);
+ * };
+ */
+#define __pt_guarded_by(...) __attribute__((pt_guarded_by(__VA_ARGS__)))
+
+/**
+ * context_guard_struct() - declare or define a context guard struct
+ * @name: struct name
+ *
+ * Helper to declare or define a struct type that is also a context guard.
+ *
+ * .. code-block:: c
+ *
+ * context_guard_struct(my_handle) {
+ * int foo;
+ * long bar;
+ * };
+ *
+ * struct some_state {
+ * ...
+ * };
+ * // ... declared elsewhere ...
+ * context_guard_struct(some_state);
+ *
+ * Note: The implementation defines several helper functions that can acquire
+ * and release the context guard.
+ */
+#define context_guard_struct(name, ...) \
+ struct __ctx_guard_type(name) __VA_ARGS__ name; \
+ static __always_inline void __acquire_ctx_guard(const struct name *var) \
+ __attribute__((overloadable)) __no_context_analysis __acquires_ctx_guard(var) { } \
+ static __always_inline void __acquire_shared_ctx_guard(const struct name *var) \
+ __attribute__((overloadable)) __no_context_analysis __acquires_shared_ctx_guard(var) { } \
+ static __always_inline bool __try_acquire_ctx_guard(const struct name *var, bool ret) \
+ __attribute__((overloadable)) __no_context_analysis __try_acquires_ctx_guard(1, var) \
+ { return ret; } \
+ static __always_inline bool __try_acquire_shared_ctx_guard(const struct name *var, bool ret) \
+ __attribute__((overloadable)) __no_context_analysis __try_acquires_shared_ctx_guard(1, var) \
+ { return ret; } \
+ static __always_inline void __release_ctx_guard(const struct name *var) \
+ __attribute__((overloadable)) __no_context_analysis __releases_ctx_guard(var) { } \
+ static __always_inline void __release_shared_ctx_guard(const struct name *var) \
+ __attribute__((overloadable)) __no_context_analysis __releases_shared_ctx_guard(var) { } \
+ static __always_inline void __assume_ctx_guard(const struct name *var) \
+ __attribute__((overloadable)) __assumes_ctx_guard(var) { } \
+ static __always_inline void __assume_shared_ctx_guard(const struct name *var) \
+ __attribute__((overloadable)) __assumes_shared_ctx_guard(var) { } \
+ struct name
+
+/**
+ * disable_context_analysis() - disables context analysis
+ *
+ * Disables context analysis. Must be paired with a later
+ * enable_context_analysis().
+ */
+#define disable_context_analysis() \
+ __diag_push(); \
+ __diag_ignore_all("-Wunknown-warning-option", "") \
+ __diag_ignore_all("-Wthread-safety", "") \
+ __diag_ignore_all("-Wthread-safety-pointer", "")
+
+/**
+ * enable_context_analysis() - re-enables context analysis
+ *
+ * Re-enables context analysis. Must be paired with a prior
+ * disable_context_analysis().
+ */
+#define enable_context_analysis() __diag_pop()
+
+/**
+ * __no_context_analysis - function attribute, disables context analysis
+ *
+ * Function attribute denoting that context analysis is disabled for the
+ * whole function. Prefer use of `context_unsafe()` where possible.
+ */
+#define __no_context_analysis __attribute__((no_thread_safety_analysis))
+
+#endif /* _LINUX_COMPILER_CONTEXT_ANALYSIS_CLANG_H */
diff --git a/include/linux/compiler-context-analysis.h b/include/linux/compiler-context-analysis.h
index 03056f87a86f..33ad367fef3f 100644
--- a/include/linux/compiler-context-analysis.h
+++ b/include/linux/compiler-context-analysis.h
@@ -6,140 +6,7 @@
#ifndef _LINUX_COMPILER_CONTEXT_ANALYSIS_H
#define _LINUX_COMPILER_CONTEXT_ANALYSIS_H
-#if defined(WARN_CONTEXT_ANALYSIS)
-
-/*
- * These attributes define new context guard (Clang: capability) types.
- * Internal only.
- */
-# define __ctx_guard_type(name) __attribute__((capability(#name)))
-# define __reentrant_ctx_guard __attribute__((reentrant_capability))
-# define __acquires_ctx_guard(...) __attribute__((acquire_capability(__VA_ARGS__)))
-# define __acquires_shared_ctx_guard(...) __attribute__((acquire_shared_capability(__VA_ARGS__)))
-# define __try_acquires_ctx_guard(ret, var) __attribute__((try_acquire_capability(ret, var)))
-# define __try_acquires_shared_ctx_guard(ret, var) __attribute__((try_acquire_shared_capability(ret, var)))
-# define __releases_ctx_guard(...) __attribute__((release_capability(__VA_ARGS__)))
-# define __releases_shared_ctx_guard(...) __attribute__((release_shared_capability(__VA_ARGS__)))
-# define __assumes_ctx_guard(...) __attribute__((assert_capability(__VA_ARGS__)))
-# define __assumes_shared_ctx_guard(...) __attribute__((assert_shared_capability(__VA_ARGS__)))
-# define __returns_ctx_guard(var) __attribute__((lock_returned(var)))
-
-/*
- * The below are used to annotate code being checked. Internal only.
- */
-# define __excludes_ctx_guard(...) __attribute__((locks_excluded(__VA_ARGS__)))
-# define __requires_ctx_guard(...) __attribute__((requires_capability(__VA_ARGS__)))
-# define __requires_shared_ctx_guard(...) __attribute__((requires_shared_capability(__VA_ARGS__)))
-
-/**
- * __guarded_by - struct member and globals attribute, declares variable
- * only accessible within active context
- *
- * Declares that the struct member or global variable is only accessible within
- * the context entered by the given context guard. Read operations on the data
- * require shared access, while write operations require exclusive access.
- *
- * .. code-block:: c
- *
- * struct some_state {
- * spinlock_t lock;
- * long counter __guarded_by(&lock);
- * };
- */
-# define __guarded_by(...) __attribute__((guarded_by(__VA_ARGS__)))
-
-/**
- * __pt_guarded_by - struct member and globals attribute, declares pointed-to
- * data only accessible within active context
- *
- * Declares that the data pointed to by the struct member pointer or global
- * pointer is only accessible within the context entered by the given context
- * guard. Read operations on the data require shared access, while write
- * operations require exclusive access.
- *
- * .. code-block:: c
- *
- * struct some_state {
- * spinlock_t lock;
- * long *counter __pt_guarded_by(&lock);
- * };
- */
-# define __pt_guarded_by(...) __attribute__((pt_guarded_by(__VA_ARGS__)))
-
-/**
- * context_guard_struct() - declare or define a context guard struct
- * @name: struct name
- *
- * Helper to declare or define a struct type that is also a context guard.
- *
- * .. code-block:: c
- *
- * context_guard_struct(my_handle) {
- * int foo;
- * long bar;
- * };
- *
- * struct some_state {
- * ...
- * };
- * // ... declared elsewhere ...
- * context_guard_struct(some_state);
- *
- * Note: The implementation defines several helper functions that can acquire
- * and release the context guard.
- */
-# define context_guard_struct(name, ...) \
- struct __ctx_guard_type(name) __VA_ARGS__ name; \
- static __always_inline void __acquire_ctx_guard(const struct name *var) \
- __attribute__((overloadable)) __no_context_analysis __acquires_ctx_guard(var) { } \
- static __always_inline void __acquire_shared_ctx_guard(const struct name *var) \
- __attribute__((overloadable)) __no_context_analysis __acquires_shared_ctx_guard(var) { } \
- static __always_inline bool __try_acquire_ctx_guard(const struct name *var, bool ret) \
- __attribute__((overloadable)) __no_context_analysis __try_acquires_ctx_guard(1, var) \
- { return ret; } \
- static __always_inline bool __try_acquire_shared_ctx_guard(const struct name *var, bool ret) \
- __attribute__((overloadable)) __no_context_analysis __try_acquires_shared_ctx_guard(1, var) \
- { return ret; } \
- static __always_inline void __release_ctx_guard(const struct name *var) \
- __attribute__((overloadable)) __no_context_analysis __releases_ctx_guard(var) { } \
- static __always_inline void __release_shared_ctx_guard(const struct name *var) \
- __attribute__((overloadable)) __no_context_analysis __releases_shared_ctx_guard(var) { } \
- static __always_inline void __assume_ctx_guard(const struct name *var) \
- __attribute__((overloadable)) __assumes_ctx_guard(var) { } \
- static __always_inline void __assume_shared_ctx_guard(const struct name *var) \
- __attribute__((overloadable)) __assumes_shared_ctx_guard(var) { } \
- struct name
-
-/**
- * disable_context_analysis() - disables context analysis
- *
- * Disables context analysis. Must be paired with a later
- * enable_context_analysis().
- */
-# define disable_context_analysis() \
- __diag_push(); \
- __diag_ignore_all("-Wunknown-warning-option", "") \
- __diag_ignore_all("-Wthread-safety", "") \
- __diag_ignore_all("-Wthread-safety-pointer", "")
-
-/**
- * enable_context_analysis() - re-enables context analysis
- *
- * Re-enables context analysis. Must be paired with a prior
- * disable_context_analysis().
- */
-# define enable_context_analysis() __diag_pop()
-
-/**
- * __no_context_analysis - function attribute, disables context analysis
- *
- * Function attribute denoting that context analysis is disabled for the
- * whole function. Prefer use of `context_unsafe()` where possible.
- */
-# define __no_context_analysis __attribute__((no_thread_safety_analysis))
-
-#else /* !WARN_CONTEXT_ANALYSIS */
-
+#if !defined(WARN_CONTEXT_ANALYSIS)
# define __ctx_guard_type(name)
# define __reentrant_ctx_guard
# define __acquires_ctx_guard(...)
@@ -168,7 +35,6 @@
# define disable_context_analysis()
# define enable_context_analysis()
# define __no_context_analysis
-
#endif /* WARN_CONTEXT_ANALYSIS */
/**
diff --git a/scripts/Makefile.context-analysis b/scripts/Makefile.context-analysis
index cd3bb49d3f09..6f94b555af14 100644
--- a/scripts/Makefile.context-analysis
+++ b/scripts/Makefile.context-analysis
@@ -2,7 +2,8 @@
context-analysis-cflags := -DWARN_CONTEXT_ANALYSIS \
-fexperimental-late-parse-attributes -Wthread-safety \
- -Wthread-safety-pointer -Wthread-safety-beta
+ -Wthread-safety-pointer -Wthread-safety-beta \
+ -include $(srctree)/include/linux/compiler-context-analysis-clang.h
ifndef CONFIG_WARN_CONTEXT_ANALYSIS_ALL
context-analysis-cflags += --warning-suppression-mappings=$(srctree)/scripts/context-analysis-suppression.txt
--
2.52.0.rc2.455.g230fcf2819-goog
^ permalink raw reply related
* Re: Re: [BUG] landlock: sleeping function called from invalid context in hook_sb_delete()
From: 许佳凯 @ 2025-11-21 1:01 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Günther Noack, linux-kernel, linux-security-module,
Günther Noack, Serge E. Hallyn
In-Reply-To: <20251120.hoT5ye5thooR@digikod.net>
Hello Mickaël,
Thanks a lot for your reply.
Best regards,
-Jiakai
^ permalink raw reply
* Re: [RFC][PATCH] exec: Move cred computation under exec_update_lock
From: Bernd Edlinger @ 2025-11-21 2:59 UTC (permalink / raw)
To: Eric W. Biederman
Cc: Alexander Viro, Alexey Dobriyan, Oleg Nesterov, Kees Cook,
Andy Lutomirski, Will Drewry, Christian Brauner, Andrew Morton,
Michal Hocko, Serge Hallyn, James Morris, Randy Dunlap,
Suren Baghdasaryan, Yafang Shao, Helge Deller, Adrian Reber,
Thomas Gleixner, Jens Axboe, Alexei Starovoitov,
linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-kselftest, linux-mm, linux-security-module, tiozhang,
Luis Chamberlain, Paulo Alcantara (SUSE), Sergey Senozhatsky,
Frederic Weisbecker, YueHaibing, Paul Moore, Aleksa Sarai,
Stefan Roesch, Chao Yu, xu xin, Jeff Layton, Jan Kara,
David Hildenbrand, Dave Chinner, Shuah Khan, Elena Reshetova,
David Windsor, Mateusz Guzik, Ard Biesheuvel,
Joel Fernandes (Google), Matthew Wilcox (Oracle),
Hans Liljestrand, Penglei Jiang, Lorenzo Stoakes, Adrian Ratiu,
Ingo Molnar, Peter Zijlstra (Intel), Cyrill Gorcunov,
Eric Dumazet
In-Reply-To: <87a50gxo0i.fsf@email.froward.int.ebiederm.org>
Hi Eric,
thanks for you valuable input on the topic.
On 11/21/25 00:50, Eric W. Biederman wrote:
> "Eric W. Biederman" <ebiederm@xmission.com> writes:
>
>> Instead of computing the new cred before we pass the point of no
>> return compute the new cred just before we use it.
>>
>> This allows the removal of fs_struct->in_exec and cred_guard_mutex.
>>
>> I am not certain why we wanted to compute the cred for the new
>> executable so early. Perhaps I missed something but I did not see any
>> common errors being signaled. So I don't think we loose anything by
>> computing the new cred later.
>
> I should add that the permission checks happen in open_exec,
> everything that follows credential wise is just about representing in
> struct cred the credentials the new executable will have.
>
> So I am really at a loss why we have had this complicated way of
> computing of computed the credentials all of these years full of
> time of check to time of use problems.
>
Well, I think I see a problem with your patch:
When the security engine gets the LSM_UNSAFE_PTRACE flag, it might
e.g. return -EPERM in bprm_creds_for_exec in the apparmor, selinux
or the smack security engines at least. Previously that callback
was called before the point of no return, and the return code should
be returned as a return code the the caller of execve. But if we move
that check after the point of no return, the caller will get killed
due to the failed security check.
Or did I miss something?
Thanks
Bernd.
> Eric
^ permalink raw reply
* Re: [RFC][PATCH] exec: Move cred computation under exec_update_lock
From: Eric W. Biederman @ 2025-11-21 7:18 UTC (permalink / raw)
To: Bernd Edlinger
Cc: Alexander Viro, Alexey Dobriyan, Oleg Nesterov, Kees Cook,
Andy Lutomirski, Will Drewry, Christian Brauner, Andrew Morton,
Michal Hocko, Serge Hallyn, James Morris, Randy Dunlap,
Suren Baghdasaryan, Yafang Shao, Helge Deller, Adrian Reber,
Thomas Gleixner, Jens Axboe, Alexei Starovoitov,
linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-kselftest, linux-mm, linux-security-module, tiozhang,
Luis Chamberlain, Paulo Alcantara (SUSE), Sergey Senozhatsky,
Frederic Weisbecker, YueHaibing, Paul Moore, Aleksa Sarai,
Stefan Roesch, Chao Yu, xu xin, Jeff Layton, Jan Kara,
David Hildenbrand, Dave Chinner, Shuah Khan, Elena Reshetova,
David Windsor, Mateusz Guzik, Ard Biesheuvel,
Joel Fernandes (Google), Matthew Wilcox (Oracle),
Hans Liljestrand, Penglei Jiang, Lorenzo Stoakes, Adrian Ratiu,
Ingo Molnar, Peter Zijlstra (Intel), Cyrill Gorcunov,
Eric Dumazet
In-Reply-To: <GV2PPF74270EBEEAD4CACA124C05BE1CE45E4D5A@GV2PPF74270EBEE.EURP195.PROD.OUTLOOK.COM>
Bernd Edlinger <bernd.edlinger@hotmail.de> writes:
> Hi Eric,
>
> thanks for you valuable input on the topic.
>
> On 11/21/25 00:50, Eric W. Biederman wrote:
>> "Eric W. Biederman" <ebiederm@xmission.com> writes:
>>
>>> Instead of computing the new cred before we pass the point of no
>>> return compute the new cred just before we use it.
>>>
>>> This allows the removal of fs_struct->in_exec and cred_guard_mutex.
>>>
>>> I am not certain why we wanted to compute the cred for the new
>>> executable so early. Perhaps I missed something but I did not see any
>>> common errors being signaled. So I don't think we loose anything by
>>> computing the new cred later.
>>
>> I should add that the permission checks happen in open_exec,
>> everything that follows credential wise is just about representing in
>> struct cred the credentials the new executable will have.
>>
>> So I am really at a loss why we have had this complicated way of
>> computing of computed the credentials all of these years full of
>> time of check to time of use problems.
>>
>
> Well, I think I see a problem with your patch:
>
> When the security engine gets the LSM_UNSAFE_PTRACE flag, it might
> e.g. return -EPERM in bprm_creds_for_exec in the apparmor, selinux
> or the smack security engines at least. Previously that callback
> was called before the point of no return, and the return code should
> be returned as a return code the the caller of execve. But if we move
> that check after the point of no return, the caller will get killed
> due to the failed security check.
>
> Or did I miss something?
I think we definitely need to document this change in behavior. I would
call ending the exec with SIGSEGV vs -EPERM a quality of implementation
issue. The exec is failing one way or the other so I don't see it as a
correctness issue.
In the case of ptrace in general I think it is a bug if the mere act of
debugging a program changes it's behavior. So which buggy behavior
should we prefer? SIGSEGV where it is totally clear that the behavior
has changed or -EPERM and ask the debugged program to handle it.
I lean towards SIGSEGV because then it is clear the code should not
handle it.
In the case of LSM_UNSAFE_NO_NEW_PRIVS I believe the preferred way to
handle unexpected things happening is to terminate the application.
In the case of LSM_UNSAFE_SHARE -EPERM might be better. I don't know
of any good uses of any good uses of sys_clone(CLONE_FS ...) outside
of CLONE_THREAD.
Plus all of these things are only considerations if we are exec'ing a
program that transitions to a different set of credentials. Something
that happens but is quite rare itself.
In practice I don't expect there is anything that depends on the exact
behavior of what happens when exec'ing a suid executable to gain
privileges when ptraced. The closes I can imagine is upstart and
I think upstart ran as root when ptracing other programs so there is no
gaining of privilege and thus no reason for a security module to
complain.
Who knows I could be wrong, and someone could actually care. Which is
hy I think we should document it.
Eric
^ permalink raw reply
* Re: [RFC][PATCH] exec: Move cred computation under exec_update_lock
From: Bernd Edlinger @ 2025-11-21 9:35 UTC (permalink / raw)
To: Eric W. Biederman
Cc: Alexander Viro, Alexey Dobriyan, Oleg Nesterov, Kees Cook,
Andy Lutomirski, Will Drewry, Christian Brauner, Andrew Morton,
Michal Hocko, Serge Hallyn, James Morris, Randy Dunlap,
Suren Baghdasaryan, Yafang Shao, Helge Deller, Adrian Reber,
Thomas Gleixner, Jens Axboe, Alexei Starovoitov,
linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-kselftest, linux-mm, linux-security-module, tiozhang,
Luis Chamberlain, Paulo Alcantara (SUSE), Sergey Senozhatsky,
Frederic Weisbecker, YueHaibing, Paul Moore, Aleksa Sarai,
Stefan Roesch, Chao Yu, xu xin, Jeff Layton, Jan Kara,
David Hildenbrand, Dave Chinner, Shuah Khan, Elena Reshetova,
David Windsor, Mateusz Guzik, Ard Biesheuvel,
Joel Fernandes (Google), Matthew Wilcox (Oracle),
Hans Liljestrand, Penglei Jiang, Lorenzo Stoakes, Adrian Ratiu,
Ingo Molnar, Peter Zijlstra (Intel), Cyrill Gorcunov,
Eric Dumazet
In-Reply-To: <87o6ovx38h.fsf@email.froward.int.ebiederm.org>
On 11/21/25 08:18, Eric W. Biederman wrote:
> Bernd Edlinger <bernd.edlinger@hotmail.de> writes:
>
>> Hi Eric,
>>
>> thanks for you valuable input on the topic.
>>
>> On 11/21/25 00:50, Eric W. Biederman wrote:
>>> "Eric W. Biederman" <ebiederm@xmission.com> writes:
>>>
>>>> Instead of computing the new cred before we pass the point of no
>>>> return compute the new cred just before we use it.
>>>>
>>>> This allows the removal of fs_struct->in_exec and cred_guard_mutex.
>>>>
>>>> I am not certain why we wanted to compute the cred for the new
>>>> executable so early. Perhaps I missed something but I did not see any
>>>> common errors being signaled. So I don't think we loose anything by
>>>> computing the new cred later.
>>>
>>> I should add that the permission checks happen in open_exec,
>>> everything that follows credential wise is just about representing in
>>> struct cred the credentials the new executable will have.
>>>
>>> So I am really at a loss why we have had this complicated way of
>>> computing of computed the credentials all of these years full of
>>> time of check to time of use problems.
>>>
>>
>> Well, I think I see a problem with your patch:
>>
>> When the security engine gets the LSM_UNSAFE_PTRACE flag, it might
>> e.g. return -EPERM in bprm_creds_for_exec in the apparmor, selinux
>> or the smack security engines at least. Previously that callback
>> was called before the point of no return, and the return code should
>> be returned as a return code the the caller of execve. But if we move
>> that check after the point of no return, the caller will get killed
>> due to the failed security check.
>>
>> Or did I miss something?
>
> I think we definitely need to document this change in behavior. I would
> call ending the exec with SIGSEGV vs -EPERM a quality of implementation
> issue. The exec is failing one way or the other so I don't see it as a
> correctness issue.
>
> In the case of ptrace in general I think it is a bug if the mere act of
> debugging a program changes it's behavior. So which buggy behavior
> should we prefer? SIGSEGV where it is totally clear that the behavior
> has changed or -EPERM and ask the debugged program to handle it.
> I lean towards SIGSEGV because then it is clear the code should not
> handle it.
>
> In the case of LSM_UNSAFE_NO_NEW_PRIVS I believe the preferred way to
> handle unexpected things happening is to terminate the application.
>
> In the case of LSM_UNSAFE_SHARE -EPERM might be better. I don't know
> of any good uses of any good uses of sys_clone(CLONE_FS ...) outside
> of CLONE_THREAD.
>
>
> Plus all of these things are only considerations if we are exec'ing a
> program that transitions to a different set of credentials. Something
> that happens but is quite rare itself.
>
> In practice I don't expect there is anything that depends on the exact
> behavior of what happens when exec'ing a suid executable to gain
> privileges when ptraced. The closes I can imagine is upstart and
> I think upstart ran as root when ptracing other programs so there is no
> gaining of privilege and thus no reason for a security module to
> complain.
>
> Who knows I could be wrong, and someone could actually care. Which is> hy I think we should document it.
>
Well, I dont know for sure, but the security engine could deny the execution
for any reason, not only because of being ptraced.
Maybe there can be a policy which denies user X to execute e.g. any suid programs.
Bernd.
^ permalink raw reply
* Re: [RFC][PATCH] exec: Move cred computation under exec_update_lock
From: Bernd Edlinger @ 2025-11-21 11:26 UTC (permalink / raw)
To: Eric W. Biederman
Cc: Alexander Viro, Alexey Dobriyan, Oleg Nesterov, Kees Cook,
Andy Lutomirski, Will Drewry, Christian Brauner, Andrew Morton,
Michal Hocko, Serge Hallyn, James Morris, Randy Dunlap,
Suren Baghdasaryan, Yafang Shao, Helge Deller, Adrian Reber,
Thomas Gleixner, Jens Axboe, Alexei Starovoitov,
linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-kselftest, linux-mm, linux-security-module, tiozhang,
Luis Chamberlain, Paulo Alcantara (SUSE), Sergey Senozhatsky,
Frederic Weisbecker, YueHaibing, Paul Moore, Aleksa Sarai,
Stefan Roesch, Chao Yu, xu xin, Jeff Layton, Jan Kara,
David Hildenbrand, Dave Chinner, Shuah Khan, Elena Reshetova,
David Windsor, Mateusz Guzik, Ard Biesheuvel,
Joel Fernandes (Google), Matthew Wilcox (Oracle),
Hans Liljestrand, Penglei Jiang, Lorenzo Stoakes, Adrian Ratiu,
Ingo Molnar, Peter Zijlstra (Intel), Cyrill Gorcunov,
Eric Dumazet
In-Reply-To: <GV2PPF74270EBEEFA106F4EF26B087ED898E4D5A@GV2PPF74270EBEE.EURP195.PROD.OUTLOOK.COM>
On 11/21/25 10:35, Bernd Edlinger wrote:
> On 11/21/25 08:18, Eric W. Biederman wrote:
>> Bernd Edlinger <bernd.edlinger@hotmail.de> writes:
>>
>>> Hi Eric,
>>>
>>> thanks for you valuable input on the topic.
>>>
>>> On 11/21/25 00:50, Eric W. Biederman wrote:
>>>> "Eric W. Biederman" <ebiederm@xmission.com> writes:
>>>>
>>>>> Instead of computing the new cred before we pass the point of no
>>>>> return compute the new cred just before we use it.
>>>>>
>>>>> This allows the removal of fs_struct->in_exec and cred_guard_mutex.
>>>>>
>>>>> I am not certain why we wanted to compute the cred for the new
>>>>> executable so early. Perhaps I missed something but I did not see any
>>>>> common errors being signaled. So I don't think we loose anything by
>>>>> computing the new cred later.
>>>>
>>>> I should add that the permission checks happen in open_exec,
>>>> everything that follows credential wise is just about representing in
>>>> struct cred the credentials the new executable will have.
>>>>
>>>> So I am really at a loss why we have had this complicated way of
>>>> computing of computed the credentials all of these years full of
>>>> time of check to time of use problems.
>>>>
>>>
>>> Well, I think I see a problem with your patch:
>>>
>>> When the security engine gets the LSM_UNSAFE_PTRACE flag, it might
>>> e.g. return -EPERM in bprm_creds_for_exec in the apparmor, selinux
>>> or the smack security engines at least. Previously that callback
>>> was called before the point of no return, and the return code should
>>> be returned as a return code the the caller of execve. But if we move
>>> that check after the point of no return, the caller will get killed
>>> due to the failed security check.
>>>
>>> Or did I miss something?
>>
>> I think we definitely need to document this change in behavior. I would
>> call ending the exec with SIGSEGV vs -EPERM a quality of implementation
>> issue. The exec is failing one way or the other so I don't see it as a
>> correctness issue.
>>
>> In the case of ptrace in general I think it is a bug if the mere act of
>> debugging a program changes it's behavior. So which buggy behavior
>> should we prefer? SIGSEGV where it is totally clear that the behavior
>> has changed or -EPERM and ask the debugged program to handle it.
>> I lean towards SIGSEGV because then it is clear the code should not
>> handle it.
>>
>> In the case of LSM_UNSAFE_NO_NEW_PRIVS I believe the preferred way to
>> handle unexpected things happening is to terminate the application.
>>
>> In the case of LSM_UNSAFE_SHARE -EPERM might be better. I don't know
>> of any good uses of any good uses of sys_clone(CLONE_FS ...) outside
>> of CLONE_THREAD.
>>
>>
>> Plus all of these things are only considerations if we are exec'ing a
>> program that transitions to a different set of credentials. Something
>> that happens but is quite rare itself.
>>
>> In practice I don't expect there is anything that depends on the exact
>> behavior of what happens when exec'ing a suid executable to gain
>> privileges when ptraced. The closes I can imagine is upstart and
>> I think upstart ran as root when ptracing other programs so there is no
>> gaining of privilege and thus no reason for a security module to
>> complain.
>>
>> Who knows I could be wrong, and someone could actually care. Which is
>> hy I think we should document it.>>
>
>
> Well, I dont know for sure, but the security engine could deny the execution
> for any reason, not only because of being ptraced.
> Maybe there can be a policy which denies user X to execute e.g. any suid programs.
>
>
> Bernd.
>
Hmm, funny..
I installed this patch on top of
commit fd95357fd8c6778ac7dea6c57a19b8b182b6e91f (HEAD -> master, origin/master, origin/HEAD)
Merge: c966813ea120 7b6216baae75
Author: Linus Torvalds <torvalds@linux-foundation.org>
Date: Thu Nov 20 11:04:37 2025 -0800
but it does panic when I try to boot:
[ 0.870539] TERM=1inux
[ 0.870573] Starting init: /bin/sh exists but couldn't execute it (error -14) 0.8705751 Kernel panic- not syncing: No working init found. Try passing i mit= option to kernel. See Linux Documentation/admin-guide/init.rst for guidance
[ 0.870577] CPU: UID: 0 PID: 1 Comm: sh Not tainted 6.18.0-rc6+ #1 PREEMPT(voluntary)
[ 0.870579] Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBo x 12/01/2006
[ 0.870580] Call Trace:
[ 0.870590] <TASK>
[ 0.870592] vpanic+0x36d/0x380
[ 0.870607] ? __pfx_kernel_init+0x10/0x10
[ 0.870615] panic+0x5b/0x60
[ 0.870617] kernel_init+0x17d/0x1c0
[ 0.870623] ret_from_fork+0x124/0x150
[ 0.870625} ? __pfx_kernel_init+0x10/0x10
[ 0.870627] ret_from_fork_asm+0x1a/0x30
[ 0.870632] </TASK>
[ 0.8706631 Kernel Offset: 0x3a800000 from Oxffffffff81000000 (relocation ran ge: 0xffffffff80000000-0xffffffffbfffffff)
[ 0.880034] ---[ end Kernel panic - not syncing: No working init found. Try passing init option to kernel. See Linux Documentation/admin-guide/init.rst for guidance. 1---`
Is that a known problem?
Bernd.
^ permalink raw reply
* Re: [PATCH v4 02/10] landlock: Add API support and docs for the quiet flags
From: Mickaël Salaün @ 2025-11-21 15:27 UTC (permalink / raw)
To: Tingmao Wang
Cc: Günther Noack, Justin Suess, Jan Kara, Abhinav Saxena,
linux-security-module
In-Reply-To: <cd79fcf71e6d38ab4280c0de2500fa5f6b06cd9d.1763330228.git.m@maowtm.org>
On Sun, Nov 16, 2025 at 09:59:32PM +0000, Tingmao Wang wrote:
> Adds the UAPI for the quiet flags feature (but not the implementation
> yet).
>
> According to pahole, even after adding the struct access_masks quiet_masks
> in struct landlock_hierarchy, the u32 log_* bitfield still only has a size
> of 2 bytes, so there's minimal wasted space.
>
> Signed-off-by: Tingmao Wang <m@maowtm.org>
> ---
>
> Changes since v3:
> - Minor update to this commit message.
> - Fix minor formatting
>
> Changes since v2:
> - Updated docs from Mickaël's suggestions.
>
> Changes since v1:
> - Per suggestion, added support for quieting only certain access bits,
> controlled by extra quiet_access_* fields in the ruleset_attr.
> - Added docs for the extra fields and made updates to doc changes in v1.
> In particular, call out that the effect of LANDLOCK_ADD_RULE_QUIET is
> independent from the access bits passed in rule_attr
> - landlock_add_rule will return -EINVAL when LANDLOCK_ADD_RULE_QUIET is
> used but the ruleset does not have any quiet access bits set for the
> given rule type.
> - ABI version bump to v8
> - Syntactic and comment changes per suggestion.
>
> include/uapi/linux/landlock.h | 64 +++++++++++++++++
> security/landlock/domain.h | 5 ++
> security/landlock/fs.c | 4 +-
> security/landlock/fs.h | 2 +-
> security/landlock/net.c | 5 +-
> security/landlock/net.h | 3 +-
> security/landlock/ruleset.c | 10 ++-
> security/landlock/ruleset.h | 8 ++-
> security/landlock/syscalls.c | 72 +++++++++++++++-----
> tools/testing/selftests/landlock/base_test.c | 4 +-
> 10 files changed, 150 insertions(+), 27 deletions(-)
>
> diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
> index f030adc462ee..50f0806b7e33 100644
> --- a/include/uapi/linux/landlock.h
> +++ b/include/uapi/linux/landlock.h
> @@ -32,6 +32,19 @@
> * *handle* a wide range or all access rights that they know about at build time
> * (and that they have tested with a kernel that supported them all).
> *
> + * @quiet_access_fs and @quiet_access_net are bitmasks of actions for
> + * which a denial by this layer will not trigger an audit log if the
> + * corresponding object (or its children, for filesystem rules) is marked
> + * with the "quiet" bit via %LANDLOCK_ADD_RULE_QUIET, even if logging
> + * would normally take place per landlock_restrict_self() flags.
> + * quiet_scoped is similar, except that it does not require marking any
> + * objects as quiet - if the ruleset is created with any bits set in
> + * quiet_scoped, then denial of such scoped resources will not trigger any
> + * log. These 3 fields are available since Landlock ABI version 8.
> + *
> + * @quiet_access_fs, @quiet_access_net and @quiet_scoped must be a subset
> + * of @handled_access_fs, @handled_access_net and @scoped respectively.
> + *
> * This structure can grow in future Landlock versions.
> */
> struct landlock_ruleset_attr {
> @@ -51,6 +64,24 @@ struct landlock_ruleset_attr {
> * resources (e.g. IPCs).
> */
> __u64 scoped;
> +
> + /* Since ABI 8: */
> +
> + /**
> + * @quiet_access_fs: Bitmask of filesystem actions which should not be
> + * audit logged if per-object quiet flag is set.
> + */
> + __u64 quiet_access_fs;
> + /**
> + * @quiet_access_net: Bitmask of network actions which should not be
> + * audit logged if per-object quiet flag is set.
> + */
> + __u64 quiet_access_net;
> + /**
> + * @quiet_scoped: Bitmask of scoped actions which should not be audit
> + * logged.
> + */
> + __u64 quiet_scoped;
> };
>
> /**
> @@ -69,6 +100,39 @@ struct landlock_ruleset_attr {
> #define LANDLOCK_CREATE_RULESET_ERRATA (1U << 1)
> /* clang-format on */
>
> +/**
> + * DOC: landlock_add_rule_flags
> + *
> + * **Flags**
> + *
> + * %LANDLOCK_ADD_RULE_QUIET
> + * Together with the quiet_* fields in struct landlock_ruleset_attr,
> + * this flag controls whether Landlock will log audit messages when
> + * access to the objects covered by this rule is denied by this layer.
> + *
> + * If audit logging is enabled, when Landlock denies an access, it will
> + * suppress the audit log if all of the following are true:
> + *
> + * - This layer is the innermost layer that denied the access;
Because these items follows ":" they should not start with a capital
letter (e.g. "- this layer ...").
> + * - All requested accesses are part of the quiet_* fields in the
> + * related struct landlock_ruleset_attr;
This should be updated to reflect my comment in the next patch about
landlock_log_denial().
> + * - The object (or one of its parents, for filesystem rules) is
> + * marked as "quiet" via %LANDLOCK_ADD_RULE_QUIET.
> + *
> + * Because logging is only suppressed by a layer if the layer denies
> + * access, a sandboxed program cannot use this flag to "hide" access
> + * denials, without denying itself the access in the first place.
> + *
> + * The effect of this flag does not depend on the value of
> + * allowed_access in the passed in rule_attr. When this flag is
> + * present, the caller is also allowed to pass in an empty
> + * allowed_access.
> + */
> +
> +/* clang-format off */
> +#define LANDLOCK_ADD_RULE_QUIET (1U << 0)
> +/* clang-format on */
> +
> /**
> * DOC: landlock_restrict_self_flags
> *
> diff --git a/security/landlock/domain.h b/security/landlock/domain.h
> index 7fb70b25f85a..aadbf53505c0 100644
> --- a/security/landlock/domain.h
> +++ b/security/landlock/domain.h
> @@ -114,6 +114,11 @@ struct landlock_hierarchy {
> * %LANDLOCK_RESTRICT_SELF_LOG_NEW_EXEC_ON. Set to false by default.
> */
> log_new_exec : 1;
> + /**
> + * @quiet_masks: Bitmasks of access that should be quieted (i.e. not
> + * logged) if the related object is marked as quiet.
> + */
> + struct access_masks quiet_masks;
Please update the above @work_free doc.
> #endif /* CONFIG_AUDIT */
> };
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox