Skip to content

Commit 100f59d

Browse files
committed
LSM: Remove double path_rename hook calls for RENAME_EXCHANGE
In order to be able to identify a file exchange with renameat2(2) and RENAME_EXCHANGE, which will be useful for Landlock [1], propagate the rename flags to LSMs. This may also improve performance because of the switch from two set of LSM hook calls to only one, and because LSMs using this hook may optimize the double check (e.g. only one lock, reduce the number of path walks). AppArmor, Landlock and Tomoyo are updated to leverage this change. This should not change the current behavior (same check order), except (different level of) speed boosts. [1] https://lore.kernel.org/r/20220221212522.320243-1-mic@digikod.net Cc: James Morris <jmorris@namei.org> Cc: Kentaro Takeda <takedakn@nttdata.co.jp> Cc: Serge E. Hallyn <serge@hallyn.com> Acked-by: John Johansen <john.johansen@canonical.com> Acked-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> Reviewed-by: Paul Moore <paul@paul-moore.com> Signed-off-by: Mickaël Salaün <mic@digikod.net> Link: https://lore.kernel.org/r/20220506161102.525323-7-mic@digikod.net
1 parent 9da82b2 commit 100f59d

6 files changed

Lines changed: 48 additions & 16 deletions

File tree

include/linux/lsm_hook_defs.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ LSM_HOOK(int, 0, path_link, struct dentry *old_dentry,
100100
const struct path *new_dir, struct dentry *new_dentry)
101101
LSM_HOOK(int, 0, path_rename, const struct path *old_dir,
102102
struct dentry *old_dentry, const struct path *new_dir,
103-
struct dentry *new_dentry)
103+
struct dentry *new_dentry, unsigned int flags)
104104
LSM_HOOK(int, 0, path_chmod, const struct path *path, umode_t mode)
105105
LSM_HOOK(int, 0, path_chown, const struct path *path, kuid_t uid, kgid_t gid)
106106
LSM_HOOK(int, 0, path_chroot, const struct path *path)

include/linux/lsm_hooks.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -358,6 +358,7 @@
358358
* @old_dentry contains the dentry structure of the old link.
359359
* @new_dir contains the path structure for parent of the new link.
360360
* @new_dentry contains the dentry structure of the new link.
361+
* @flags may contain rename options such as RENAME_EXCHANGE.
361362
* Return 0 if permission is granted.
362363
* @path_chmod:
363364
* Check for permission to change a mode of the file @path. The new

security/apparmor/lsm.c

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -354,13 +354,16 @@ static int apparmor_path_link(struct dentry *old_dentry, const struct path *new_
354354
}
355355

356356
static int apparmor_path_rename(const struct path *old_dir, struct dentry *old_dentry,
357-
const struct path *new_dir, struct dentry *new_dentry)
357+
const struct path *new_dir, struct dentry *new_dentry,
358+
const unsigned int flags)
358359
{
359360
struct aa_label *label;
360361
int error = 0;
361362

362363
if (!path_mediated_fs(old_dentry))
363364
return 0;
365+
if ((flags & RENAME_EXCHANGE) && !path_mediated_fs(new_dentry))
366+
return 0;
364367

365368
label = begin_current_label_crit_section();
366369
if (!unconfined(label)) {
@@ -374,10 +377,27 @@ static int apparmor_path_rename(const struct path *old_dir, struct dentry *old_d
374377
d_backing_inode(old_dentry)->i_mode
375378
};
376379

377-
error = aa_path_perm(OP_RENAME_SRC, label, &old_path, 0,
378-
MAY_READ | AA_MAY_GETATTR | MAY_WRITE |
379-
AA_MAY_SETATTR | AA_MAY_DELETE,
380-
&cond);
380+
if (flags & RENAME_EXCHANGE) {
381+
struct path_cond cond_exchange = {
382+
i_uid_into_mnt(mnt_userns, d_backing_inode(new_dentry)),
383+
d_backing_inode(new_dentry)->i_mode
384+
};
385+
386+
error = aa_path_perm(OP_RENAME_SRC, label, &new_path, 0,
387+
MAY_READ | AA_MAY_GETATTR | MAY_WRITE |
388+
AA_MAY_SETATTR | AA_MAY_DELETE,
389+
&cond_exchange);
390+
if (!error)
391+
error = aa_path_perm(OP_RENAME_DEST, label, &old_path,
392+
0, MAY_WRITE | AA_MAY_SETATTR |
393+
AA_MAY_CREATE, &cond_exchange);
394+
}
395+
396+
if (!error)
397+
error = aa_path_perm(OP_RENAME_SRC, label, &old_path, 0,
398+
MAY_READ | AA_MAY_GETATTR | MAY_WRITE |
399+
AA_MAY_SETATTR | AA_MAY_DELETE,
400+
&cond);
381401
if (!error)
382402
error = aa_path_perm(OP_RENAME_DEST, label, &new_path,
383403
0, MAY_WRITE | AA_MAY_SETATTR |

security/landlock/fs.c

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -622,23 +622,32 @@ static int hook_path_link(struct dentry *const old_dentry,
622622
static int hook_path_rename(const struct path *const old_dir,
623623
struct dentry *const old_dentry,
624624
const struct path *const new_dir,
625-
struct dentry *const new_dentry)
625+
struct dentry *const new_dentry,
626+
const unsigned int flags)
626627
{
627628
const struct landlock_ruleset *const dom =
628629
landlock_get_current_domain();
630+
u32 exchange_access = 0;
629631

630632
if (!dom)
631633
return 0;
632634
/* The mount points are the same for old and new paths, cf. EXDEV. */
633635
if (old_dir->dentry != new_dir->dentry)
634636
/* Gracefully forbids reparenting. */
635637
return -EXDEV;
638+
if (flags & RENAME_EXCHANGE) {
639+
if (unlikely(d_is_negative(new_dentry)))
640+
return -ENOENT;
641+
exchange_access =
642+
get_mode_access(d_backing_inode(new_dentry)->i_mode);
643+
}
636644
if (unlikely(d_is_negative(old_dentry)))
637645
return -ENOENT;
638646
/* RENAME_EXCHANGE is handled because directories are the same. */
639647
return check_access_path(
640648
dom, old_dir,
641649
maybe_remove(old_dentry) | maybe_remove(new_dentry) |
650+
exchange_access |
642651
get_mode_access(d_backing_inode(old_dentry)->i_mode));
643652
}
644653

security/security.c

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1197,15 +1197,8 @@ int security_path_rename(const struct path *old_dir, struct dentry *old_dentry,
11971197
(d_is_positive(new_dentry) && IS_PRIVATE(d_backing_inode(new_dentry)))))
11981198
return 0;
11991199

1200-
if (flags & RENAME_EXCHANGE) {
1201-
int err = call_int_hook(path_rename, 0, new_dir, new_dentry,
1202-
old_dir, old_dentry);
1203-
if (err)
1204-
return err;
1205-
}
1206-
12071200
return call_int_hook(path_rename, 0, old_dir, old_dentry, new_dir,
1208-
new_dentry);
1201+
new_dentry, flags);
12091202
}
12101203
EXPORT_SYMBOL(security_path_rename);
12111204

security/tomoyo/tomoyo.c

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -264,17 +264,26 @@ static int tomoyo_path_link(struct dentry *old_dentry, const struct path *new_di
264264
* @old_dentry: Pointer to "struct dentry".
265265
* @new_parent: Pointer to "struct path".
266266
* @new_dentry: Pointer to "struct dentry".
267+
* @flags: Rename options.
267268
*
268269
* Returns 0 on success, negative value otherwise.
269270
*/
270271
static int tomoyo_path_rename(const struct path *old_parent,
271272
struct dentry *old_dentry,
272273
const struct path *new_parent,
273-
struct dentry *new_dentry)
274+
struct dentry *new_dentry,
275+
const unsigned int flags)
274276
{
275277
struct path path1 = { .mnt = old_parent->mnt, .dentry = old_dentry };
276278
struct path path2 = { .mnt = new_parent->mnt, .dentry = new_dentry };
277279

280+
if (flags & RENAME_EXCHANGE) {
281+
const int err = tomoyo_path2_perm(TOMOYO_TYPE_RENAME, &path2,
282+
&path1);
283+
284+
if (err)
285+
return err;
286+
}
278287
return tomoyo_path2_perm(TOMOYO_TYPE_RENAME, &path1, &path2);
279288
}
280289

0 commit comments

Comments
 (0)