github-mirror/linux
1
0
Fork 0
mirror of https://github.com/torvalds/linux.git synced 2026-09-13 05:27:32 +02:00
Code Issues Actions Packages Projects Releases Wiki Activity Graph

landlock: Fix use-after-free of the source's parent directory

Browse Source 
current_check_refer_path() reads old_dentry->d_parent without holding a
reference nor a lock on it, and then dereferences it in
collect_domain_accesses() and in the audit record.

A reference on a child does not pin its parent: __d_move() reassigns
dentry->d_parent and drops the reference the child held on its former
parent.  hook_path_rename() is not affected because the rename path
calls lock_rename() before the hook, so the source cannot be reparented
under it.  hook_path_link() has no such protection: filename_linkat()
holds a reference on the source dentry but neither locks nor references
its parent, so a concurrent rename(2) can reparent the source while
security_path_link() runs, and the former parent can then be removed and
freed while the hook walks it.

A process can trigger this after entering a Landlock domain that handles
at least one filesystem access right.  The process can then race a
linkat(2) loop against rename(2) and rmdir(2):

  BUG: KASAN: slab-use-after-free in collect_domain_accesses+0x278/0x290
  Read of size 4 at addr ffff888160bd53f4 by task llrepro2/549
   collect_domain_accesses+0x278/0x290
   current_check_refer_path+0x952/0x1120
   security_path_link+0x1be/0x320
   filename_linkat+0x342/0x6d0
   __x64_sys_linkat+0xfa/0x150
  Freed by task 562:
   kmem_cache_free+0x139/0x4c0
   i_callback+0x4b/0x80
   rcu_core+0x7dc/0x10a0

Take a reference on the dentry selected as the source parent, using
dget() for the common-mount-root case and dget_parent() otherwise.
Release it after the hierarchy walk and synchronous audit logging.

Cc: stable@vger.kernel.org
Fixes: b91c3e4ea7 ("landlock: Add support for file reparenting with LANDLOCK_ACCESS_FS_REFER")
Signed-off-by: Norbert Szetei <norbert@doyensec.com>
Reviewed-by: Günther Noack <gnoack3000@gmail.com>
Tested-by: Günther Noack <gnoack3000@gmail.com>
Link: https://patch.msgid.link/E9CDD9E6-E960-4DE2-B1AC-5667D52ABB3E@doyensec.com
[mic: Clarify the caller, reachability, and reference handling]
Signed-off-by: Mickaël Salaün <mic@digikod.net>
This commit is contained in:
Norbert Szetei 2026-08-22 14:29:00 +02:00 committed by Mickaël Salaün
parent df2908090c
commit 2c6dc79253
No known key found for this signature in database
GPG Key ID: E5E3D0E88C82F6D2
1 changed files with 12 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
security/landlock/fs.c
View File

@ -1298,11 +1298,12 @@ static int current_check_refer_path(struct dentry *const old_dentry,
/*
* old_dentry may be the root of the common mount point and
* !IS_ROOT(old_dentry) at the same time (e.g. with open_tree() and
* OPEN_TREE_CLONE). We do not need to call dget(old_parent) because
* we keep a reference to old_dentry.
* OPEN_TREE_CLONE). Pin the dentry used as old_parent in either case.
* Otherwise, dget_parent() safely fetches and pins the current parent
* against a concurrent rename(2).
*/
old_parent = (old_dentry == mnt_dir.dentry) ? old_dentry :
old_dentry->d_parent;
old_parent = (old_dentry == mnt_dir.dentry) ? dget(old_dentry) :
dget_parent(old_dentry);
/* new_dir->dentry is equal to new_dentry->d_parent */
allow_parent1 = collect_domain_accesses(subject->domain, mnt_dir.dentry,
@ -1311,8 +1312,10 @@ static int current_check_refer_path(struct dentry *const old_dentry,
allow_parent2 = collect_domain_accesses(subject->domain, mnt_dir.dentry,
new_dir->dentry,
&layer_masks_parent2);
if (allow_parent1 && allow_parent2)
if (allow_parent1 && allow_parent2) {
dput(old_parent);
return 0;
}
/*
* To be able to compare source and destination domain access rights,
@ -1324,8 +1327,10 @@ static int current_check_refer_path(struct dentry *const old_dentry,
subject->domain, &mnt_dir, access_request_parent1,
&layer_masks_parent1, &request1, old_dentry,
access_request_parent2, &layer_masks_parent2, &request2,
exchange ? new_dentry : NULL))
exchange ? new_dentry : NULL)) {
dput(old_parent);
return 0;
}
if (request1.access) {
request1.audit.u.path.dentry = old_parent;
@ -1335,6 +1340,7 @@ static int current_check_refer_path(struct dentry *const old_dentry,
request2.audit.u.path.dentry = new_dir->dentry;
landlock_log_denial(subject, &request2);
}
dput(old_parent);
/*
* This prioritizes EACCES over EXDEV for all actions, including