Re: [PATCH] fs-writeback: fix NULL pointer dereference in __mark_inode_dirty

["Andreas Bießmann" <andreas.devel@googlemail.com>]

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@corscience.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