From: Stefan Berger <stefanb@linux.ibm.com>
To: linux-integrity@vger.kernel.org, linux-security-module@vger.kernel.org
Cc: linux-kernel@vger.kernel.org, zohar@linux.ibm.com,
roberto.sassu@huawei.com, Stefan Berger <stefanb@linux.ibm.com>,
Al Viro <viro@zeniv.linux.org.uk>
Subject: [PATCH] ima: Fix use-after-free on a dentry's dname.name
Date: Fri, 22 Mar 2024 10:03:12 -0400 [thread overview]
Message-ID: <20240322140312.1804404-1-stefanb@linux.ibm.com> (raw)
->d_name.name can change on rename and the earlier value can be freed;
there are conditions sufficient to stabilize it (->d_lock on dentry,
->d_lock on its parent, ->i_rwsem exclusive on the parent's inode,
rename_lock), but none of those are met at any of the sites. Take a stable
snapshot of the name instead.
Link: https://lore.kernel.org/all/20240202182732.GE2087318@ZenIV/
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
---
security/integrity/ima/ima_api.c | 16 ++++++++++++----
security/integrity/ima/ima_template_lib.c | 17 ++++++++++++++---
2 files changed, 26 insertions(+), 7 deletions(-)
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index b37d043d5748..1856981e33df 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -245,8 +245,8 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
const char *audit_cause = "failed";
struct inode *inode = file_inode(file);
struct inode *real_inode = d_real_inode(file_dentry(file));
- const char *filename = file->f_path.dentry->d_name.name;
struct ima_max_digest_data hash;
+ struct name_snapshot filename;
struct kstat stat;
int result = 0;
int length;
@@ -317,9 +317,13 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
if (file->f_flags & O_DIRECT)
audit_cause = "failed(directio)";
+ take_dentry_name_snapshot(&filename, file->f_path.dentry);
+
integrity_audit_msg(AUDIT_INTEGRITY_DATA, inode,
- filename, "collect_data", audit_cause,
- result, 0);
+ filename.name.name, "collect_data",
+ audit_cause, result, 0);
+
+ release_dentry_name_snapshot(&filename);
}
return result;
}
@@ -432,6 +436,7 @@ void ima_audit_measurement(struct ima_iint_cache *iint,
*/
const char *ima_d_path(const struct path *path, char **pathbuf, char *namebuf)
{
+ struct name_snapshot filename;
char *pathname = NULL;
*pathbuf = __getname();
@@ -445,7 +450,10 @@ const char *ima_d_path(const struct path *path, char **pathbuf, char *namebuf)
}
if (!pathname) {
- strscpy(namebuf, path->dentry->d_name.name, NAME_MAX);
+ take_dentry_name_snapshot(&filename, path->dentry);
+ strscpy(namebuf, filename.name.name, NAME_MAX);
+ release_dentry_name_snapshot(&filename);
+
pathname = namebuf;
}
diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c
index 6cd0add524cd..3b2cb8f1002e 100644
--- a/security/integrity/ima/ima_template_lib.c
+++ b/security/integrity/ima/ima_template_lib.c
@@ -483,7 +483,10 @@ static int ima_eventname_init_common(struct ima_event_data *event_data,
bool size_limit)
{
const char *cur_filename = NULL;
+ struct name_snapshot filename;
u32 cur_filename_len = 0;
+ bool snapshot = false;
+ int ret;
BUG_ON(event_data->filename == NULL && event_data->file == NULL);
@@ -496,7 +499,10 @@ static int ima_eventname_init_common(struct ima_event_data *event_data,
}
if (event_data->file) {
- cur_filename = event_data->file->f_path.dentry->d_name.name;
+ take_dentry_name_snapshot(&filename,
+ event_data->file->f_path.dentry);
+ snapshot = true;
+ cur_filename = filename.name.name;
cur_filename_len = strlen(cur_filename);
} else
/*
@@ -505,8 +511,13 @@ static int ima_eventname_init_common(struct ima_event_data *event_data,
*/
cur_filename_len = IMA_EVENT_NAME_LEN_MAX;
out:
- return ima_write_template_field_data(cur_filename, cur_filename_len,
- DATA_FMT_STRING, field_data);
+ ret = ima_write_template_field_data(cur_filename, cur_filename_len,
+ DATA_FMT_STRING, field_data);
+
+ if (snapshot)
+ release_dentry_name_snapshot(&filename);
+
+ return ret;
}
/*
--
2.43.0
next reply other threads:[~2024-03-22 14:03 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-22 14:03 Stefan Berger [this message]
2024-03-29 1:02 ` [PATCH] ima: Fix use-after-free on a dentry's dname.name Mimi Zohar
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240322140312.1804404-1-stefanb@linux.ibm.com \
--to=stefanb@linux.ibm.com \
--cc=linux-integrity@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-security-module@vger.kernel.org \
--cc=roberto.sassu@huawei.com \
--cc=viro@zeniv.linux.org.uk \
--cc=zohar@linux.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).