Message ID | 20240207205740.541084-2-georgia.garcia@canonical.com |
---|---|
State | New |
Headers | show |
Series | apparmor: Fix move_mount mediation by detecting if source is detached | expand |
Hey John, On 24/02/07 05:57PM, Georgia Garcia wrote: > From: John Johansen <john.johansen@canonical.com> We require a Buglink in all the included patches (not only the cover letter). Also, the LP bug doesn't have "Mantic" selected as the target series. I recommend addressing the above and pushing a v2 to the mailing list. > > Prevent move_mount from applying the attach_disconnected flag > to move_mount(). This prevents detached mounts from appearing > as / when applying mount mediation, which is not only incorrect > but could result in bad policy being generated. > > Basic mount rules like > allow mount, > allow mount options=(move) -> /target/, > > will allow detached mounts, allowing older policy to continue > to function. New policy gains the ability to specify `detached` as > a source option > allow mount detached -> /target/, > > In addition make sure support of move_mount is advertised as > a feature to userspace so that applications that generate policy > can respond to the addition. > > Note: this fixes mediation of move_mount when a detached mount is used, > it does not fix the broader regression of apparmor mediation of > mounts under the new mount api. > > Link: https://lore.kernel.org/all/68c166b8-5b4d-4612-8042-1dee3334385b@leemhuis.info/T/#mb35fdde37f999f08f0b02d58dc1bf4e6b65b8da2 > Fixes: 157a3537d6bc ("apparmor: Fix regression in mount mediation") > Reviewed-by: Georgia Garcia <georgia.garcia@canonical.com> > Signed-off-by: John Johansen <john.johansen@canonical.com> > (cherry picked from commit 8026e40608b4d552216d2a818ca7080a4264bb44) > Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com> > --- > security/apparmor/apparmorfs.c | 1 + > security/apparmor/mount.c | 4 ++++ > 2 files changed, 5 insertions(+) > > diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c > index ab7aa97fe7e38..cfc1741916e08 100644 > --- a/security/apparmor/apparmorfs.c > +++ b/security/apparmor/apparmorfs.c > @@ -2619,6 +2619,7 @@ static struct aa_sfs_entry aa_sfs_entry_policy[] = { > > static struct aa_sfs_entry aa_sfs_entry_mount[] = { > AA_SFS_FILE_STRING("mask", "mount umount pivot_root"), > + AA_SFS_FILE_STRING("move_mount", "detached"), > { } > }; > > diff --git a/security/apparmor/mount.c b/security/apparmor/mount.c > index fb30204c761ad..49fe8da6fea45 100644 > --- a/security/apparmor/mount.c > +++ b/security/apparmor/mount.c > @@ -499,6 +499,10 @@ int aa_move_mount(const struct cred *subj_cred, > error = -ENOMEM; > if (!to_buffer || !from_buffer) > goto out; > + > + if (!our_mnt(from_path->mnt)) > + /* moving a mount detached from the namespace */ > + from_path = NULL; > error = fn_for_each_confined(label, profile, > match_mnt(subj_cred, profile, to_path, to_buffer, > from_path, from_buffer, Thanks,
diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c index ab7aa97fe7e38..cfc1741916e08 100644 --- a/security/apparmor/apparmorfs.c +++ b/security/apparmor/apparmorfs.c @@ -2619,6 +2619,7 @@ static struct aa_sfs_entry aa_sfs_entry_policy[] = { static struct aa_sfs_entry aa_sfs_entry_mount[] = { AA_SFS_FILE_STRING("mask", "mount umount pivot_root"), + AA_SFS_FILE_STRING("move_mount", "detached"), { } }; diff --git a/security/apparmor/mount.c b/security/apparmor/mount.c index fb30204c761ad..49fe8da6fea45 100644 --- a/security/apparmor/mount.c +++ b/security/apparmor/mount.c @@ -499,6 +499,10 @@ int aa_move_mount(const struct cred *subj_cred, error = -ENOMEM; if (!to_buffer || !from_buffer) goto out; + + if (!our_mnt(from_path->mnt)) + /* moving a mount detached from the namespace */ + from_path = NULL; error = fn_for_each_confined(label, profile, match_mnt(subj_cred, profile, to_path, to_buffer, from_path, from_buffer,