On 1/28/2024 2:00 AM, Ronald Monthero wrote:
> As with kfree_sensitive() it does kfree() and memzero_explicit()
> internally to clear sensitive data. The patch includes some of
> the code paths to free data such as keys, hash table and
> scontext and tcontext of selinux, which would benefit
> from kfree_sensitive() to replace kfree()
What about the data being freed makes it "sensitive"? Generally
security attributes are not themselves considered sensitive.
>
> Signed-off-by: Ronald Monthero <debug.penguin32@xxxxxxxxx>
> ---
> security/selinux/avc.c | 4 ++--
> security/selinux/ima.c | 2 +-
> security/selinux/selinuxfs.c | 16 ++++++++--------
> security/selinux/ss/conditional.c | 4 ++--
> security/selinux/ss/hashtab.c | 2 +-
> security/selinux/ss/policydb.c | 6 +++---
> 6 files changed, 17 insertions(+), 17 deletions(-)
>
> diff --git a/security/selinux/avc.c b/security/selinux/avc.c
> index 32eb67fb3e42..c6d7f52a11c1 100644
> --- a/security/selinux/avc.c
> +++ b/security/selinux/avc.c
> @@ -723,8 +723,8 @@ static void avc_audit_post_callback(struct audit_buffer *ab, void *a)
> audit_log_format(ab, " permissive=%u", sad->result ? 0 : 1);
>
> trace_selinux_audited(sad, scontext, tcontext, tclass);
> - kfree(tcontext);
> - kfree(scontext);
> + kfree_sensitive(tcontext);
> + kfree_sensitive(scontext);
>
> /* in case of invalid context report also the actual context string */
> rc = security_sid_to_context_inval(sad->ssid, &scontext,
> diff --git a/security/selinux/ima.c b/security/selinux/ima.c
> index aa34da9b0aeb..b5d923d272a0 100644
> --- a/security/selinux/ima.c
> +++ b/security/selinux/ima.c
> @@ -86,7 +86,7 @@ void selinux_ima_measure_state_locked(void)
> state_str, strlen(state_str), false,
> NULL, 0);
>
> - kfree(state_str);
> + kfree_sensitive(state_str);
>
> /*
> * Measure SELinux policy only after initialization is completed.
> diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
> index 6c596ae7fef9..6c2490dbfaa6 100644
> --- a/security/selinux/selinuxfs.c
> +++ b/security/selinux/selinuxfs.c
> @@ -183,7 +183,7 @@ static ssize_t sel_write_enforce(struct file *file, const char __user *buf,
> }
> length = count;
> out:
> - kfree(page);
> + kfree_sensitive(page);
> return length;
> }
> #else
> @@ -299,7 +299,7 @@ static ssize_t sel_write_disable(struct file *file, const char __user *buf,
> }
>
> out:
> - kfree(page);
> + kfree_sensitive(page);
> return length;
> }
>
> @@ -725,7 +725,7 @@ static ssize_t sel_write_checkreqprot(struct file *file, const char __user *buf,
> selinux_ima_measure_state();
>
> out:
> - kfree(page);
> + kfree_sensitive(page);
> return length;
> }
> static const struct file_operations sel_checkreqprot_ops = {
> @@ -1290,7 +1290,7 @@ static ssize_t sel_write_bool(struct file *filep, const char __user *buf,
>
> out:
> mutex_unlock(&selinux_state.policy_mutex);
> - kfree(page);
> + kfree_sensitive(page);
> return length;
> }
>
> @@ -1342,7 +1342,7 @@ static ssize_t sel_commit_bools_write(struct file *filep,
>
> out:
> mutex_unlock(&selinux_state.policy_mutex);
> - kfree(page);
> + kfree_sensitive(page);
> return length;
> }
>
> @@ -1481,7 +1481,7 @@ static ssize_t sel_write_avc_cache_threshold(struct file *file,
>
> ret = count;
> out:
> - kfree(page);
> + kfree_sensitive(page);
> return ret;
> }
>
> @@ -1831,7 +1831,7 @@ static int sel_make_perm_files(struct selinux_policy *newpolicy,
> out:
> for (i = 0; i < nperms; i++)
> kfree(perms[i]);
> - kfree(perms);
> + kfree_sensitive(perms);
> return rc;
> }
>
> @@ -1900,7 +1900,7 @@ static int sel_make_classes(struct selinux_policy *newpolicy,
> out:
> for (i = 0; i < nclasses; i++)
> kfree(classes[i]);
> - kfree(classes);
> + kfree_sensitive(classes);
> return rc;
> }
>
> diff --git a/security/selinux/ss/conditional.c b/security/selinux/ss/conditional.c
> index 81ff676f209a..51409a74eb74 100644
> --- a/security/selinux/ss/conditional.c
> +++ b/security/selinux/ss/conditional.c
> @@ -176,8 +176,8 @@ int cond_init_bool_indexes(struct policydb *p)
>
> int cond_destroy_bool(void *key, void *datum, void *p)
> {
> - kfree(key);
> - kfree(datum);
> + kfree_sensitive(key);
> + kfree_sensitive(datum);
> return 0;
> }
>
> diff --git a/security/selinux/ss/hashtab.c b/security/selinux/ss/hashtab.c
> index c05d8346a94a..73b867fa4726 100644
> --- a/security/selinux/ss/hashtab.c
> +++ b/security/selinux/ss/hashtab.c
> @@ -79,7 +79,7 @@ void hashtab_destroy(struct hashtab *h)
> h->htable[i] = NULL;
> }
>
> - kfree(h->htable);
> + kfree_sensitive(h->htable);
> h->htable = NULL;
> }
>
> diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
> index 595a435ea9c8..fd58a6a1ef8f 100644
> --- a/security/selinux/ss/policydb.c
> +++ b/security/selinux/ss/policydb.c
> @@ -179,8 +179,8 @@ static const struct policydb_compat_info *policydb_lookup_compat(unsigned int ve
>
> static int perm_destroy(void *key, void *datum, void *p)
> {
> - kfree(key);
> - kfree(datum);
> + kfree_sensitive(key);
> + kfree_sensitive(datum);
> return 0;
> }
>
> @@ -369,7 +369,7 @@ static void ocontext_destroy(struct ocontext *c, unsigned int i)
> if (i == OCON_ISID || i == OCON_FS ||
> i == OCON_NETIF || i == OCON_FSUSE)
> kfree(c->u.name);
> - kfree(c);
> + kfree_sensitive(c);
> }
>
> /*
