diff mbox series

[RFC] integrity: fix null ptr dereference in integrity_inode_free()

Message ID 20210419181850.885349-1-zohar@linux.ibm.com (mailing list archive)
State New, archived
Headers show
Series [RFC] integrity: fix null ptr dereference in integrity_inode_free() | expand

Commit Message

Mimi Zohar April 19, 2021, 6:18 p.m. UTC
__destroy_inode() -> security_inode_free() should only be called once to
free an inode.  With the provided ecryptfs test.sh script the same inode
is freed twice.

With some debugging info and dump_stack():
[  417.586522][T13423] integrity_inode_get: pid=13423 ppid=13404 ino:7221545
	count: 1 write:0 read:1
[  417.724446][T13428] integrity_inode_get: pid=13428 ppid=13404 ino:7221545
	count: 2 write:0 read:1
[  417.796269][T13430] integrity_inode_free: pid=13430 ppid=13404 ino:7221545
	count: 0 write:0 read:0
[  417.799101][T13430] integrity_inode_free: pid=13430 ppid=13404 ino:7221545
	count: 0 write:0 read:0

But only when executing both the provided ecryptfs test.sh and test2.sh
scripts, causes the null pointer dereferencing.

[  420.896059][T13532] integrity_inode_get: pid=13532 ppid=13514 ino:7604534
	count: 1 write:0 read:1
[  421.020910][T13541] integrity_inode_get: pid=13541 ppid=13514 ino:7604534
	count: 2 write:0 read:1
[  421.055163][T13543] integrity_inode_free: pid=13543 ppid=13514 ino:7604534
	count: 0 write:0 read:0
[  421.056274][T13543] integrity_inode_free null: pid=13543 ppid=13514 ino:7604534
	count: 0 write:0 read:0
[  421.058096][T13543] CPU: 3 PID: 13543 Comm: rm Not tainted 5.12.0-rc6+ #47
[  421.059039][T13543] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.13.0-48-gd9c812dda519-prebuilt.qemu.org 04/01/2014
[  421.060277][T13543] Call Trace:
[  421.060645][T13543]  dump_stack+0x141/0x1d7
[  421.061156][T13543]  integrity_inode_free+0x3e7/0xa70
[  421.061726][T13543]  security_inode_free+0x1e/0xc0
[  421.062245][T13543]  __destroy_inode+0x22f/0x700
[  421.062807][T13543]  destroy_inode+0x91/0x1b0
[  421.063301][T13543]  iput.part.0+0x57e/0x810
[  421.063749][T13543]  iput+0x58/0x70
[  421.064123][T13543]  do_unlinkat+0x431/0x690
[  421.064553][T13543]  ? __ia32_sys_rmdir+0x100/0x100
[  421.065066][T13543]  ? getname_flags.part.0+0x1dd/0x4f0
[  421.065765][T13543]  __x64_sys_unlinkat+0xbd/0x130
[  421.066412][T13543]  do_syscall_64+0x2d/0x70
[  421.067082][T13543]  entry_SYSCALL_64_after_hwframe+0x44/0xae
[  421.068065][T13543] RIP: 0033:0x7fdaaaa0d17b
[  421.068590][T13543] Code: 73 01 c3 48 8b 0d fd fc 0c 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa b8 07 01 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d cd fc 0c 00 f7 d8 64 89 01 48
[  421.071401][T13543] RSP: 002b:00007ffdc00e5948 EFLAGS: 00000246 ORIG_RAX: 0000000000000107
[  421.072321][T13543] RAX: ffffffffffffffda RBX: 0000562e27c0e820 RCX: 00007fdaaaa0d17b
[  421.073211][T13543] RDX: 0000000000000000 RSI: 0000562e27c0e920 RDI: 0000000000000004
[  421.074083][T13543] RBP: 0000562e27c0d440 R08: 0000000000000000 R09: 0000000000000000
[  421.074973][T13543] R10: 0000000000000231 R11: 0000000000000246 R12: 0000000000000000
[  421.075854][T13543] R13: 00007ffdc00e5a60 R14: 0000562e25ddca80 R15: 0000000000000002

Before accessing the "iint", make sure it hasn't already been freed.

Link: https://lore.kernel.org/linux-integrity/630cc0a27a0db8fd790a767300ae0a7f2540bdc2.camel@linux.ibm.com/
Reported-by: Christian Brauner <christian.brauner@ubuntu.com>
Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
---
 security/integrity/iint.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
diff mbox series

Patch

diff --git a/security/integrity/iint.c b/security/integrity/iint.c
index fca8a9409e4a..f2af4afbd94b 100644
--- a/security/integrity/iint.c
+++ b/security/integrity/iint.c
@@ -152,9 +152,13 @@  void integrity_inode_free(struct inode *inode)
 
 	write_lock(&integrity_iint_lock);
 	iint = __integrity_iint_find(inode);
-	rb_erase(&iint->rb_node, &integrity_iint_tree);
+	if (iint)
+		rb_erase(&iint->rb_node, &integrity_iint_tree);
 	write_unlock(&integrity_iint_lock);
 
+	if (!iint)
+		return;
+
 	iint_free(iint);
 }