linux-kernel - Re: [PATCH] fs-writeback: fix NULL pointer dereference in __mark_inode

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]

Date:	Mon, 28 Feb 2011 16:59:34 +0100
From:	"Andreas Bießmann" <andreas.devel@...glemail.com>
To:	Sergey Senozhatsky <sergey.senozhatsky@...il.com>
CC:	Andreas Bießmann <biessmann@...science.de>,
	linux-kernel@...r.kernel.org,
	Alexander Viro <viro@...iv.linux.org.uk>,
	linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH] fs-writeback: fix NULL pointer dereference in __mark_inode_dirty

Dear Sergey Senozhatsky,

Am 28.02.2011 16:43, schrieb Sergey Senozhatsky:
> On (02/28/11 16:25), Andreas Bießmann wrote:

>> The reference to sb->s_bdi may be deleted from mmc_blk_remove() ->
>> del_gendisk() -> unlink_gendisk() -> bdi_unregister() -> bdi_prune_sb() while
>> another instance try to write some data to the device.
>>
>> Signed-off-by: Andreas Bießmann <biessmann@...science.de>
>> ---
>>  fs/fs-writeback.c |    3 +++
>>  1 files changed, 3 insertions(+), 0 deletions(-)
>>
>> diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
>> index cdbf7ac..96b4b25 100644
>> --- a/fs/fs-writeback.c
>> +++ b/fs/fs-writeback.c
>> @@ -1047,6 +1047,9 @@ void __mark_inode_dirty(struct inode *inode, int flags)
>>  		if (!was_dirty) {
>>  			bdi = inode_to_bdi(inode);
>>  
>> +			if (!bdi)
>> +				goto out;
>> +
>>  			if (bdi_cap_writeback_dirty(bdi)) {
>>  				WARN(!test_bit(BDI_registered, &bdi->state),
>>  				     "bdi-%s not registered\n", bdi->name);
> 
> Hello,
> I had something very similar to this some time ago
> https://lkml.org/lkml/2010/12/9/436

Sorry, I did not see that patch.

> However, I'm not sure that this check is sufficient.

Why are you think this is not sufficient?
If an instance try to write that specific inode to an physical device
which is not longer available how should we react then?

Another solution could be to clean up all instances referring to that
superblock in del_/unlink_gendisk(). But I think to check the return of
inode_to_bdi() is needed in any case.

regards

Andreas Bießmann
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/