Skip to content

Commit be84f32

Browse files
stefanbergermimizohar
authored andcommitted
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 <[email protected]> Signed-off-by: Stefan Berger <[email protected]> Signed-off-by: Mimi Zohar <[email protected]>
1 parent fec50db commit be84f32

File tree

2 files changed

+26
-7
lines changed

2 files changed

+26
-7
lines changed

security/integrity/ima/ima_api.c

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -245,8 +245,8 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
245245
const char *audit_cause = "failed";
246246
struct inode *inode = file_inode(file);
247247
struct inode *real_inode = d_real_inode(file_dentry(file));
248-
const char *filename = file->f_path.dentry->d_name.name;
249248
struct ima_max_digest_data hash;
249+
struct name_snapshot filename;
250250
struct kstat stat;
251251
int result = 0;
252252
int length;
@@ -317,9 +317,13 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
317317
if (file->f_flags & O_DIRECT)
318318
audit_cause = "failed(directio)";
319319

320+
take_dentry_name_snapshot(&filename, file->f_path.dentry);
321+
320322
integrity_audit_msg(AUDIT_INTEGRITY_DATA, inode,
321-
filename, "collect_data", audit_cause,
322-
result, 0);
323+
filename.name.name, "collect_data",
324+
audit_cause, result, 0);
325+
326+
release_dentry_name_snapshot(&filename);
323327
}
324328
return result;
325329
}
@@ -432,6 +436,7 @@ void ima_audit_measurement(struct ima_iint_cache *iint,
432436
*/
433437
const char *ima_d_path(const struct path *path, char **pathbuf, char *namebuf)
434438
{
439+
struct name_snapshot filename;
435440
char *pathname = NULL;
436441

437442
*pathbuf = __getname();
@@ -445,7 +450,10 @@ const char *ima_d_path(const struct path *path, char **pathbuf, char *namebuf)
445450
}
446451

447452
if (!pathname) {
448-
strscpy(namebuf, path->dentry->d_name.name, NAME_MAX);
453+
take_dentry_name_snapshot(&filename, path->dentry);
454+
strscpy(namebuf, filename.name.name, NAME_MAX);
455+
release_dentry_name_snapshot(&filename);
456+
449457
pathname = namebuf;
450458
}
451459

security/integrity/ima/ima_template_lib.c

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -483,7 +483,10 @@ static int ima_eventname_init_common(struct ima_event_data *event_data,
483483
bool size_limit)
484484
{
485485
const char *cur_filename = NULL;
486+
struct name_snapshot filename;
486487
u32 cur_filename_len = 0;
488+
bool snapshot = false;
489+
int ret;
487490

488491
BUG_ON(event_data->filename == NULL && event_data->file == NULL);
489492

@@ -496,7 +499,10 @@ static int ima_eventname_init_common(struct ima_event_data *event_data,
496499
}
497500

498501
if (event_data->file) {
499-
cur_filename = event_data->file->f_path.dentry->d_name.name;
502+
take_dentry_name_snapshot(&filename,
503+
event_data->file->f_path.dentry);
504+
snapshot = true;
505+
cur_filename = filename.name.name;
500506
cur_filename_len = strlen(cur_filename);
501507
} else
502508
/*
@@ -505,8 +511,13 @@ static int ima_eventname_init_common(struct ima_event_data *event_data,
505511
*/
506512
cur_filename_len = IMA_EVENT_NAME_LEN_MAX;
507513
out:
508-
return ima_write_template_field_data(cur_filename, cur_filename_len,
509-
DATA_FMT_STRING, field_data);
514+
ret = ima_write_template_field_data(cur_filename, cur_filename_len,
515+
DATA_FMT_STRING, field_data);
516+
517+
if (snapshot)
518+
release_dentry_name_snapshot(&filename);
519+
520+
return ret;
510521
}
511522

512523
/*

0 commit comments

Comments
 (0)