From mboxrd@z Thu Jan 1 00:00:00 1970 From: "=?ISO-8859-15?Q?Andreas_Bie=DFmann?=" Subject: Re: [PATCH] fs-writeback: fix NULL pointer dereference in __mark_inode_dirty Date: Mon, 28 Feb 2011 16:59:34 +0100 Message-ID: <4D6BC666.4010603@gmail.com> References: <1298906733-31427-1-git-send-email-biessmann@corscience.de> <20110228154314.GA4675@swordfish.minsk.epam.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: =?ISO-8859-15?Q?Andreas_Bie=DFmann?= , linux-kernel@vger.kernel.org, Alexander Viro , linux-fsdevel@vger.kernel.org To: Sergey Senozhatsky Return-path: In-Reply-To: <20110228154314.GA4675@swordfish.minsk.epam.com> Sender: linux-kernel-owner@vger.kernel.org List-Id: linux-fsdevel.vger.kernel.org Dear Sergey Senozhatsky, Am 28.02.2011 16:43, schrieb Sergey Senozhatsky: > On (02/28/11 16:25), Andreas Bie=DFmann wrote: >> The reference to sb->s_bdi may be deleted from mmc_blk_remove() -> >> del_gendisk() -> unlink_gendisk() -> bdi_unregister() -> bdi_prune_s= b() while >> another instance try to write some data to the device. >> >> Signed-off-by: Andreas Bie=DFmann >> --- >> 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, i= nt flags) >> if (!was_dirty) { >> bdi =3D inode_to_bdi(inode); >> =20 >> + if (!bdi) >> + goto out; >> + >> if (bdi_cap_writeback_dirty(bdi)) { >> WARN(!test_bit(BDI_registered, &bdi->state), >> "bdi-%s not registered\n", bdi->name); >=20 > 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=DFmann