Thanks for the review. Will respin with the Fixes tag added.
On Thu, Apr 23, 2026 at 1:24 AM Georgia Garcia <[email protected]>
wrote:
> Hello,
>
> On Mon, 2026-04-13 at 17:03 +0800, Dudu Lu wrote:
> > In apparmor_path_rename(), when handling RENAME_EXCHANGE, the
> > cond_exchange structure is supposed to carry the attributes of the
> > *new* dentry (since it is used to authorize moving new_dentry to the
> > old location). However, line 412 reads:
> >
> > vfsuid = i_uid_into_vfsuid(idmap, d_backing_inode(old_dentry));
> >
> > This fetches the uid of old_dentry instead of new_dentry. As a result,
> > the RENAME_EXCHANGE permission check uses the wrong file owner, which
> > can allow a rename that should be denied (if old_dentry's owner has
> > more privileges) or deny one that should be allowed.
> >
> > Note that cond_exchange.mode on the line above correctly uses
> > new_dentry. Only the uid lookup is wrong.
> >
> > Fix by changing old_dentry to new_dentry in the i_uid_into_vfsuid call.
> >
>
> This looks correct to me, but could you add a Fixes tag?
>
> Fixes: 5e26a01e56fd ("apparmor: use type safe idmapping helpers")
>
> Thank you,
> Georgia
>
> > Signed-off-by: Dudu Lu <[email protected]>
> > ---
> > security/apparmor/lsm.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
> > index c1d42fc72fdb..e8de919fbea6 100644
> > --- a/security/apparmor/lsm.c
> > +++ b/security/apparmor/lsm.c
> > @@ -409,7 +409,7 @@ static int apparmor_path_rename(const struct path
> *old_dir, struct dentry *old_d
> > struct path_cond cond_exchange = {
> > .mode =
> d_backing_inode(new_dentry)->i_mode,
> > };
> > - vfsuid = i_uid_into_vfsuid(idmap,
> d_backing_inode(old_dentry));
> > + vfsuid = i_uid_into_vfsuid(idmap,
> d_backing_inode(new_dentry));
> > cond_exchange.uid = vfsuid_into_kuid(vfsuid);
> >
> > error = aa_path_perm(OP_RENAME_SRC, current_cred(),
>
>
