[PATCH] smack: do not override the subject label in overlayfs hooks

[PATCH] smack: do not override the subject label in overlayfs hooks

[Mateusz Moscicki]

Overlayfs performs operations on the upper, lower and work directories
with the credentials of the mounter and uses the inode_copy_up() and
dentry_create_files_as() hooks only to let the LSM choose the label of
the inode it is about to create. Smack implemented both hooks by
replacing smk_task in the override credentials, so the label of the
copied file, or of the transmuting directory, became the subject of all
access checks done in the underlying filesystems on behalf of overlayfs:
the lookup and creation of the temporary inode in the workdir, the
rename into the upper directory, setting the overlay xattrs.

Unless that label happens to have access to the workdir and upper
directories, or is allowed to use CAP_MAC_OVERRIDE, these operations
fail with EACCES:

  lsm=SMACK fn=smack_inode_permission action=denied subject="somelabel"
    object="<mounter label>" requested=x ... name="work"

With onlycap configured this makes copy-up and creation in transmuting
directories fail for every label not listed there, and a file labeled
"*" can never be copied up at all, as a "*" subject is not granted
access to anything.

Keep smk_task untouched and store the label the new inode should get in
a new task_smack field, smk_create, consumed by
smack_inode_alloc_security() and smack_inode_init_security(). This is
the same split SELinux uses with create_sid. Labels assigned to new and
copied up inodes do not change; only the subject of the access checks
done on behalf of overlayfs does, and it is now the mounter, as it is
for DAC. The transmute handling added in commit 2c085f3a8f23 ("smack:
Record transmuting in smk_transmuted") is preserved: smk_transmuted is
compared against smk_create.

Outside overlayfs smk_create is never set, so there is no behaviour
change there.

Fixes: d6d80cb57be4 ("Smack: Base support for overlayfs")
Signed-off-by: Mateusz Moscicki <m.moscicki2@samsung.com>
---
The analysis, the patch and the reproducer below were drafted with the
help of Claude Code (Claude Opus 5.5 / Fable 5.1) and then reviewed,
adjusted and tested by me. Tested before and after the change on a
6.12-based kernel on Raspberry Pi 4 and on linux-next (next-20261028).

Reproducer (needs root and setfattr/getfattr). The bug only shows when
the override credentials cannot use CAP_MAC_OVERRIDE, so the script
sets onlycap to the mounter's label for its duration if it is empty.
Before the fix both checks print FAIL with EACCES and Smack reports
"subject=somelabel object=<mounter label> requested=x" on the workdir;
after the fix both print SUCCESS:

  #!/bin/sh
  # overlayfs + Smack: copy-up and create run with the label of the copied file
  # or of the transmuting directory instead of the mounter's label.
  # Run as root. onlycap must be set, otherwise CAP_MAC_OVERRIDE of the
  # mounter hides the bug; if it is empty, it is set for the duration of
  # the test and cleared afterwards.
  ME=$(cat /proc/self/attr/current)
  OC=$(cat /sys/fs/smackfs/onlycap)
  [ -z "$OC" ] && echo "$ME" > /sys/fs/smackfs/onlycap
  D=/tmp/ovltest
  rm -rf $D; mkdir -p $D/lower/dir $D/upper $D/work $D/merged
  echo x > $D/lower/file
  setfattr -n security.SMACK64 -v somelabel $D/lower/file
  setfattr -n security.SMACK64 -v somelabel $D/lower/dir
  setfattr -n security.SMACK64TRANSMUTE -v TRUE $D/lower/dir
  echo "$ME somelabel rwxat" > /sys/fs/smackfs/load2
  mount -t overlay overlay \
  	-o lowerdir=$D/lower,upperdir=$D/upper,workdir=$D/work $D/merged
  
  if echo y >> $D/merged/file; then
  	echo "copy-up of 'somelabel' file: SUCCESS"
  else
  	echo "copy-up of 'somelabel' file: FAIL"
  fi
  if touch $D/merged/dir/new; then
  	l=$(getfattr -n security.SMACK64 --only-values --absolute-names \
  		$D/merged/dir/new)
  	echo "create in transmuting dir:   SUCCESS (new file labeled '$l')"
  else
  	echo "create in transmuting dir:   FAIL"
  fi
  
  umount $D/merged; rm -rf $D
  echo "$ME somelabel -" > /sys/fs/smackfs/load2
  [ -z "$OC" ] && echo - > /sys/fs/smackfs/onlycap

 security/smack/smack.h     |  1 +
 security/smack/smack_lsm.c | 42 +++++++++++++++++++++++++++-----------
 2 files changed, 31 insertions(+), 12 deletions(-)

diff --git a/security/smack/smack.h b/security/smack/smack.h
index deb2ef31b63a4..456f282acc251 100644
--- a/security/smack/smack.h
+++ b/security/smack/smack.h
@@ -121,6 +121,7 @@ struct task_smack {
 	struct smack_known	*smk_task;	/* label for access control */
 	struct smack_known	*smk_forked;	/* label when forked */
 	struct smack_known	*smk_transmuted;/* label when transmuted */
+	struct smack_known	*smk_create;	/* label for new inodes */
 	struct list_head	smk_rules;	/* per task access rules */
 	struct mutex		smk_rules_lock;	/* lock for the rules */
 	struct list_head	smk_relabel;	/* transit allowed labels */
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index c243adb137402..7bd63005e9631 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -965,6 +965,19 @@ static int smack_bprm_creds_for_exec(struct linux_binprm *bprm)
  * Inode hooks
  */
 
+/**
+ * smk_of_create - the label new inodes get
+ * @tsp: the task security blob
+ *
+ * Overlayfs sets smk_create in smack_inode_copy_up() and
+ * smack_dentry_create_files_as(), while smk_task stays
+ * the mounter's label used for access control.
+ */
+static struct smack_known *smk_of_create(const struct task_smack *tsp)
+{
+	return tsp->smk_create ? tsp->smk_create : tsp->smk_task;
+}
+
 /**
  * smack_inode_alloc_security - allocate an inode blob
  * @inode: the inode in need of a blob
@@ -973,7 +986,7 @@ static int smack_bprm_creds_for_exec(struct linux_binprm *bprm)
  */
 static int smack_inode_alloc_security(struct inode *inode)
 {
-	struct smack_known *skp = smk_of_current();
+	struct smack_known *skp = smk_of_create(smack_cred(current_cred()));
 
 	init_inode_smack(inode, skp);
 	return 0;
@@ -1030,6 +1043,7 @@ static int smack_inode_init_security(struct inode *inode, struct inode *dir,
 				     struct xattr *xattrs, int *xattr_count)
 {
 	struct task_smack *tsp = smack_cred(current_cred());
+	struct smack_known *skp = smk_of_create(tsp);
 	struct inode_smack * const issp = smack_inode(inode);
 	struct smack_known *dsp = smk_of_inode(dir);
 	int rc = 0;
@@ -1051,12 +1065,12 @@ static int smack_inode_init_security(struct inode *inode, struct inode *dir,
 	 * If equal, transmuting already occurred in
 	 * smack_dentry_create_files_as(). No need to check again.
 	 */
-	trans_cred = (tsp->smk_task == tsp->smk_transmuted);
+	trans_cred = (skp == tsp->smk_transmuted);
 	if (!trans_cred)
-		trans_rule = smk_rule_transmutes(smk_of_task(tsp), dsp);
+		trans_rule = smk_rule_transmutes(skp, dsp);
 
 	/*
-	 * In addition to having smk_task equal to smk_transmuted,
+	 * In addition to having smk_create equal to smk_transmuted,
 	 * if the access rule allows transmutation and the directory
 	 * requests transmutation then by all means transmute.
 	 * Mark the inode as changed.
@@ -4974,11 +4988,13 @@ static int smack_inode_copy_up(struct dentry *dentry, struct cred **new)
 	tsp = smack_cred(new_creds);
 
 	/*
-	 * Get label from overlay inode and set it in create_sid
+	 * Get label from overlay inode and set it in smk_create.
+	 * Keep smk_task, overlayfs accesses the underlying
+	 * filesystems with the mounter's credentials.
 	 */
 	isp = smack_inode(d_inode(dentry));
 	skp = isp->smk_inode;
-	tsp->smk_task = skp;
+	tsp->smk_create = skp;
 	*new = new_creds;
 	return 0;
 }
@@ -5004,10 +5020,12 @@ static int smack_dentry_create_files_as(struct dentry *dentry, int mode,
 	struct inode_smack *isp;
 
 	/*
-	 * Use the process credential unless all of
-	 * the transmuting criteria are met
+	 * Label the new inode with the process label unless all of
+	 * the transmuting criteria are met. Keep smk_task, overlayfs
+	 * accesses the underlying filesystems with the mounter's
+	 * credentials.
 	 */
-	ntsp->smk_task = otsp->smk_task;
+	ntsp->smk_create = smk_of_create(otsp);
 
 	/*
 	 * the attribute of the containing directory
@@ -5020,9 +5038,9 @@ static int smack_dentry_create_files_as(struct dentry *dentry, int mode,
 		 * providing access is transmuting use the containing
 		 * directory label instead of the process label.
 		 */
-		if (smk_rule_transmutes(otsp->smk_task, isp->smk_inode)) {
-			ntsp->smk_task = isp->smk_inode;
-			ntsp->smk_transmuted = ntsp->smk_task;
+		if (smk_rule_transmutes(ntsp->smk_create, isp->smk_inode)) {
+			ntsp->smk_create = isp->smk_inode;
+			ntsp->smk_transmuted = isp->smk_inode;
 		}
 	}
 	return 0;
-- 
2.34.1

Re: [PATCH] smack: do not override the subject label in overlayfs hooks

[sashiko-bot]

> Overlayfs performs operations on the upper, lower and work directories
> with the credentials of the mounter and uses the inode_copy_up() and
> dentry_create_files_as() hooks only to let the LSM choose the label of
> the inode it is about to create. Smack implemented both hooks by
> replacing smk_task in the override credentials, so the label of the
> copied file, or of the transmuting directory, became the subject of all
> access checks done in the underlying filesystems on behalf of overlayfs:
> the lookup and creation of the temporary inode in the workdir, the
> rename into the upper directory, setting the overlay xattrs.
> 
> Unless that label happens to have access to the workdir and upper
> directories, or is allowed to use CAP_MAC_OVERRIDE, these operations
> fail with EACCES:
> 
>   lsm=SMACK fn=smack_inode_permission action=denied subject="somelabel"
> [ ... ]
> Outside overlayfs smk_create is never set, so there is no behaviour
> change there.
> 
> Fixes: d6d80cb57be4 ("Smack: Base support for overlayfs")
> Signed-off-by: Mateusz Moscicki <m.moscicki2@samsung.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261029155422.578322-1-m.moscicki2@samsung.com?part=1