* Re: [PATCH v2] rcu: Remove redundant rcu_read_lock/unlock() in spin_lock critical sections
From: Waiman Long @ 2025-09-15 14:13 UTC (permalink / raw)
To: pengdonglin, tj, tony.luck, jani.nikula, ap420073, jv, freude,
bcrl, trondmy, kees
Cc: bigeasy, linux-kernel, linux-rt-devel, linux-nfs, linux-aio,
linux-fsdevel, linux-security-module, netdev, intel-gfx,
linux-acpi, linux-s390, cgroups, Hillf Danton, Paul E . McKenney,
pengdonglin
In-Reply-To: <20250915134729.1801557-1-dolinux.peng@gmail.com>
On 9/15/25 9:47 AM, pengdonglin wrote:
> From: pengdonglin <pengdonglin@xiaomi.com>
>
> Per Documentation/RCU/rcu_dereference.rst [1], since Linux 4.20's RCU
> consolidation [2][3], RCU read-side critical sections include:
> - Explicit rcu_read_lock()
> - BH/interrupt/preemption-disabling regions
> - Spinlock critical sections (including CONFIG_PREEMPT_RT kernels [4])
>
> Thus, explicit rcu_read_lock()/unlock() calls within spin_lock*() regions are redundant.
> This patch removes them, simplifying locking semantics while preserving RCU protection.
>
> [1] https://elixir.bootlin.com/linux/v6.17-rc5/source/Documentation/RCU/rcu_dereference.rst#L407
> [2] https://lore.kernel.org/lkml/20180829222021.GA29944@linux.vnet.ibm.com/
> [3] https://lwn.net/Articles/777036/
> [4] https://lore.kernel.org/lkml/6435833a-bdcb-4114-b29d-28b7f436d47d@paulmck-laptop/
>
> Cc: Waiman Long <longman@redhat.com>
> Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Cc: Hillf Danton <hdanton@sina.com>
> Cc: Paul E. McKenney <paulmck@kernel.org>
> Signed-off-by: pengdonglin <pengdonglin@xiaomi.com>
> Signed-off-by: pengdonglin <dolinux.peng@gmail.com>
> ---
> Changes in v2:
> - Clarified commit message to prevent accidental backport to older kernels
> - Added lockdep_is_held() to avoid false positives
> ---
> drivers/acpi/apei/ghes.c | 2 --
> drivers/gpu/drm/i915/gt/intel_ring_submission.c | 2 --
> drivers/net/amt.c | 8 --------
> drivers/net/bonding/bond_3ad.c | 2 --
> drivers/net/wireless/ath/ath9k/xmit.c | 2 --
> drivers/s390/crypto/pkey_base.c | 3 ---
> fs/aio.c | 6 ++----
> fs/nfs/callback_proc.c | 2 --
> fs/nfs/nfs4state.c | 2 --
> fs/nfs/pnfs.c | 9 ---------
> fs/nfs/pnfs_dev.c | 4 ----
> ipc/msg.c | 1 -
> ipc/sem.c | 1 -
> ipc/shm.c | 1 -
> ipc/util.c | 2 --
> kernel/cgroup/cgroup.c | 2 --
> kernel/cgroup/cpuset.c | 6 ------
> kernel/cgroup/debug.c | 4 ----
> net/mac80211/cfg.c | 2 --
> net/mac80211/debugfs.c | 2 --
> net/mac80211/debugfs_netdev.c | 2 --
> net/mac80211/debugfs_sta.c | 2 --
> net/mac80211/sta_info.c | 2 --
> net/ncsi/ncsi-manage.c | 2 --
> security/yama/yama_lsm.c | 4 ----
> 25 files changed, 2 insertions(+), 73 deletions(-)
>
...
> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index 27adb04df675..9b7e8e8e9411 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -4073,7 +4073,6 @@ void cpuset_cpus_allowed(struct task_struct *tsk, struct cpumask *pmask)
> struct cpuset *cs;
>
> spin_lock_irqsave(&callback_lock, flags);
> - rcu_read_lock();
>
> cs = task_cs(tsk);
> if (cs != &top_cpuset)
> @@ -4095,7 +4094,6 @@ void cpuset_cpus_allowed(struct task_struct *tsk, struct cpumask *pmask)
> cpumask_copy(pmask, possible_mask);
> }
>
> - rcu_read_unlock();
> spin_unlock_irqrestore(&callback_lock, flags);
> }
>
> @@ -4168,9 +4166,7 @@ nodemask_t cpuset_mems_allowed(struct task_struct *tsk)
> unsigned long flags;
>
> spin_lock_irqsave(&callback_lock, flags);
> - rcu_read_lock();
> guarantee_online_mems(task_cs(tsk), &mask);
> - rcu_read_unlock();
> spin_unlock_irqrestore(&callback_lock, flags);
>
> return mask;
> @@ -4265,10 +4261,8 @@ bool cpuset_current_node_allowed(int node, gfp_t gfp_mask)
> /* Not hardwall and node outside mems_allowed: scan up cpusets */
> spin_lock_irqsave(&callback_lock, flags);
>
> - rcu_read_lock();
> cs = nearest_hardwall_ancestor(task_cs(current));
> allowed = node_isset(node, cs->mems_allowed);
> - rcu_read_unlock();
>
> spin_unlock_irqrestore(&callback_lock, flags);
> return allowed;
For cpuset,
Acked-by: Waiman Long <longman@redhat.com>
^ permalink raw reply
* Re: [PATCH] pid: use ns_capable_noaudit() when determining net sysctl permissions
From: Christian Brauner @ 2025-09-15 14:13 UTC (permalink / raw)
To: Serge E. Hallyn
Cc: Paul Moore, Christian Göttsche, linux-kernel,
Christian Göttsche, linux-security-module, selinux
In-Reply-To: <aMR2EfsdxmIXIgTW@mail.hallyn.com>
On Fri, Sep 12, 2025 at 02:35:45PM -0500, Serge Hallyn wrote:
> On Thu, Sep 11, 2025 at 06:05:05PM -0500, Serge E. Hallyn wrote:
> > On Thu, Sep 11, 2025 at 04:46:20PM -0400, Paul Moore wrote:
> > > On Sep 10, 2025 =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgoettsche@seltendoof.de> wrote:
> > > >
> > > > The capability check should not be audited since it is only being used
> > > > to determine the inode permissions. A failed check does not indicate a
> > > > violation of security policy but, when an LSM is enabled, a denial audit
> > > > message was being generated.
> > > >
> > > > The denial audit message can either lead to the capability being
> > > > unnecessarily allowed in a security policy, or being silenced potentially
> > > > masking a legitimate capability check at a later point in time.
> > > >
> > > > Similar to commit d6169b0206db ("net: Use ns_capable_noaudit() when
> > > > determining net sysctl permissions")
> > > >
> > > > Fixes: 7863dcc72d0f ("pid: allow pid_max to be set per pid namespace")
> > > > CC: Christian Brauner <brauner@kernel.org>
> > > > CC: linux-security-module@vger.kernel.org
> > > > CC: selinux@vger.kernel.org
> > > > Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> > > > ---
> > > > kernel/pid.c | 2 +-
> > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > Reviewed-by: Paul Moore <paul@paul-moore.com>
> >
> > Thanks.
> >
> > Acked-by: Serge Hallyn <serge@hallyn.com>
> >
> > I'll queue this up in the capability tree, unless Christian wanted
> > it in his.
> >
> > -serge
>
> I've included this in git://git.kernel.org/pub/scm/linux/kernel/git/sergeh/linux.git#caps-next
What's easier for you? Just want me to stuff this in the pidfs pile?
^ permalink raw reply
* [PATCH v2] rcu: Remove redundant rcu_read_lock/unlock() in spin_lock critical sections
From: pengdonglin @ 2025-09-15 13:47 UTC (permalink / raw)
To: tj, tony.luck, jani.nikula, ap420073, jv, freude, bcrl, trondmy,
longman, kees
Cc: bigeasy, linux-kernel, linux-rt-devel, linux-nfs, linux-aio,
linux-fsdevel, linux-security-module, netdev, intel-gfx,
linux-acpi, linux-s390, cgroups, pengdonglin, Hillf Danton,
Paul E . McKenney, pengdonglin
From: pengdonglin <pengdonglin@xiaomi.com>
Per Documentation/RCU/rcu_dereference.rst [1], since Linux 4.20's RCU
consolidation [2][3], RCU read-side critical sections include:
- Explicit rcu_read_lock()
- BH/interrupt/preemption-disabling regions
- Spinlock critical sections (including CONFIG_PREEMPT_RT kernels [4])
Thus, explicit rcu_read_lock()/unlock() calls within spin_lock*() regions are redundant.
This patch removes them, simplifying locking semantics while preserving RCU protection.
[1] https://elixir.bootlin.com/linux/v6.17-rc5/source/Documentation/RCU/rcu_dereference.rst#L407
[2] https://lore.kernel.org/lkml/20180829222021.GA29944@linux.vnet.ibm.com/
[3] https://lwn.net/Articles/777036/
[4] https://lore.kernel.org/lkml/6435833a-bdcb-4114-b29d-28b7f436d47d@paulmck-laptop/
Cc: Waiman Long <longman@redhat.com>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Hillf Danton <hdanton@sina.com>
Cc: Paul E. McKenney <paulmck@kernel.org>
Signed-off-by: pengdonglin <pengdonglin@xiaomi.com>
Signed-off-by: pengdonglin <dolinux.peng@gmail.com>
---
Changes in v2:
- Clarified commit message to prevent accidental backport to older kernels
- Added lockdep_is_held() to avoid false positives
---
drivers/acpi/apei/ghes.c | 2 --
drivers/gpu/drm/i915/gt/intel_ring_submission.c | 2 --
drivers/net/amt.c | 8 --------
drivers/net/bonding/bond_3ad.c | 2 --
drivers/net/wireless/ath/ath9k/xmit.c | 2 --
drivers/s390/crypto/pkey_base.c | 3 ---
fs/aio.c | 6 ++----
fs/nfs/callback_proc.c | 2 --
fs/nfs/nfs4state.c | 2 --
fs/nfs/pnfs.c | 9 ---------
fs/nfs/pnfs_dev.c | 4 ----
ipc/msg.c | 1 -
ipc/sem.c | 1 -
ipc/shm.c | 1 -
ipc/util.c | 2 --
kernel/cgroup/cgroup.c | 2 --
kernel/cgroup/cpuset.c | 6 ------
kernel/cgroup/debug.c | 4 ----
net/mac80211/cfg.c | 2 --
net/mac80211/debugfs.c | 2 --
net/mac80211/debugfs_netdev.c | 2 --
net/mac80211/debugfs_sta.c | 2 --
net/mac80211/sta_info.c | 2 --
net/ncsi/ncsi-manage.c | 2 --
security/yama/yama_lsm.c | 4 ----
25 files changed, 2 insertions(+), 73 deletions(-)
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index a0d54993edb3..97ee19f2cae0 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -1207,12 +1207,10 @@ static int ghes_notify_hed(struct notifier_block *this, unsigned long event,
int ret = NOTIFY_DONE;
spin_lock_irqsave(&ghes_notify_lock_irq, flags);
- rcu_read_lock();
list_for_each_entry_rcu(ghes, &ghes_hed, list) {
if (!ghes_proc(ghes))
ret = NOTIFY_OK;
}
- rcu_read_unlock();
spin_unlock_irqrestore(&ghes_notify_lock_irq, flags);
return ret;
diff --git a/drivers/gpu/drm/i915/gt/intel_ring_submission.c b/drivers/gpu/drm/i915/gt/intel_ring_submission.c
index 2a6d79abf25b..bf73166a1337 100644
--- a/drivers/gpu/drm/i915/gt/intel_ring_submission.c
+++ b/drivers/gpu/drm/i915/gt/intel_ring_submission.c
@@ -392,14 +392,12 @@ static void reset_rewind(struct intel_engine_cs *engine, bool stalled)
rq = NULL;
spin_lock_irqsave(&engine->sched_engine->lock, flags);
- rcu_read_lock();
list_for_each_entry(pos, &engine->sched_engine->requests, sched.link) {
if (!__i915_request_is_complete(pos)) {
rq = pos;
break;
}
}
- rcu_read_unlock();
/*
* The guilty request will get skipped on a hung engine.
diff --git a/drivers/net/amt.c b/drivers/net/amt.c
index ed86537b2f61..aaed9fbc7526 100644
--- a/drivers/net/amt.c
+++ b/drivers/net/amt.c
@@ -295,7 +295,6 @@ static void amt_source_work(struct work_struct *work)
tunnel = gnode->tunnel_list;
spin_lock_bh(&tunnel->lock);
- rcu_read_lock();
if (gnode->filter_mode == MCAST_INCLUDE) {
amt_destroy_source(snode);
if (!gnode->nr_sources)
@@ -306,7 +305,6 @@ static void amt_source_work(struct work_struct *work)
*/
snode->status = AMT_SOURCE_STATUS_D_FWD;
}
- rcu_read_unlock();
spin_unlock_bh(&tunnel->lock);
}
@@ -426,7 +424,6 @@ static void amt_group_work(struct work_struct *work)
goto out;
}
- rcu_read_lock();
for (i = 0; i < buckets; i++) {
hlist_for_each_entry_safe(snode, t,
&gnode->sources[i], node) {
@@ -443,7 +440,6 @@ static void amt_group_work(struct work_struct *work)
amt_del_group(amt, gnode);
else
gnode->filter_mode = MCAST_INCLUDE;
- rcu_read_unlock();
spin_unlock_bh(&tunnel->lock);
out:
dev_put(amt->dev);
@@ -1327,11 +1323,9 @@ static void amt_clear_groups(struct amt_tunnel_list *tunnel)
int i;
spin_lock_bh(&tunnel->lock);
- rcu_read_lock();
for (i = 0; i < amt->hash_buckets; i++)
hlist_for_each_entry_safe(gnode, t, &tunnel->groups[i], node)
amt_del_group(amt, gnode);
- rcu_read_unlock();
spin_unlock_bh(&tunnel->lock);
}
@@ -1343,11 +1337,9 @@ static void amt_tunnel_expire(struct work_struct *work)
struct amt_dev *amt = tunnel->amt;
spin_lock_bh(&amt->lock);
- rcu_read_lock();
list_del_rcu(&tunnel->list);
amt->nr_tunnels--;
amt_clear_groups(tunnel);
- rcu_read_unlock();
spin_unlock_bh(&amt->lock);
kfree_rcu(tunnel, rcu);
}
diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index 4edc8e6b6b64..c53ea73f103a 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -2485,7 +2485,6 @@ void bond_3ad_state_machine_handler(struct work_struct *work)
* concurrently due to incoming LACPDU as well.
*/
spin_lock_bh(&bond->mode_lock);
- rcu_read_lock();
/* check if there are any slaves */
if (!bond_has_slaves(bond))
@@ -2537,7 +2536,6 @@ void bond_3ad_state_machine_handler(struct work_struct *work)
break;
}
}
- rcu_read_unlock();
spin_unlock_bh(&bond->mode_lock);
if (update_slave_arr)
diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
index 0ac9212e42f7..4a0f465aa2fe 100644
--- a/drivers/net/wireless/ath/ath9k/xmit.c
+++ b/drivers/net/wireless/ath/ath9k/xmit.c
@@ -1993,7 +1993,6 @@ void ath_txq_schedule(struct ath_softc *sc, struct ath_txq *txq)
ieee80211_txq_schedule_start(hw, txq->mac80211_qnum);
spin_lock_bh(&sc->chan_lock);
- rcu_read_lock();
if (sc->cur_chan->stopped)
goto out;
@@ -2011,7 +2010,6 @@ void ath_txq_schedule(struct ath_softc *sc, struct ath_txq *txq)
}
out:
- rcu_read_unlock();
spin_unlock_bh(&sc->chan_lock);
ieee80211_txq_schedule_end(hw, txq->mac80211_qnum);
}
diff --git a/drivers/s390/crypto/pkey_base.c b/drivers/s390/crypto/pkey_base.c
index b15741461a63..4c4a9feecccc 100644
--- a/drivers/s390/crypto/pkey_base.c
+++ b/drivers/s390/crypto/pkey_base.c
@@ -48,16 +48,13 @@ int pkey_handler_register(struct pkey_handler *handler)
spin_lock(&handler_list_write_lock);
- rcu_read_lock();
list_for_each_entry_rcu(h, &handler_list, list) {
if (h == handler) {
- rcu_read_unlock();
spin_unlock(&handler_list_write_lock);
module_put(handler->module);
return -EEXIST;
}
}
- rcu_read_unlock();
list_add_rcu(&handler->list, &handler_list);
spin_unlock(&handler_list_write_lock);
diff --git a/fs/aio.c b/fs/aio.c
index 7fc7b6221312..e3f9a5a391b5 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -359,15 +359,14 @@ static int aio_ring_mremap(struct vm_area_struct *vma)
int i, res = -EINVAL;
spin_lock(&mm->ioctx_lock);
- rcu_read_lock();
- table = rcu_dereference(mm->ioctx_table);
+ table = rcu_dereference_check(mm->ioctx_table, lockdep_is_held(&mm->ioctx_lock));
if (!table)
goto out_unlock;
for (i = 0; i < table->nr; i++) {
struct kioctx *ctx;
- ctx = rcu_dereference(table->table[i]);
+ ctx = rcu_dereference_check(table->table[i], lockdep_is_held(&mm->ioctx_lock));
if (ctx && ctx->aio_ring_file == file) {
if (!atomic_read(&ctx->dead)) {
ctx->user_id = ctx->mmap_base = vma->vm_start;
@@ -378,7 +377,6 @@ static int aio_ring_mremap(struct vm_area_struct *vma)
}
out_unlock:
- rcu_read_unlock();
spin_unlock(&mm->ioctx_lock);
return res;
}
diff --git a/fs/nfs/callback_proc.c b/fs/nfs/callback_proc.c
index 8397c43358bd..16144db39335 100644
--- a/fs/nfs/callback_proc.c
+++ b/fs/nfs/callback_proc.c
@@ -721,7 +721,6 @@ __be32 nfs4_callback_offload(void *data, void *dummy,
return cpu_to_be32(NFS4ERR_DELAY);
spin_lock(&cps->clp->cl_lock);
- rcu_read_lock();
list_for_each_entry_rcu(server, &cps->clp->cl_superblocks,
client_link) {
list_for_each_entry(tmp_copy, &server->ss_copies, copies) {
@@ -736,7 +735,6 @@ __be32 nfs4_callback_offload(void *data, void *dummy,
}
}
out:
- rcu_read_unlock();
if (!found) {
memcpy(©->stateid, &args->coa_stateid, NFS4_STATEID_SIZE);
nfs4_copy_cb_args(copy, args);
diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index 7612e977e80b..598229fc07ed 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -241,13 +241,11 @@ const struct cred *nfs4_get_renew_cred(struct nfs_client *clp)
goto out;
spin_lock(&clp->cl_lock);
- rcu_read_lock();
list_for_each_entry_rcu(server, &clp->cl_superblocks, client_link) {
cred = nfs4_get_renew_cred_server_locked(server);
if (cred != NULL)
break;
}
- rcu_read_unlock();
spin_unlock(&clp->cl_lock);
out:
diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
index a3135b5af7ee..c9399452bcfd 100644
--- a/fs/nfs/pnfs.c
+++ b/fs/nfs/pnfs.c
@@ -862,16 +862,13 @@ pnfs_layout_bulk_destroy_byserver_locked(struct nfs_client *clp,
if (pnfs_layout_add_bulk_destroy_list(inode,
layout_list))
continue;
- rcu_read_unlock();
spin_unlock(&clp->cl_lock);
iput(inode);
} else {
- rcu_read_unlock();
spin_unlock(&clp->cl_lock);
}
nfs_sb_deactive(server->super);
spin_lock(&clp->cl_lock);
- rcu_read_lock();
return -EAGAIN;
}
return 0;
@@ -922,7 +919,6 @@ int pnfs_layout_destroy_byfsid(struct nfs_client *clp, struct nfs_fsid *fsid,
LIST_HEAD(layout_list);
spin_lock(&clp->cl_lock);
- rcu_read_lock();
restart:
list_for_each_entry_rcu(server, &clp->cl_superblocks, client_link) {
if (memcmp(&server->fsid, fsid, sizeof(*fsid)) != 0)
@@ -932,7 +928,6 @@ int pnfs_layout_destroy_byfsid(struct nfs_client *clp, struct nfs_fsid *fsid,
&layout_list) != 0)
goto restart;
}
- rcu_read_unlock();
spin_unlock(&clp->cl_lock);
return pnfs_layout_free_bulk_destroy_list(&layout_list, mode);
@@ -944,14 +939,12 @@ static void pnfs_layout_build_destroy_list_byclient(struct nfs_client *clp,
struct nfs_server *server;
spin_lock(&clp->cl_lock);
- rcu_read_lock();
restart:
list_for_each_entry_rcu(server, &clp->cl_superblocks, client_link) {
if (pnfs_layout_bulk_destroy_byserver_locked(clp, server,
list) != 0)
goto restart;
}
- rcu_read_unlock();
spin_unlock(&clp->cl_lock);
}
@@ -990,7 +983,6 @@ static void pnfs_layout_build_recover_list_byclient(struct nfs_client *clp,
struct nfs_server *server;
spin_lock(&clp->cl_lock);
- rcu_read_lock();
restart:
list_for_each_entry_rcu(server, &clp->cl_superblocks, client_link) {
if (!(server->caps & NFS_CAP_REBOOT_LAYOUTRETURN))
@@ -999,7 +991,6 @@ static void pnfs_layout_build_recover_list_byclient(struct nfs_client *clp,
list) != 0)
goto restart;
}
- rcu_read_unlock();
spin_unlock(&clp->cl_lock);
}
diff --git a/fs/nfs/pnfs_dev.c b/fs/nfs/pnfs_dev.c
index bf0f2d67e96c..d19752ec1a95 100644
--- a/fs/nfs/pnfs_dev.c
+++ b/fs/nfs/pnfs_dev.c
@@ -231,9 +231,7 @@ nfs4_delete_deviceid(const struct pnfs_layoutdriver_type *ld,
struct nfs4_deviceid_node *d;
spin_lock(&nfs4_deviceid_lock);
- rcu_read_lock();
d = _lookup_deviceid(ld, clp, id, nfs4_deviceid_hash(id));
- rcu_read_unlock();
if (!d) {
spin_unlock(&nfs4_deviceid_lock);
return;
@@ -331,14 +329,12 @@ _deviceid_purge_client(const struct nfs_client *clp, long hash)
HLIST_HEAD(tmp);
spin_lock(&nfs4_deviceid_lock);
- rcu_read_lock();
hlist_for_each_entry_rcu(d, &nfs4_deviceid_cache[hash], node)
if (d->nfs_client == clp && atomic_read(&d->ref)) {
hlist_del_init_rcu(&d->node);
hlist_add_head(&d->tmpnode, &tmp);
clear_bit(NFS_DEVICEID_NOCACHE, &d->flags);
}
- rcu_read_unlock();
spin_unlock(&nfs4_deviceid_lock);
if (hlist_empty(&tmp))
diff --git a/ipc/msg.c b/ipc/msg.c
index ee6af4fe52bf..1e579b57023f 100644
--- a/ipc/msg.c
+++ b/ipc/msg.c
@@ -179,7 +179,6 @@ static int newque(struct ipc_namespace *ns, struct ipc_params *params)
}
ipc_unlock_object(&msq->q_perm);
- rcu_read_unlock();
return msq->q_perm.id;
}
diff --git a/ipc/sem.c b/ipc/sem.c
index a39cdc7bf88f..38ad57b2b558 100644
--- a/ipc/sem.c
+++ b/ipc/sem.c
@@ -579,7 +579,6 @@ static int newary(struct ipc_namespace *ns, struct ipc_params *params)
ns->used_sems += nsems;
sem_unlock(sma, -1);
- rcu_read_unlock();
return sma->sem_perm.id;
}
diff --git a/ipc/shm.c b/ipc/shm.c
index a9310b6dbbc3..61fae1b6a18e 100644
--- a/ipc/shm.c
+++ b/ipc/shm.c
@@ -795,7 +795,6 @@ static int newseg(struct ipc_namespace *ns, struct ipc_params *params)
error = shp->shm_perm.id;
ipc_unlock_object(&shp->shm_perm);
- rcu_read_unlock();
return error;
no_id:
diff --git a/ipc/util.c b/ipc/util.c
index cae60f11d9c2..1be691b5dcad 100644
--- a/ipc/util.c
+++ b/ipc/util.c
@@ -293,7 +293,6 @@ int ipc_addid(struct ipc_ids *ids, struct kern_ipc_perm *new, int limit)
idr_preload(GFP_KERNEL);
spin_lock_init(&new->lock);
- rcu_read_lock();
spin_lock(&new->lock);
current_euid_egid(&euid, &egid);
@@ -316,7 +315,6 @@ int ipc_addid(struct ipc_ids *ids, struct kern_ipc_perm *new, int limit)
if (idx < 0) {
new->deleted = true;
spin_unlock(&new->lock);
- rcu_read_unlock();
return idx;
}
diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index 312c6a8b55bb..db9e00a559df 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -2944,14 +2944,12 @@ int cgroup_attach_task(struct cgroup *dst_cgrp, struct task_struct *leader,
/* look up all src csets */
spin_lock_irq(&css_set_lock);
- rcu_read_lock();
task = leader;
do {
cgroup_migrate_add_src(task_css_set(task), dst_cgrp, &mgctx);
if (!threadgroup)
break;
} while_each_thread(leader, task);
- rcu_read_unlock();
spin_unlock_irq(&css_set_lock);
/* prepare dst csets and commit */
diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index 27adb04df675..9b7e8e8e9411 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -4073,7 +4073,6 @@ void cpuset_cpus_allowed(struct task_struct *tsk, struct cpumask *pmask)
struct cpuset *cs;
spin_lock_irqsave(&callback_lock, flags);
- rcu_read_lock();
cs = task_cs(tsk);
if (cs != &top_cpuset)
@@ -4095,7 +4094,6 @@ void cpuset_cpus_allowed(struct task_struct *tsk, struct cpumask *pmask)
cpumask_copy(pmask, possible_mask);
}
- rcu_read_unlock();
spin_unlock_irqrestore(&callback_lock, flags);
}
@@ -4168,9 +4166,7 @@ nodemask_t cpuset_mems_allowed(struct task_struct *tsk)
unsigned long flags;
spin_lock_irqsave(&callback_lock, flags);
- rcu_read_lock();
guarantee_online_mems(task_cs(tsk), &mask);
- rcu_read_unlock();
spin_unlock_irqrestore(&callback_lock, flags);
return mask;
@@ -4265,10 +4261,8 @@ bool cpuset_current_node_allowed(int node, gfp_t gfp_mask)
/* Not hardwall and node outside mems_allowed: scan up cpusets */
spin_lock_irqsave(&callback_lock, flags);
- rcu_read_lock();
cs = nearest_hardwall_ancestor(task_cs(current));
allowed = node_isset(node, cs->mems_allowed);
- rcu_read_unlock();
spin_unlock_irqrestore(&callback_lock, flags);
return allowed;
diff --git a/kernel/cgroup/debug.c b/kernel/cgroup/debug.c
index 80aa3f027ac3..81ea38dd6f9d 100644
--- a/kernel/cgroup/debug.c
+++ b/kernel/cgroup/debug.c
@@ -49,7 +49,6 @@ static int current_css_set_read(struct seq_file *seq, void *v)
return -ENODEV;
spin_lock_irq(&css_set_lock);
- rcu_read_lock();
cset = task_css_set(current);
refcnt = refcount_read(&cset->refcount);
seq_printf(seq, "css_set %pK %d", cset, refcnt);
@@ -67,7 +66,6 @@ static int current_css_set_read(struct seq_file *seq, void *v)
seq_printf(seq, "%2d: %-4s\t- %p[%d]\n", ss->id, ss->name,
css, css->id);
}
- rcu_read_unlock();
spin_unlock_irq(&css_set_lock);
cgroup_kn_unlock(of->kn);
return 0;
@@ -95,7 +93,6 @@ static int current_css_set_cg_links_read(struct seq_file *seq, void *v)
return -ENOMEM;
spin_lock_irq(&css_set_lock);
- rcu_read_lock();
cset = task_css_set(current);
list_for_each_entry(link, &cset->cgrp_links, cgrp_link) {
struct cgroup *c = link->cgrp;
@@ -104,7 +101,6 @@ static int current_css_set_cg_links_read(struct seq_file *seq, void *v)
seq_printf(seq, "Root %d group %s\n",
c->root->hierarchy_id, name_buf);
}
- rcu_read_unlock();
spin_unlock_irq(&css_set_lock);
kfree(name_buf);
return 0;
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 2ed07fa121ab..4fe50d4c461d 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -4825,7 +4825,6 @@ static int ieee80211_get_txq_stats(struct wiphy *wiphy,
int ret = 0;
spin_lock_bh(&local->fq.lock);
- rcu_read_lock();
if (wdev) {
sdata = IEEE80211_WDEV_TO_SUB_IF(wdev);
@@ -4851,7 +4850,6 @@ static int ieee80211_get_txq_stats(struct wiphy *wiphy,
}
out:
- rcu_read_unlock();
spin_unlock_bh(&local->fq.lock);
return ret;
diff --git a/net/mac80211/debugfs.c b/net/mac80211/debugfs.c
index e8b78ec682da..82099f4cedbe 100644
--- a/net/mac80211/debugfs.c
+++ b/net/mac80211/debugfs.c
@@ -82,7 +82,6 @@ static ssize_t aqm_read(struct file *file,
int len = 0;
spin_lock_bh(&local->fq.lock);
- rcu_read_lock();
len = scnprintf(buf, sizeof(buf),
"access name value\n"
@@ -105,7 +104,6 @@ static ssize_t aqm_read(struct file *file,
fq->limit,
fq->quantum);
- rcu_read_unlock();
spin_unlock_bh(&local->fq.lock);
return simple_read_from_buffer(user_buf, count, ppos,
diff --git a/net/mac80211/debugfs_netdev.c b/net/mac80211/debugfs_netdev.c
index 1dac78271045..30a5a978a678 100644
--- a/net/mac80211/debugfs_netdev.c
+++ b/net/mac80211/debugfs_netdev.c
@@ -625,7 +625,6 @@ static ssize_t ieee80211_if_fmt_aqm(
txqi = to_txq_info(sdata->vif.txq);
spin_lock_bh(&local->fq.lock);
- rcu_read_lock();
len = scnprintf(buf,
buflen,
@@ -642,7 +641,6 @@ static ssize_t ieee80211_if_fmt_aqm(
txqi->tin.tx_bytes,
txqi->tin.tx_packets);
- rcu_read_unlock();
spin_unlock_bh(&local->fq.lock);
return len;
diff --git a/net/mac80211/debugfs_sta.c b/net/mac80211/debugfs_sta.c
index 49061bd4151b..ef75255d47d5 100644
--- a/net/mac80211/debugfs_sta.c
+++ b/net/mac80211/debugfs_sta.c
@@ -148,7 +148,6 @@ static ssize_t sta_aqm_read(struct file *file, char __user *userbuf,
return -ENOMEM;
spin_lock_bh(&local->fq.lock);
- rcu_read_lock();
p += scnprintf(p,
bufsz + buf - p,
@@ -178,7 +177,6 @@ static ssize_t sta_aqm_read(struct file *file, char __user *userbuf,
test_bit(IEEE80211_TXQ_DIRTY, &txqi->flags) ? " DIRTY" : "");
}
- rcu_read_unlock();
spin_unlock_bh(&local->fq.lock);
rv = simple_read_from_buffer(userbuf, count, ppos, buf, p - buf);
diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index 8c550aab9bdc..663318a75d7f 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -2637,13 +2637,11 @@ static void sta_set_tidstats(struct sta_info *sta,
if (link_id < 0 && tid < IEEE80211_NUM_TIDS) {
spin_lock_bh(&local->fq.lock);
- rcu_read_lock();
tidstats->filled |= BIT(NL80211_TID_STATS_TXQ_STATS);
ieee80211_fill_txq_stats(&tidstats->txq_stats,
to_txq_info(sta->sta.txq[tid]));
- rcu_read_unlock();
spin_unlock_bh(&local->fq.lock);
}
}
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index 446e4e3b9553..6e36cd64a31e 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -650,7 +650,6 @@ static int set_one_vid(struct ncsi_dev_priv *ndp, struct ncsi_channel *nc,
spin_lock_irqsave(&nc->lock, flags);
- rcu_read_lock();
list_for_each_entry_rcu(vlan, &ndp->vlan_vids, list) {
vid = vlan->vid;
for (i = 0; i < ncf->n_vids; i++)
@@ -661,7 +660,6 @@ static int set_one_vid(struct ncsi_dev_priv *ndp, struct ncsi_channel *nc,
if (vid)
break;
}
- rcu_read_unlock();
if (!vid) {
/* No VLAN ID is not set */
diff --git a/security/yama/yama_lsm.c b/security/yama/yama_lsm.c
index 3d064dd4e03f..60d38deb181b 100644
--- a/security/yama/yama_lsm.c
+++ b/security/yama/yama_lsm.c
@@ -117,14 +117,12 @@ static void yama_relation_cleanup(struct work_struct *work)
struct ptrace_relation *relation;
spin_lock(&ptracer_relations_lock);
- rcu_read_lock();
list_for_each_entry_rcu(relation, &ptracer_relations, node) {
if (relation->invalid) {
list_del_rcu(&relation->node);
kfree_rcu(relation, rcu);
}
}
- rcu_read_unlock();
spin_unlock(&ptracer_relations_lock);
}
@@ -152,7 +150,6 @@ static int yama_ptracer_add(struct task_struct *tracer,
added->invalid = false;
spin_lock(&ptracer_relations_lock);
- rcu_read_lock();
list_for_each_entry_rcu(relation, &ptracer_relations, node) {
if (relation->invalid)
continue;
@@ -166,7 +163,6 @@ static int yama_ptracer_add(struct task_struct *tracer,
list_add_rcu(&added->node, &ptracer_relations);
out:
- rcu_read_unlock();
spin_unlock(&ptracer_relations_lock);
return 0;
}
--
2.34.1
^ permalink raw reply related
* Re: [PATCH v2 0/7] fs/9p: Reuse inode based on path (in addition to qid)
From: Tingmao Wang @ 2025-09-15 13:44 UTC (permalink / raw)
To: Dominique Martinet
Cc: Mickaël Salaün, Eric Van Hensbergen, Latchesar Ionkov,
Christian Schoenebeck, v9fs, Günther Noack,
linux-security-module, Jan Kara, Amir Goldstein,
Matthew Bobrowski, Al Viro, Christian Brauner, linux-fsdevel
In-Reply-To: <aMgMOnrAOrwQyVbp@codewreck.org>
On 9/15/25 13:53, Dominique Martinet wrote:
> Hi Tingmao,
>
> thanks for pushing this forward, I still have very little time so
> haven't been able to review this properly
No worries and thanks for the quick reply :)
>
> Tingmao Wang wrote on Sun, Sep 14, 2025 at 10:25:02PM +0100:
>> I had a chat with Mickaël earlier this week and some discussion following
>> that, and we thought of a potential alternative to what I was proposing
>> here that might work for Landlock: using the inode number (or more
>> correctly, qid.path) directly as the keys for Landlock rules when
>> accessing 9p files. I'm not sure how sound this is from the perspective
>> of 9pfs (there are pros and caveats), and I would like to gather some
>> thoughts on this idea.
>
> I'm honestly split on this:
> - I really don't like tracking the full path of each file around;
> there are various corner cases with files being removed (possibly server
> side!) or hard links; and it's potentially slowing down all operations a
> bit...
The way I see it, this tracking is really a best-effort, pragmatic
solution, and in the presence of server-side changes, it's not going to
always be correct (but then, because of the possibility of qid collisions,
there is not really a fully fool-proof way in any case). In some sense
currently hard links doesn't "work" anyway in uncached mode since each
inode is separate - this doesn't change that. Files being removed is not
a problem, and if another file with the same name is recreated, if it has
a different qid we will create a new inode anyway (since this patch really
matches against path _and_ qid). I've not measured the slowdown, but I
think in uncached mode the time is mostly domainated communication
overhead in any case... I think.
However, alternatives do exist (as discussed above and below)
> - OTOH as you pointed out qid isn't as reliable, and having file paths
> around opens the way to rebuilding fids on reconnect for non-local
> servers, which could potentially be interesting (not that I ever see
> myself having time to work on this as I no longer have any stake there,
> I just know that would have interested my previous employer when they
> were still using 9p/rdma...)
If you / anyone else would be interested in that I can implement it, but
that would probably be best done by landing the ino_path struct in this
patchset first.
>
>> In discussion with Mickaël he thought that it would be acceptable for
>> Landlock to assume that the server is well-behaved, and Landlock could
>> specialize for 9pfs to allow access if the qid matches what's previously
>> seen when creating the Landlock ruleset (by using the qid as the key of
>> the rule, instead of a pointer to the inode).
>
> I'm not familiar at all with landlock so forgive this question: what is
> this key about exactly?
> When a program loads a ruleset, paths referred in that ruleset are
> looked up by the kernel and the inodes involved kept around in some hash
> table for lookup on further accesses?
Yes, that is correct (but it uses rbtree, not hash table, currently).
>
> I'm fuzzy on the details but I don't see how inode pointers would be
> stable for other filesystems as well, what prevents
> e.g. vm.drop_caches=3 to drop these inodes on ext4?
Landlock holds a reference to the inode in the ruleset, so they shouldn't
be dropped. On security_sb_delete Landlock will iput those inodes so they
won't cause issue with unmounting. There is some special mechanism
("landlock objects") to decouple the ruleset themselves from the actual
inodes, so that previously Landlocked things can keep running even after
the inode has disappeared as a result of unmounting.
>
> In general I'd see the file handle (as exposed to userspace by
> name_to_handle_at) as a stable key, that works for all filesystems
> supporting fhandles (... so, not 9p, right... But in general it's
> something like inode number + generation, and we could expose that as
> handle and "just" return ENOTSUP on open_by_handle_at if that helps)
Hmm, I think this would be a good way for 9pfs to expose the qid to
Landlock, by exposing it as a handle, since that is standardized.
>
> Although looking at the patches what 9p seems to need isn't a new stable
> handle, but "just" not allocating new inodes in iget5...
> This was attempted in 724a08450f74 ("fs/9p: simplify iget to remove
> unnecessary paths"), but later reverted in be2ca3825372 ("Revert "fs/9p:
> simplify iget to remove unnecessary paths"") because it broke too many
> users, but if you're comfortable with a new mount option for the lookup
> by path I think we could make a new option saying
> "yes_my_server_has_unique_qids"... Which I assume would work for
> landlock/fsnotify?
I noticed that, but assumed that simply reverting them without additional
work (such as tracking the string path) would be a no go given the reason
why they are reverted.
>
> If you'd like to try, you can re-revert these 4 patches:
> Fixes: be2ca3825372 ("Revert "fs/9p: simplify iget to remove unnecessary paths"")
> Fixes: 26f8dd2dde68 ("Revert "fs/9p: fix uaf in in v9fs_stat2inode_dotl"")
> Fixes: fedd06210b14 ("Revert "fs/9p: remove redundant pointer v9ses"")
> Fixes: f69999b5f9b4 ("Revert " fs/9p: mitigate inode collisions"")
>
> If that works, and having this only work when a non-default option is
> set is acceptable, I think that's as good a way forward as we'll find.
Well, if you think there is no other possibility for a default solution
(and tracking paths by default is not feasible) I think it might also be
alright if we expose the qid as a handle to Landlock (without any need for
mount options), and figure out a way for Landlock to keep a fid open.
>
>> 1. The qid is 9pfs internal data, and we may need extra API for 9pfs to
>> expose this to Landlock. On 64bit, this is easy as it's just the inode
>> number (offset by 2), which we can already get from the struct inode.
>> But perhaps on 32bit we need a way to expose the full 64bit server-sent
>> qid to Landlock (or other kernel subsystems), if we're going to do
>> this.
>
> I'm not sure how much effort we want to spend on 32bit: as far as I
> know, if we have inode number collision on 32 bit we're already in
> trouble (tools like tar will consider such files to be hardlink of each
> other and happily skip reading data, producing corrupted archives);
> this is not a happy state but I don't know how to do better in any
> reasonable way, so we can probably keep a similar limitation for 32bit
> and use inode number directly...
I think if 9pfs export a handle it can be the full 64bit qid.path on any
platform, right?
>
>> 2. Even though qids are supposed to be unique across the lifetime of a
>> filesystem (including deleted files), this is not the case even for
>> QEMU in multidevs=remap mode, when running on ext4, as tested on QEMU
>> 10.1.0.
>
> I'm not familiar with the qid remap implementation in qemu, but I'm
> curious in what case you hit that.
> Deleting and recreating files? Or as you seem to say below the 'qid' is
> "freed" when fd is closed qemu-side and re-used by later open of other
> files?
I tried mounting a qemu-exported 9pfs backed on ext4, with
multidevs=remap, and created a file, used stat to note its inode number,
deleted the file, created another file (of the same OR different name),
and that new file will have the same inode number.
(If I don't delete the file, then a newly created file would of course
have a different ext4 inode number, and in that case QEMU exposes a
different qid)
>
> If this is understood I think this can be improved, reusing the qid on
> different files could yield problems with caching as well so I think
> it's something that warrants investigations.
>
>> Unfortunately, holding a dentry in Landlock prevents the filesystem
>> from being unmounted (causes WARNs), with no (proper) chance for
>> Landlock to release those dentries. We might do it in
>> security_sb_umount, but then at that point it is not guaranteed that
>> the unmount will happen - perhaps we would need a new security_ hooks
>> in the umount path?
>
> Hmm yeah that is problematic, I don't see how to take "weak" refs that
> wouldn't cause a warning for the umount to free yet still prevent
> recycling the inode, so another hook to free up resources when really
> umounting sounds appropriate if we go that way... At least umount isn't
> usually performance sensitive so it would probably be acceptable?
>
>
^ permalink raw reply
* Re: [PATCH v2 0/7] fs/9p: Reuse inode based on path (in addition to qid)
From: Dominique Martinet @ 2025-09-15 12:53 UTC (permalink / raw)
To: Tingmao Wang
Cc: Mickaël Salaün, Eric Van Hensbergen, Latchesar Ionkov,
Christian Schoenebeck, v9fs, Günther Noack,
linux-security-module, Jan Kara, Amir Goldstein,
Matthew Bobrowski, Al Viro, Christian Brauner, linux-fsdevel
In-Reply-To: <2acd6ae7-caf5-4fe7-8306-b92f5903d9c0@maowtm.org>
Hi Tingmao,
thanks for pushing this forward, I still have very little time so
haven't been able to review this properly
Tingmao Wang wrote on Sun, Sep 14, 2025 at 10:25:02PM +0100:
> I had a chat with Mickaël earlier this week and some discussion following
> that, and we thought of a potential alternative to what I was proposing
> here that might work for Landlock: using the inode number (or more
> correctly, qid.path) directly as the keys for Landlock rules when
> accessing 9p files. I'm not sure how sound this is from the perspective
> of 9pfs (there are pros and caveats), and I would like to gather some
> thoughts on this idea.
I'm honestly split on this:
- I really don't like tracking the full path of each file around;
there are various corner cases with files being removed (possibly server
side!) or hard links; and it's potentially slowing down all operations a
bit...
- OTOH as you pointed out qid isn't as reliable, and having file paths
around opens the way to rebuilding fids on reconnect for non-local
servers, which could potentially be interesting (not that I ever see
myself having time to work on this as I no longer have any stake there,
I just know that would have interested my previous employer when they
were still using 9p/rdma...)
> In discussion with Mickaël he thought that it would be acceptable for
> Landlock to assume that the server is well-behaved, and Landlock could
> specialize for 9pfs to allow access if the qid matches what's previously
> seen when creating the Landlock ruleset (by using the qid as the key of
> the rule, instead of a pointer to the inode).
I'm not familiar at all with landlock so forgive this question: what is
this key about exactly?
When a program loads a ruleset, paths referred in that ruleset are
looked up by the kernel and the inodes involved kept around in some hash
table for lookup on further accesses?
I'm fuzzy on the details but I don't see how inode pointers would be
stable for other filesystems as well, what prevents
e.g. vm.drop_caches=3 to drop these inodes on ext4?
In general I'd see the file handle (as exposed to userspace by
name_to_handle_at) as a stable key, that works for all filesystems
supporting fhandles (... so, not 9p, right... But in general it's
something like inode number + generation, and we could expose that as
handle and "just" return ENOTSUP on open_by_handle_at if that helps)
Although looking at the patches what 9p seems to need isn't a new stable
handle, but "just" not allocating new inodes in iget5...
This was attempted in 724a08450f74 ("fs/9p: simplify iget to remove
unnecessary paths"), but later reverted in be2ca3825372 ("Revert "fs/9p:
simplify iget to remove unnecessary paths"") because it broke too many
users, but if you're comfortable with a new mount option for the lookup
by path I think we could make a new option saying
"yes_my_server_has_unique_qids"... Which I assume would work for
landlock/fsnotify?
If you'd like to try, you can re-revert these 4 patches:
Fixes: be2ca3825372 ("Revert "fs/9p: simplify iget to remove unnecessary paths"")
Fixes: 26f8dd2dde68 ("Revert "fs/9p: fix uaf in in v9fs_stat2inode_dotl"")
Fixes: fedd06210b14 ("Revert "fs/9p: remove redundant pointer v9ses"")
Fixes: f69999b5f9b4 ("Revert " fs/9p: mitigate inode collisions"")
If that works, and having this only work when a non-default option is
set is acceptable, I think that's as good a way forward as we'll find.
> 1. The qid is 9pfs internal data, and we may need extra API for 9pfs to
> expose this to Landlock. On 64bit, this is easy as it's just the inode
> number (offset by 2), which we can already get from the struct inode.
> But perhaps on 32bit we need a way to expose the full 64bit server-sent
> qid to Landlock (or other kernel subsystems), if we're going to do
> this.
I'm not sure how much effort we want to spend on 32bit: as far as I
know, if we have inode number collision on 32 bit we're already in
trouble (tools like tar will consider such files to be hardlink of each
other and happily skip reading data, producing corrupted archives);
this is not a happy state but I don't know how to do better in any
reasonable way, so we can probably keep a similar limitation for 32bit
and use inode number directly...
> 2. Even though qids are supposed to be unique across the lifetime of a
> filesystem (including deleted files), this is not the case even for
> QEMU in multidevs=remap mode, when running on ext4, as tested on QEMU
> 10.1.0.
I'm not familiar with the qid remap implementation in qemu, but I'm
curious in what case you hit that.
Deleting and recreating files? Or as you seem to say below the 'qid' is
"freed" when fd is closed qemu-side and re-used by later open of other
files?
If this is understood I think this can be improved, reusing the qid on
different files could yield problems with caching as well so I think
it's something that warrants investigations.
> Unfortunately, holding a dentry in Landlock prevents the filesystem
> from being unmounted (causes WARNs), with no (proper) chance for
> Landlock to release those dentries. We might do it in
> security_sb_umount, but then at that point it is not guaranteed that
> the unmount will happen - perhaps we would need a new security_ hooks
> in the umount path?
Hmm yeah that is problematic, I don't see how to take "weak" refs that
wouldn't cause a warning for the umount to free yet still prevent
recycling the inode, so another hook to free up resources when really
umounting sounds appropriate if we go that way... At least umount isn't
usually performance sensitive so it would probably be acceptable?
--
Dominique Martinet | Asmadeus
^ permalink raw reply
* Re: [PATCH 6/6] make it easier to catch those who try to modify ->d_name
From: Christian Brauner @ 2025-09-15 12:46 UTC (permalink / raw)
To: Al Viro
Cc: linux-fsdevel, torvalds, jack, neil, linux-security-module,
dhowells, linkinjeon
In-Reply-To: <20250911050534.3116491-6-viro@zeniv.linux.org.uk>
On Thu, Sep 11, 2025 at 06:05:34AM +0100, Al Viro wrote:
> Turn d_name into an anon union of const struct qstr d_name with
> struct qstr __d_name. Very few places need to modify it (all
> in fs/dcache.c); those are switched to use of ->__d_name.
>
> Note that ->d_name can actually change under you unless you have
> the right locking environment; this const just prohibits accidentally
> doing stores without being easily spotted.
>
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
Reviewed-by: Christian Brauner <brauner@kernel.org>
^ permalink raw reply
* Re: [PATCH 5/6] generic_ci_validate_strict_name(): constify name argument
From: Christian Brauner @ 2025-09-15 12:46 UTC (permalink / raw)
To: Al Viro
Cc: linux-fsdevel, torvalds, jack, neil, linux-security-module,
dhowells, linkinjeon
In-Reply-To: <20250911050534.3116491-5-viro@zeniv.linux.org.uk>
On Thu, Sep 11, 2025 at 06:05:33AM +0100, Al Viro wrote:
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
Reviewed-by: Christian Brauner <brauner@kernel.org>
^ permalink raw reply
* Re: [PATCH 4/6] afs_dir_search: constify qstr argument
From: Christian Brauner @ 2025-09-15 12:46 UTC (permalink / raw)
To: Al Viro
Cc: linux-fsdevel, torvalds, jack, neil, linux-security-module,
dhowells, linkinjeon
In-Reply-To: <20250911050534.3116491-4-viro@zeniv.linux.org.uk>
On Thu, Sep 11, 2025 at 06:05:32AM +0100, Al Viro wrote:
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
Reviewed-by: Christian Brauner <brauner@kernel.org>
^ permalink raw reply
* Re: [PATCH 3/6] afs_edit_dir_{add,remove}(): constify qstr argument
From: Christian Brauner @ 2025-09-15 12:45 UTC (permalink / raw)
To: Al Viro
Cc: linux-fsdevel, torvalds, jack, neil, linux-security-module,
dhowells, linkinjeon
In-Reply-To: <20250911050534.3116491-3-viro@zeniv.linux.org.uk>
On Thu, Sep 11, 2025 at 06:05:31AM +0100, Al Viro wrote:
> Nothing outside of fs/dcache.c has any business modifying
> dentry names; passing &dentry->d_name as an argument should
> have that argument declared as a const pointer.
>
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
Reviewed-by: Christian Brauner <brauner@kernel.org>
^ permalink raw reply
* Re: [PATCH 2/6] exfat_find(): constify qstr argument
From: Christian Brauner @ 2025-09-15 12:45 UTC (permalink / raw)
To: Al Viro
Cc: linux-fsdevel, torvalds, jack, neil, linux-security-module,
dhowells, linkinjeon
In-Reply-To: <20250911050534.3116491-2-viro@zeniv.linux.org.uk>
On Thu, Sep 11, 2025 at 06:05:30AM +0100, Al Viro wrote:
> Nothing outside of fs/dcache.c has any business modifying
> dentry names; passing &dentry->d_name as an argument should
> have that argument declared as a const pointer.
>
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
Reviewed-by: Christian Brauner <brauner@kernel.org>
^ permalink raw reply
* Re: [PATCH 1/6] security_dentry_init_security(): constify qstr argument
From: Christian Brauner @ 2025-09-15 12:45 UTC (permalink / raw)
To: Al Viro
Cc: linux-fsdevel, torvalds, jack, neil, linux-security-module,
dhowells, linkinjeon
In-Reply-To: <20250911050534.3116491-1-viro@zeniv.linux.org.uk>
On Thu, Sep 11, 2025 at 06:05:29AM +0100, Al Viro wrote:
> Nothing outside of fs/dcache.c has any business modifying
> dentry names; passing &dentry->d_name as an argument should
> have that argument declared as a const pointer.
>
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
Reviewed-by: Christian Brauner <brauner@kernel.org>
^ permalink raw reply
* Re: [PATCH] ima: don't clear IMA_DIGSIG flag when setting non-IMA xattr
From: Coiby Xu @ 2025-09-15 6:07 UTC (permalink / raw)
To: Mimi Zohar
Cc: linux-integrity, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg,
Paul Moore, James Morris, Serge E. Hallyn,
open list:SECURITY SUBSYSTEM, open list
In-Reply-To: <gw7xopwfzrp5qatwqa4n4i3miztbvbdolfaa2jxslb5pktc7fs@56r7n6feu55m>
On Mon, Sep 15, 2025 at 12:06:14PM +0800, Coiby Xu wrote:
>On Wed, Sep 10, 2025 at 08:21:33AM -0400, Mimi Zohar wrote:
>>On Wed, 2025-09-10 at 09:36 +0800, Coiby Xu wrote:
>>>On Mon, Sep 08, 2025 at 04:58:05PM -0400, Mimi Zohar wrote:
>>>> On Mon, 2025-09-08 at 10:53 -0400, Mimi Zohar wrote:
>>>> > Hi Coiby,
>>>> >
>>>> > On Mon, 2025-09-08 at 19:12 +0800, Coiby Xu wrote:
>>>> > > >
>>>> > > > Even without an IMA appraise policy, the security xattrs are written out to the
>>>> > > > filesystem, but the IMA_DIGSIG flag is not cached.
>>>> > >
>>>> > > It seems I miss some context for the above sentence. If no IMA policy is
>>>> > > configured, no ima_iint_cache will be created. If you mean non-appraisal
>>>> > > policy, will not caching IMA_DIGSIG flag cause any problem?
>>>> >
>>>> > Sorry. What I was trying to say is that your test program illustrates the
>>>> > problem both with or without any of the boot command line options as you
>>>> > suggested - "ima_appraise=fix evm=fix ima_policy=appraise_tcb". Writing some
>>>> > other security xattr is a generic problem, whether the file is in policy or not,
>>>> > whether IMA or EVM are in fix mode or not. The rpm-plugin-ima should install
>>>> > the IMA signature regardless.
>>>>
>>>> My mistake. An appraise policy indeed needs to be defined for the file
>>>> signature to be replaced with a file hash.
>>>
>>>Thanks for the clarification! rpm-plugin-ima does try to install IMA
>>>signature as shown from the following strace output,
>>
>>Agreed. I was referring to the SELinux label, which would be installed for new
>>files, but not necessarily re-installed on existing files. The test program
>>simplified testing. Thank you.
>
>My pleasure! Note reinstalling a package using dnf/rpm is equivalent to
>installing a new package in terms of this issue. Because according to
>the strace output and rpm's source code, when reinstalling a package,
>the following steps happens, taking lnstat as an example,
>
>1. A temporary file "lnstat;68aee3f4" is created
>2. Read the content from RPM and write it to lnstat;68aee3f4
>3. Set file permission
>4. Set security.ima by rpm-plugin-ima
>5. Set security.selinux by rpm-plugin-selinux
>6. Rename "lnstat;68aee3f4" to lnstat
>
>And here's the strace output,
>
> # strace rpm --reinstall ip*.rpm
> openat(11, "lnstat;68aee3f4", O_WRONLY|O_CREAT|O_EXCL, 0200) = 12
> dup(12) = 13
> write(13, "\177ELF\2\1\1\0\0\0\0\0\0\0\0\0\3\0>\0\1\0\0\0\0'\0\0\0\0\0\0"..., 19256) = 19256
> close(13) = 0
> getuid() = 0
> fchown(12, 0, 0) = 0
> fchmod(12, 0755) = 0
> getuid() = 0
> utimensat(12, NULL, [{tv_sec=1734480000, tv_nsec=0} /* 2024-12-17T19:00:00-0500 */, {tv_sec=1734480000, tv_nsec=0} /* 2024-12-17T19:00:00-0500 */], 0) = 0
> fsetxattr(12, "security.ima", "\3\2\4\3232\4I\0f0d\0020O\231\341q\323Q\322\235\341\7\323\224\205\2104\24\241\331#"..., 111, 0) = 0
> fsetxattr(12, "security.selinux", "system_u:object_r:bin_t:s0", 27, 0) = 0
> close(12) = 0
> ...
> renameat(11, "lnstat;68aee3f4", 11, "lnstat") = 0
Btw, I realize my commit message that says the problem happens when reinstalling a
package can be a bit misleading. So in v3, I rephrase it as
"... installing/reinstalling a package will not make good reference IMA generated"
^ permalink raw reply
* Re: [PATCH v2] ima: don't clear IMA_DIGSIG flag when setting non-IMA xattr
From: Coiby Xu @ 2025-09-15 6:03 UTC (permalink / raw)
To: Mimi Zohar
Cc: linux-integrity, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg,
Paul Moore, James Morris, Serge E. Hallyn,
open list:SECURITY SUBSYSTEM, open list
In-Reply-To: <7790048d4dc468792b428e80ceae7261a97a896d.camel@linux.ibm.com>
On Wed, Sep 10, 2025 at 08:21:03AM -0400, Mimi Zohar wrote:
>On Mon, 2025-09-08 at 18:58 +0800, Coiby Xu wrote:
>> Currently when both IMA and EVM are in fix mode, the IMA signature will
>> be reset to IMA hash if a program first stores IMA signature in
>> security.ima and then sets security.selinux for a file.
>
>The problem description should be generic.
>
>-> and then writes some other security xattr for the file.
Good advice! I've applied it to v3 with a slight change,
"... then writes/removes some other security xattr" in v3,
>
>Start a new paragraph here for the example.
>> For example, on
>> Fedora, after booting the kernel with "ima_appraise=fix evm=fix
>> ima_policy=appraise_tcb" and installing rpm-plugin-ima, reinstalling a
>> package will not make good reference IMA signature generated. Instead
>> IMA hash is generated,
>> # getfattr -m - -d -e hex /usr/bin/bash
>> # file: usr/bin/bash
>> security.ima=0x0404...
>>
>> This happens because when setting selinux.selinux, the IMA_DIGSIG flag
>> that had been set early was cleared. As a result, IMA hash is generated
>> when the file is closed.
>
>Start a new paragraph here, adding a sentence describing the solution to the
>problem. For example,
>
>Prevent replacing the IMA file signature with a file hash, by preventing the
>IMA_DIGSIG flag from being reset.
Thanks for the suggestion, applied to v3.
>
>>
>> Here's a minimal C reproducer,
>>
>> #include <stdio.h>
>> #include <sys/xattr.h>
>> #include <fcntl.h>
>> #include <unistd.h>
>> #include <string.h>
>> #include <stdlib.h>
>>
>> int main() {
>> const char* file_path = "/usr/sbin/test_binary";
>> const char* hex_string = "030204d33204490066306402304";
>> int length = strlen(hex_string);
>> char* ima_attr_value;
>> int fd;
>>
>> fd = open(file_path, O_WRONLY|O_CREAT|O_EXCL, 0644);
>> if (fd == -1) {
>> perror("Error opening file");
>> return 1;
>> }
>>
>> ima_attr_value = (char*)malloc(length / 2 );
>> for (int i = 0, j = 0; i < length; i += 2, j++) {
>> sscanf(hex_string + i, "%2hhx", &ima_attr_value[j]);
>> }
>>
>> if (fsetxattr(fd, "security.ima", ima_attr_value, length/2, 0) == -1) {
>> perror("Error setting extended attribute");
>> close(fd);
>> return 1;
>> }
>>
>> const char* selinux_value= "system_u:object_r:bin_t:s0";
>> if (fsetxattr(fd, "security.selinux", selinux_value, strlen(selinux_value), 0) == -1) {
>> perror("Error setting extended attribute");
>> close(fd);
>> return 1;
>> }
>>
>> close(fd);
>>
>> return 0;
>> }
>>
>> Signed-off-by: Coiby Xu <coxu@redhat.com>
>
>Thanks, Coiby. The updated patch looks good. Have you looked at the other
>calls to ima_reset_appraise_flags() to make sure they don't need to be adjusted?
Great question! I've updated the other two places in v3 which can
address two additional cases,
- remove xattr like security.evm
- set/remove ACL
Note I tried to write a C program to set/remove ACL but somehow the C
version of "setfacl -m u:test:r" always lead to "acl_set_fd: Invalid
argument". I bypass this issue by setting system.posix_acl_access xattr
directly. We can get the value by "getfattr -m system.posix_acl_access
-d -e hex" after "setfacl -m u:test:r /usr/sbin/test_binary".
>
>thanks,
>
>Mimi
>
>> ---
>> security/integrity/ima/ima_appraise.c | 15 +++++++++++++--
>> 1 file changed, 13 insertions(+), 2 deletions(-)
>>
>> diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
>> index f435eff4667f..4e4750ea41ad 100644
>> --- a/security/integrity/ima/ima_appraise.c
>> +++ b/security/integrity/ima/ima_appraise.c
>> @@ -694,6 +694,15 @@ static int ima_protect_xattr(struct dentry *dentry, const char *xattr_name,
>> return 0;
>> }
>>
>> +/*
>> + * ima_reset_appraise_flags - reset ima_iint_cache flags
>> + *
>> + * @digsig: whether to clear/set IMA_DIGSIG flag, tristate values
>> + * 0: clear IMA_DIGSIG
>> + * 1: set IMA_DIGSIG
>> + * -1: don't change IMA_DIGSIG
>> + *
>> + */
>> static void ima_reset_appraise_flags(struct inode *inode, int digsig)
>> {
>> struct ima_iint_cache *iint;
>> @@ -706,9 +715,9 @@ static void ima_reset_appraise_flags(struct inode *inode, int digsig)
>> return;
>> iint->measured_pcrs = 0;
>> set_bit(IMA_CHANGE_XATTR, &iint->atomic_flags);
>> - if (digsig)
>> + if (digsig == 1)
>> set_bit(IMA_DIGSIG, &iint->atomic_flags);
>> - else
>> + else if (digsig == 0)
>> clear_bit(IMA_DIGSIG, &iint->atomic_flags);
>> }
>>
>> @@ -794,6 +803,8 @@ static int ima_inode_setxattr(struct mnt_idmap *idmap, struct dentry *dentry,
>> digsig = (xvalue->type == EVM_IMA_XATTR_DIGSIG);
>> } else if (!strcmp(xattr_name, XATTR_NAME_EVM) && xattr_value_len > 0) {
>> digsig = (xvalue->type == EVM_XATTR_PORTABLE_DIGSIG);
>> + } else {
>> + digsig = -1;
>> }
>> if (result == 1 || evm_revalidate_status(xattr_name)) {
>> ima_reset_appraise_flags(d_backing_inode(dentry), digsig);
>
--
Best regards,
Coiby
^ permalink raw reply
* [PATCH v3] ima: don't clear IMA_DIGSIG flag when setting or removing non-IMA xattr
From: Coiby Xu @ 2025-09-15 5:55 UTC (permalink / raw)
To: linux-integrity, Mimi Zohar
Cc: Roberto Sassu, Dmitry Kasatkin, Eric Snowberg, Paul Moore,
James Morris, Serge E. Hallyn, open list:SECURITY SUBSYSTEM,
open list
In-Reply-To: <20250902042515.759750-1-coxu@redhat.com>
Currently when both IMA and EVM are in fix mode, the IMA signature will
be reset to IMA hash if a program first stores IMA signature in
security.ima and then writes/removes some other security xattr for the
file.
For example, on Fedora, after booting the kernel with "ima_appraise=fix
evm=fix ima_policy=appraise_tcb" and installing rpm-plugin-ima,
installing/reinstalling a package will not make good reference IMA
signature generated. Instead IMA hash is generated,
# getfattr -m - -d -e hex /usr/bin/bash
# file: usr/bin/bash
security.ima=0x0404...
This happens because when setting security.selinux, the IMA_DIGSIG flag
that had been set early was cleared. As a result, IMA hash is generated
when the file is closed.
Similarly, IMA signature can be cleared on file close after removing
security xattr like security.evm or setting/removing ACL.
Prevent replacing the IMA file signature with a file hash, by preventing
the IMA_DIGSIG flag from being reset.
Here's a minimal C reproducer which sets security.selinux as the last
step which can also replaced by removing security.evm or setting ACL,
#include <stdio.h>
#include <sys/xattr.h>
#include <fcntl.h>
#include <unistd.h>
#include <string.h>
#include <stdlib.h>
int main() {
const char* file_path = "/usr/sbin/test_binary";
const char* hex_string = "030204d33204490066306402304";
int length = strlen(hex_string);
char* ima_attr_value;
int fd;
fd = open(file_path, O_WRONLY|O_CREAT|O_EXCL, 0644);
if (fd == -1) {
perror("Error opening file");
return 1;
}
ima_attr_value = (char*)malloc(length / 2 );
for (int i = 0, j = 0; i < length; i += 2, j++) {
sscanf(hex_string + i, "%2hhx", &ima_attr_value[j]);
}
if (fsetxattr(fd, "security.ima", ima_attr_value, length/2, 0) == -1) {
perror("Error setting extended attribute");
close(fd);
return 1;
}
const char* selinux_value= "system_u:object_r:bin_t:s0";
if (fsetxattr(fd, "security.selinux", selinux_value, strlen(selinux_value), 0) == -1) {
perror("Error setting extended attribute");
close(fd);
return 1;
}
close(fd);
return 0;
}
Signed-off-by: Coiby Xu <coxu@redhat.com>
---
security/integrity/ima/ima_appraise.c | 23 ++++++++++++++++++-----
1 file changed, 18 insertions(+), 5 deletions(-)
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index f435eff4667f..5149ff4fd50d 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -694,6 +694,15 @@ static int ima_protect_xattr(struct dentry *dentry, const char *xattr_name,
return 0;
}
+/*
+ * ima_reset_appraise_flags - reset ima_iint_cache flags
+ *
+ * @digsig: whether to clear/set IMA_DIGSIG flag, tristate values
+ * 0: clear IMA_DIGSIG
+ * 1: set IMA_DIGSIG
+ * -1: don't change IMA_DIGSIG
+ *
+ */
static void ima_reset_appraise_flags(struct inode *inode, int digsig)
{
struct ima_iint_cache *iint;
@@ -706,9 +715,9 @@ static void ima_reset_appraise_flags(struct inode *inode, int digsig)
return;
iint->measured_pcrs = 0;
set_bit(IMA_CHANGE_XATTR, &iint->atomic_flags);
- if (digsig)
+ if (digsig == 1)
set_bit(IMA_DIGSIG, &iint->atomic_flags);
- else
+ else if (digsig == 0)
clear_bit(IMA_DIGSIG, &iint->atomic_flags);
}
@@ -794,6 +803,8 @@ static int ima_inode_setxattr(struct mnt_idmap *idmap, struct dentry *dentry,
digsig = (xvalue->type == EVM_IMA_XATTR_DIGSIG);
} else if (!strcmp(xattr_name, XATTR_NAME_EVM) && xattr_value_len > 0) {
digsig = (xvalue->type == EVM_XATTR_PORTABLE_DIGSIG);
+ } else {
+ digsig = -1;
}
if (result == 1 || evm_revalidate_status(xattr_name)) {
ima_reset_appraise_flags(d_backing_inode(dentry), digsig);
@@ -807,7 +818,7 @@ static int ima_inode_set_acl(struct mnt_idmap *idmap, struct dentry *dentry,
const char *acl_name, struct posix_acl *kacl)
{
if (evm_revalidate_status(acl_name))
- ima_reset_appraise_flags(d_backing_inode(dentry), 0);
+ ima_reset_appraise_flags(d_backing_inode(dentry), -1);
return 0;
}
@@ -815,11 +826,13 @@ static int ima_inode_set_acl(struct mnt_idmap *idmap, struct dentry *dentry,
static int ima_inode_removexattr(struct mnt_idmap *idmap, struct dentry *dentry,
const char *xattr_name)
{
- int result;
+ int result, digsig = -1;
result = ima_protect_xattr(dentry, xattr_name, NULL, 0);
if (result == 1 || evm_revalidate_status(xattr_name)) {
- ima_reset_appraise_flags(d_backing_inode(dentry), 0);
+ if (!strcmp(xattr_name, XATTR_NAME_IMA))
+ digsig = 0;
+ ima_reset_appraise_flags(d_backing_inode(dentry), digsig);
if (result == 1)
result = 0;
}
base-commit: 7aac71907bdea16e2754a782b9d9155449a9d49d
--
2.51.0
^ permalink raw reply related
* Re: [PATCH] ima: don't clear IMA_DIGSIG flag when setting non-IMA xattr
From: Coiby Xu @ 2025-09-15 4:06 UTC (permalink / raw)
To: Mimi Zohar
Cc: linux-integrity, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg,
Paul Moore, James Morris, Serge E. Hallyn,
open list:SECURITY SUBSYSTEM, open list
In-Reply-To: <6f2b7320b691669068dda110c29c25f8aae2f244.camel@linux.ibm.com>
On Wed, Sep 10, 2025 at 08:21:33AM -0400, Mimi Zohar wrote:
>On Wed, 2025-09-10 at 09:36 +0800, Coiby Xu wrote:
>> On Mon, Sep 08, 2025 at 04:58:05PM -0400, Mimi Zohar wrote:
>> > On Mon, 2025-09-08 at 10:53 -0400, Mimi Zohar wrote:
>> > > Hi Coiby,
>> > >
>> > > On Mon, 2025-09-08 at 19:12 +0800, Coiby Xu wrote:
>> > > > >
>> > > > > Even without an IMA appraise policy, the security xattrs are written out to the
>> > > > > filesystem, but the IMA_DIGSIG flag is not cached.
>> > > >
>> > > > It seems I miss some context for the above sentence. If no IMA policy is
>> > > > configured, no ima_iint_cache will be created. If you mean non-appraisal
>> > > > policy, will not caching IMA_DIGSIG flag cause any problem?
>> > >
>> > > Sorry. What I was trying to say is that your test program illustrates the
>> > > problem both with or without any of the boot command line options as you
>> > > suggested - "ima_appraise=fix evm=fix ima_policy=appraise_tcb". Writing some
>> > > other security xattr is a generic problem, whether the file is in policy or not,
>> > > whether IMA or EVM are in fix mode or not. The rpm-plugin-ima should install
>> > > the IMA signature regardless.
>> >
>> > My mistake. An appraise policy indeed needs to be defined for the file
>> > signature to be replaced with a file hash.
>>
>> Thanks for the clarification! rpm-plugin-ima does try to install IMA
>> signature as shown from the following strace output,
>
>Agreed. I was referring to the SELinux label, which would be installed for new
>files, but not necessarily re-installed on existing files. The test program
>simplified testing. Thank you.
My pleasure! Note reinstalling a package using dnf/rpm is equivalent to
installing a new package in terms of this issue. Because according to
the strace output and rpm's source code, when reinstalling a package,
the following steps happens, taking lnstat as an example,
1. A temporary file "lnstat;68aee3f4" is created
2. Read the content from RPM and write it to lnstat;68aee3f4
3. Set file permission
4. Set security.ima by rpm-plugin-ima
5. Set security.selinux by rpm-plugin-selinux
6. Rename "lnstat;68aee3f4" to lnstat
And here's the strace output,
# strace rpm --reinstall ip*.rpm
openat(11, "lnstat;68aee3f4", O_WRONLY|O_CREAT|O_EXCL, 0200) = 12
dup(12) = 13
write(13, "\177ELF\2\1\1\0\0\0\0\0\0\0\0\0\3\0>\0\1\0\0\0\0'\0\0\0\0\0\0"..., 19256) = 19256
close(13) = 0
getuid() = 0
fchown(12, 0, 0) = 0
fchmod(12, 0755) = 0
getuid() = 0
utimensat(12, NULL, [{tv_sec=1734480000, tv_nsec=0} /* 2024-12-17T19:00:00-0500 */, {tv_sec=1734480000, tv_nsec=0} /* 2024-12-17T19:00:00-0500 */], 0) = 0
fsetxattr(12, "security.ima", "\3\2\4\3232\4I\0f0d\0020O\231\341q\323Q\322\235\341\7\323\224\205\2104\24\241\331#"..., 111, 0) = 0
fsetxattr(12, "security.selinux", "system_u:object_r:bin_t:s0", 27, 0) = 0
close(12) = 0
...
renameat(11, "lnstat;68aee3f4", 11, "lnstat") = 0
>
>Mimi
>
--
Best regards,
Coiby
^ permalink raw reply
* [PATCH v4 12/12] selftests/bpf: Enable signature verification for some lskel tests
From: KP Singh @ 2025-09-14 21:51 UTC (permalink / raw)
To: bpf, linux-security-module
Cc: bboscaccy, paul, kys, ast, daniel, andrii, KP Singh
In-Reply-To: <20250914215141.15144-1-kpsingh@kernel.org>
The test harness uses the verify_sig_setup.sh to generate the required
key material for program signing.
Generate key material for signing LSKEL some lskel programs and use
xxd to convert the verification certificate into a C header file.
Finally, update the main test runner to load this
certificate into the session keyring via the add_key() syscall before
executing any tests. Use the session keyring in the tests with signed
programs.
Signed-off-by: KP Singh <kpsingh@kernel.org>
---
tools/testing/selftests/bpf/.gitignore | 1 +
tools/testing/selftests/bpf/Makefile | 35 ++++++++++++++++---
.../selftests/bpf/prog_tests/atomics.c | 10 ++++--
.../selftests/bpf/prog_tests/fentry_fexit.c | 15 ++++++--
.../selftests/bpf/prog_tests/fentry_test.c | 9 +++--
.../selftests/bpf/prog_tests/fexit_test.c | 9 +++--
tools/testing/selftests/bpf/test_progs.c | 13 +++++++
.../testing/selftests/bpf/verify_sig_setup.sh | 11 ++++--
8 files changed, 89 insertions(+), 14 deletions(-)
diff --git a/tools/testing/selftests/bpf/.gitignore b/tools/testing/selftests/bpf/.gitignore
index 3d8378972d26..be1ee7ba7ce0 100644
--- a/tools/testing/selftests/bpf/.gitignore
+++ b/tools/testing/selftests/bpf/.gitignore
@@ -44,3 +44,4 @@ xdp_redirect_multi
xdp_synproxy
xdp_hw_metadata
xdp_features
+verification_cert.h
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 11d2a368db3e..0b6ee902bce5 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -496,15 +496,16 @@ LINKED_SKELS := test_static_linked.skel.h linked_funcs.skel.h \
test_subskeleton.skel.h test_subskeleton_lib.skel.h \
test_usdt.skel.h
-LSKELS := fentry_test.c fexit_test.c fexit_sleep.c atomics.c \
- trace_printk.c trace_vprintk.c map_ptr_kern.c \
+LSKELS := fexit_sleep.c trace_printk.c trace_vprintk.c map_ptr_kern.c \
core_kern.c core_kern_overflow.c test_ringbuf.c \
test_ringbuf_n.c test_ringbuf_map_key.c test_ringbuf_write.c
+LSKELS_SIGNED := fentry_test.c fexit_test.c atomics.c
+
# Generate both light skeleton and libbpf skeleton for these
LSKELS_EXTRA := test_ksyms_module.c test_ksyms_weak.c kfunc_call_test.c \
kfunc_call_test_subprog.c
-SKEL_BLACKLIST += $$(LSKELS)
+SKEL_BLACKLIST += $$(LSKELS) $$(LSKELS_SIGNED)
test_static_linked.skel.h-deps := test_static_linked1.bpf.o test_static_linked2.bpf.o
linked_funcs.skel.h-deps := linked_funcs1.bpf.o linked_funcs2.bpf.o
@@ -535,6 +536,7 @@ HEADERS_FOR_BPF_OBJS := $(wildcard $(BPFDIR)/*.bpf.h) \
# $2 - test runner extra "flavor" (e.g., no_alu32, cpuv4, bpf_gcc, etc)
define DEFINE_TEST_RUNNER
+LSKEL_SIGN := -S -k $(PRIVATE_KEY) -i $(VERIFICATION_CERT)
TRUNNER_OUTPUT := $(OUTPUT)$(if $2,/)$2
TRUNNER_BINARY := $1$(if $2,-)$2
TRUNNER_TEST_OBJS := $$(patsubst %.c,$$(TRUNNER_OUTPUT)/%.test.o, \
@@ -550,6 +552,7 @@ TRUNNER_BPF_SKELS := $$(patsubst %.c,$$(TRUNNER_OUTPUT)/%.skel.h, \
$$(TRUNNER_BPF_SRCS)))
TRUNNER_BPF_LSKELS := $$(patsubst %.c,$$(TRUNNER_OUTPUT)/%.lskel.h, $$(LSKELS) $$(LSKELS_EXTRA))
TRUNNER_BPF_SKELS_LINKED := $$(addprefix $$(TRUNNER_OUTPUT)/,$(LINKED_SKELS))
+TRUNNER_BPF_LSKELS_SIGNED := $$(patsubst %.c,$$(TRUNNER_OUTPUT)/%.lskel.h, $$(LSKELS_SIGNED))
TEST_GEN_FILES += $$(TRUNNER_BPF_OBJS)
# Evaluate rules now with extra TRUNNER_XXX variables above already defined
@@ -604,6 +607,15 @@ $(TRUNNER_BPF_LSKELS): %.lskel.h: %.bpf.o $(BPFTOOL) | $(TRUNNER_OUTPUT)
$(Q)$$(BPFTOOL) gen skeleton -L $$(<:.o=.llinked3.o) name $$(notdir $$(<:.bpf.o=_lskel)) > $$@
$(Q)rm -f $$(<:.o=.llinked1.o) $$(<:.o=.llinked2.o) $$(<:.o=.llinked3.o)
+$(TRUNNER_BPF_LSKELS_SIGNED): %.lskel.h: %.bpf.o $(BPFTOOL) | $(TRUNNER_OUTPUT)
+ $$(call msg,GEN-SKEL,$(TRUNNER_BINARY) (signed),$$@)
+ $(Q)$$(BPFTOOL) gen object $$(<:.o=.llinked1.o) $$<
+ $(Q)$$(BPFTOOL) gen object $$(<:.o=.llinked2.o) $$(<:.o=.llinked1.o)
+ $(Q)$$(BPFTOOL) gen object $$(<:.o=.llinked3.o) $$(<:.o=.llinked2.o)
+ $(Q)diff $$(<:.o=.llinked2.o) $$(<:.o=.llinked3.o)
+ $(Q)$$(BPFTOOL) gen skeleton $(LSKEL_SIGN) $$(<:.o=.llinked3.o) name $$(notdir $$(<:.bpf.o=_lskel)) > $$@
+ $(Q)rm -f $$(<:.o=.llinked1.o) $$(<:.o=.llinked2.o) $$(<:.o=.llinked3.o)
+
$(LINKED_BPF_OBJS): %: $(TRUNNER_OUTPUT)/%
# .SECONDEXPANSION here allows to correctly expand %-deps variables as prerequisites
@@ -653,6 +665,7 @@ $(TRUNNER_TEST_OBJS:.o=.d): $(TRUNNER_OUTPUT)/%.test.d: \
$(TRUNNER_EXTRA_HDRS) \
$(TRUNNER_BPF_SKELS) \
$(TRUNNER_BPF_LSKELS) \
+ $(TRUNNER_BPF_LSKELS_SIGNED) \
$(TRUNNER_BPF_SKELS_LINKED) \
$$(BPFOBJ) | $(TRUNNER_OUTPUT)
@@ -667,6 +680,7 @@ $(foreach N,$(patsubst $(TRUNNER_OUTPUT)/%.o,%,$(TRUNNER_EXTRA_OBJS)), \
$(TRUNNER_EXTRA_OBJS): $(TRUNNER_OUTPUT)/%.o: \
%.c \
$(TRUNNER_EXTRA_HDRS) \
+ $(VERIFY_SIG_HDR) \
$(TRUNNER_TESTS_HDR) \
$$(BPFOBJ) | $(TRUNNER_OUTPUT)
$$(call msg,EXT-OBJ,$(TRUNNER_BINARY),$$@)
@@ -697,6 +711,18 @@ $(OUTPUT)/$(TRUNNER_BINARY): $(TRUNNER_TEST_OBJS) \
endef
+VERIFY_SIG_SETUP := $(CURDIR)/verify_sig_setup.sh
+VERIFY_SIG_HDR := verification_cert.h
+VERIFICATION_CERT := $(BUILD_DIR)/signing_key.der
+PRIVATE_KEY := $(BUILD_DIR)/signing_key.pem
+
+$(VERIFICATION_CERT) $(PRIVATE_KEY): $(VERIFY_SIG_SETUP)
+ $(Q)mkdir -p $(BUILD_DIR)
+ $(Q)$(VERIFY_SIG_SETUP) genkey $(BUILD_DIR)
+
+$(VERIFY_SIG_HDR): $(VERIFICATION_CERT)
+ $(Q)xxd -i -n test_progs_verification_cert $< > $@
+
# Define test_progs test runner.
TRUNNER_TESTS_DIR := prog_tests
TRUNNER_BPF_PROGS_DIR := progs
@@ -716,6 +742,7 @@ TRUNNER_EXTRA_SOURCES := test_progs.c \
disasm.c \
disasm_helpers.c \
json_writer.c \
+ $(VERIFY_SIG_HDR) \
flow_dissector_load.h \
ip_check_defrag_frags.h
TRUNNER_EXTRA_FILES := $(OUTPUT)/urandom_read \
@@ -725,7 +752,7 @@ TRUNNER_EXTRA_FILES := $(OUTPUT)/urandom_read \
$(OUTPUT)/uprobe_multi \
$(TEST_KMOD_TARGETS) \
ima_setup.sh \
- verify_sig_setup.sh \
+ $(VERIFY_SIG_SETUP) \
$(wildcard progs/btf_dump_test_case_*.c) \
$(wildcard progs/*.bpf.o)
TRUNNER_BPF_BUILD_RULE := CLANG_BPF_BUILD_RULE
diff --git a/tools/testing/selftests/bpf/prog_tests/atomics.c b/tools/testing/selftests/bpf/prog_tests/atomics.c
index 13e101f370a1..92b5f378bfb8 100644
--- a/tools/testing/selftests/bpf/prog_tests/atomics.c
+++ b/tools/testing/selftests/bpf/prog_tests/atomics.c
@@ -165,11 +165,17 @@ static void test_xchg(struct atomics_lskel *skel)
void test_atomics(void)
{
struct atomics_lskel *skel;
+ int err;
- skel = atomics_lskel__open_and_load();
- if (!ASSERT_OK_PTR(skel, "atomics skeleton load"))
+ skel = atomics_lskel__open();
+ if (!ASSERT_OK_PTR(skel, "atomics skeleton open"))
return;
+ skel->keyring_id = KEY_SPEC_SESSION_KEYRING;
+ err = atomics_lskel__load(skel);
+ if (!ASSERT_OK(err, "atomics skeleton load"))
+ goto cleanup;
+
if (skel->data->skip_tests) {
printf("%s:SKIP:no ENABLE_ATOMICS_TESTS (missing Clang BPF atomics support)",
__func__);
diff --git a/tools/testing/selftests/bpf/prog_tests/fentry_fexit.c b/tools/testing/selftests/bpf/prog_tests/fentry_fexit.c
index 130f5b82d2e6..5ef1804e44df 100644
--- a/tools/testing/selftests/bpf/prog_tests/fentry_fexit.c
+++ b/tools/testing/selftests/bpf/prog_tests/fentry_fexit.c
@@ -12,13 +12,24 @@ void test_fentry_fexit(void)
int err, prog_fd, i;
LIBBPF_OPTS(bpf_test_run_opts, topts);
- fentry_skel = fentry_test_lskel__open_and_load();
+ fentry_skel = fentry_test_lskel__open();
if (!ASSERT_OK_PTR(fentry_skel, "fentry_skel_load"))
goto close_prog;
- fexit_skel = fexit_test_lskel__open_and_load();
+
+ fentry_skel->keyring_id = KEY_SPEC_SESSION_KEYRING;
+ err = fentry_test_lskel__load(fentry_skel);
+ if (!ASSERT_OK(err, "fentry_skel_load"))
+ goto close_prog;
+
+ fexit_skel = fexit_test_lskel__open();
if (!ASSERT_OK_PTR(fexit_skel, "fexit_skel_load"))
goto close_prog;
+ fexit_skel->keyring_id = KEY_SPEC_SESSION_KEYRING;
+ err = fexit_test_lskel__load(fexit_skel);
+ if (!ASSERT_OK(err, "fexit_skel_load"))
+ goto close_prog;
+
err = fentry_test_lskel__attach(fentry_skel);
if (!ASSERT_OK(err, "fentry_attach"))
goto close_prog;
diff --git a/tools/testing/selftests/bpf/prog_tests/fentry_test.c b/tools/testing/selftests/bpf/prog_tests/fentry_test.c
index aee1bc77a17f..ec882328eb59 100644
--- a/tools/testing/selftests/bpf/prog_tests/fentry_test.c
+++ b/tools/testing/selftests/bpf/prog_tests/fentry_test.c
@@ -43,8 +43,13 @@ static void fentry_test(void)
struct fentry_test_lskel *fentry_skel = NULL;
int err;
- fentry_skel = fentry_test_lskel__open_and_load();
- if (!ASSERT_OK_PTR(fentry_skel, "fentry_skel_load"))
+ fentry_skel = fentry_test_lskel__open();
+ if (!ASSERT_OK_PTR(fentry_skel, "fentry_skel_open"))
+ goto cleanup;
+
+ fentry_skel->keyring_id = KEY_SPEC_SESSION_KEYRING;
+ err = fentry_test_lskel__load(fentry_skel);
+ if (!ASSERT_OK(err, "fentry_skel_load"))
goto cleanup;
err = fentry_test_common(fentry_skel);
diff --git a/tools/testing/selftests/bpf/prog_tests/fexit_test.c b/tools/testing/selftests/bpf/prog_tests/fexit_test.c
index 1c13007e37dd..94eed753560c 100644
--- a/tools/testing/selftests/bpf/prog_tests/fexit_test.c
+++ b/tools/testing/selftests/bpf/prog_tests/fexit_test.c
@@ -43,8 +43,13 @@ static void fexit_test(void)
struct fexit_test_lskel *fexit_skel = NULL;
int err;
- fexit_skel = fexit_test_lskel__open_and_load();
- if (!ASSERT_OK_PTR(fexit_skel, "fexit_skel_load"))
+ fexit_skel = fexit_test_lskel__open();
+ if (!ASSERT_OK_PTR(fexit_skel, "fexit_skel_open"))
+ goto cleanup;
+
+ fexit_skel->keyring_id = KEY_SPEC_SESSION_KEYRING;
+ err = fexit_test_lskel__load(fexit_skel);
+ if (!ASSERT_OK(err, "fexit_skel_load"))
goto cleanup;
err = fexit_test_common(fexit_skel);
diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
index 309d9d4a8ace..02a85dda30e6 100644
--- a/tools/testing/selftests/bpf/test_progs.c
+++ b/tools/testing/selftests/bpf/test_progs.c
@@ -14,12 +14,14 @@
#include <netinet/in.h>
#include <sys/select.h>
#include <sys/socket.h>
+#include <linux/keyctl.h>
#include <sys/un.h>
#include <bpf/btf.h>
#include <time.h>
#include "json_writer.h"
#include "network_helpers.h"
+#include "verification_cert.h"
/* backtrace() and backtrace_symbols_fd() are glibc specific,
* use header file when glibc is available and provide stub
@@ -1928,6 +1930,13 @@ static void free_test_states(void)
}
}
+static __u32 register_session_key(const char *key_data, size_t key_data_size)
+{
+ return syscall(__NR_add_key, "asymmetric", "libbpf_session_key",
+ (const void *)key_data, key_data_size,
+ KEY_SPEC_SESSION_KEYRING);
+}
+
int main(int argc, char **argv)
{
static const struct argp argp = {
@@ -1961,6 +1970,10 @@ int main(int argc, char **argv)
/* Use libbpf 1.0 API mode */
libbpf_set_strict_mode(LIBBPF_STRICT_ALL);
libbpf_set_print(libbpf_print_fn);
+ err = register_session_key((const char *)test_progs_verification_cert,
+ test_progs_verification_cert_len);
+ if (err < 0)
+ return err;
traffic_monitor_set_print(traffic_monitor_print_fn);
diff --git a/tools/testing/selftests/bpf/verify_sig_setup.sh b/tools/testing/selftests/bpf/verify_sig_setup.sh
index f2cac42298ba..09179fb551f0 100755
--- a/tools/testing/selftests/bpf/verify_sig_setup.sh
+++ b/tools/testing/selftests/bpf/verify_sig_setup.sh
@@ -32,7 +32,7 @@ usage()
exit 1
}
-setup()
+genkey()
{
local tmp_dir="$1"
@@ -45,9 +45,14 @@ setup()
openssl x509 -in ${tmp_dir}/signing_key.pem -out \
${tmp_dir}/signing_key.der -outform der
+}
- key_id=$(cat ${tmp_dir}/signing_key.der | keyctl padd asymmetric ebpf_testing_key @s)
+setup()
+{
+ local tmp_dir="$1"
+ genkey "${tmp_dir}"
+ key_id=$(cat ${tmp_dir}/signing_key.der | keyctl padd asymmetric ebpf_testing_key @s)
keyring_id=$(keyctl newring ebpf_testing_keyring @s)
keyctl link $key_id $keyring_id
}
@@ -105,6 +110,8 @@ main()
if [[ "${action}" == "setup" ]]; then
setup "${tmp_dir}"
+ elif [[ "${action}" == "genkey" ]]; then
+ genkey "${tmp_dir}"
elif [[ "${action}" == "cleanup" ]]; then
cleanup "${tmp_dir}"
elif [[ "${action}" == "fsverity-create-sign" ]]; then
--
2.43.0
^ permalink raw reply related
* [PATCH v4 11/12] bpftool: Add support for signing BPF programs
From: KP Singh @ 2025-09-14 21:51 UTC (permalink / raw)
To: bpf, linux-security-module
Cc: bboscaccy, paul, kys, ast, daniel, andrii, KP Singh
In-Reply-To: <20250914215141.15144-1-kpsingh@kernel.org>
Two modes of operation being added:
Add two modes of operation:
* For prog load, allow signing a program immediately before loading. This
is essential for command-line testing and administration.
bpftool prog load -S -k <private_key> -i <identity_cert> fentry_test.bpf.o
* For gen skeleton, embed a pre-generated signature into the C skeleton
file. This supports the use of signed programs in compiled applications.
bpftool gen skeleton -S -k <private_key> -i <identity_cert> fentry_test.bpf.o
Generation of the loader program and its metadata map is implemented in
libbpf (bpf_obj__gen_loader). bpftool generates a skeleton that loads
the program and automates the required steps: freezing the map, creating
an exclusive map, loading, and running. Users can use standard libbpf
APIs directly or integrate loader program generation into their own
toolchains.
Signed-off-by: KP Singh <kpsingh@kernel.org>
---
.../bpf/bpftool/Documentation/bpftool-gen.rst | 16 +-
.../bpftool/Documentation/bpftool-prog.rst | 18 +-
tools/bpf/bpftool/Makefile | 6 +-
tools/bpf/bpftool/cgroup.c | 4 +
tools/bpf/bpftool/gen.c | 66 +++++-
tools/bpf/bpftool/main.c | 26 ++-
tools/bpf/bpftool/main.h | 11 +
tools/bpf/bpftool/prog.c | 27 ++-
tools/bpf/bpftool/sign.c | 212 ++++++++++++++++++
9 files changed, 373 insertions(+), 13 deletions(-)
create mode 100644 tools/bpf/bpftool/sign.c
diff --git a/tools/bpf/bpftool/Documentation/bpftool-gen.rst b/tools/bpf/bpftool/Documentation/bpftool-gen.rst
index ca860fd97d8d..cef469d758ed 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-gen.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-gen.rst
@@ -16,7 +16,8 @@ SYNOPSIS
**bpftool** [*OPTIONS*] **gen** *COMMAND*
-*OPTIONS* := { |COMMON_OPTIONS| | { **-L** | **--use-loader** } }
+*OPTIONS* := { |COMMON_OPTIONS| [ { **-L** | **--use-loader** } ]
+[ { { **-S** | **--sign** } **-k** <private_key.pem> **-i** <certificate.x509> } ] }}
*COMMAND* := { **object** | **skeleton** | **help** }
@@ -186,6 +187,19 @@ OPTIONS
skeleton). A light skeleton contains a loader eBPF program. It does not use
the majority of the libbpf infrastructure, and does not need libelf.
+-S, --sign
+ For skeletons, generate a signed skeleton. This option must be used with
+ **-k** and **-i**. Using this flag implicitly enables **--use-loader**.
+ See the "Signed Skeletons" section in the description of the
+ **gen skeleton** command for more details.
+
+-k <private_key.pem>
+ Path to the private key file in PEM format, required for signing.
+
+-i <certificate.x509>
+ Path to the X.509 certificate file in PEM or DER format, required for
+ signing.
+
EXAMPLES
========
**$ cat example1.bpf.c**
diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
index f69fd92df8d8..55b812761df2 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
@@ -16,9 +16,9 @@ SYNOPSIS
**bpftool** [*OPTIONS*] **prog** *COMMAND*
-*OPTIONS* := { |COMMON_OPTIONS| |
-{ **-f** | **--bpffs** } | { **-m** | **--mapcompat** } | { **-n** | **--nomount** } |
-{ **-L** | **--use-loader** } }
+*OPTIONS* := { |COMMON_OPTIONS| [ { **-f** | **--bpffs** } ] [ { **-m** | **--mapcompat** } ]
+[ { **-n** | **--nomount** } ] [ { **-L** | **--use-loader** } ]
+[ { { **-S** | **--sign** } **-k** <private_key.pem> **-i** <certificate.x509> } ] }
*COMMANDS* :=
{ **show** | **list** | **dump xlated** | **dump jited** | **pin** | **load** |
@@ -248,6 +248,18 @@ OPTIONS
creating the maps, and loading the programs (see **bpftool prog tracelog**
as a way to dump those messages).
+-S, --sign
+ Enable signing of the BPF program before loading. This option must be
+ used with **-k** and **-i**. Using this flag implicitly enables
+ **--use-loader**.
+
+-k <private_key.pem>
+ Path to the private key file in PEM format, required when signing.
+
+-i <certificate.x509>
+ Path to the X.509 certificate file in PEM or DER format, required when
+ signing.
+
EXAMPLES
========
**# bpftool prog show**
diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
index 9e9a5f006cd2..586d1b2595d1 100644
--- a/tools/bpf/bpftool/Makefile
+++ b/tools/bpf/bpftool/Makefile
@@ -130,8 +130,8 @@ include $(FEATURES_DUMP)
endif
endif
-LIBS = $(LIBBPF) -lelf -lz
-LIBS_BOOTSTRAP = $(LIBBPF_BOOTSTRAP) -lelf -lz
+LIBS = $(LIBBPF) -lelf -lz -lcrypto
+LIBS_BOOTSTRAP = $(LIBBPF_BOOTSTRAP) -lelf -lz -lcrypto
ifeq ($(feature-libelf-zstd),1)
LIBS += -lzstd
@@ -194,7 +194,7 @@ endif
BPFTOOL_BOOTSTRAP := $(BOOTSTRAP_OUTPUT)bpftool
-BOOTSTRAP_OBJS = $(addprefix $(BOOTSTRAP_OUTPUT),main.o common.o json_writer.o gen.o btf.o)
+BOOTSTRAP_OBJS = $(addprefix $(BOOTSTRAP_OUTPUT),main.o common.o json_writer.o gen.o btf.o sign.o)
$(BOOTSTRAP_OBJS): $(LIBBPF_BOOTSTRAP)
OBJS = $(patsubst %.c,$(OUTPUT)%.o,$(SRCS)) $(OUTPUT)disasm.o
diff --git a/tools/bpf/bpftool/cgroup.c b/tools/bpf/bpftool/cgroup.c
index 944ebe21a216..ec356deb27c9 100644
--- a/tools/bpf/bpftool/cgroup.c
+++ b/tools/bpf/bpftool/cgroup.c
@@ -2,6 +2,10 @@
// Copyright (C) 2017 Facebook
// Author: Roman Gushchin <guro@fb.com>
+#undef GCC_VERSION
+#ifndef _GNU_SOURCE
+#define _GNU_SOURCE
+#endif
#define _XOPEN_SOURCE 500
#include <errno.h>
#include <fcntl.h>
diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
index 67a60114368f..694e61f1909e 100644
--- a/tools/bpf/bpftool/gen.c
+++ b/tools/bpf/bpftool/gen.c
@@ -688,10 +688,17 @@ static void codegen_destroy(struct bpf_object *obj, const char *obj_name)
static int gen_trace(struct bpf_object *obj, const char *obj_name, const char *header_guard)
{
DECLARE_LIBBPF_OPTS(gen_loader_opts, opts);
+ struct bpf_load_and_run_opts sopts = {};
+ char sig_buf[MAX_SIG_SIZE];
+ __u8 prog_sha[SHA256_DIGEST_LENGTH];
struct bpf_map *map;
+
char ident[256];
int err = 0;
+ if (sign_progs)
+ opts.gen_hash = true;
+
err = bpf_object__gen_loader(obj, &opts);
if (err)
return err;
@@ -701,6 +708,7 @@ static int gen_trace(struct bpf_object *obj, const char *obj_name, const char *h
p_err("failed to load object file");
goto out;
}
+
/* If there was no error during load then gen_loader_opts
* are populated with the loader program.
*/
@@ -780,8 +788,50 @@ static int gen_trace(struct bpf_object *obj, const char *obj_name, const char *h
print_hex(opts.insns, opts.insns_sz);
codegen("\
\n\
- \"; \n\
- \n\
+ \";\n");
+
+ if (sign_progs) {
+ sopts.insns = opts.insns;
+ sopts.insns_sz = opts.insns_sz;
+ sopts.excl_prog_hash = prog_sha;
+ sopts.excl_prog_hash_sz = sizeof(prog_sha);
+ sopts.signature = sig_buf;
+ sopts.signature_sz = MAX_SIG_SIZE;
+
+ err = bpftool_prog_sign(&sopts);
+ if (err < 0)
+ return err;
+
+ codegen("\
+ \n\
+ static const char opts_sig[] __attribute__((__aligned__(8))) = \"\\\n\
+ ");
+ print_hex((const void *)sig_buf, sopts.signature_sz);
+ codegen("\
+ \n\
+ \";\n");
+
+ codegen("\
+ \n\
+ static const char opts_excl_hash[] __attribute__((__aligned__(8))) = \"\\\n\
+ ");
+ print_hex((const void *)prog_sha, sizeof(prog_sha));
+ codegen("\
+ \n\
+ \";\n");
+
+ codegen("\
+ \n\
+ opts.signature = (void *)opts_sig; \n\
+ opts.signature_sz = sizeof(opts_sig) - 1; \n\
+ opts.excl_prog_hash = (void *)opts_excl_hash; \n\
+ opts.excl_prog_hash_sz = sizeof(opts_excl_hash) - 1; \n\
+ opts.keyring_id = skel->keyring_id; \n\
+ ");
+ }
+
+ codegen("\
+ \n\
opts.ctx = (struct bpf_loader_ctx *)skel; \n\
opts.data_sz = sizeof(opts_data) - 1; \n\
opts.data = (void *)opts_data; \n\
@@ -1240,7 +1290,7 @@ static int do_skeleton(int argc, char **argv)
err = -errno;
libbpf_strerror(err, err_buf, sizeof(err_buf));
p_err("failed to open BPF object file: %s", err_buf);
- goto out;
+ goto out_obj;
}
bpf_object__for_each_map(map, obj) {
@@ -1355,6 +1405,13 @@ static int do_skeleton(int argc, char **argv)
printf("\t} links;\n");
}
+ if (sign_progs) {
+ codegen("\
+ \n\
+ __s32 keyring_id; \n\
+ ");
+ }
+
if (btf) {
err = codegen_datasecs(obj, obj_name);
if (err)
@@ -1552,6 +1609,7 @@ static int do_skeleton(int argc, char **argv)
err = 0;
out:
bpf_object__close(obj);
+out_obj:
if (obj_data)
munmap(obj_data, mmap_sz);
close(fd);
@@ -1930,7 +1988,7 @@ static int do_help(int argc, char **argv)
" %1$s %2$s help\n"
"\n"
" " HELP_SPEC_OPTIONS " |\n"
- " {-L|--use-loader} }\n"
+ " {-L|--use-loader} | [ {-S|--sign } {-k} <private_key.pem> {-i} <certificate.x509> ]}\n"
"",
bin_name, "gen");
diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c
index 0f1183b2ed0a..c78eb80b9c94 100644
--- a/tools/bpf/bpftool/main.c
+++ b/tools/bpf/bpftool/main.c
@@ -33,6 +33,9 @@ bool relaxed_maps;
bool use_loader;
struct btf *base_btf;
struct hashmap *refs_table;
+bool sign_progs;
+const char *private_key_path;
+const char *cert_path;
static void __noreturn clean_and_exit(int i)
{
@@ -448,6 +451,7 @@ int main(int argc, char **argv)
{ "nomount", no_argument, NULL, 'n' },
{ "debug", no_argument, NULL, 'd' },
{ "use-loader", no_argument, NULL, 'L' },
+ { "sign", no_argument, NULL, 'S' },
{ "base-btf", required_argument, NULL, 'B' },
{ 0 }
};
@@ -474,7 +478,7 @@ int main(int argc, char **argv)
bin_name = "bpftool";
opterr = 0;
- while ((opt = getopt_long(argc, argv, "VhpjfLmndB:l",
+ while ((opt = getopt_long(argc, argv, "VhpjfLmndSi:k:B:l",
options, NULL)) >= 0) {
switch (opt) {
case 'V':
@@ -520,6 +524,16 @@ int main(int argc, char **argv)
case 'L':
use_loader = true;
break;
+ case 'S':
+ sign_progs = true;
+ use_loader = true;
+ break;
+ case 'k':
+ private_key_path = optarg;
+ break;
+ case 'i':
+ cert_path = optarg;
+ break;
default:
p_err("unrecognized option '%s'", argv[optind - 1]);
if (json_output)
@@ -534,6 +548,16 @@ int main(int argc, char **argv)
if (argc < 0)
usage();
+ if (sign_progs && (private_key_path == NULL || cert_path == NULL)) {
+ p_err("-i <identity_x509_cert> and -k <private> key must be supplied with -S for signing");
+ return -EINVAL;
+ }
+
+ if (!sign_progs && (private_key_path != NULL || cert_path != NULL)) {
+ p_err("-i <identity_x509_cert> and -k <private> also need --sign to be used for sign programs");
+ return -EINVAL;
+ }
+
if (version_requested)
ret = do_version(argc, argv);
else
diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
index 374cac2a8c66..1130299cede0 100644
--- a/tools/bpf/bpftool/main.h
+++ b/tools/bpf/bpftool/main.h
@@ -6,9 +6,14 @@
/* BFD and kernel.h both define GCC_VERSION, differently */
#undef GCC_VERSION
+#ifndef _GNU_SOURCE
+#define _GNU_SOURCE
+#endif
#include <stdbool.h>
#include <stdio.h>
+#include <errno.h>
#include <stdlib.h>
+#include <bpf/skel_internal.h>
#include <linux/bpf.h>
#include <linux/compiler.h>
#include <linux/kernel.h>
@@ -52,6 +57,7 @@ static inline void *u64_to_ptr(__u64 ptr)
})
#define ERR_MAX_LEN 1024
+#define MAX_SIG_SIZE 4096
#define BPF_TAG_FMT "%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx"
@@ -85,6 +91,9 @@ extern bool relaxed_maps;
extern bool use_loader;
extern struct btf *base_btf;
extern struct hashmap *refs_table;
+extern bool sign_progs;
+extern const char *private_key_path;
+extern const char *cert_path;
void __printf(1, 2) p_err(const char *fmt, ...);
void __printf(1, 2) p_info(const char *fmt, ...);
@@ -284,4 +293,6 @@ struct kernel_config_option {
int read_kernel_config(const struct kernel_config_option *requested_options,
size_t num_options, char **out_values,
const char *define_prefix);
+int bpftool_prog_sign(struct bpf_load_and_run_opts *opts);
+__u32 register_session_key(const char *key_der_path);
#endif
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index cf18c3879680..f78a5135f104 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -23,6 +23,7 @@
#include <linux/err.h>
#include <linux/perf_event.h>
#include <linux/sizes.h>
+#include <linux/keyctl.h>
#include <bpf/bpf.h>
#include <bpf/btf.h>
@@ -1930,6 +1931,8 @@ static int try_loader(struct gen_loader_opts *gen)
{
struct bpf_load_and_run_opts opts = {};
struct bpf_loader_ctx *ctx;
+ char sig_buf[MAX_SIG_SIZE];
+ __u8 prog_sha[SHA256_DIGEST_LENGTH];
int ctx_sz = sizeof(*ctx) + 64 * max(sizeof(struct bpf_map_desc),
sizeof(struct bpf_prog_desc));
int log_buf_sz = (1u << 24) - 1;
@@ -1953,6 +1956,24 @@ static int try_loader(struct gen_loader_opts *gen)
opts.insns = gen->insns;
opts.insns_sz = gen->insns_sz;
fds_before = count_open_fds();
+
+ if (sign_progs) {
+ opts.excl_prog_hash = prog_sha;
+ opts.excl_prog_hash_sz = sizeof(prog_sha);
+ opts.signature = sig_buf;
+ opts.signature_sz = MAX_SIG_SIZE;
+ opts.keyring_id = KEY_SPEC_SESSION_KEYRING;
+
+ err = bpftool_prog_sign(&opts);
+ if (err < 0)
+ return err;
+
+ err = register_session_key(cert_path);
+ if (err < 0) {
+ p_err("failed to add session key");
+ goto out;
+ }
+ }
err = bpf_load_and_run(&opts);
fd_delta = count_open_fds() - fds_before;
if (err < 0 || verifier_logs) {
@@ -1961,6 +1982,7 @@ static int try_loader(struct gen_loader_opts *gen)
fprintf(stderr, "loader prog leaked %d FDs\n",
fd_delta);
}
+out:
free(log_buf);
return err;
}
@@ -1988,6 +2010,9 @@ static int do_loader(int argc, char **argv)
goto err_close_obj;
}
+ if (sign_progs)
+ gen.gen_hash = true;
+
err = bpf_object__gen_loader(obj, &gen);
if (err)
goto err_close_obj;
@@ -2562,7 +2587,7 @@ static int do_help(int argc, char **argv)
" METRIC := { cycles | instructions | l1d_loads | llc_misses | itlb_misses | dtlb_misses }\n"
" " HELP_SPEC_OPTIONS " |\n"
" {-f|--bpffs} | {-m|--mapcompat} | {-n|--nomount} |\n"
- " {-L|--use-loader} }\n"
+ " {-L|--use-loader} | [ {-S|--sign } {-k} <private_key.pem> {-i} <certificate.x509> ] \n"
"",
bin_name, argv[-2]);
diff --git a/tools/bpf/bpftool/sign.c b/tools/bpf/bpftool/sign.c
new file mode 100644
index 000000000000..b29d825bb1d4
--- /dev/null
+++ b/tools/bpf/bpftool/sign.c
@@ -0,0 +1,212 @@
+// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+/*
+ * Copyright (C) 2025 Google LLC.
+ */
+
+#ifndef _GNU_SOURCE
+#define _GNU_SOURCE
+#endif
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdint.h>
+#include <stdbool.h>
+#include <string.h>
+#include <string.h>
+#include <getopt.h>
+#include <err.h>
+#include <openssl/opensslv.h>
+#include <openssl/bio.h>
+#include <openssl/evp.h>
+#include <openssl/pem.h>
+#include <openssl/err.h>
+#include <openssl/cms.h>
+#include <linux/keyctl.h>
+#include <errno.h>
+
+#include <bpf/skel_internal.h>
+
+#include "main.h"
+
+#define OPEN_SSL_ERR_BUF_LEN 256
+
+static void display_openssl_errors(int l)
+{
+ char buf[OPEN_SSL_ERR_BUF_LEN];
+ const char *file;
+ const char *data;
+ unsigned long e;
+ int flags;
+ int line;
+
+ while ((e = ERR_get_error_all(&file, &line, NULL, &data, &flags))) {
+ ERR_error_string_n(e, buf, sizeof(buf));
+ if (data && (flags & ERR_TXT_STRING)) {
+ p_err("OpenSSL %s: %s:%d: %s", buf, file, line, data);
+ } else {
+ p_err("OpenSSL %s: %s:%d", buf, file, line);
+ }
+ }
+}
+
+#define DISPLAY_OSSL_ERR(cond) \
+ do { \
+ bool __cond = (cond); \
+ if (__cond && ERR_peek_error()) \
+ display_openssl_errors(__LINE__);\
+ } while (0)
+
+static EVP_PKEY *read_private_key(const char *pkey_path)
+{
+ EVP_PKEY *private_key = NULL;
+ BIO *b;
+
+ b = BIO_new_file(pkey_path, "rb");
+ private_key = PEM_read_bio_PrivateKey(b, NULL, NULL, NULL);
+ BIO_free(b);
+ DISPLAY_OSSL_ERR(!private_key);
+ return private_key;
+}
+
+static X509 *read_x509(const char *x509_name)
+{
+ unsigned char buf[2];
+ X509 *x509 = NULL;
+ BIO *b;
+ int n;
+
+ b = BIO_new_file(x509_name, "rb");
+ if (!b)
+ goto cleanup;
+
+ /* Look at the first two bytes of the file to determine the encoding */
+ n = BIO_read(b, buf, 2);
+ if (n != 2)
+ goto cleanup;
+
+ if (BIO_reset(b) != 0)
+ goto cleanup;
+
+ if (buf[0] == 0x30 && buf[1] >= 0x81 && buf[1] <= 0x84)
+ /* Assume raw DER encoded X.509 */
+ x509 = d2i_X509_bio(b, NULL);
+ else
+ /* Assume PEM encoded X.509 */
+ x509 = PEM_read_bio_X509(b, NULL, NULL, NULL);
+
+cleanup:
+ BIO_free(b);
+ DISPLAY_OSSL_ERR(!x509);
+ return x509;
+}
+
+__u32 register_session_key(const char *key_der_path)
+{
+ unsigned char *der_buf = NULL;
+ X509 *x509 = NULL;
+ int key_id = -1;
+ int der_len;
+
+ if (!key_der_path)
+ return key_id;
+ x509 = read_x509(key_der_path);
+ if (!x509)
+ goto cleanup;
+ der_len = i2d_X509(x509, &der_buf);
+ if (der_len < 0)
+ goto cleanup;
+ key_id = syscall(__NR_add_key, "asymmetric", key_der_path, der_buf,
+ (size_t)der_len, KEY_SPEC_SESSION_KEYRING);
+cleanup:
+ X509_free(x509);
+ OPENSSL_free(der_buf);
+ DISPLAY_OSSL_ERR(key_id == -1);
+ return key_id;
+}
+
+int bpftool_prog_sign(struct bpf_load_and_run_opts *opts)
+{
+ BIO *bd_in = NULL, *bd_out = NULL;
+ EVP_PKEY *private_key = NULL;
+ CMS_ContentInfo *cms = NULL;
+ long actual_sig_len = 0;
+ X509 *x509 = NULL;
+ int err = 0;
+
+ bd_in = BIO_new_mem_buf(opts->insns, opts->insns_sz);
+ if (!bd_in) {
+ err = -ENOMEM;
+ goto cleanup;
+ }
+
+ private_key = read_private_key(private_key_path);
+ if (!private_key) {
+ err = -EINVAL;
+ goto cleanup;
+ }
+
+ x509 = read_x509(cert_path);
+ if (!x509) {
+ err = -EINVAL;
+ goto cleanup;
+ }
+
+ cms = CMS_sign(NULL, NULL, NULL, NULL,
+ CMS_NOCERTS | CMS_PARTIAL | CMS_BINARY | CMS_DETACHED |
+ CMS_STREAM);
+ if (!cms) {
+ err = -EINVAL;
+ goto cleanup;
+ }
+
+ if (!CMS_add1_signer(cms, x509, private_key, EVP_sha256(),
+ CMS_NOCERTS | CMS_BINARY | CMS_NOSMIMECAP |
+ CMS_USE_KEYID | CMS_NOATTR)) {
+ err = -EINVAL;
+ goto cleanup;
+ }
+
+ if (CMS_final(cms, bd_in, NULL, CMS_NOCERTS | CMS_BINARY) != 1) {
+ err = -EIO;
+ goto cleanup;
+ }
+
+ EVP_Digest(opts->insns, opts->insns_sz, opts->excl_prog_hash,
+ &opts->excl_prog_hash_sz, EVP_sha256(), NULL);
+
+ bd_out = BIO_new(BIO_s_mem());
+ if (!bd_out) {
+ err = -ENOMEM;
+ goto cleanup;
+ }
+
+ if (!i2d_CMS_bio_stream(bd_out, cms, NULL, 0)) {
+ err = -EIO;
+ goto cleanup;
+ }
+
+ actual_sig_len = BIO_get_mem_data(bd_out, NULL);
+ if (actual_sig_len <= 0) {
+ err = -EIO;
+ goto cleanup;
+ }
+
+ if ((size_t)actual_sig_len > opts->signature_sz) {
+ err = -ENOSPC;
+ goto cleanup;
+ }
+
+ if (BIO_read(bd_out, opts->signature, actual_sig_len) != actual_sig_len) {
+ err = -EIO;
+ goto cleanup;
+ }
+
+ opts->signature_sz = actual_sig_len;
+cleanup:
+ BIO_free(bd_out);
+ CMS_ContentInfo_free(cms);
+ X509_free(x509);
+ EVP_PKEY_free(private_key);
+ BIO_free(bd_in);
+ DISPLAY_OSSL_ERR(err < 0);
+ return err;
+}
--
2.43.0
^ permalink raw reply related
* [PATCH v4 10/12] libbpf: Embed and verify the metadata hash in the loader
From: KP Singh @ 2025-09-14 21:51 UTC (permalink / raw)
To: bpf, linux-security-module
Cc: bboscaccy, paul, kys, ast, daniel, andrii, KP Singh
In-Reply-To: <20250914215141.15144-1-kpsingh@kernel.org>
To fulfill the BPF signing contract, represented as Sig(I_loader ||
H_meta), the generated trusted loader program must verify the integrity
of the metadata. This signature cryptographically binds the loader's
instructions (I_loader) to a hash of the metadata (H_meta).
The verification process is embedded directly into the loader program.
Upon execution, the loader loads the runtime hash from struct bpf_map
i.e. BPF_PSEUDO_MAP_IDX and compares this runtime hash against an
expected hash value that has been hardcoded directly by
bpf_obj__gen_loader.
The load from bpf_map can be improved by calling
BPF_OBJ_GET_INFO_BY_FD from the kernel context after BPF_OBJ_GET_INFO_BY_FD
has been updated for being called from the kernel context.
The following instructions are generated:
ld_imm64 r1, const_ptr_to_map // insn[0].src_reg == BPF_PSEUDO_MAP_IDX
r2 = *(u64 *)(r1 + 0);
ld_imm64 r3, sha256_of_map_part1 // constant precomputed by
bpftool (part of H_meta)
if r2 != r3 goto out;
r2 = *(u64 *)(r1 + 8);
ld_imm64 r3, sha256_of_map_part2 // (part of H_meta)
if r2 != r3 goto out;
r2 = *(u64 *)(r1 + 16);
ld_imm64 r3, sha256_of_map_part3 // (part of H_meta)
if r2 != r3 goto out;
r2 = *(u64 *)(r1 + 24);
ld_imm64 r3, sha256_of_map_part4 // (part of H_meta)
if r2 != r3 goto out;
...
Signed-off-by: KP Singh <kpsingh@kernel.org>
---
tools/lib/bpf/bpf_gen_internal.h | 2 ++
tools/lib/bpf/gen_loader.c | 55 ++++++++++++++++++++++++++++++++
tools/lib/bpf/libbpf.h | 3 +-
3 files changed, 59 insertions(+), 1 deletion(-)
diff --git a/tools/lib/bpf/bpf_gen_internal.h b/tools/lib/bpf/bpf_gen_internal.h
index 6ff963a491d9..49af4260b8e6 100644
--- a/tools/lib/bpf/bpf_gen_internal.h
+++ b/tools/lib/bpf/bpf_gen_internal.h
@@ -4,6 +4,7 @@
#define __BPF_GEN_INTERNAL_H
#include "bpf.h"
+#include "libbpf_internal.h"
struct ksym_relo_desc {
const char *name;
@@ -50,6 +51,7 @@ struct bpf_gen {
__u32 nr_ksyms;
int fd_array;
int nr_fd_array;
+ int hash_insn_offset[SHA256_DWORD_SIZE];
};
void bpf_gen__init(struct bpf_gen *gen, int log_level, int nr_progs, int nr_maps);
diff --git a/tools/lib/bpf/gen_loader.c b/tools/lib/bpf/gen_loader.c
index 113ae4abd345..376eef292d3a 100644
--- a/tools/lib/bpf/gen_loader.c
+++ b/tools/lib/bpf/gen_loader.c
@@ -110,6 +110,7 @@ static void emit2(struct bpf_gen *gen, struct bpf_insn insn1, struct bpf_insn in
static int add_data(struct bpf_gen *gen, const void *data, __u32 size);
static void emit_sys_close_blob(struct bpf_gen *gen, int blob_off);
+static void emit_signature_match(struct bpf_gen *gen);
void bpf_gen__init(struct bpf_gen *gen, int log_level, int nr_progs, int nr_maps)
{
@@ -152,6 +153,8 @@ void bpf_gen__init(struct bpf_gen *gen, int log_level, int nr_progs, int nr_maps
/* R7 contains the error code from sys_bpf. Copy it into R0 and exit. */
emit(gen, BPF_MOV64_REG(BPF_REG_0, BPF_REG_7));
emit(gen, BPF_EXIT_INSN());
+ if (OPTS_GET(gen->opts, gen_hash, false))
+ emit_signature_match(gen);
}
static int add_data(struct bpf_gen *gen, const void *data, __u32 size)
@@ -368,6 +371,8 @@ static void emit_sys_close_blob(struct bpf_gen *gen, int blob_off)
__emit_sys_close(gen);
}
+static int compute_sha_udpate_offsets(struct bpf_gen *gen);
+
int bpf_gen__finish(struct bpf_gen *gen, int nr_progs, int nr_maps)
{
int i;
@@ -394,6 +399,12 @@ int bpf_gen__finish(struct bpf_gen *gen, int nr_progs, int nr_maps)
blob_fd_array_off(gen, i));
emit(gen, BPF_MOV64_IMM(BPF_REG_0, 0));
emit(gen, BPF_EXIT_INSN());
+ if (OPTS_GET(gen->opts, gen_hash, false)) {
+ gen->error = compute_sha_udpate_offsets(gen);
+ if (gen->error)
+ return gen->error;
+ }
+
pr_debug("gen: finish %s\n", errstr(gen->error));
if (!gen->error) {
struct gen_loader_opts *opts = gen->opts;
@@ -446,6 +457,27 @@ void bpf_gen__free(struct bpf_gen *gen)
_val; \
})
+static int compute_sha_udpate_offsets(struct bpf_gen *gen)
+{
+ __u64 sha[SHA256_DWORD_SIZE];
+ __u64 sha_dw;
+ int i, err;
+
+ err = libbpf_sha256(gen->data_start, gen->data_cur - gen->data_start, sha, SHA256_DIGEST_LENGTH);
+ if (err < 0) {
+ pr_warn("sha256 computation of the metadata failed");
+ return err;
+ }
+ for (i = 0; i < SHA256_DWORD_SIZE; i++) {
+ struct bpf_insn *insn =
+ (struct bpf_insn *)(gen->insn_start + gen->hash_insn_offset[i]);
+ sha_dw = tgt_endian(sha[i]);
+ insn[0].imm = (__u32)sha_dw;
+ insn[1].imm = sha_dw >> 32;
+ }
+ return 0;
+}
+
void bpf_gen__load_btf(struct bpf_gen *gen, const void *btf_raw_data,
__u32 btf_raw_size)
{
@@ -557,6 +589,29 @@ void bpf_gen__map_create(struct bpf_gen *gen,
emit_sys_close_stack(gen, stack_off(inner_map_fd));
}
+static void emit_signature_match(struct bpf_gen *gen)
+{
+ __s64 off;
+ int i;
+
+ for (i = 0; i < SHA256_DWORD_SIZE; i++) {
+ emit2(gen, BPF_LD_IMM64_RAW_FULL(BPF_REG_1, BPF_PSEUDO_MAP_IDX,
+ 0, 0, 0, 0));
+ emit(gen, BPF_LDX_MEM(BPF_DW, BPF_REG_2, BPF_REG_1, i * sizeof(__u64)));
+ gen->hash_insn_offset[i] = gen->insn_cur - gen->insn_start;
+ emit2(gen, BPF_LD_IMM64_RAW_FULL(BPF_REG_3, 0, 0, 0, 0, 0));
+
+ off = -(gen->insn_cur - gen->insn_start - gen->cleanup_label) / 8 - 1;
+ if (is_simm16(off)) {
+ emit(gen, BPF_MOV64_IMM(BPF_REG_7, -EINVAL));
+ emit(gen, BPF_JMP_REG(BPF_JNE, BPF_REG_2, BPF_REG_3, off));
+ } else {
+ gen->error = -ERANGE;
+ emit(gen, BPF_JMP_IMM(BPF_JA, 0, 0, -1));
+ }
+ }
+}
+
void bpf_gen__record_attach_target(struct bpf_gen *gen, const char *attach_name,
enum bpf_attach_type type)
{
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index e978bc093c39..5118d0a90e24 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -1857,9 +1857,10 @@ struct gen_loader_opts {
const char *insns;
__u32 data_sz;
__u32 insns_sz;
+ bool gen_hash;
};
-#define gen_loader_opts__last_field insns_sz
+#define gen_loader_opts__last_field gen_hash
LIBBPF_API int bpf_object__gen_loader(struct bpf_object *obj,
struct gen_loader_opts *opts);
--
2.43.0
^ permalink raw reply related
* [PATCH v4 09/12] libbpf: Update light skeleton for signing
From: KP Singh @ 2025-09-14 21:51 UTC (permalink / raw)
To: bpf, linux-security-module
Cc: bboscaccy, paul, kys, ast, daniel, andrii, KP Singh
In-Reply-To: <20250914215141.15144-1-kpsingh@kernel.org>
* The metadata map is created with as an exclusive map (with an
excl_prog_hash) This restricts map access exclusively to the signed
loader program, preventing tampering by other processes.
* The map is then frozen, making it read-only from userspace.
* BPF_OBJ_GET_INFO_BY_ID instructs the kernel to compute the hash of the
metadata map (H') and store it in bpf_map->sha.
* The loader is then loaded with the signature which is then verified by
the kernel.
loading signed programs prebuilt into the kernel are not currently
supported. These can supported by enabling BPF_OBJ_GET_INFO_BY_ID to be
called from the kernel.
Signed-off-by: KP Singh <kpsingh@kernel.org>
---
tools/lib/bpf/skel_internal.h | 76 +++++++++++++++++++++++++++++++++--
1 file changed, 72 insertions(+), 4 deletions(-)
diff --git a/tools/lib/bpf/skel_internal.h b/tools/lib/bpf/skel_internal.h
index 4d5fa079b5d6..7f784c32b967 100644
--- a/tools/lib/bpf/skel_internal.h
+++ b/tools/lib/bpf/skel_internal.h
@@ -13,10 +13,15 @@
#include <unistd.h>
#include <sys/syscall.h>
#include <sys/mman.h>
+#include <linux/keyctl.h>
#include <stdlib.h>
#include "bpf.h"
#endif
+#ifndef SHA256_DIGEST_LENGTH
+#define SHA256_DIGEST_LENGTH 32
+#endif
+
#ifndef __NR_bpf
# if defined(__mips__) && defined(_ABIO32)
# define __NR_bpf 4355
@@ -64,6 +69,11 @@ struct bpf_load_and_run_opts {
__u32 data_sz;
__u32 insns_sz;
const char *errstr;
+ void *signature;
+ __u32 signature_sz;
+ __s32 keyring_id;
+ void * excl_prog_hash;
+ __u32 excl_prog_hash_sz;
};
long kern_sys_bpf(__u32 cmd, void *attr, __u32 attr_size);
@@ -220,14 +230,19 @@ static inline int skel_map_create(enum bpf_map_type map_type,
const char *map_name,
__u32 key_size,
__u32 value_size,
- __u32 max_entries)
+ __u32 max_entries,
+ const void *excl_prog_hash,
+ __u32 excl_prog_hash_sz)
{
- const size_t attr_sz = offsetofend(union bpf_attr, map_extra);
+ const size_t attr_sz = offsetofend(union bpf_attr, excl_prog_hash_size);
union bpf_attr attr;
memset(&attr, 0, attr_sz);
attr.map_type = map_type;
+ attr.excl_prog_hash = (unsigned long) excl_prog_hash;
+ attr.excl_prog_hash_size = excl_prog_hash_sz;
+
strncpy(attr.map_name, map_name, sizeof(attr.map_name));
attr.key_size = key_size;
attr.value_size = value_size;
@@ -300,6 +315,35 @@ static inline int skel_link_create(int prog_fd, int target_fd,
return skel_sys_bpf(BPF_LINK_CREATE, &attr, attr_sz);
}
+static inline int skel_obj_get_info_by_fd(int fd)
+{
+ const size_t attr_sz = offsetofend(union bpf_attr, info);
+ __u8 sha[SHA256_DIGEST_LENGTH];
+ struct bpf_map_info info;
+ __u32 info_len = sizeof(info);
+ union bpf_attr attr;
+
+ memset(&info, 0, sizeof(info));
+ info.hash = (long) &sha;
+ info.hash_size = SHA256_DIGEST_LENGTH;
+
+ memset(&attr, 0, attr_sz);
+ attr.info.bpf_fd = fd;
+ attr.info.info = (long) &info;
+ attr.info.info_len = info_len;
+ return skel_sys_bpf(BPF_OBJ_GET_INFO_BY_FD, &attr, attr_sz);
+}
+
+static inline int skel_map_freeze(int fd)
+{
+ const size_t attr_sz = offsetofend(union bpf_attr, map_fd);
+ union bpf_attr attr;
+
+ memset(&attr, 0, attr_sz);
+ attr.map_fd = fd;
+
+ return skel_sys_bpf(BPF_MAP_FREEZE, &attr, attr_sz);
+}
#ifdef __KERNEL__
#define set_err
#else
@@ -308,12 +352,13 @@ static inline int skel_link_create(int prog_fd, int target_fd,
static inline int bpf_load_and_run(struct bpf_load_and_run_opts *opts)
{
- const size_t prog_load_attr_sz = offsetofend(union bpf_attr, fd_array);
+ const size_t prog_load_attr_sz = offsetofend(union bpf_attr, keyring_id);
const size_t test_run_attr_sz = offsetofend(union bpf_attr, test);
int map_fd = -1, prog_fd = -1, key = 0, err;
union bpf_attr attr;
- err = map_fd = skel_map_create(BPF_MAP_TYPE_ARRAY, "__loader.map", 4, opts->data_sz, 1);
+ err = map_fd = skel_map_create(BPF_MAP_TYPE_ARRAY, "__loader.map", 4, opts->data_sz, 1,
+ opts->excl_prog_hash, opts->excl_prog_hash_sz);
if (map_fd < 0) {
opts->errstr = "failed to create loader map";
set_err;
@@ -327,11 +372,34 @@ static inline int bpf_load_and_run(struct bpf_load_and_run_opts *opts)
goto out;
}
+#ifndef __KERNEL__
+ err = skel_map_freeze(map_fd);
+ if (err < 0) {
+ opts->errstr = "failed to freeze map";
+ set_err;
+ goto out;
+ }
+ err = skel_obj_get_info_by_fd(map_fd);
+ if (err < 0) {
+ opts->errstr = "failed to fetch obj info";
+ set_err;
+ goto out;
+ }
+#endif
+
memset(&attr, 0, prog_load_attr_sz);
attr.prog_type = BPF_PROG_TYPE_SYSCALL;
attr.insns = (long) opts->insns;
attr.insn_cnt = opts->insns_sz / sizeof(struct bpf_insn);
attr.license = (long) "Dual BSD/GPL";
+#ifndef __KERNEL__
+ attr.signature = (long) opts->signature;
+ attr.signature_size = opts->signature_sz;
+#else
+ if (opts->signature || opts->signature_sz)
+ pr_warn("signatures are not supported from bpf_preload\n");
+#endif
+ attr.keyring_id = opts->keyring_id;
memcpy(attr.prog_name, "__loader.prog", sizeof("__loader.prog"));
attr.fd_array = (long) &map_fd;
attr.log_level = opts->ctx->log_level;
--
2.43.0
^ permalink raw reply related
* [PATCH v4 08/12] bpf: Implement signature verification for BPF programs
From: KP Singh @ 2025-09-14 21:51 UTC (permalink / raw)
To: bpf, linux-security-module
Cc: bboscaccy, paul, kys, ast, daniel, andrii, syzbot, KP Singh
In-Reply-To: <20250914215141.15144-1-kpsingh@kernel.org>
This patch extends the BPF_PROG_LOAD command by adding three new fields
to `union bpf_attr` in the user-space API:
- signature: A pointer to the signature blob.
- signature_size: The size of the signature blob.
- keyring_id: The serial number of a loaded kernel keyring (e.g.,
the user or session keyring) containing the trusted public keys.
When a BPF program is loaded with a signature, the kernel:
1. Retrieves the trusted keyring using the provided `keyring_id`.
2. Verifies the supplied signature against the BPF program's
instruction buffer.
3. If the signature is valid and was generated by a key in the trusted
keyring, the program load proceeds.
4. If no signature is provided, the load proceeds as before, allowing
for backward compatibility. LSMs can chose to restrict unsigned
programs and implement a security policy.
5. If signature verification fails for any reason,
the program is not loaded.
Tested-by: syzbot@syzkaller.appspotmail.com
Signed-off-by: KP Singh <kpsingh@kernel.org>
---
crypto/asymmetric_keys/pkcs7_verify.c | 1 +
include/linux/verification.h | 1 +
include/uapi/linux/bpf.h | 10 ++++++
kernel/bpf/helpers.c | 2 +-
kernel/bpf/syscall.c | 45 ++++++++++++++++++++++++++-
tools/include/uapi/linux/bpf.h | 10 ++++++
tools/lib/bpf/bpf.c | 2 +-
7 files changed, 68 insertions(+), 3 deletions(-)
diff --git a/crypto/asymmetric_keys/pkcs7_verify.c b/crypto/asymmetric_keys/pkcs7_verify.c
index f0d4ff3c20a8..6d6475e3a9bf 100644
--- a/crypto/asymmetric_keys/pkcs7_verify.c
+++ b/crypto/asymmetric_keys/pkcs7_verify.c
@@ -429,6 +429,7 @@ int pkcs7_verify(struct pkcs7_message *pkcs7,
/* Authattr presence checked in parser */
break;
case VERIFYING_UNSPECIFIED_SIGNATURE:
+ case VERIFYING_BPF_SIGNATURE:
if (pkcs7->data_type != OID_data) {
pr_warn("Invalid unspecified sig (not pkcs7-data)\n");
return -EKEYREJECTED;
diff --git a/include/linux/verification.h b/include/linux/verification.h
index 4f3022d081c3..dec7f2beabfd 100644
--- a/include/linux/verification.h
+++ b/include/linux/verification.h
@@ -36,6 +36,7 @@ enum key_being_used_for {
VERIFYING_KEY_SIGNATURE,
VERIFYING_KEY_SELF_SIGNATURE,
VERIFYING_UNSPECIFIED_SIGNATURE,
+ VERIFYING_BPF_SIGNATURE,
NR__KEY_BEING_USED_FOR
};
#ifdef CONFIG_SYSTEM_DATA_VERIFICATION
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 0987b52d5648..f3b173e48b0f 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -1611,6 +1611,16 @@ union bpf_attr {
* continuous.
*/
__u32 fd_array_cnt;
+ /* Pointer to a buffer containing the signature of the BPF
+ * program.
+ */
+ __aligned_u64 signature;
+ /* Size of the signature buffer in bytes. */
+ __u32 signature_size;
+ /* ID of the kernel keyring to be used for signature
+ * verification.
+ */
+ __s32 keyring_id;
};
struct { /* anonymous struct used by BPF_OBJ_* commands */
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index 922dae3afa63..03bea5343f4b 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -3898,7 +3898,7 @@ __bpf_kfunc int bpf_verify_pkcs7_signature(struct bpf_dynptr *data_p,
return verify_pkcs7_signature(data, data_len, sig, sig_len,
trusted_keyring->key,
- VERIFYING_UNSPECIFIED_SIGNATURE, NULL,
+ VERIFYING_BPF_SIGNATURE, NULL,
NULL);
#else
return -EOPNOTSUPP;
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index cf7173b1bb83..8a3c3d26f6e2 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -39,6 +39,7 @@
#include <linux/tracepoint.h>
#include <linux/overflow.h>
#include <linux/cookie.h>
+#include <linux/verification.h>
#include <net/netfilter/nf_bpf_link.h>
#include <net/netkit.h>
@@ -2785,8 +2786,44 @@ static bool is_perfmon_prog_type(enum bpf_prog_type prog_type)
}
}
+static int bpf_prog_verify_signature(struct bpf_prog *prog, union bpf_attr *attr,
+ bool is_kernel)
+{
+ bpfptr_t usig = make_bpfptr(attr->signature, is_kernel);
+ struct bpf_dynptr_kern sig_ptr, insns_ptr;
+ struct bpf_key *key = NULL;
+ void *sig;
+ int err = 0;
+
+ if (system_keyring_id_check(attr->keyring_id) == 0)
+ key = bpf_lookup_system_key(attr->keyring_id);
+ else
+ key = bpf_lookup_user_key(attr->keyring_id, 0);
+
+ if (!key)
+ return -EINVAL;
+
+ sig = kvmemdup_bpfptr(usig, attr->signature_size);
+ if (IS_ERR(sig)) {
+ bpf_key_put(key);
+ return -ENOMEM;
+ }
+
+ bpf_dynptr_init(&sig_ptr, sig, BPF_DYNPTR_TYPE_LOCAL, 0,
+ attr->signature_size);
+ bpf_dynptr_init(&insns_ptr, prog->insnsi, BPF_DYNPTR_TYPE_LOCAL, 0,
+ prog->len * sizeof(struct bpf_insn));
+
+ err = bpf_verify_pkcs7_signature((struct bpf_dynptr *)&insns_ptr,
+ (struct bpf_dynptr *)&sig_ptr, key);
+
+ bpf_key_put(key);
+ kvfree(sig);
+ return err;
+}
+
/* last field in 'union bpf_attr' used by this command */
-#define BPF_PROG_LOAD_LAST_FIELD fd_array_cnt
+#define BPF_PROG_LOAD_LAST_FIELD keyring_id
static int bpf_prog_load(union bpf_attr *attr, bpfptr_t uattr, u32 uattr_size)
{
@@ -2950,6 +2987,12 @@ static int bpf_prog_load(union bpf_attr *attr, bpfptr_t uattr, u32 uattr_size)
/* eBPF programs must be GPL compatible to use GPL-ed functions */
prog->gpl_compatible = license_is_gpl_compatible(license) ? 1 : 0;
+ if (attr->signature) {
+ err = bpf_prog_verify_signature(prog, attr, uattr.is_kernel);
+ if (err)
+ goto free_prog;
+ }
+
prog->orig_prog = NULL;
prog->jited = 0;
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 0987b52d5648..f3b173e48b0f 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -1611,6 +1611,16 @@ union bpf_attr {
* continuous.
*/
__u32 fd_array_cnt;
+ /* Pointer to a buffer containing the signature of the BPF
+ * program.
+ */
+ __aligned_u64 signature;
+ /* Size of the signature buffer in bytes. */
+ __u32 signature_size;
+ /* ID of the kernel keyring to be used for signature
+ * verification.
+ */
+ __s32 keyring_id;
};
struct { /* anonymous struct used by BPF_OBJ_* commands */
diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
index 19ad7bcf0c2f..339b19797237 100644
--- a/tools/lib/bpf/bpf.c
+++ b/tools/lib/bpf/bpf.c
@@ -240,7 +240,7 @@ int bpf_prog_load(enum bpf_prog_type prog_type,
const struct bpf_insn *insns, size_t insn_cnt,
struct bpf_prog_load_opts *opts)
{
- const size_t attr_sz = offsetofend(union bpf_attr, fd_array_cnt);
+ const size_t attr_sz = offsetofend(union bpf_attr, keyring_id);
void *finfo = NULL, *linfo = NULL;
const char *func_info, *line_info;
__u32 log_size, log_level, attach_prog_fd, attach_btf_obj_fd;
--
2.43.0
^ permalink raw reply related
* [PATCH v4 07/12] bpf: Move the signature kfuncs to helpers.c
From: KP Singh @ 2025-09-14 21:51 UTC (permalink / raw)
To: bpf, linux-security-module
Cc: bboscaccy, paul, kys, ast, daniel, andrii, KP Singh
In-Reply-To: <20250914215141.15144-1-kpsingh@kernel.org>
No functional changes, except for the addition of the headers for the
kfuncs so that they can be used for signature verification.
Signed-off-by: KP Singh <kpsingh@kernel.org>
---
include/linux/bpf.h | 32 +++++++
kernel/bpf/helpers.c | 166 +++++++++++++++++++++++++++++++++++
kernel/trace/bpf_trace.c | 183 ---------------------------------------
3 files changed, 198 insertions(+), 183 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index e0c2c78a5faa..dfc1a27b56d5 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -3424,6 +3424,38 @@ static inline int bpf_fd_reuseport_array_update_elem(struct bpf_map *map,
#endif /* CONFIG_BPF_SYSCALL */
#endif /* defined(CONFIG_INET) && defined(CONFIG_BPF_SYSCALL) */
+#if defined(CONFIG_KEYS) && defined(CONFIG_BPF_SYSCALL)
+
+struct bpf_key *bpf_lookup_user_key(s32 serial, u64 flags);
+struct bpf_key *bpf_lookup_system_key(u64 id);
+void bpf_key_put(struct bpf_key *bkey);
+int bpf_verify_pkcs7_signature(struct bpf_dynptr *data_p,
+ struct bpf_dynptr *sig_p,
+ struct bpf_key *trusted_keyring);
+
+#else
+static inline struct bpf_key *bpf_lookup_user_key(u32 serial, u64 flags)
+{
+ return NULL;
+}
+
+static inline struct bpf_key *bpf_lookup_system_key(u64 id)
+{
+ return NULL;
+}
+
+static inline void bpf_key_put(struct bpf_key *bkey)
+{
+}
+
+static inline int bpf_verify_pkcs7_signature(struct bpf_dynptr *data_p,
+ struct bpf_dynptr *sig_p,
+ struct bpf_key *trusted_keyring)
+{
+ return -EOPNOTSUPP;
+}
+#endif /* defined(CONFIG_KEYS) && defined(CONFIG_BPF_SYSCALL) */
+
/* verifier prototypes for helper functions called from eBPF programs */
extern const struct bpf_func_proto bpf_map_lookup_elem_proto;
extern const struct bpf_func_proto bpf_map_update_elem_proto;
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index c0c0764a2025..922dae3afa63 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -25,6 +25,7 @@
#include <linux/kasan.h>
#include <linux/bpf_verifier.h>
#include <linux/uaccess.h>
+#include <linux/verification.h>
#include "../../lib/kstrtox.h"
@@ -3747,6 +3748,163 @@ __bpf_kfunc int bpf_strstr(const char *s1__ign, const char *s2__ign)
{
return bpf_strnstr(s1__ign, s2__ign, XATTR_SIZE_MAX);
}
+#ifdef CONFIG_KEYS
+/**
+ * bpf_lookup_user_key - lookup a key by its serial
+ * @serial: key handle serial number
+ * @flags: lookup-specific flags
+ *
+ * Search a key with a given *serial* and the provided *flags*.
+ * If found, increment the reference count of the key by one, and
+ * return it in the bpf_key structure.
+ *
+ * The bpf_key structure must be passed to bpf_key_put() when done
+ * with it, so that the key reference count is decremented and the
+ * bpf_key structure is freed.
+ *
+ * Permission checks are deferred to the time the key is used by
+ * one of the available key-specific kfuncs.
+ *
+ * Set *flags* with KEY_LOOKUP_CREATE, to attempt creating a requested
+ * special keyring (e.g. session keyring), if it doesn't yet exist.
+ * Set *flags* with KEY_LOOKUP_PARTIAL, to lookup a key without waiting
+ * for the key construction, and to retrieve uninstantiated keys (keys
+ * without data attached to them).
+ *
+ * Return: a bpf_key pointer with a valid key pointer if the key is found, a
+ * NULL pointer otherwise.
+ */
+__bpf_kfunc struct bpf_key *bpf_lookup_user_key(s32 serial, u64 flags)
+{
+ key_ref_t key_ref;
+ struct bpf_key *bkey;
+
+ if (flags & ~KEY_LOOKUP_ALL)
+ return NULL;
+
+ /*
+ * Permission check is deferred until the key is used, as the
+ * intent of the caller is unknown here.
+ */
+ key_ref = lookup_user_key(serial, flags, KEY_DEFER_PERM_CHECK);
+ if (IS_ERR(key_ref))
+ return NULL;
+
+ bkey = kmalloc(sizeof(*bkey), GFP_KERNEL);
+ if (!bkey) {
+ key_put(key_ref_to_ptr(key_ref));
+ return NULL;
+ }
+
+ bkey->key = key_ref_to_ptr(key_ref);
+ bkey->has_ref = true;
+
+ return bkey;
+}
+
+/**
+ * bpf_lookup_system_key - lookup a key by a system-defined ID
+ * @id: key ID
+ *
+ * Obtain a bpf_key structure with a key pointer set to the passed key ID.
+ * The key pointer is marked as invalid, to prevent bpf_key_put() from
+ * attempting to decrement the key reference count on that pointer. The key
+ * pointer set in such way is currently understood only by
+ * verify_pkcs7_signature().
+ *
+ * Set *id* to one of the values defined in include/linux/verification.h:
+ * 0 for the primary keyring (immutable keyring of system keys);
+ * VERIFY_USE_SECONDARY_KEYRING for both the primary and secondary keyring
+ * (where keys can be added only if they are vouched for by existing keys
+ * in those keyrings); VERIFY_USE_PLATFORM_KEYRING for the platform
+ * keyring (primarily used by the integrity subsystem to verify a kexec'ed
+ * kerned image and, possibly, the initramfs signature).
+ *
+ * Return: a bpf_key pointer with an invalid key pointer set from the
+ * pre-determined ID on success, a NULL pointer otherwise
+ */
+__bpf_kfunc struct bpf_key *bpf_lookup_system_key(u64 id)
+{
+ struct bpf_key *bkey;
+
+ if (system_keyring_id_check(id) < 0)
+ return NULL;
+
+ bkey = kmalloc(sizeof(*bkey), GFP_ATOMIC);
+ if (!bkey)
+ return NULL;
+
+ bkey->key = (struct key *)(unsigned long)id;
+ bkey->has_ref = false;
+
+ return bkey;
+}
+
+/**
+ * bpf_key_put - decrement key reference count if key is valid and free bpf_key
+ * @bkey: bpf_key structure
+ *
+ * Decrement the reference count of the key inside *bkey*, if the pointer
+ * is valid, and free *bkey*.
+ */
+__bpf_kfunc void bpf_key_put(struct bpf_key *bkey)
+{
+ if (bkey->has_ref)
+ key_put(bkey->key);
+
+ kfree(bkey);
+}
+
+/**
+ * bpf_verify_pkcs7_signature - verify a PKCS#7 signature
+ * @data_p: data to verify
+ * @sig_p: signature of the data
+ * @trusted_keyring: keyring with keys trusted for signature verification
+ *
+ * Verify the PKCS#7 signature *sig_ptr* against the supplied *data_ptr*
+ * with keys in a keyring referenced by *trusted_keyring*.
+ *
+ * Return: 0 on success, a negative value on error.
+ */
+__bpf_kfunc int bpf_verify_pkcs7_signature(struct bpf_dynptr *data_p,
+ struct bpf_dynptr *sig_p,
+ struct bpf_key *trusted_keyring)
+{
+#ifdef CONFIG_SYSTEM_DATA_VERIFICATION
+ struct bpf_dynptr_kern *data_ptr = (struct bpf_dynptr_kern *)data_p;
+ struct bpf_dynptr_kern *sig_ptr = (struct bpf_dynptr_kern *)sig_p;
+ const void *data, *sig;
+ u32 data_len, sig_len;
+ int ret;
+
+ if (trusted_keyring->has_ref) {
+ /*
+ * Do the permission check deferred in bpf_lookup_user_key().
+ * See bpf_lookup_user_key() for more details.
+ *
+ * A call to key_task_permission() here would be redundant, as
+ * it is already done by keyring_search() called by
+ * find_asymmetric_key().
+ */
+ ret = key_validate(trusted_keyring->key);
+ if (ret < 0)
+ return ret;
+ }
+
+ data_len = __bpf_dynptr_size(data_ptr);
+ data = __bpf_dynptr_data(data_ptr, data_len);
+ sig_len = __bpf_dynptr_size(sig_ptr);
+ sig = __bpf_dynptr_data(sig_ptr, sig_len);
+
+ return verify_pkcs7_signature(data, data_len, sig, sig_len,
+ trusted_keyring->key,
+ VERIFYING_UNSPECIFIED_SIGNATURE, NULL,
+ NULL);
+#else
+ return -EOPNOTSUPP;
+#endif /* CONFIG_SYSTEM_DATA_VERIFICATION */
+}
+#endif /* CONFIG_KEYS */
__bpf_kfunc_end_defs();
@@ -3788,6 +3946,14 @@ BTF_ID_FLAGS(func, bpf_throw)
#ifdef CONFIG_BPF_EVENTS
BTF_ID_FLAGS(func, bpf_send_signal_task, KF_TRUSTED_ARGS)
#endif
+#ifdef CONFIG_KEYS
+BTF_ID_FLAGS(func, bpf_lookup_user_key, KF_ACQUIRE | KF_RET_NULL | KF_SLEEPABLE)
+BTF_ID_FLAGS(func, bpf_lookup_system_key, KF_ACQUIRE | KF_RET_NULL)
+BTF_ID_FLAGS(func, bpf_key_put, KF_RELEASE)
+#ifdef CONFIG_SYSTEM_DATA_VERIFICATION
+BTF_ID_FLAGS(func, bpf_verify_pkcs7_signature, KF_SLEEPABLE)
+#endif
+#endif
BTF_KFUNCS_END(generic_btf_ids)
static const struct btf_kfunc_id_set generic_kfunc_set = {
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 606007c387c5..f2360579658e 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -22,7 +22,6 @@
#include <linux/bsearch.h>
#include <linux/sort.h>
#include <linux/key.h>
-#include <linux/verification.h>
#include <linux/namei.h>
#include <net/bpf_sk_storage.h>
@@ -1241,188 +1240,6 @@ static const struct bpf_func_proto bpf_get_func_arg_cnt_proto = {
.arg1_type = ARG_PTR_TO_CTX,
};
-#ifdef CONFIG_KEYS
-__bpf_kfunc_start_defs();
-
-/**
- * bpf_lookup_user_key - lookup a key by its serial
- * @serial: key handle serial number
- * @flags: lookup-specific flags
- *
- * Search a key with a given *serial* and the provided *flags*.
- * If found, increment the reference count of the key by one, and
- * return it in the bpf_key structure.
- *
- * The bpf_key structure must be passed to bpf_key_put() when done
- * with it, so that the key reference count is decremented and the
- * bpf_key structure is freed.
- *
- * Permission checks are deferred to the time the key is used by
- * one of the available key-specific kfuncs.
- *
- * Set *flags* with KEY_LOOKUP_CREATE, to attempt creating a requested
- * special keyring (e.g. session keyring), if it doesn't yet exist.
- * Set *flags* with KEY_LOOKUP_PARTIAL, to lookup a key without waiting
- * for the key construction, and to retrieve uninstantiated keys (keys
- * without data attached to them).
- *
- * Return: a bpf_key pointer with a valid key pointer if the key is found, a
- * NULL pointer otherwise.
- */
-__bpf_kfunc struct bpf_key *bpf_lookup_user_key(s32 serial, u64 flags)
-{
- key_ref_t key_ref;
- struct bpf_key *bkey;
-
- if (flags & ~KEY_LOOKUP_ALL)
- return NULL;
-
- /*
- * Permission check is deferred until the key is used, as the
- * intent of the caller is unknown here.
- */
- key_ref = lookup_user_key(serial, flags, KEY_DEFER_PERM_CHECK);
- if (IS_ERR(key_ref))
- return NULL;
-
- bkey = kmalloc(sizeof(*bkey), GFP_KERNEL);
- if (!bkey) {
- key_put(key_ref_to_ptr(key_ref));
- return NULL;
- }
-
- bkey->key = key_ref_to_ptr(key_ref);
- bkey->has_ref = true;
-
- return bkey;
-}
-
-/**
- * bpf_lookup_system_key - lookup a key by a system-defined ID
- * @id: key ID
- *
- * Obtain a bpf_key structure with a key pointer set to the passed key ID.
- * The key pointer is marked as invalid, to prevent bpf_key_put() from
- * attempting to decrement the key reference count on that pointer. The key
- * pointer set in such way is currently understood only by
- * verify_pkcs7_signature().
- *
- * Set *id* to one of the values defined in include/linux/verification.h:
- * 0 for the primary keyring (immutable keyring of system keys);
- * VERIFY_USE_SECONDARY_KEYRING for both the primary and secondary keyring
- * (where keys can be added only if they are vouched for by existing keys
- * in those keyrings); VERIFY_USE_PLATFORM_KEYRING for the platform
- * keyring (primarily used by the integrity subsystem to verify a kexec'ed
- * kerned image and, possibly, the initramfs signature).
- *
- * Return: a bpf_key pointer with an invalid key pointer set from the
- * pre-determined ID on success, a NULL pointer otherwise
- */
-__bpf_kfunc struct bpf_key *bpf_lookup_system_key(u64 id)
-{
- struct bpf_key *bkey;
-
- if (system_keyring_id_check(id) < 0)
- return NULL;
-
- bkey = kmalloc(sizeof(*bkey), GFP_ATOMIC);
- if (!bkey)
- return NULL;
-
- bkey->key = (struct key *)(unsigned long)id;
- bkey->has_ref = false;
-
- return bkey;
-}
-
-/**
- * bpf_key_put - decrement key reference count if key is valid and free bpf_key
- * @bkey: bpf_key structure
- *
- * Decrement the reference count of the key inside *bkey*, if the pointer
- * is valid, and free *bkey*.
- */
-__bpf_kfunc void bpf_key_put(struct bpf_key *bkey)
-{
- if (bkey->has_ref)
- key_put(bkey->key);
-
- kfree(bkey);
-}
-
-#ifdef CONFIG_SYSTEM_DATA_VERIFICATION
-/**
- * bpf_verify_pkcs7_signature - verify a PKCS#7 signature
- * @data_p: data to verify
- * @sig_p: signature of the data
- * @trusted_keyring: keyring with keys trusted for signature verification
- *
- * Verify the PKCS#7 signature *sig_ptr* against the supplied *data_ptr*
- * with keys in a keyring referenced by *trusted_keyring*.
- *
- * Return: 0 on success, a negative value on error.
- */
-__bpf_kfunc int bpf_verify_pkcs7_signature(struct bpf_dynptr *data_p,
- struct bpf_dynptr *sig_p,
- struct bpf_key *trusted_keyring)
-{
- struct bpf_dynptr_kern *data_ptr = (struct bpf_dynptr_kern *)data_p;
- struct bpf_dynptr_kern *sig_ptr = (struct bpf_dynptr_kern *)sig_p;
- const void *data, *sig;
- u32 data_len, sig_len;
- int ret;
-
- if (trusted_keyring->has_ref) {
- /*
- * Do the permission check deferred in bpf_lookup_user_key().
- * See bpf_lookup_user_key() for more details.
- *
- * A call to key_task_permission() here would be redundant, as
- * it is already done by keyring_search() called by
- * find_asymmetric_key().
- */
- ret = key_validate(trusted_keyring->key);
- if (ret < 0)
- return ret;
- }
-
- data_len = __bpf_dynptr_size(data_ptr);
- data = __bpf_dynptr_data(data_ptr, data_len);
- sig_len = __bpf_dynptr_size(sig_ptr);
- sig = __bpf_dynptr_data(sig_ptr, sig_len);
-
- return verify_pkcs7_signature(data, data_len, sig, sig_len,
- trusted_keyring->key,
- VERIFYING_UNSPECIFIED_SIGNATURE, NULL,
- NULL);
-}
-#endif /* CONFIG_SYSTEM_DATA_VERIFICATION */
-
-__bpf_kfunc_end_defs();
-
-BTF_KFUNCS_START(key_sig_kfunc_set)
-BTF_ID_FLAGS(func, bpf_lookup_user_key, KF_ACQUIRE | KF_RET_NULL | KF_SLEEPABLE)
-BTF_ID_FLAGS(func, bpf_lookup_system_key, KF_ACQUIRE | KF_RET_NULL)
-BTF_ID_FLAGS(func, bpf_key_put, KF_RELEASE)
-#ifdef CONFIG_SYSTEM_DATA_VERIFICATION
-BTF_ID_FLAGS(func, bpf_verify_pkcs7_signature, KF_SLEEPABLE)
-#endif
-BTF_KFUNCS_END(key_sig_kfunc_set)
-
-static const struct btf_kfunc_id_set bpf_key_sig_kfunc_set = {
- .owner = THIS_MODULE,
- .set = &key_sig_kfunc_set,
-};
-
-static int __init bpf_key_sig_kfuncs_init(void)
-{
- return register_btf_kfunc_id_set(BPF_PROG_TYPE_TRACING,
- &bpf_key_sig_kfunc_set);
-}
-
-late_initcall(bpf_key_sig_kfuncs_init);
-#endif /* CONFIG_KEYS */
-
static const struct bpf_func_proto *
bpf_tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
{
--
2.43.0
^ permalink raw reply related
* [PATCH v4 06/12] bpf: Return hashes of maps in BPF_OBJ_GET_INFO_BY_FD
From: KP Singh @ 2025-09-14 21:51 UTC (permalink / raw)
To: bpf, linux-security-module
Cc: bboscaccy, paul, kys, ast, daniel, andrii, KP Singh
In-Reply-To: <20250914215141.15144-1-kpsingh@kernel.org>
Currently only array maps are supported, but the implementation can be
extended for other maps and objects. The hash is memoized only for
exclusive and frozen maps as their content is stable until the exclusive
program modifies the map.
This is required for BPF signing, enabling a trusted loader program to
verify a map's integrity. The loader retrieves
the map's runtime hash from the kernel and compares it against an
expected hash computed at build time.
Signed-off-by: KP Singh <kpsingh@kernel.org>
---
include/linux/bpf.h | 3 +++
include/uapi/linux/bpf.h | 2 ++
kernel/bpf/arraymap.c | 13 +++++++++++
kernel/bpf/syscall.c | 23 +++++++++++++++++++
tools/include/uapi/linux/bpf.h | 2 ++
.../selftests/bpf/progs/verifier_map_ptr.c | 7 ++++--
6 files changed, 48 insertions(+), 2 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index c6a6ee1b2938..e0c2c78a5faa 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -7,6 +7,7 @@
#include <uapi/linux/bpf.h>
#include <uapi/linux/filter.h>
+#include <crypto/sha2.h>
#include <linux/workqueue.h>
#include <linux/file.h>
#include <linux/percpu.h>
@@ -110,6 +111,7 @@ struct bpf_map_ops {
long (*map_pop_elem)(struct bpf_map *map, void *value);
long (*map_peek_elem)(struct bpf_map *map, void *value);
void *(*map_lookup_percpu_elem)(struct bpf_map *map, void *key, u32 cpu);
+ int (*map_get_hash)(struct bpf_map *map, u32 hash_buf_size, void *hash_buf);
/* funcs called by prog_array and perf_event_array map */
void *(*map_fd_get_ptr)(struct bpf_map *map, struct file *map_file,
@@ -289,6 +291,7 @@ struct bpf_map_owner {
};
struct bpf_map {
+ u8 sha[SHA256_DIGEST_SIZE];
const struct bpf_map_ops *ops;
struct bpf_map *inner_map_meta;
#ifdef CONFIG_SECURITY
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 57687b2e1c47..0987b52d5648 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -6672,6 +6672,8 @@ struct bpf_map_info {
__u32 btf_value_type_id;
__u32 btf_vmlinux_id;
__u64 map_extra;
+ __aligned_u64 hash;
+ __u32 hash_size;
} __attribute__((aligned(8)));
struct bpf_btf_info {
diff --git a/kernel/bpf/arraymap.c b/kernel/bpf/arraymap.c
index 3d080916faf9..26d5dda989bc 100644
--- a/kernel/bpf/arraymap.c
+++ b/kernel/bpf/arraymap.c
@@ -12,6 +12,7 @@
#include <uapi/linux/btf.h>
#include <linux/rcupdate_trace.h>
#include <linux/btf_ids.h>
+#include <crypto/sha2.h>
#include "map_in_map.h"
@@ -174,6 +175,17 @@ static void *array_map_lookup_elem(struct bpf_map *map, void *key)
return array->value + (u64)array->elem_size * (index & array->index_mask);
}
+static int array_map_get_hash(struct bpf_map *map, u32 hash_buf_size,
+ void *hash_buf)
+{
+ struct bpf_array *array = container_of(map, struct bpf_array, map);
+
+ sha256(array->value, (u64)array->elem_size * array->map.max_entries,
+ hash_buf);
+ memcpy(array->map.sha, hash_buf, sizeof(array->map.sha));
+ return 0;
+}
+
static int array_map_direct_value_addr(const struct bpf_map *map, u64 *imm,
u32 off)
{
@@ -800,6 +812,7 @@ const struct bpf_map_ops array_map_ops = {
.map_mem_usage = array_map_mem_usage,
.map_btf_id = &array_map_btf_ids[0],
.iter_seq_info = &iter_seq_info,
+ .map_get_hash = &array_map_get_hash,
};
const struct bpf_map_ops percpu_array_map_ops = {
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index c8ef91acfe98..cf7173b1bb83 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: GPL-2.0-only
/* Copyright (c) 2011-2014 PLUMgrid, http://plumgrid.com
*/
+#include <crypto/sha2.h>
#include <linux/bpf.h>
#include <linux/bpf-cgroup.h>
#include <linux/bpf_trace.h>
@@ -5184,6 +5185,9 @@ static int bpf_map_get_info_by_fd(struct file *file,
info_len = min_t(u32, sizeof(info), info_len);
memset(&info, 0, sizeof(info));
+ if (copy_from_user(&info, uinfo, info_len))
+ return -EFAULT;
+
info.type = map->map_type;
info.id = map->id;
info.key_size = map->key_size;
@@ -5208,6 +5212,25 @@ static int bpf_map_get_info_by_fd(struct file *file,
return err;
}
+ if (info.hash) {
+ char __user *uhash = u64_to_user_ptr(info.hash);
+
+ if (!map->ops->map_get_hash)
+ return -EINVAL;
+
+ if (info.hash_size != SHA256_DIGEST_SIZE)
+ return -EINVAL;
+
+ err = map->ops->map_get_hash(map, SHA256_DIGEST_SIZE, map->sha);
+ if (err != 0)
+ return err;
+
+ if (copy_to_user(uhash, map->sha, SHA256_DIGEST_SIZE) != 0)
+ return -EFAULT;
+ } else if (info.hash_size) {
+ return -EINVAL;
+ }
+
if (copy_to_user(uinfo, &info, info_len) ||
put_user(info_len, &uattr->info.info_len))
return -EFAULT;
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 57687b2e1c47..0987b52d5648 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -6672,6 +6672,8 @@ struct bpf_map_info {
__u32 btf_value_type_id;
__u32 btf_vmlinux_id;
__u64 map_extra;
+ __aligned_u64 hash;
+ __u32 hash_size;
} __attribute__((aligned(8)));
struct bpf_btf_info {
diff --git a/tools/testing/selftests/bpf/progs/verifier_map_ptr.c b/tools/testing/selftests/bpf/progs/verifier_map_ptr.c
index 11a079145966..e2767d27d8aa 100644
--- a/tools/testing/selftests/bpf/progs/verifier_map_ptr.c
+++ b/tools/testing/selftests/bpf/progs/verifier_map_ptr.c
@@ -70,10 +70,13 @@ __naked void bpf_map_ptr_write_rejected(void)
: __clobber_all);
}
+/* The first element of struct bpf_map is a SHA256 hash of 32 bytes, accessing
+ * into this array is valid. The opts field is now at offset 33.
+ */
SEC("socket")
__description("bpf_map_ptr: read non-existent field rejected")
__failure
-__msg("cannot access ptr member ops with moff 0 in struct bpf_map with off 1 size 4")
+__msg("cannot access ptr member ops with moff 32 in struct bpf_map with off 33 size 4")
__failure_unpriv
__msg_unpriv("access is allowed only to CAP_PERFMON and CAP_SYS_ADMIN")
__flag(BPF_F_ANY_ALIGNMENT)
@@ -82,7 +85,7 @@ __naked void read_non_existent_field_rejected(void)
asm volatile (" \
r6 = 0; \
r1 = %[map_array_48b] ll; \
- r6 = *(u32*)(r1 + 1); \
+ r6 = *(u32*)(r1 + 33); \
r0 = 1; \
exit; \
" :
--
2.43.0
^ permalink raw reply related
* [PATCH v4 05/12] selftests/bpf: Add tests for exclusive maps
From: KP Singh @ 2025-09-14 21:51 UTC (permalink / raw)
To: bpf, linux-security-module
Cc: bboscaccy, paul, kys, ast, daniel, andrii, KP Singh
In-Reply-To: <20250914215141.15144-1-kpsingh@kernel.org>
Check if access is denied to another program for an exclusive map
Signed-off-by: KP Singh <kpsingh@kernel.org>
---
.../selftests/bpf/prog_tests/map_excl.c | 54 +++++++++++++++++++
tools/testing/selftests/bpf/progs/map_excl.c | 34 ++++++++++++
2 files changed, 88 insertions(+)
create mode 100644 tools/testing/selftests/bpf/prog_tests/map_excl.c
create mode 100644 tools/testing/selftests/bpf/progs/map_excl.c
diff --git a/tools/testing/selftests/bpf/prog_tests/map_excl.c b/tools/testing/selftests/bpf/prog_tests/map_excl.c
new file mode 100644
index 000000000000..6bdc6d6de0da
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/map_excl.c
@@ -0,0 +1,54 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (C) 2025 Google LLC. */
+#define _GNU_SOURCE
+#include <unistd.h>
+#include <sys/syscall.h>
+#include <test_progs.h>
+#include <bpf/btf.h>
+
+#include "map_excl.skel.h"
+
+static void test_map_excl_allowed(void)
+{
+ struct map_excl *skel = map_excl__open();
+ int err;
+
+ err = bpf_map__set_exclusive_program(skel->maps.excl_map, skel->progs.should_have_access);
+ if (!ASSERT_OK(err, "bpf_map__set_exclusive_program"))
+ goto out;
+
+ bpf_program__set_autoload(skel->progs.should_have_access, true);
+ bpf_program__set_autoload(skel->progs.should_not_have_access, false);
+
+ err = map_excl__load(skel);
+ ASSERT_OK(err, "map_excl__load");
+out:
+ map_excl__destroy(skel);
+}
+
+static void test_map_excl_denied(void)
+{
+ struct map_excl *skel = map_excl__open();
+ int err;
+
+ err = bpf_map__set_exclusive_program(skel->maps.excl_map, skel->progs.should_have_access);
+ if (!ASSERT_OK(err, "bpf_map__make_exclusive"))
+ goto out;
+
+ bpf_program__set_autoload(skel->progs.should_have_access, false);
+ bpf_program__set_autoload(skel->progs.should_not_have_access, true);
+
+ err = map_excl__load(skel);
+ ASSERT_EQ(err, -EACCES, "exclusive map access not denied\n");
+out:
+ map_excl__destroy(skel);
+
+}
+
+void test_map_excl(void)
+{
+ if (test__start_subtest("map_excl_allowed"))
+ test_map_excl_allowed();
+ if (test__start_subtest("map_excl_denied"))
+ test_map_excl_denied();
+}
diff --git a/tools/testing/selftests/bpf/progs/map_excl.c b/tools/testing/selftests/bpf/progs/map_excl.c
new file mode 100644
index 000000000000..d461684728e4
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/map_excl.c
@@ -0,0 +1,34 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (C) 2025 Google LLC. */
+#include <linux/bpf.h>
+#include <time.h>
+#include <bpf/bpf_helpers.h>
+
+#include "bpf_misc.h"
+
+struct {
+ __uint(type, BPF_MAP_TYPE_ARRAY);
+ __type(key, __u32);
+ __type(value, __u32);
+ __uint(max_entries, 1);
+} excl_map SEC(".maps");
+
+char _license[] SEC("license") = "GPL";
+
+SEC("?fentry.s/" SYS_PREFIX "sys_getpgid")
+int should_have_access(void *ctx)
+{
+ int key = 0, value = 0xdeadbeef;
+
+ bpf_map_update_elem(&excl_map, &key, &value, 0);
+ return 0;
+}
+
+SEC("?fentry.s/" SYS_PREFIX "sys_getpgid")
+int should_not_have_access(void *ctx)
+{
+ int key = 0, value = 0xdeadbeef;
+
+ bpf_map_update_elem(&excl_map, &key, &value, 0);
+ return 0;
+}
--
2.43.0
^ permalink raw reply related
* [PATCH v4 04/12] libbpf: Support exclusive map creation
From: KP Singh @ 2025-09-14 21:51 UTC (permalink / raw)
To: bpf, linux-security-module
Cc: bboscaccy, paul, kys, ast, daniel, andrii, KP Singh
In-Reply-To: <20250914215141.15144-1-kpsingh@kernel.org>
Implement setters and getters that allow map to be registered as
exclusive to the specified program. The registration should be done
before the exclusive program is loaded.
Signed-off-by: KP Singh <kpsingh@kernel.org>
---
tools/lib/bpf/bpf.c | 4 ++-
tools/lib/bpf/bpf.h | 5 ++-
tools/lib/bpf/libbpf.c | 69 ++++++++++++++++++++++++++++++++++++++++
tools/lib/bpf/libbpf.h | 22 +++++++++++++
tools/lib/bpf/libbpf.map | 3 ++
5 files changed, 101 insertions(+), 2 deletions(-)
diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
index ab40dbf9f020..19ad7bcf0c2f 100644
--- a/tools/lib/bpf/bpf.c
+++ b/tools/lib/bpf/bpf.c
@@ -172,7 +172,7 @@ int bpf_map_create(enum bpf_map_type map_type,
__u32 max_entries,
const struct bpf_map_create_opts *opts)
{
- const size_t attr_sz = offsetofend(union bpf_attr, map_token_fd);
+ const size_t attr_sz = offsetofend(union bpf_attr, excl_prog_hash_size);
union bpf_attr attr;
int fd;
@@ -203,6 +203,8 @@ int bpf_map_create(enum bpf_map_type map_type,
attr.map_ifindex = OPTS_GET(opts, map_ifindex, 0);
attr.map_token_fd = OPTS_GET(opts, token_fd, 0);
+ attr.excl_prog_hash = ptr_to_u64(OPTS_GET(opts, excl_prog_hash, NULL));
+ attr.excl_prog_hash_size = OPTS_GET(opts, excl_prog_hash_size, 0);
fd = sys_bpf_fd(BPF_MAP_CREATE, &attr, attr_sz);
return libbpf_err_errno(fd);
diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
index 7252150e7ad3..e983a3e40d61 100644
--- a/tools/lib/bpf/bpf.h
+++ b/tools/lib/bpf/bpf.h
@@ -54,9 +54,12 @@ struct bpf_map_create_opts {
__s32 value_type_btf_obj_fd;
__u32 token_fd;
+
+ const void *excl_prog_hash;
+ __u32 excl_prog_hash_size;
size_t :0;
};
-#define bpf_map_create_opts__last_field token_fd
+#define bpf_map_create_opts__last_field excl_prog_hash_size
LIBBPF_API int bpf_map_create(enum bpf_map_type map_type,
const char *map_name,
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index a39640bd5448..5161c2b39875 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -499,6 +499,7 @@ struct bpf_program {
__u32 line_info_rec_size;
__u32 line_info_cnt;
__u32 prog_flags;
+ __u8 hash[SHA256_DIGEST_LENGTH];
};
struct bpf_struct_ops {
@@ -578,6 +579,7 @@ struct bpf_map {
bool autocreate;
bool autoattach;
__u64 map_extra;
+ struct bpf_program *excl_prog;
};
enum extern_type {
@@ -4488,6 +4490,44 @@ bpf_object__section_to_libbpf_map_type(const struct bpf_object *obj, int shndx)
}
}
+static int bpf_prog_compute_hash(struct bpf_program *prog)
+{
+ struct bpf_insn *purged;
+ int i, err;
+
+ purged = calloc(prog->insns_cnt, BPF_INSN_SZ);
+ if (!purged)
+ return -ENOMEM;
+
+ /* If relocations have been done, the map_fd needs to be
+ * discarded for the digest calculation.
+ */
+ for (i = 0; i < prog->insns_cnt; i++) {
+ purged[i] = prog->insns[i];
+ if (purged[i].code == (BPF_LD | BPF_IMM | BPF_DW) &&
+ (purged[i].src_reg == BPF_PSEUDO_MAP_FD ||
+ purged[i].src_reg == BPF_PSEUDO_MAP_VALUE)) {
+ purged[i].imm = 0;
+ i++;
+ if (i >= prog->insns_cnt ||
+ prog->insns[i].code != 0 ||
+ prog->insns[i].dst_reg != 0 ||
+ prog->insns[i].src_reg != 0 ||
+ prog->insns[i].off != 0) {
+ err = -EINVAL;
+ goto out;
+ }
+ purged[i] = prog->insns[i];
+ purged[i].imm = 0;
+ }
+ }
+ err = libbpf_sha256(purged, prog->insns_cnt * sizeof(struct bpf_insn),
+ prog->hash, SHA256_DIGEST_LENGTH);
+out:
+ free(purged);
+ return err;
+}
+
static int bpf_program__record_reloc(struct bpf_program *prog,
struct reloc_desc *reloc_desc,
__u32 insn_idx, const char *sym_name,
@@ -5237,6 +5277,14 @@ static int bpf_object__create_map(struct bpf_object *obj, struct bpf_map *map, b
create_attr.token_fd = obj->token_fd;
if (obj->token_fd)
create_attr.map_flags |= BPF_F_TOKEN_FD;
+ if (map->excl_prog) {
+ err = bpf_prog_compute_hash(map->excl_prog);
+ if (err)
+ return err;
+
+ create_attr.excl_prog_hash = map->excl_prog->hash;
+ create_attr.excl_prog_hash_size = SHA256_DIGEST_LENGTH;
+ }
if (bpf_map__is_struct_ops(map)) {
create_attr.btf_vmlinux_value_type_id = map->btf_vmlinux_value_type_id;
@@ -10527,6 +10575,27 @@ int bpf_map__set_inner_map_fd(struct bpf_map *map, int fd)
return 0;
}
+int bpf_map__set_exclusive_program(struct bpf_map *map, struct bpf_program *prog)
+{
+ if (map_is_created(map)) {
+ pr_warn("exclusive programs must be set before map creation\n");
+ return libbpf_err(-EINVAL);
+ }
+
+ if (map->obj != prog->obj) {
+ pr_warn("excl_prog and map must be from the same bpf object\n");
+ return libbpf_err(-EINVAL);
+ }
+
+ map->excl_prog = prog;
+ return 0;
+}
+
+struct bpf_program *bpf_map__exclusive_program(struct bpf_map *map)
+{
+ return map->excl_prog;
+}
+
static struct bpf_map *
__bpf_map__iter(const struct bpf_map *m, const struct bpf_object *obj, int i)
{
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 2e91148d9b44..e978bc093c39 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -1291,6 +1291,28 @@ LIBBPF_API int bpf_map__lookup_and_delete_elem(const struct bpf_map *map,
*/
LIBBPF_API int bpf_map__get_next_key(const struct bpf_map *map,
const void *cur_key, void *next_key, size_t key_sz);
+/**
+ * @brief **bpf_map__set_exclusive_program()** sets a map to be exclusive to the
+ * specified program. This must be called *before* the map is created.
+ *
+ * @param map BPF map to make exclusive.
+ * @param prog BPF program to be the exclusive user of the map. Must belong
+ * to the same bpf_object as the map.
+ * @return 0 on success; a negative error code otherwise.
+ *
+ * This function must be called after the BPF object is opened but before
+ * it is loaded. Once the object is loaded, only the specified program
+ * will be able to access the map's contents.
+ */
+LIBBPF_API int bpf_map__set_exclusive_program(struct bpf_map *map, struct bpf_program *prog);
+
+/**
+ * @brief **bpf_map__exclusive_program()** returns the exclusive program
+ * that is registered with the map (if any).
+ * @param map BPF map to which the exclusive program is registered.
+ * @return the registered exclusive program.
+ */
+LIBBPF_API struct bpf_program *bpf_map__exclusive_program(struct bpf_map *map);
struct bpf_xdp_set_link_opts {
size_t sz;
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index d7bd463e7017..8ed8749907d4 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -448,4 +448,7 @@ LIBBPF_1.6.0 {
} LIBBPF_1.5.0;
LIBBPF_1.7.0 {
+ global:
+ bpf_map__set_exclusive_program;
+ bpf_map__exclusive_program;
} LIBBPF_1.6.0;
--
2.43.0
^ permalink raw reply related
* [PATCH v4 03/12] libbpf: Implement SHA256 internal helper
From: KP Singh @ 2025-09-14 21:51 UTC (permalink / raw)
To: bpf, linux-security-module
Cc: bboscaccy, paul, kys, ast, daniel, andrii, KP Singh
In-Reply-To: <20250914215141.15144-1-kpsingh@kernel.org>
Use AF_ALG sockets to not have libbpf depend on OpenSSL. The helper is
used for the loader generation code to embed the metadata hash in the
loader program and also by the bpf_map__make_exclusive API to calculate
the hash of the program the map is exclusive to.
Acked-by: Andrii Nakryiko <andrii@kernel.org>
Signed-off-by: KP Singh <kpsingh@kernel.org>
---
tools/lib/bpf/libbpf.c | 59 +++++++++++++++++++++++++++++++++
tools/lib/bpf/libbpf_internal.h | 4 +++
2 files changed, 63 insertions(+)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index fe4fc5438678..a39640bd5448 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -43,6 +43,9 @@
#include <sys/vfs.h>
#include <sys/utsname.h>
#include <sys/resource.h>
+#include <sys/socket.h>
+#include <linux/if_alg.h>
+#include <linux/socket.h>
#include <libelf.h>
#include <gelf.h>
#include <zlib.h>
@@ -14217,3 +14220,59 @@ void bpf_object__destroy_skeleton(struct bpf_object_skeleton *s)
free(s->progs);
free(s);
}
+
+int libbpf_sha256(const void *data, size_t data_sz, void *sha_out, size_t sha_out_sz)
+{
+ struct sockaddr_alg sa = {
+ .salg_family = AF_ALG,
+ .salg_type = "hash",
+ .salg_name = "sha256"
+ };
+ int sock_fd = -1;
+ int op_fd = -1;
+ int err = 0;
+
+ if (sha_out_sz != SHA256_DIGEST_LENGTH) {
+ pr_warn("sha_out_sz should be exactly 32 bytes for a SHA256 digest");
+ return -EINVAL;
+ }
+
+ sock_fd = socket(AF_ALG, SOCK_SEQPACKET, 0);
+ if (sock_fd < 0) {
+ err = -errno;
+ pr_warn("failed to create AF_ALG socket for SHA256: %s\n", errstr(err));
+ return err;
+ }
+
+ if (bind(sock_fd, (struct sockaddr *)&sa, sizeof(sa)) < 0) {
+ err = -errno;
+ pr_warn("failed to bind to AF_ALG socket for SHA256: %s\n", errstr(err));
+ goto out;
+ }
+
+ op_fd = accept(sock_fd, NULL, 0);
+ if (op_fd < 0) {
+ err = -errno;
+ pr_warn("failed to accept from AF_ALG socket for SHA256: %s\n", errstr(err));
+ goto out;
+ }
+
+ if (write(op_fd, data, data_sz) != data_sz) {
+ err = -errno;
+ pr_warn("failed to write data to AF_ALG socket for SHA256: %s\n", errstr(err));
+ goto out;
+ }
+
+ if (read(op_fd, sha_out, SHA256_DIGEST_LENGTH) != SHA256_DIGEST_LENGTH) {
+ err = -errno;
+ pr_warn("failed to read SHA256 from AF_ALG socket: %s\n", errstr(err));
+ goto out;
+ }
+
+out:
+ if (op_fd >= 0)
+ close(op_fd);
+ if (sock_fd >= 0)
+ close(sock_fd);
+ return err;
+}
diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h
index 477a3b3389a0..8a055de0d324 100644
--- a/tools/lib/bpf/libbpf_internal.h
+++ b/tools/lib/bpf/libbpf_internal.h
@@ -736,4 +736,8 @@ int elf_resolve_pattern_offsets(const char *binary_path, const char *pattern,
int probe_fd(int fd);
+#define SHA256_DIGEST_LENGTH 32
+#define SHA256_DWORD_SIZE SHA256_DIGEST_LENGTH / sizeof(__u64)
+
+int libbpf_sha256(const void *data, size_t data_sz, void *sha_out, size_t sha_out_sz);
#endif /* __LIBBPF_LIBBPF_INTERNAL_H */
--
2.43.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox