linux-hardening - Re: [PATCH] pstore: Fix uaf when backend is unregistered

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <6077c924-f975-49a0-8ee4-4e481664ac38@gmail.com>
Date: Wed, 9 Oct 2024 21:36:33 +0800
From: Zach Wade <zachwade.k@...il.com>
To: Kees Cook <kees@...nel.org>
Cc: tony.luck@...el.com, gpiccoli@...lia.com,
 linux-hardening@...r.kernel.org, linux-kernel@...r.kernel.org,
 stable@...r.kernel.org, lixingyang1@...com
Subject: Re: [PATCH] pstore: Fix uaf when backend is unregistered



On 2024/10/9 1:23, Kees Cook wrote:
> On Wed, Oct 09, 2024 at 01:10:14AM +0800, Zach Wade wrote:
>> when unload pstore_blk, we will unlink the pstore file and
>> set pos->dentry to NULL, but simple_unlink(d_inode(root), pos->dentry)
>> may free inode of pos->dentry and free pos by free_pstore_private,
>> this may trigger uaf. kasan report:
> 
> Thanks for this! I need to double check what happening here a bit more
> closely, as maybe some ordering changed after a43e0fc5e913 ("pstore:
> inode: Only d_invalidate() is needed")
> 
>> @@ -316,9 +316,10 @@ int pstore_put_backend_records(struct pstore_info *psi)
>>   		list_for_each_entry_safe(pos, tmp, &records_list, list) {
>>   			if (pos->record->psi == psi) {
>>   				list_del_init(&pos->list);
>> -				d_invalidate(pos->dentry);
>> -				simple_unlink(d_inode(root), pos->dentry);
>> +				unlink_dentry = pos->dentry;
>>   				pos->dentry = NULL;
>> +				d_invalidate(unlink_dentry);
>> +				simple_unlink(d_inode(root), unlink_dentry);
> 
> But on the face of it, this does solve the UAF. I'll double-check that
> this isn't a result of the mentioned commit.

Hi Cook,

However, I think a more appropriate solution to the problem here is to
check the reference count when uninstalling the mount point.

At the same time, the test script is attached:
#!/bin/bash
modprobe pstore_blk
blkdev=/dev/sda2
best_effort=on
mount -t pstore pstore /sys/fs/pstore
sleep 2
umount /sys/fs/pstore
modprobe -r pstore_blk

Thanks,
Zach

> 

> -Kees
>