From 674e2b24791cbe8fd5dc8a0aed4cb4404fcd2028 Mon Sep 17 00:00:00 2001 From: Konstantin Andreev Date: Tue, 17 Jun 2025 00:32:17 +0300 Subject: smack: fix bug: setting task label silently ignores input garbage This command: # echo foo/bar >/proc/$$/attr/smack/current gives the task a label 'foo' w/o indication that label does not match input. Setting the label with lsm_set_self_attr() syscall behaves identically. This occures because: 1) smk_parse_smack() is used to convert input to a label 2) smk_parse_smack() takes only that part from the beginning of the input that looks like a label. 3) `/' is prohibited in labels, so only "foo" is taken. (2) is by design, because smk_parse_smack() is used for parsing strings which are more than just a label. Silent failure is not a good thing, and there are two indicators that this was not done intentionally: (size >= SMK_LONGLABEL) ~> invalid clause at the beginning of the do_setattr() and the "Returns the length of the smack label" claim in the do_setattr() description. So I fixed this by adding one tiny check: the taken label length == input length. Since input length is now strictly controlled, I changed the two ways of setting label smack_setselfattr(): lsm_set_self_attr() syscall smack_setprocattr(): > /proc/.../current to accommodate the divergence in what they understand by "input length": smack_setselfattr counts mandatory \0 into input length, smack_setprocattr does not. smack_setprocattr allows various trailers after label Related changes: * fixed description for smk_parse_smack * allow unprivileged tasks validate label syntax. * extract smk_parse_label_len() from smk_parse_smack() so parsing may be done w/o string allocation. * extract smk_import_valid_label() from smk_import_entry() to avoid repeated parsing. * smk_parse_smack(): scan null-terminated strings for no more than SMK_LONGLABEL(256) characters * smack_setselfattr(): require struct lsm_ctx . flags == 0 to reserve them for future. Fixes: e114e473771c ("Smack: Simplified Mandatory Access Control Kernel") Signed-off-by: Konstantin Andreev Signed-off-by: Casey Schaufler --- security/smack/smack_access.c | 93 +++++++++++++++++++++++++++++++++---------- 1 file changed, 71 insertions(+), 22 deletions(-) (limited to 'security/smack/smack_access.c') diff --git a/security/smack/smack_access.c b/security/smack/smack_access.c index 2e4a0cb22782..a289cb6672bd 100644 --- a/security/smack/smack_access.c +++ b/security/smack/smack_access.c @@ -443,19 +443,19 @@ struct smack_known *smk_find_entry(const char *string) } /** - * smk_parse_smack - parse smack label from a text string - * @string: a text string that might contain a Smack label - * @len: the maximum size, or zero if it is NULL terminated. + * smk_parse_label_len - calculate the length of the starting segment + * in the string that constitutes a valid smack label + * @string: a text string that might contain a Smack label at the beginning + * @len: the maximum size to look into, may be zero if string is null-terminated * - * Returns a pointer to the clean label or an error code. + * Returns the length of the segment (0 < L < SMK_LONGLABEL) or an error code. */ -char *smk_parse_smack(const char *string, int len) +int smk_parse_label_len(const char *string, int len) { - char *smack; int i; - if (len <= 0) - len = strlen(string) + 1; + if (len <= 0 || len > SMK_LONGLABEL) + len = SMK_LONGLABEL; /* * Reserve a leading '-' as an indicator that @@ -463,7 +463,7 @@ char *smk_parse_smack(const char *string, int len) * including /smack/cipso and /smack/cipso2 */ if (string[0] == '-') - return ERR_PTR(-EINVAL); + return -EINVAL; for (i = 0; i < len; i++) if (string[i] > '~' || string[i] <= ' ' || string[i] == '/' || @@ -471,6 +471,25 @@ char *smk_parse_smack(const char *string, int len) break; if (i == 0 || i >= SMK_LONGLABEL) + return -EINVAL; + + return i; +} + +/** + * smk_parse_smack - copy the starting segment in the string + * that constitutes a valid smack label + * @string: a text string that might contain a Smack label at the beginning + * @len: the maximum size to look into, may be zero if string is null-terminated + * + * Returns a pointer to the copy of the label or an error code. + */ +char *smk_parse_smack(const char *string, int len) +{ + char *smack; + int i = smk_parse_label_len(string, len); + + if (i < 0) return ERR_PTR(-EINVAL); smack = kstrndup(string, i, GFP_NOFS); @@ -554,31 +573,25 @@ int smack_populate_secattr(struct smack_known *skp) } /** - * smk_import_entry - import a label, return the list entry - * @string: a text string that might be a Smack label - * @len: the maximum size, or zero if it is NULL terminated. + * smk_import_valid_allocated_label - import a label, return the list entry + * @smack: a text string that is a valid Smack label and may be kfree()ed. + * It is consumed: either becomes a part of the entry or kfree'ed. * - * Returns a pointer to the entry in the label list that - * matches the passed string, adding it if necessary, - * or an error code. + * Returns: see description of smk_import_entry() */ -struct smack_known *smk_import_entry(const char *string, int len) +static struct smack_known * +smk_import_allocated_label(char *smack, gfp_t gfp) { struct smack_known *skp; - char *smack; int rc; - smack = smk_parse_smack(string, len); - if (IS_ERR(smack)) - return ERR_CAST(smack); - mutex_lock(&smack_known_lock); skp = smk_find_entry(smack); if (skp != NULL) goto freeout; - skp = kzalloc(sizeof(*skp), GFP_NOFS); + skp = kzalloc(sizeof(*skp), gfp); if (skp == NULL) { skp = ERR_PTR(-ENOMEM); goto freeout; @@ -608,6 +621,42 @@ unlockout: return skp; } +/** + * smk_import_entry - import a label, return the list entry + * @string: a text string that might contain a Smack label at the beginning + * @len: the maximum size to look into, may be zero if string is null-terminated + * + * Returns a pointer to the entry in the label list that + * matches the passed string, adding it if necessary, + * or an error code. + */ +struct smack_known *smk_import_entry(const char *string, int len) +{ + char *smack = smk_parse_smack(string, len); + + if (IS_ERR(smack)) + return ERR_CAST(smack); + + return smk_import_allocated_label(smack, GFP_NOFS); +} + +/** + * smk_import_valid_label - import a label, return the list entry + * @label a text string that is a valid Smack label, not null-terminated + * + * Returns: see description of smk_import_entry() + */ +struct smack_known * +smk_import_valid_label(const char *label, int label_len, gfp_t gfp) +{ + char *smack = kstrndup(label, label_len, gfp); + + if (!smack) + return ERR_PTR(-ENOMEM); + + return smk_import_allocated_label(smack, gfp); +} + /** * smack_from_secid - find the Smack label associated with a secid * @secid: an integer that might be associated with a Smack label -- cgit From 6ddd169d0288ebee8ec9afcd75825ce8476edb00 Mon Sep 17 00:00:00 2001 From: Konstantin Andreev Date: Thu, 26 Jun 2025 19:47:29 +0300 Subject: smack: fix kernel-doc warnings for smk_import_valid_label() Reported-by: kernel test robot Closes: https://lore.kernel.org/oe-kbuild-all/202506251712.x5SJiNlh-lkp@intel.com/ Signed-off-by: Konstantin Andreev Signed-off-by: Casey Schaufler --- security/smack/smack_access.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'security/smack/smack_access.c') diff --git a/security/smack/smack_access.c b/security/smack/smack_access.c index a289cb6672bd..09167be79122 100644 --- a/security/smack/smack_access.c +++ b/security/smack/smack_access.c @@ -642,9 +642,11 @@ struct smack_known *smk_import_entry(const char *string, int len) /** * smk_import_valid_label - import a label, return the list entry - * @label a text string that is a valid Smack label, not null-terminated + * @label: a text string that is a valid Smack label, not null-terminated + * @label_len: the length of the text string in the @label + * @gfp: the GFP mask used for allocating memory for the @label text string copy * - * Returns: see description of smk_import_entry() + * Return: see description of smk_import_entry() */ struct smack_known * smk_import_valid_label(const char *label, int label_len, gfp_t gfp) -- cgit From 29c701f90b9341f1f9c1854a9c22b71c2318457d Mon Sep 17 00:00:00 2001 From: Casey Schaufler Date: Tue, 11 Nov 2025 12:00:18 -0800 Subject: Smack: function parameter 'gfp' not described Add a descrition of the gfp parameter to smk_import_allocated_label(). Signed-off-by: Casey Schaufler Reported-by: kernel test robot Closes: https://lore.kernel.org/oe-kbuild-all/202511061746.dPegBnNf-lkp@intel.com/ --- security/smack/smack_access.c | 1 + 1 file changed, 1 insertion(+) (limited to 'security/smack/smack_access.c') diff --git a/security/smack/smack_access.c b/security/smack/smack_access.c index 09167be79122..fc507dcc7ea5 100644 --- a/security/smack/smack_access.c +++ b/security/smack/smack_access.c @@ -576,6 +576,7 @@ int smack_populate_secattr(struct smack_known *skp) * smk_import_valid_allocated_label - import a label, return the list entry * @smack: a text string that is a valid Smack label and may be kfree()ed. * It is consumed: either becomes a part of the entry or kfree'ed. + * @gfp: Allocation type * * Returns: see description of smk_import_entry() */ -- cgit