FAILED: patch "[PATCH] ima: Fix use-after-free on a dentry's dname.name" failed to apply to 5.15-stable tree

FAILED: patch "[PATCH] ima: Fix use-after-free on a dentry's dname.name" failed to apply to 5.15-stable tree

[gregkh]

The patch below does not apply to the 5.15-stable tree.
If someone wants it applied there, or to any other stable or longterm
tree, then please email the backport, including the original git commit
id to <stable@vger.kernel.org>.

To reproduce the conflict and resubmit, you may use the following commands:

git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-5.15.y
git checkout FETCH_HEAD
git cherry-pick -x be84f32bb2c981ca6709
# <resolve conflicts, build, test, etc.>
git commit -s
git send-email --to '<stable@vger.kernel.org>' --in-reply-to '2024061919-angelic-granny-6472@gregkh' --subject-prefix 'PATCH 5.15.y' HEAD^..

Possible dependencies:



thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From be84f32bb2c981ca670922e047cdde1488b233de Mon Sep 17 00:00:00 2001
From: Stefan Berger <stefanb@linux.ibm.com>
Date: Fri, 22 Mar 2024 10:03:12 -0400
Subject: [PATCH] ima: Fix use-after-free on a dentry's dname.name

->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>
Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>

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;
 }
 
 /*

[PATCH 5.15.y] ima: Fix use-after-free on a dentry's dname.name

[libo.chen.cn]

From: Stefan Berger <stefanb@linux.ibm.com>

[ Upstream commit be84f32bb2c981ca670922e047cdde1488b233de ]

->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>
Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
Signed-off-by: Libo Chen <libo.chen.cn@windriver.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 04b9e465463b..fa7abe4bde61 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -217,7 +217,7 @@ int ima_collect_measurement(struct integrity_iint_cache *iint,
 	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 name_snapshot filename;
 	int result = 0;
 	int length;
 	void *tmpbuf;
@@ -280,9 +280,13 @@ int ima_collect_measurement(struct integrity_iint_cache *iint,
 		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;
 }
@@ -395,6 +399,7 @@ void ima_audit_measurement(struct integrity_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();
@@ -408,7 +413,10 @@ const char *ima_d_path(const struct path *path, char **pathbuf, char *namebuf)
 	}
 
 	if (!pathname) {
-		strlcpy(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 ca017cae73eb..dd7beaa0e787 100644
--- a/security/integrity/ima/ima_template_lib.c
+++ b/security/integrity/ima/ima_template_lib.c
@@ -426,7 +426,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);
 
@@ -439,7 +442,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
 		/*
@@ -448,8 +454,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.25.1

Re: [PATCH 5.15.y] ima: Fix use-after-free on a dentry's dname.name

[Sasha Levin]

[ Sasha's backport helper bot ]

Hi,

The upstream commit SHA1 provided is correct: be84f32bb2c981ca670922e047cdde1488b233de

WARNING: Author mismatch between patch and upstream commit:
Backport author: libo.chen.cn@windriver.com
Commit author: Stefan Berger <stefanb@linux.ibm.com>


Status in newer kernel trees:
6.12.y | Present (exact SHA1)
6.6.y | Present (different SHA1: dd431c3ac1fc)
6.1.y | Present (different SHA1: 7fb374981e31)
5.15.y | Not found

Note: The patch differs from the upstream commit:
---
1:  be84f32bb2c98 ! 1:  033c0527f3846 ima: Fix use-after-free on a dentry's dname.name
    @@ Metadata
      ## Commit message ##
         ima: Fix use-after-free on a dentry's dname.name
     
    +    [ Upstream commit be84f32bb2c981ca670922e047cdde1488b233de ]
    +
         ->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,
    @@ Commit message
         Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
         Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
         Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
    +    Signed-off-by: Sasha Levin <sashal@kernel.org>
    +    Signed-off-by: Libo Chen <libo.chen.cn@windriver.com>
     
      ## security/integrity/ima/ima_api.c ##
    -@@ security/integrity/ima/ima_api.c: int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
    +@@ security/integrity/ima/ima_api.c: int ima_collect_measurement(struct integrity_iint_cache *iint,
      	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;
    -@@ security/integrity/ima/ima_api.c: int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
    + 	void *tmpbuf;
    +@@ security/integrity/ima/ima_api.c: int ima_collect_measurement(struct integrity_iint_cache *iint,
      		if (file->f_flags & O_DIRECT)
      			audit_cause = "failed(directio)";
      
    @@ security/integrity/ima/ima_api.c: int ima_collect_measurement(struct ima_iint_ca
      	}
      	return result;
      }
    -@@ security/integrity/ima/ima_api.c: void ima_audit_measurement(struct ima_iint_cache *iint,
    +@@ security/integrity/ima/ima_api.c: void ima_audit_measurement(struct integrity_iint_cache *iint,
       */
      const char *ima_d_path(const struct path *path, char **pathbuf, char *namebuf)
      {
    @@ security/integrity/ima/ima_api.c: const char *ima_d_path(const struct path *path
      	}
      
      	if (!pathname) {
    --		strscpy(namebuf, path->dentry->d_name.name, NAME_MAX);
    +-		strlcpy(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);
---

Results of testing on various branches:

| Branch                    | Patch Apply | Build Test |
|---------------------------|-------------|------------|
| stable/linux-5.15.y       |  Success    |  Success   |

Re: [PATCH 5.15.y] ima: Fix use-after-free on a dentry's dname.name

[Greg KH]

On Tue, Dec 10, 2024 at 10:02:52AM +0800, libo.chen.cn@windriver.com wrote:
> From: Stefan Berger <stefanb@linux.ibm.com>
> 
> [ Upstream commit be84f32bb2c981ca670922e047cdde1488b233de ]

Please cc: all relevant people on backports.

[PATCH 5.15.y] ima: Fix use-after-free on a dentry's dname.name

[libo.chen.cn]

From: Stefan Berger <stefanb@linux.ibm.com>

[ Upstream commit be84f32bb2c981ca670922e047cdde1488b233de ]

->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>
Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
Signed-off-by: Libo Chen <libo.chen.cn@windriver.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 04b9e465463b..fa7abe4bde61 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -217,7 +217,7 @@ int ima_collect_measurement(struct integrity_iint_cache *iint,
 	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 name_snapshot filename;
 	int result = 0;
 	int length;
 	void *tmpbuf;
@@ -280,9 +280,13 @@ int ima_collect_measurement(struct integrity_iint_cache *iint,
 		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;
 }
@@ -395,6 +399,7 @@ void ima_audit_measurement(struct integrity_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();
@@ -408,7 +413,10 @@ const char *ima_d_path(const struct path *path, char **pathbuf, char *namebuf)
 	}
 
 	if (!pathname) {
-		strlcpy(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 ca017cae73eb..dd7beaa0e787 100644
--- a/security/integrity/ima/ima_template_lib.c
+++ b/security/integrity/ima/ima_template_lib.c
@@ -426,7 +426,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);
 
@@ -439,7 +442,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
 		/*
@@ -448,8 +454,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.25.1

Re: [PATCH 5.15.y] ima: Fix use-after-free on a dentry's dname.name

[Mimi Zohar]

On Wed, 2024-12-11 at 16:28 +0800, libo.chen.cn@eng.windriver.com wrote:
> From: Stefan Berger <stefanb@linux.ibm.com>
> 
> [ Upstream commit be84f32bb2c981ca670922e047cdde1488b233de ]
> 
> ->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.

Thanks, Libo.  The backport looks fine.

Mimi

Re: [PATCH 5.15.y] ima: Fix use-after-free on a dentry's dname.name

[Sasha Levin]

[ Sasha's backport helper bot ]

Hi,

The upstream commit SHA1 provided is correct: be84f32bb2c981ca670922e047cdde1488b233de

WARNING: Author mismatch between patch and upstream commit:
Backport author: libo.chen.cn@eng.windriver.com
Commit author: Stefan Berger <stefanb@linux.ibm.com>


Status in newer kernel trees:
6.12.y | Present (exact SHA1)
6.6.y | Present (different SHA1: dd431c3ac1fc)
6.1.y | Present (different SHA1: 7fb374981e31)
5.15.y | Not found

Note: The patch differs from the upstream commit:
---
1:  be84f32bb2c98 ! 1:  fd4816135a9e5 ima: Fix use-after-free on a dentry's dname.name
    @@ Metadata
      ## Commit message ##
         ima: Fix use-after-free on a dentry's dname.name
     
    +    [ Upstream commit be84f32bb2c981ca670922e047cdde1488b233de ]
    +
         ->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,
    @@ Commit message
         Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
         Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
         Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
    +    Signed-off-by: Sasha Levin <sashal@kernel.org>
    +    Signed-off-by: Libo Chen <libo.chen.cn@windriver.com>
     
      ## security/integrity/ima/ima_api.c ##
    -@@ security/integrity/ima/ima_api.c: int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
    +@@ security/integrity/ima/ima_api.c: int ima_collect_measurement(struct integrity_iint_cache *iint,
      	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;
    -@@ security/integrity/ima/ima_api.c: int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
    + 	void *tmpbuf;
    +@@ security/integrity/ima/ima_api.c: int ima_collect_measurement(struct integrity_iint_cache *iint,
      		if (file->f_flags & O_DIRECT)
      			audit_cause = "failed(directio)";
      
    @@ security/integrity/ima/ima_api.c: int ima_collect_measurement(struct ima_iint_ca
      	}
      	return result;
      }
    -@@ security/integrity/ima/ima_api.c: void ima_audit_measurement(struct ima_iint_cache *iint,
    +@@ security/integrity/ima/ima_api.c: void ima_audit_measurement(struct integrity_iint_cache *iint,
       */
      const char *ima_d_path(const struct path *path, char **pathbuf, char *namebuf)
      {
    @@ security/integrity/ima/ima_api.c: const char *ima_d_path(const struct path *path
      	}
      
      	if (!pathname) {
    --		strscpy(namebuf, path->dentry->d_name.name, NAME_MAX);
    +-		strlcpy(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);
---

Results of testing on various branches:

| Branch                    | Patch Apply | Build Test |
|---------------------------|-------------|------------|
| stable/linux-5.15.y       |  Success    |  Success   |