* [PATCH v3] smack: preserve low-integrity labels across file copy via whitelist
@ 2026-10-28 16:15 Tang Pengke
2026-10-28 16:37 ` sashiko-bot
0 siblings, 1 reply; 2+ messages in thread
From: Tang Pengke @ 2026-10-28 16:15 UTC (permalink / raw)
To: Casey Schaufler; +Cc: linux-security-module, gaolong, Tang Pengke
Files copied from an untrusted channel (USB, network, serial) inherit
the creating process's Smack label, losing the source's label. That
breaks provenance and lets a file from an untrusted source silently
acquire a trusted label.
Record a "preserve" label when a process opens a whitelisted regular
file read-only and the read check passes. The next inode it creates
inherits that label instead of the process label, unless transmutation
applies. The record is one-shot: consumed on creation, dropped when
transmutation takes precedence, cleared on exec, and not inherited
across fork.
The record lives in a per-task LSM blob rather than in the cred. A cred
is immutable once committed and is shared by every thread of a process;
writing the record there would be a data race between threads, and one
thread could consume a label another thread had just recorded. The
copy intent belongs to a single thread, so a task blob is the right
home for it.
The preservable labels form a whitelist exposed through
/smack/preserve-whitelist. Only untrusted-channel / low-integrity
labels should be added. Recording happens only after a successful
read check, so a process cannot create a file with a label it could
not read (which would be a write-down primitive), and because the
whitelist holds only low labels the mechanism can only ever preserve a
low label, never a system trusted or sensitive one. An empty
whitelist is also the fast path in smack_file_open().
Known limitation: the kernel cannot distinguish a copy from an
unrelated create, so a task that opens a whitelisted file and then
creates an unrelated file will label the latter with the preserved low
label. Restricting the whitelist to low-integrity labels bounds the
impact.
The whole mechanism is guarded by CONFIG_SECURITY_SMACK_COPYWHITELIST,
which defaults to no, so the default Smack behavior is unchanged.
Signed-off-by: Tang Pengke <tangpengke@yhkylin.cn>
---
Changes in v3:
- Move the preserve record out of the cred into a per-task LSM blob.
A cred is immutable once committed and is shared by all threads of a
process; storing the record there raced between threads and violated
the immutability contract.
- Drop a pending record when transmutation applies. Previously the
record survived that path and leaked into the next create in a
non-transmutable directory.
- Clear the record on exec; it is not inherited across fork.
- Document the known limitation that a create unrelated to the copy
still inherits the preserved low label.
security/smack/Kconfig | 21 ++++++++
security/smack/smack.h | 47 +++++++++++++++++
security/smack/smack_access.c | 77 +++++++++++++++++++++++++++
security/smack/smack_lsm.c | 61 +++++++++++++++++++++
security/smack/smackfs.c | 99 +++++++++++++++++++++++++++++++++++
5 files changed, 305 insertions(+)
diff --git a/security/smack/Kconfig b/security/smack/Kconfig
index 5a8dfad46..134449fb6 100644
--- a/security/smack/Kconfig
+++ b/security/smack/Kconfig
@@ -53,3 +53,24 @@ config SECURITY_SMACK_APPEND_SIGNALS
to differentiate between delivering a network packet and
delivering a signal in the Smack rules.
If you are unsure how to answer this question, answer N.
+
+config SECURITY_SMACK_COPYWHITELIST
+ bool "Preserve low-integrity labels when copying files"
+ depends on SECURITY_SMACK
+ default n
+ help
+ Enable a whitelist of labels that are preserved when a file is
+ copied. When a process reads a file whose label is on the
+ whitelist and then creates a new file, the new file inherits
+ the source label instead of the process label. This is useful
+ for tracking the provenance of data copied from untrusted
+ sources such as removable media or network mounts, where the
+ filesystem cannot store a per-file label.
+
+ Only low-integrity labels should be placed on the whitelist,
+ so that a process can only ever preserve a low label and never
+ a trusted or sensitive one. The whitelist is managed through
+ the /smack/preserve-whitelist interface.
+
+ If you are unsure how to answer this question, answer N.
+
diff --git a/security/smack/smack.h b/security/smack/smack.h
index 9b9eb262f..03c83f338 100644
--- a/security/smack/smack.h
+++ b/security/smack/smack.h
@@ -73,6 +73,25 @@ struct smack_known {
struct mutex smk_rules_lock; /* lock for rules */
};
+#ifdef CONFIG_SECURITY_SMACK_COPYWHITELIST
+/*
+ * An entry in the preserve whitelist.
+ *
+ * The whitelist is the single source of truth for which labels are
+ * preservable. Its emptiness is the fast path in smack_file_open(),
+ * and membership is tested by walking the list.
+ *
+ * Only untrusted-channel / low-integrity labels should be added, so a
+ * process can only ever preserve a low label and never a system trusted
+ * label (which would be a write-down primitive and let shared-library
+ * loads pollute the record).
+ */
+struct smk_preserve_wl {
+ struct list_head list;
+ struct smack_known *skp;
+};
+#endif
+
/*
* Maximum number of bytes for the levels in a CIPSO IP option.
* Why 23? CIPSO is constrained to 30, so a 32 byte buffer is
@@ -126,6 +145,19 @@ struct task_smack {
struct list_head smk_relabel; /* transit allowed labels */
};
+#ifdef CONFIG_SECURITY_SMACK_COPYWHITELIST
+/*
+ * Per-thread state for a single copy operation. This is deliberately
+ * kept out of the cred: credentials are immutable once committed and are
+ * shared by every thread of a process, while the label to preserve
+ * belongs to one copy and must not be visible to, or raced by, the other
+ * threads.
+ */
+struct smack_task {
+ struct smack_known *smk_preserve; /* label to inherit on next create */
+};
+#endif
+
#define SMK_INODE_INSTANT 0x01 /* inode is instantiated */
#define SMK_INODE_TRANSMUTE 0x02 /* directory is transmuting */
#define SMK_INODE_CHANGED 0x04 /* smack was transmuted (unused) */
@@ -312,6 +344,10 @@ bool smack_privileged(int cap);
bool smack_privileged_cred(int cap, const struct cred *cred);
void smk_destroy_label_list(struct list_head *list);
int smack_populate_secattr(struct smack_known *skp);
+#ifdef CONFIG_SECURITY_SMACK_COPYWHITELIST
+bool smk_preserve_allowed(struct smack_known *skp);
+void smk_preserve_set(struct smack_known *skp, bool on);
+#endif
/*
* Shared data.
@@ -326,6 +362,10 @@ extern struct smack_known *smack_unconfined;
#endif
extern int smack_ptrace_rule;
extern struct lsm_blob_sizes smack_blob_sizes;
+#ifdef CONFIG_SECURITY_SMACK_COPYWHITELIST
+extern struct list_head smk_preserve_wl;
+extern struct mutex smk_preserve_wl_lock;
+#endif
extern struct smack_known smack_known_floor;
extern struct smack_known smack_known_hat;
@@ -352,6 +392,13 @@ static inline struct task_smack *smack_cred(const struct cred *cred)
return cred->security + smack_blob_sizes.lbs_cred;
}
+#ifdef CONFIG_SECURITY_SMACK_COPYWHITELIST
+static inline struct smack_task *smack_task(const struct task_struct *task)
+{
+ return task->security + smack_blob_sizes.lbs_task;
+}
+#endif
+
static inline struct smack_known **smack_file(const struct file *file)
{
return (struct smack_known **)(file->f_security +
diff --git a/security/smack/smack_access.c b/security/smack/smack_access.c
index 350b88d58..5bb75e2ec 100644
--- a/security/smack/smack_access.c
+++ b/security/smack/smack_access.c
@@ -53,6 +53,83 @@ static u32 smack_next_secid = 10;
int log_policy = SMACK_AUDIT_DENIED;
#endif /* CONFIG_AUDIT */
+#ifdef CONFIG_SECURITY_SMACK_COPYWHITELIST
+/*
+ * Preserve whitelist: only labels on this list are preserved on copy.
+ *
+ * The list is the single source of truth for "which labels are
+ * preservable". It also lets smack_file_open() take a plain
+ * list_empty() fast path.
+ *
+ * Only untrusted-channel / low-integrity labels should be added, so
+ * that a process can only ever preserve a low label -- never a system
+ * trusted or sensitive label -- which would otherwise be a write-down
+ * primitive (and would let shared-library loads pollute the record).
+ *
+ * Entries hold raw smack_known pointers. SMACK global labels are never
+ * freed at runtime (smk_destroy_label_list frees list elements, not the
+ * label objects), so these pointers stay valid for the life of the
+ * system.
+ */
+LIST_HEAD(smk_preserve_wl);
+DEFINE_MUTEX(smk_preserve_wl_lock);
+
+bool smk_preserve_allowed(struct smack_known *skp)
+{
+ struct smk_preserve_wl *e;
+
+ mutex_lock(&smk_preserve_wl_lock);
+ list_for_each_entry(e, &smk_preserve_wl, list) {
+ if (e->skp == skp) {
+ mutex_unlock(&smk_preserve_wl_lock);
+ return true;
+ }
+ }
+ mutex_unlock(&smk_preserve_wl_lock);
+ return false;
+}
+
+/*
+ * Add or remove a label from the preserve whitelist.
+ *
+ * The sysfs interface (preserve-whitelist) calls this with on=true for
+ * a plain label, and on=false for a label prefixed with '-'.
+ */
+void smk_preserve_set(struct smack_known *skp, bool on)
+{
+ struct smk_preserve_wl *e, *new = NULL;
+
+ if (on) {
+ new = kzalloc_obj(*new, GFP_KERNEL);
+ if (!new)
+ return;
+ new->skp = skp;
+ }
+
+ mutex_lock(&smk_preserve_wl_lock);
+ if (on) {
+ list_for_each_entry(e, &smk_preserve_wl, list) {
+ if (e->skp == skp) {
+ kfree(new);
+ new = NULL;
+ break;
+ }
+ }
+ if (new)
+ list_add_tail(&new->list, &smk_preserve_wl);
+ } else {
+ list_for_each_entry(e, &smk_preserve_wl, list) {
+ if (e->skp == skp) {
+ list_del(&e->list);
+ kfree(e);
+ break;
+ }
+ }
+ }
+ mutex_unlock(&smk_preserve_wl_lock);
+}
+#endif /* CONFIG_SECURITY_SMACK_COPYWHITELIST */
+
/**
* smk_access_entry - look up matching access rule
* @subject_label: a pointer to the subject's Smack label
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index ff115068c..2ec48b932 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -906,6 +906,11 @@ static int smack_bprm_creds_for_exec(struct linux_binprm *bprm)
struct superblock_smack *sbsp;
int rc;
+#ifdef CONFIG_SECURITY_SMACK_COPYWHITELIST
+ /* a new program starts with a clean copy record */
+ smack_task(current)->smk_preserve = NULL;
+#endif
+
isp = smack_inode(inode);
if (isp->smk_task == NULL || isp->smk_task == bsp->smk_task)
return 0;
@@ -1066,6 +1071,26 @@ static int smack_inode_init_security(struct inode *inode, struct inode *dir,
}
}
+#ifdef CONFIG_SECURITY_SMACK_COPYWHITELIST
+ /*
+ * If a whitelisted label was recorded on file open, inherit it so
+ * the new file keeps the source's (low-integrity) label across the
+ * copy. Transmutation takes precedence: when it applies the record
+ * is dropped, otherwise it would leak into a later create in a
+ * non-transmutable directory.
+ */
+ if (trans_cred || (trans_rule && smk_inode_transmutable(dir))) {
+ smack_task(current)->smk_preserve = NULL;
+ } else {
+ struct smack_known *skp = smack_task(current)->smk_preserve;
+
+ if (skp != NULL) {
+ issp->smk_inode = skp;
+ smack_task(current)->smk_preserve = NULL;
+ }
+ }
+#endif
+
if (rc == 0)
if (xattr_dupval(xattrs, xattr_count,
XATTR_SMACK_SUFFIX,
@@ -2068,6 +2093,39 @@ static int smack_file_open(struct file *file)
smk_ad_setfield_u_fs_path(&ad, file->f_path);
rc = smk_tskacc(tsp, smk_of_inode(inode), MAY_READ, &ad);
rc = smk_bu_credfile(file->f_cred, file, MAY_READ, rc);
+ if (rc)
+ return rc;
+
+#ifdef CONFIG_SECURITY_SMACK_COPYWHITELIST
+ /*
+ * Record a whitelisted label after a successful read check so the
+ * next file create (smack_inode_init_security) can inherit it.
+ *
+ * The record must happen only after the read check passes;
+ * otherwise a process without read access to the source could
+ * create a file with that label without ever reading it, which
+ * would be a write-down primitive.
+ *
+ * The whitelist must only hold untrusted-channel / low-integrity
+ * labels (peripheral, network, serial, and other external
+ * sources), never system trusted labels (system binaries, shared
+ * libraries, etc.) -- otherwise shared-library loads would pollute
+ * the record. Because of this constraint no library path
+ * filtering is needed here: the whitelist check already excludes
+ * every non-whitelisted file, including shared libraries.
+ *
+ * The whitelist is read-only after configuration, so the
+ * list_empty() fast path needs no locking.
+ */
+ if (!list_empty(&smk_preserve_wl) &&
+ (file->f_flags & O_ACCMODE) == O_RDONLY &&
+ S_ISREG(inode->i_mode)) {
+ struct smack_known *skp = smk_of_inode(inode);
+
+ if (smk_preserve_allowed(skp))
+ smack_task(current)->smk_preserve = skp;
+ }
+#endif
return rc;
}
@@ -5106,6 +5164,9 @@ static int smack_uring_cmd(struct io_uring_cmd *ioucmd)
struct lsm_blob_sizes smack_blob_sizes __ro_after_init = {
.lbs_cred = sizeof(struct task_smack),
+#ifdef CONFIG_SECURITY_SMACK_COPYWHITELIST
+ .lbs_task = sizeof(struct smack_task),
+#endif
.lbs_file = sizeof(struct smack_known *),
.lbs_inode = sizeof(struct inode_smack),
.lbs_ipc = sizeof(struct smack_known *),
diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c
index 6e62dcb36..b1a350365 100644
--- a/security/smack/smackfs.c
+++ b/security/smack/smackfs.c
@@ -62,6 +62,9 @@ enum smk_inos {
SMK_NET6ADDR = 23, /* single label IPv6 hosts */
#endif /* CONFIG_IPV6 */
SMK_RELABEL_SELF = 24, /* relabel possible without CAP_MAC_ADMIN */
+#ifdef CONFIG_SECURITY_SMACK_COPYWHITELIST
+ SMK_PRESERVE_WL = 25, /* preserve whitelist labels */
+#endif
};
/*
@@ -2867,6 +2870,97 @@ static const struct file_operations smk_ptrace_ops = {
.llseek = default_llseek,
};
+#ifdef CONFIG_SECURITY_SMACK_COPYWHITELIST
+/*
+ * Seq_file operations for /smack/preserve-whitelist.
+ * Iterates the whitelist and shows each preservable label.
+ */
+static void *preserve_wl_seq_start(struct seq_file *s, loff_t *pos)
+{
+ mutex_lock(&smk_preserve_wl_lock);
+ return seq_list_start(&smk_preserve_wl, *pos);
+}
+
+static void *preserve_wl_seq_next(struct seq_file *s, void *v, loff_t *pos)
+{
+ return seq_list_next(v, &smk_preserve_wl, pos);
+}
+
+static void preserve_wl_seq_stop(struct seq_file *s, void *v)
+{
+ mutex_unlock(&smk_preserve_wl_lock);
+}
+
+static int preserve_wl_seq_show(struct seq_file *s, void *v)
+{
+ struct smk_preserve_wl *e = list_entry(v, struct smk_preserve_wl, list);
+
+ seq_printf(s, "%s\n", e->skp->smk_known);
+ return 0;
+}
+
+static const struct seq_operations preserve_wl_seq_ops = {
+ .start = preserve_wl_seq_start,
+ .next = preserve_wl_seq_next,
+ .stop = preserve_wl_seq_stop,
+ .show = preserve_wl_seq_show,
+};
+
+static int smk_open_preserve_wl(struct inode *inode, struct file *file)
+{
+ return seq_open(file, &preserve_wl_seq_ops);
+}
+
+static ssize_t smk_write_preserve_wl(struct file *file, const char __user *buf,
+ size_t count, loff_t *ppos)
+{
+ struct smack_known *skp;
+ char *data, *label;
+ bool on = true;
+
+ if (!smack_privileged(CAP_MAC_ADMIN))
+ return -EPERM;
+
+ data = memdup_user_nul(buf, count);
+ if (IS_ERR(data))
+ return PTR_ERR(data);
+
+ label = strim(data);
+ if (!label[0]) {
+ kfree(data);
+ return -EINVAL;
+ }
+
+ /* A leading '-' removes the label (same convention as revoke-subject). */
+ if (label[0] == '-') {
+ on = false;
+ label = strim(label + 1);
+ if (!label[0]) {
+ kfree(data);
+ return -EINVAL;
+ }
+ }
+
+ skp = smk_find_entry(label);
+ if (!skp) {
+ kfree(data);
+ return -ENOENT;
+ }
+
+ smk_preserve_set(skp, on);
+ kfree(data);
+ return count;
+}
+
+static const struct file_operations smk_preserve_wl_ops = {
+ .open = smk_open_preserve_wl,
+ .read = seq_read,
+ .write = smk_write_preserve_wl,
+ .llseek = seq_lseek,
+ .release = seq_release,
+};
+#endif /* CONFIG_SECURITY_SMACK_COPYWHITELIST */
+
/**
* smk_fill_super - fill the smackfs superblock
* @sb: the empty superblock
@@ -2933,6 +3027,11 @@ static int smk_fill_super(struct super_block *sb, struct fs_context *fc)
[SMK_RELABEL_SELF] = {
"relabel-self", &smk_relabel_self_ops,
S_IRUGO|S_IWUGO},
+#ifdef CONFIG_SECURITY_SMACK_COPYWHITELIST
+ [SMK_PRESERVE_WL] = {
+ "preserve-whitelist", &smk_preserve_wl_ops,
+ S_IRUGO|S_IWUSR},
+#endif
/* last one */
{""}
};
--
2.43.0
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH v3] smack: preserve low-integrity labels across file copy via whitelist
2026-10-28 16:15 [PATCH v3] smack: preserve low-integrity labels across file copy via whitelist Tang Pengke
@ 2026-10-28 16:37 ` sashiko-bot
0 siblings, 0 replies; 2+ messages in thread
From: sashiko-bot @ 2026-10-28 16:37 UTC (permalink / raw)
Cc: linux-security-module
Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
- [High] smack: broken exec handling for smk_preserve in smack_bprm_creds_for_exec()
- [High] smack: missing MAY_WRITE check in smack_inode_init_security() for smk_preserve
- [High] smack: smk_preserve corrupts inode labels across OverlayFS copy-up
- [Medium] smack: silent allocation failure in smk_preserve_set()
- [Medium] smack: smack_file_open() re-records smk_preserve on O_RDONLY | O_CREAT
- [Medium] smack: unimported label rejection and unbounded write in smk_write_preserve_wl()
--
commit d4423a161be1eb02cab93ddd7185fcd574c8aaf2
Author: Tang Pengke <tangpengke@yhkylin.cn>
smack: preserve low-integrity labels across file copy via whitelist
This commit introduces a configurable whitelist (/smack/preserve-whitelist) of
low-integrity Smack labels that are recorded in a per-task LSM blob when a
process opens a whitelisted regular file read-only, so that the next inode
created by that thread inherits the preserved label instead of the process
label.
> diff --git a/security/smack/smack_access.c b/security/smack/smack_access.c
> index fb85356266e53..4982127ce985f 100644
> --- a/security/smack/smack_access.c
> +++ b/security/smack/smack_access.c
> @@ -53,6 +53,83 @@ static u32 smack_next_secid = 10;
[ ... ]
> +void smk_preserve_set(struct smack_known *skp, bool on)
> +{
> + struct smk_preserve_wl *e, *new = NULL;
> +
> + if (on) {
> + new = kzalloc_obj(*new, GFP_KERNEL);
> + if (!new)
> + return;
> + new->skp = skp;
> + }
[Severity: Medium]
When kzalloc_obj(*new, GFP_KERNEL) fails in smk_preserve_set(), this returns
early without an error code, and smk_write_preserve_wl() in
security/smack/smackfs.c then unconditionally returns count to userspace:
smk_preserve_set(skp, on);
kfree(data);
return count;
Should smk_preserve_set() return an int so that smk_write_preserve_wl() can
propagate -ENOMEM to userspace when the whitelist allocation fails?
[ ... ]
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index 8e88ac65fd7fc..094d26c943b52 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -915,6 +915,11 @@ static int smack_bprm_creds_for_exec(struct linux_binprm *bprm)
> struct superblock_smack *sbsp;
> int rc;
>
> +#ifdef CONFIG_SECURITY_SMACK_COPYWHITELIST
> + /* a new program starts with a clean copy record */
> + smack_task(current)->smk_preserve = NULL;
> +#endif
[Severity: High]
Can clearing smack_task(current)->smk_preserve in smack_bprm_creds_for_exec()
allow a task to drop its pending preserve record without actually executing a
new program, or leave smk_preserve armed across exec?
In fs/exec.c:bprm_execve(), security_bprm_creds_for_exec() runs before the
bprm->is_check early return and before exec_binprm():
retval = security_bprm_creds_for_exec(bprm);
if (retval || bprm->is_check)
goto out;
retval = exec_binprm(bprm);
If a task that has opened a whitelisted low-integrity file calls
execveat(..., AT_EXECVE_CHECK) or invokes execve() on a target that fails in
smack_bprm_creds_for_exec() or exec_binprm(), smk_preserve is cleared on
current even though no exec took place, allowing subsequent file creations by
that task to use its trusted process label.
In addition, fs/exec.c:do_open_execat() and open_exec() open the executable
and any interpreter with O_LARGEFILE | O_RDONLY | __FMODE_EXEC. Because
smack_file_open() checks (file->f_flags & O_ACCMODE) == O_RDONLY without
excluding __FMODE_EXEC (or file->f_mode & FMODE_EXEC):
1. If do_open_execat() opens a whitelisted file and then fails before
bprm_execve() (for example, at path_noexec(&file->f_path) on a noexec
mount), smack_file_open() has already set smk_preserve on the calling task.
2. If exec_binprm() opens a script interpreter via open_exec() after
security_bprm_creds_for_exec() has already run, smack_file_open() will
record smk_preserve for the newly executed program if the interpreter's
label is on the whitelist.
Should smack_file_open() ignore FMODE_EXEC opens, and should smk_preserve be
cleared when the exec commits in a bprm_committing_creds or
bprm_committed_creds hook instead of smack_bprm_creds_for_exec()?
> isp = smack_inode(inode);
> if (isp->smk_task == NULL || isp->smk_task == bsp->smk_task)
> return 0;
> @@ -1075,6 +1080,26 @@ static int smack_inode_init_security(struct inode *inode, struct inode *dir,
> }
> }
>
> +#ifdef CONFIG_SECURITY_SMACK_COPYWHITELIST
> + /*
> + * If a whitelisted label was recorded on file open, inherit it so
> + * the new file keeps the source's (low-integrity) label across the
> + * copy. Transmutation takes precedence: when it applies the record
> + * is dropped, otherwise it would leak into a later create in a
> + * non-transmutable directory.
> + */
> + if (trans_cred || (trans_rule && smk_inode_transmutable(dir))) {
> + smack_task(current)->smk_preserve = NULL;
> + } else {
> + struct smack_known *skp = smack_task(current)->smk_preserve;
> +
> + if (skp != NULL) {
> + issp->smk_inode = skp;
> + smack_task(current)->smk_preserve = NULL;
> + }
> + }
> +#endif
[Severity: High]
What happens here when the creating task has MAY_READ access to the
whitelisted label skp, but does not have MAY_WRITE access to skp?
In smack_file_open(), smk_preserve is recorded after checking only MAY_READ on
the source inode. When the task then creates a new file via open(dst,
O_CREAT | O_WRONLY, mode) in a writable non-transmuting directory,
may_o_create() checks MAY_WRITE | MAY_EXEC on the parent directory, and
smack_inode_init_security() assigns issp->smk_inode = skp without checking
whether the task has MAY_WRITE access to skp.
Because FMODE_CREATED is set on the struct file, fs/namei.c:do_open() sets
acc_mode = 0 before may_open():
if (file->f_mode & FMODE_CREATED) {
/* Don't check for write permission, don't truncate */
open_flag &= ~O_TRUNC;
acc_mode = 0;
}
may_open() then passes only MAY_OPEN to inode_permission(), which
smack_inode_permission() masks out, and smack_file_open() checks only
MAY_READ. Since Smack does not hook file_permission on write(), the task
receives a writable file descriptor and can write arbitrary content to an
inode labeled skp despite policy denying it write access to skp. Meanwhile,
follow-up operations that copy utilities perform on the newly created file
(such as fchmod(), fchown(), futimens(), setxattr(), rename(), or unlink())
check MAY_WRITE against issp->smk_inode and fail with -EACCES.
[Severity: High]
Also in smack_inode_init_security(), how does smk_preserve interact with
OverlayFS copy-up and credential overrides?
When a task with a pending smk_preserve creates a file in an OverlayFS
directory whose parent is still on a lower layer,
ovl_create_object() -> ovl_copy_up() -> ovl_copy_up_workdir() calls
security_inode_copy_up() -> smack_inode_copy_up() and installs copy_up_cred
via override_creds():
isp = smack_inode(d_inode(dentry));
skp = isp->smk_inode;
tsp->smk_task = skp;
*new = new_creds;
smack_inode_copy_up() leaves tsp->smk_transmuted as NULL, so when
ovl_create_temp() creates the upper directory, smack_inode_init_security()
sees trans_cred == false, overwrites the copied-up upper directory's label
with smack_task(current)->smk_preserve, and clears smk_preserve = NULL.
Because smack_inode_copy_up_xattr() returns -ECANCELED for XATTR_NAME_SMACK,
the upper directory is permanently relabeled to the low-integrity label, and
when ovl_create_object() then creates the actual target file, smk_preserve has
already been consumed so the copied file receives the task's trusted label.
Similarly, if a task with a pending smk_preserve modifies an existing lower
file or directory on OverlayFS (for example via chmod(), truncate(), or
open(O_WRONLY)), the copy-up creation in ovl_copy_up_tmpfile() or
ovl_copy_up_workdir() consumes smk_preserve and relabels the copied-up inode.
Furthermore, ovl_copy_up_file() opens the lower file internally via
ovl_path_open(..., O_LARGEFILE | O_RDONLY), which reaches smack_file_open()
and records smk_preserve on current even though the user only truncated or
opened the file for writing.
> if (rc == 0)
> if (xattr_dupval(xattrs, xattr_count,
> XATTR_SMACK_SUFFIX,
> @@ -2077,6 +2102,39 @@ static int smack_file_open(struct file *file)
[ ... ]
> + if (!list_empty(&smk_preserve_wl) &&
> + (file->f_flags & O_ACCMODE) == O_RDONLY &&
> + S_ISREG(inode->i_mode)) {
> + struct smack_known *skp = smk_of_inode(inode);
> +
> + if (smk_preserve_allowed(skp))
> + smack_task(current)->smk_preserve = skp;
> + }
> +#endif
[Severity: Medium]
If a task with a pending smk_preserve creates a new regular file using
open(path, O_RDONLY | O_CREAT, mode) (or open(path, O_CREAT | O_EXCL, mode),
since O_RDONLY is 0), lookup_open() sets FMODE_CREATED and invokes
smack_inode_init_security(), which labels the new inode with skp and clears
smack_task(current)->smk_preserve = NULL.
Later in that same open() syscall, do_open() -> vfs_open() -> do_dentry_open()
calls security_file_open() -> smack_file_open() on the newly created file.
Because (file->f_flags & O_ACCMODE) == O_RDONLY is true and FMODE_CREATED is
not excluded here, won't smack_file_open() see the newly inherited skp on
inode and immediately set smack_task(current)->smk_preserve = skp again,
defeating the one-shot consumption?
> diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c
> index a72bc7fabea99..7a6dc3e2eb812 100644
> --- a/security/smack/smackfs.c
> +++ b/security/smack/smackfs.c
[ ... ]
> @@ -2777,6 +2780,97 @@ static const struct file_operations smk_ptrace_ops = {
[ ... ]
> +static ssize_t smk_write_preserve_wl(struct file *file, const char __user *buf,
> + size_t count, loff_t *ppos)
> +{
> + struct smack_known *skp;
> + char *data, *label;
> + bool on = true;
> +
> + if (!smack_privileged(CAP_MAC_ADMIN))
> + return -EPERM;
> +
> + data = memdup_user_nul(buf, count);
> + if (IS_ERR(data))
> + return PTR_ERR(data);
> +
> + label = strim(data);
> + if (!label[0]) {
> + kfree(data);
> + return -EINVAL;
> + }
> +
> + /* A leading '-' removes the label (same convention as revoke-subject). */
> + if (label[0] == '-') {
> + on = false;
> + label = strim(label + 1);
> + if (!label[0]) {
> + kfree(data);
> + return -EINVAL;
> + }
> + }
> +
> + skp = smk_find_entry(label);
> + if (!skp) {
> + kfree(data);
> + return -ENOENT;
> + }
[Severity: Medium]
When adding a label to /smack/preserve-whitelist (for example at boot before a
removable filesystem with smackfsdef= is mounted or before policy rules
referencing that label are loaded), smk_find_entry(label) returns NULL and
smk_write_preserve_wl() fails with -ENOENT if the label has not already been
imported into smack_known_hash.
Other smackfs label configuration writers such as smk_write_onlycap(),
smk_write_ambient(), smk_write_unconfined(), smk_write_syslog(), and
smk_write_relabel_self() check *ppos != 0, bound count (to SMK_LONGLABEL /
PAGE_SIZE), and call smk_import_entry(label, 0) when adding a label so valid
labels are imported on demand. Should smk_write_preserve_wl() also bound count
and use smk_import_entry() when on is true?
--
Sashiko AI review · https://sashiko.dev/#/patchset/684C81A1C8289709+20261028161521.925644-1-tangpengke@yhkylin.cn?part=1
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-10-28 16:37 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-28 16:15 [PATCH v3] smack: preserve low-integrity labels across file copy via whitelist Tang Pengke
2026-10-28 16:37 ` sashiko-bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox