* [PATCH] fs-writeback: fix NULL pointer dereference in __mark_inode_dirty @ 2011-02-28 15:25 Andreas Bießmann 2011-02-28 15:43 ` Sergey Senozhatsky 0 siblings, 1 reply; 12+ messages in thread From: Andreas Bießmann @ 2011-02-28 15:25 UTC (permalink / raw) To: linux-kernel; +Cc: Andreas Bießmann, Alexander Viro, linux-fsdevel This patch fixes a kernel NULL pointer dereference as mentioned in this log: ---8<--- [ 43.044000] mmc0: card c556 removed [ 43.059000] mmcblk0: error -123 sending status comand [ 43.064000] mmcblk0: error -123 sending read/write command, response 0x0, card status 0x0 [ 43.089000] mmcblk0: error -123 requesting status [ 43.096000] end_request: I/O error, dev mmcblk0, sector 1667989 <snip repeated error> [ 43.830000] end_request: I/O error, dev mmcblk0, sector 1667988 [ 44.679000] Unable to handle kernel NULL pointer dereference at virtual address 00000010 [ 44.688000] ptbr = 93ec0000 pgd = 93ebf000 [ 44.692000] Oops: Kernel access of bad area, sig: 11 [#1] [ 44.692000] FRAME_POINTER chip: 0x01f:0x1e82 rev 2 [ 44.692000] Modules linked in: [ 44.692000] PC is at __mark_inode_dirty+0x8a/0x11c [ 44.692000] LR is at __mark_inode_dirty+0x7c/0x11c <snip stack trace> [ 44.692000] Call trace: [ 44.692000] [<900780a4>] file_update_time+0x96/0xaa [ 44.692000] [<9005439a>] __generic_file_aio_write+0x212/0x330 [ 44.692000] [<900544f4>] generic_file_aio_write+0x3c/0x74 [ 44.692000] [<9006b82c>] do_sync_readv_writev+0x68/0x90 [ 44.692000] [<9006b8c0>] do_readv_writev+0x6c/0x108 [ 44.692000] [<9006b98a>] vfs_writev+0x2e/0x34 [ 44.692000] [<9006be60>] sys_writev+0x2c/0x4c [ 44.692000] [<90023132>] syscall_return+0x0/0x12 [ 44.692000] --->8--- 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); -- 1.7.2.3 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] fs-writeback: fix NULL pointer dereference in __mark_inode_dirty 2011-02-28 15:25 [PATCH] fs-writeback: fix NULL pointer dereference in __mark_inode_dirty Andreas Bießmann @ 2011-02-28 15:43 ` Sergey Senozhatsky 2011-02-28 15:59 ` Andreas Bießmann 0 siblings, 1 reply; 12+ messages in thread From: Sergey Senozhatsky @ 2011-02-28 15:43 UTC (permalink / raw) To: Andreas Bießmann; +Cc: linux-kernel, Alexander Viro, linux-fsdevel [-- Attachment #1: Type: text/plain, Size: 2552 bytes --] On (02/28/11 16:25), Andreas Bießmann wrote: > This patch fixes a kernel NULL pointer dereference as mentioned in this log: > > ---8<--- > [ 43.044000] mmc0: card c556 removed > [ 43.059000] mmcblk0: error -123 sending status comand > [ 43.064000] mmcblk0: error -123 sending read/write command, response 0x0, card status 0x0 > [ 43.089000] mmcblk0: error -123 requesting status > [ 43.096000] end_request: I/O error, dev mmcblk0, sector 1667989 > <snip repeated error> > [ 43.830000] end_request: I/O error, dev mmcblk0, sector 1667988 > [ 44.679000] Unable to handle kernel NULL pointer dereference at virtual address 00000010 > [ 44.688000] ptbr = 93ec0000 pgd = 93ebf000 > [ 44.692000] Oops: Kernel access of bad area, sig: 11 [#1] > [ 44.692000] FRAME_POINTER chip: 0x01f:0x1e82 rev 2 > [ 44.692000] Modules linked in: > [ 44.692000] PC is at __mark_inode_dirty+0x8a/0x11c > [ 44.692000] LR is at __mark_inode_dirty+0x7c/0x11c > <snip stack trace> > [ 44.692000] Call trace: > [ 44.692000] [<900780a4>] file_update_time+0x96/0xaa > [ 44.692000] [<9005439a>] __generic_file_aio_write+0x212/0x330 > [ 44.692000] [<900544f4>] generic_file_aio_write+0x3c/0x74 > [ 44.692000] [<9006b82c>] do_sync_readv_writev+0x68/0x90 > [ 44.692000] [<9006b8c0>] do_readv_writev+0x6c/0x108 > [ 44.692000] [<9006b98a>] vfs_writev+0x2e/0x34 > [ 44.692000] [<9006be60>] sys_writev+0x2c/0x4c > [ 44.692000] [<90023132>] syscall_return+0x0/0x12 > [ 44.692000] > --->8--- > > 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 However, I'm not sure that this check is sufficient. Sergey [-- Attachment #2: Type: application/pgp-signature, Size: 316 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] fs-writeback: fix NULL pointer dereference in __mark_inode_dirty 2011-02-28 15:43 ` Sergey Senozhatsky @ 2011-02-28 15:59 ` Andreas Bießmann 2011-02-28 16:29 ` Sergey Senozhatsky 0 siblings, 1 reply; 12+ messages in thread From: Andreas Bießmann @ 2011-02-28 15:59 UTC (permalink / raw) To: Sergey Senozhatsky Cc: Andreas Bießmann, linux-kernel, Alexander Viro, linux-fsdevel 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] fs-writeback: fix NULL pointer dereference in __mark_inode_dirty 2011-02-28 15:59 ` Andreas Bießmann @ 2011-02-28 16:29 ` Sergey Senozhatsky [not found] ` <AANLkTimARkrtmBBgtXmNA=MOD94FWQ8x-qcmLJ8mdQ6o@mail.gmail.com> 0 siblings, 1 reply; 12+ messages in thread From: Sergey Senozhatsky @ 2011-02-28 16:29 UTC (permalink / raw) To: Andreas Bießmann; +Cc: linux-kernel, Alexander Viro, linux-fsdevel [-- Attachment #1: Type: text/plain, Size: 2346 bytes --] On (02/28/11 16:59), Andreas Bießmann wrote: > 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. > No problem :-) > > 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? > I think the path which `kills' the device should take care of that. Otherwise, IMHO, we have: - ok, we're falling on line 42 -- let's fix line 42 ignoring the fact that there are reasons which led to faulty line 42, which are, for example: #0 604 spin_lock(&sb_lock); 605 list_for_each_entry(sb, &super_blocks, s_list) { 606 if (sb->s_bdi == bdi) 607 sb->s_bdi = NULL; 608 } 609 spin_unlock(&sb_lock); #1 bdi_prune_sb #2 bdi_unregister #3 del_gendisk Of course, I may be wrong. > 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. > Sergey [-- Attachment #2: Type: application/pgp-signature, Size: 316 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
[parent not found: <AANLkTimARkrtmBBgtXmNA=MOD94FWQ8x-qcmLJ8mdQ6o@mail.gmail.com>]
* Re: [PATCH] fs-writeback: fix NULL pointer dereference in __mark_inode_dirty [not found] ` <AANLkTimARkrtmBBgtXmNA=MOD94FWQ8x-qcmLJ8mdQ6o@mail.gmail.com> @ 2011-03-02 8:35 ` Andreas Bießmann 2011-03-03 13:58 ` Andreas Bießmann 2011-03-17 21:04 ` Andrew Morton 0 siblings, 2 replies; 12+ messages in thread From: Andreas Bießmann @ 2011-03-02 8:35 UTC (permalink / raw) To: Jason A. Donenfeld Cc: Sergey Senozhatsky, Andreas Bießmann, linux-kernel, linux-fsdevel Dear Jason A. Donenfeld, Am 01.03.2011 10:00, schrieb Jason A. Donenfeld: > Can you make an isolated test case to trigger this bug? in my case it is easily reproduceable. I have an SD-card in our embedded device (AVR32 AP7000). Some random data is continuously written to an FAT filesystem on that device. When you pull the card out of the slot you trigger that NULL pointer dereference. I will try to reproduce that error on my workstation but this will need some time. Maybe I can not hit that race on my quad core workstation but I will give it a try. regards Andreas Bießmann -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] fs-writeback: fix NULL pointer dereference in __mark_inode_dirty 2011-03-02 8:35 ` Andreas Bießmann @ 2011-03-03 13:58 ` Andreas Bießmann 2011-03-17 21:04 ` Andrew Morton 1 sibling, 0 replies; 12+ messages in thread From: Andreas Bießmann @ 2011-03-03 13:58 UTC (permalink / raw) To: Andreas Bießmann Cc: Jason A. Donenfeld, Sergey Senozhatsky, linux-kernel, linux-fsdevel Dear Jason A. Donenfeld, Am 02.03.2011 09:35, schrieb Andreas Bießmann: > Dear Jason A. Donenfeld, > > Am 01.03.2011 10:00, schrieb Jason A. Donenfeld: >> Can you make an isolated test case to trigger this bug? > > in my case it is easily reproduceable. I have an SD-card in our embedded > device (AVR32 AP7000). Some random data is continuously written to an > FAT filesystem on that device. When you pull the card out of the slot > you trigger that NULL pointer dereference. > > I will try to reproduce that error on my workstation but this will need > some time. Maybe I can not hit that race on my quad core workstation but > I will give it a try. unfortunately I can not reproduce this error on my workstation. I tested an 35 in 1 USB card reader and a single USB stick. I will try to test on another architecture uniprocessor system (e.g. one of our ARM eval boards). regards Andreas Bießmann -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] fs-writeback: fix NULL pointer dereference in __mark_inode_dirty 2011-03-02 8:35 ` Andreas Bießmann 2011-03-03 13:58 ` Andreas Bießmann @ 2011-03-17 21:04 ` Andrew Morton 2011-03-17 21:06 ` George Spelvin 2011-03-17 22:19 ` Sergey Senozhatsky 1 sibling, 2 replies; 12+ messages in thread From: Andrew Morton @ 2011-03-17 21:04 UTC (permalink / raw) To: Andreas Bießmann Cc: Jason A. Donenfeld, Sergey Senozhatsky, linux-kernel, linux-fsdevel, Jens Axboe, Christoph Hellwig, Anton Altaparmakov, George Spelvin On Wed, 02 Mar 2011 09:35:55 +0100 "Andreas Bie__mann" <andreas.devel@googlemail.com> wrote: > Dear Jason A. Donenfeld, > > Am 01.03.2011 10:00, schrieb Jason A. Donenfeld: > > Can you make an isolated test case to trigger this bug? > > in my case it is easily reproduceable. I have an SD-card in our embedded > device (AVR32 AP7000). Some random data is continuously written to an > FAT filesystem on that device. When you pull the card out of the slot > you trigger that NULL pointer dereference. > > I will try to reproduce that error on my workstation but this will need > some time. Maybe I can not hit that race on my quad core workstation but > I will give it a try. > afaik this regression didn't get fixed. Jens put out a patch for George to test but there hasn't been any feedback on that yet. Could you guys please give it a spin? From: Jens Axboe <axboe@kernel.dk> When we move the potential dirty list entries to the default_backing_dev_info, reassign the sb->s_bdi as well. default_backing_dev_info will always be around. I hope this can fix it up for 2.6.38 and we can add the proper ref counting for .39. Cc: Anton Altaparmakov <aia21@cam.ac.uk> Cc: George Spelvin <linux@horizon.com> Cc: Christoph Hellwig <hch@lst.de> Cc: Andreas Biemann <biessmann@corscience.de> Cc: Sergey Senozhatsky <sergey.senozhatsky@gmail.com> Tested-by: Torsten Hilbrich <torsten.hilbrich@secunet.com> Cc: <stable@kernel.org> [2.6.38.x] Signed-off-by: Andrew Morton <akpm@linux-foundation.org> --- fs/super.c | 2 ++ fs/sync.c | 4 ++-- mm/backing-dev.c | 2 +- 3 files changed, 5 insertions(+), 3 deletions(-) diff -puN fs/super.c~vfs-fix-null-pointer-oops-in-sync_inodes_sb fs/super.c --- a/fs/super.c~vfs-fix-null-pointer-oops-in-sync_inodes_sb +++ a/fs/super.c @@ -72,6 +72,7 @@ static struct super_block *alloc_super(s #else INIT_LIST_HEAD(&s->s_files); #endif + s->s_bdi = &default_backing_dev_info; INIT_LIST_HEAD(&s->s_instances); INIT_HLIST_BL_HEAD(&s->s_anon); INIT_LIST_HEAD(&s->s_inodes); @@ -1006,6 +1007,7 @@ vfs_kern_mount(struct file_system_type * } BUG_ON(!mnt->mnt_sb); WARN_ON(!mnt->mnt_sb->s_bdi); + WARN_ON(mnt->mnt_sb->s_bdi == &default_backing_dev_info); mnt->mnt_sb->s_flags |= MS_BORN; error = security_sb_kern_mount(mnt->mnt_sb, flags, secdata); diff -puN fs/sync.c~vfs-fix-null-pointer-oops-in-sync_inodes_sb fs/sync.c --- a/fs/sync.c~vfs-fix-null-pointer-oops-in-sync_inodes_sb +++ a/fs/sync.c @@ -33,7 +33,7 @@ static int __sync_filesystem(struct supe * This should be safe, as we require bdi backing to actually * write out data in the first place */ - if (!sb->s_bdi || sb->s_bdi == &noop_backing_dev_info) + if (sb->s_bdi == &noop_backing_dev_info) return 0; if (sb->s_qcop && sb->s_qcop->quota_sync) @@ -79,7 +79,7 @@ EXPORT_SYMBOL_GPL(sync_filesystem); static void sync_one_sb(struct super_block *sb, void *arg) { - if (!(sb->s_flags & MS_RDONLY) && sb->s_bdi) + if (!(sb->s_flags & MS_RDONLY)) __sync_filesystem(sb, *(int *)arg); } /* diff -puN mm/backing-dev.c~vfs-fix-null-pointer-oops-in-sync_inodes_sb mm/backing-dev.c --- a/mm/backing-dev.c~vfs-fix-null-pointer-oops-in-sync_inodes_sb +++ a/mm/backing-dev.c @@ -598,7 +598,7 @@ static void bdi_prune_sb(struct backing_ spin_lock(&sb_lock); list_for_each_entry(sb, &super_blocks, s_list) { if (sb->s_bdi == bdi) - sb->s_bdi = NULL; + sb->s_bdi = &default_backing_dev_info; } spin_unlock(&sb_lock); } _ btw, Christoph: would this not have been be a less hacky hack? --- a/fs/fs-writeback.c~a +++ a/fs/fs-writeback.c @@ -73,7 +73,7 @@ static inline struct backing_dev_info *i { struct super_block *sb = inode->i_sb; - if (strcmp(sb->s_type->name, "bdev") == 0) + if (sb == blockdev_superblock) return inode->i_mapping->backing_dev_info; return sb->s_bdi; _ ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] fs-writeback: fix NULL pointer dereference in __mark_inode_dirty 2011-03-17 21:04 ` Andrew Morton @ 2011-03-17 21:06 ` George Spelvin 2011-03-17 22:19 ` Sergey Senozhatsky 1 sibling, 0 replies; 12+ messages in thread From: George Spelvin @ 2011-03-17 21:06 UTC (permalink / raw) To: akpm, andreas.devel Cc: aia21, axboe, hch, Jason, linux-fsdevel, linux-kernel, linux, sergey.senozhatsky > afaik this regression didn't get fixed. Jens put out a patch for > George to test but there hasn't been any feedback on that yet. Could > you guys please give it a spin? I'm running with it without problems, but the bug was hard (for me) to trigger before. So it's a very weak "Works For Me." ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] fs-writeback: fix NULL pointer dereference in __mark_inode_dirty 2011-03-17 21:04 ` Andrew Morton 2011-03-17 21:06 ` George Spelvin @ 2011-03-17 22:19 ` Sergey Senozhatsky 1 sibling, 0 replies; 12+ messages in thread From: Sergey Senozhatsky @ 2011-03-17 22:19 UTC (permalink / raw) To: Andrew Morton Cc: Andreas Bießmann, Jason A. Donenfeld, linux-kernel, linux-fsdevel, Jens Axboe, Christoph Hellwig, Anton Altaparmakov, George Spelvin [-- Attachment #1: Type: text/plain, Size: 3679 bytes --] Hello, On (03/17/11 14:04), Andrew Morton wrote: >[..] > afaik this regression didn't get fixed. Jens put out a patch for > George to test but there hasn't been any feedback on that yet. Could > you guys please give it a spin? > Sorry for rather long reply. Seem to work fine for me. Tested-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com> Thanks, Sergey > From: Jens Axboe <axboe@kernel.dk> > > When we move the potential dirty list entries to the > default_backing_dev_info, reassign the sb->s_bdi as well. > default_backing_dev_info will always be around. I hope this can fix it up > for 2.6.38 and we can add the proper ref counting for .39. > > Cc: Anton Altaparmakov <aia21@cam.ac.uk> > Cc: George Spelvin <linux@horizon.com> > Cc: Christoph Hellwig <hch@lst.de> > Cc: Andreas Biemann <biessmann@corscience.de> > Cc: Sergey Senozhatsky <sergey.senozhatsky@gmail.com> > Tested-by: Torsten Hilbrich <torsten.hilbrich@secunet.com> > Cc: <stable@kernel.org> [2.6.38.x] > Signed-off-by: Andrew Morton <akpm@linux-foundation.org> > --- > > fs/super.c | 2 ++ > fs/sync.c | 4 ++-- > mm/backing-dev.c | 2 +- > 3 files changed, 5 insertions(+), 3 deletions(-) > > diff -puN fs/super.c~vfs-fix-null-pointer-oops-in-sync_inodes_sb fs/super.c > --- a/fs/super.c~vfs-fix-null-pointer-oops-in-sync_inodes_sb > +++ a/fs/super.c > @@ -72,6 +72,7 @@ static struct super_block *alloc_super(s > #else > INIT_LIST_HEAD(&s->s_files); > #endif > + s->s_bdi = &default_backing_dev_info; > INIT_LIST_HEAD(&s->s_instances); > INIT_HLIST_BL_HEAD(&s->s_anon); > INIT_LIST_HEAD(&s->s_inodes); > @@ -1006,6 +1007,7 @@ vfs_kern_mount(struct file_system_type * > } > BUG_ON(!mnt->mnt_sb); > WARN_ON(!mnt->mnt_sb->s_bdi); > + WARN_ON(mnt->mnt_sb->s_bdi == &default_backing_dev_info); > mnt->mnt_sb->s_flags |= MS_BORN; > > error = security_sb_kern_mount(mnt->mnt_sb, flags, secdata); > diff -puN fs/sync.c~vfs-fix-null-pointer-oops-in-sync_inodes_sb fs/sync.c > --- a/fs/sync.c~vfs-fix-null-pointer-oops-in-sync_inodes_sb > +++ a/fs/sync.c > @@ -33,7 +33,7 @@ static int __sync_filesystem(struct supe > * This should be safe, as we require bdi backing to actually > * write out data in the first place > */ > - if (!sb->s_bdi || sb->s_bdi == &noop_backing_dev_info) > + if (sb->s_bdi == &noop_backing_dev_info) > return 0; > > if (sb->s_qcop && sb->s_qcop->quota_sync) > @@ -79,7 +79,7 @@ EXPORT_SYMBOL_GPL(sync_filesystem); > > static void sync_one_sb(struct super_block *sb, void *arg) > { > - if (!(sb->s_flags & MS_RDONLY) && sb->s_bdi) > + if (!(sb->s_flags & MS_RDONLY)) > __sync_filesystem(sb, *(int *)arg); > } > /* > diff -puN mm/backing-dev.c~vfs-fix-null-pointer-oops-in-sync_inodes_sb mm/backing-dev.c > --- a/mm/backing-dev.c~vfs-fix-null-pointer-oops-in-sync_inodes_sb > +++ a/mm/backing-dev.c > @@ -598,7 +598,7 @@ static void bdi_prune_sb(struct backing_ > spin_lock(&sb_lock); > list_for_each_entry(sb, &super_blocks, s_list) { > if (sb->s_bdi == bdi) > - sb->s_bdi = NULL; > + sb->s_bdi = &default_backing_dev_info; > } > spin_unlock(&sb_lock); > } > _ > > > btw, Christoph: would this not have been be a less hacky hack? > > --- a/fs/fs-writeback.c~a > +++ a/fs/fs-writeback.c > @@ -73,7 +73,7 @@ static inline struct backing_dev_info *i > { > struct super_block *sb = inode->i_sb; > > - if (strcmp(sb->s_type->name, "bdev") == 0) > + if (sb == blockdev_superblock) > return inode->i_mapping->backing_dev_info; > > return sb->s_bdi; > _ > [-- Attachment #2: Type: application/pgp-signature, Size: 316 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
[parent not found: <fa.8ND6HoGS9dKYUuniFCAoNq+1TFY@ifi.uio.no>]
[parent not found: <4D78AA72.3060202@secunet.com>]
[parent not found: <4D7F3CD5.9030105@secunet.com>]
* Re: [PATCH] fs-writeback: fix NULL pointer dereference in, __mark_inode_dirty [not found] ` <4D7F3CD5.9030105@secunet.com> @ 2011-03-15 12:11 ` Anton Altaparmakov 2011-03-15 12:35 ` Jens Axboe 2011-03-15 12:40 ` Torsten Hilbrich 0 siblings, 2 replies; 12+ messages in thread From: Anton Altaparmakov @ 2011-03-15 12:11 UTC (permalink / raw) To: Torsten Hilbrich Cc: LKML, Andreas Bießmann, Sergey Senozhatsky, Christoph Hellwig, Jens Axboe, Linus Torvalds, linux-fsdevel Hi, On 15 Mar 2011, at 10:17, Torsten Hilbrich wrote: > On 10.03.2011 11:39, Torsten Hilbrich wrote: >> I ran into the same problem and successfully applied your fix. >> >> I was able to reproduce this panic and bisected it to the following commit: >> >> commit aaead25b954879e1a708ff2f3602f494c18d20b5 >> Author: Christoph Hellwig <hch@lst.de> >> Date: Mon Oct 4 14:25:33 2010 +0200 >> >> writeback: always use sb->s_bdi for writeback purposes >> >> The steps to reproduce it on my test system (T60p with Intel Core Duo) were. > > Added Christoph to CC. I also open a bug report at https://bugzilla.kernel.org/show_bug.cgi?id=31112 This is already being handled. It is the same as other bug reports, i.e. the fact that sb->s_bdi is made NULL on device removal and if it happens at the wrong time you then get a NULL pointer dereference. Jens Axboe just only yesterday posted an initial patch for this. Can you please test it and report back if it does indeed cure the problem? The patch can be found here for example: https://lkml.org/lkml/2011/3/14/25 Best regards, Anton -- Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @) Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK Linux NTFS maintainer, http://www.linux-ntfs.org/ ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] fs-writeback: fix NULL pointer dereference in, __mark_inode_dirty 2011-03-15 12:11 ` [PATCH] fs-writeback: fix NULL pointer dereference in, __mark_inode_dirty Anton Altaparmakov @ 2011-03-15 12:35 ` Jens Axboe 2011-03-15 12:40 ` Torsten Hilbrich 1 sibling, 0 replies; 12+ messages in thread From: Jens Axboe @ 2011-03-15 12:35 UTC (permalink / raw) To: Anton Altaparmakov Cc: Torsten Hilbrich, LKML, Andreas Bießmann, Sergey Senozhatsky, Christoph Hellwig, Linus Torvalds, linux-fsdevel On 2011-03-15 13:11, Anton Altaparmakov wrote: > Hi, > > On 15 Mar 2011, at 10:17, Torsten Hilbrich wrote: >> On 10.03.2011 11:39, Torsten Hilbrich wrote: >>> I ran into the same problem and successfully applied your fix. >>> >>> I was able to reproduce this panic and bisected it to the following commit: >>> >>> commit aaead25b954879e1a708ff2f3602f494c18d20b5 >>> Author: Christoph Hellwig <hch@lst.de> >>> Date: Mon Oct 4 14:25:33 2010 +0200 >>> >>> writeback: always use sb->s_bdi for writeback purposes >>> >>> The steps to reproduce it on my test system (T60p with Intel Core Duo) were. >> >> Added Christoph to CC. I also open a bug report at https://bugzilla.kernel.org/show_bug.cgi?id=31112 > > This is already being handled. It is the same as other bug reports, > i.e. the fact that sb->s_bdi is made NULL on device removal and if it > happens at the wrong time you then get a NULL pointer dereference. > > Jens Axboe just only yesterday posted an initial patch for this. Can > you please test it and report back if it does indeed cure the problem? > > The patch can be found here for example: > > https://lkml.org/lkml/2011/3/14/25 Yes, any testing of that patch would be greatly appreciated. -- Jens Axboe ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] fs-writeback: fix NULL pointer dereference in, __mark_inode_dirty 2011-03-15 12:11 ` [PATCH] fs-writeback: fix NULL pointer dereference in, __mark_inode_dirty Anton Altaparmakov 2011-03-15 12:35 ` Jens Axboe @ 2011-03-15 12:40 ` Torsten Hilbrich 1 sibling, 0 replies; 12+ messages in thread From: Torsten Hilbrich @ 2011-03-15 12:40 UTC (permalink / raw) To: Anton Altaparmakov Cc: LKML, Andreas Bießmann, Sergey Senozhatsky, Christoph Hellwig, Jens Axboe, Linus Torvalds, linux-fsdevel On 15.03.2011 13:11, Anton Altaparmakov wrote: > This is already being handled. It is the same as other bug reports, i.e. the fact that sb->s_bdi is made NULL on device removal and if it happens at the wrong time you then get a NULL pointer dereference. > > Jens Axboe just only yesterday posted an initial patch for this. Can you please test it and report back if it does indeed cure the problem? > > The patch can be found here for example: > > https://lkml.org/lkml/2011/3/14/25 > This patch fixes the problem for my test szenario. Torsten ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2011-03-17 22:19 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-02-28 15:25 [PATCH] fs-writeback: fix NULL pointer dereference in __mark_inode_dirty Andreas Bießmann 2011-02-28 15:43 ` Sergey Senozhatsky 2011-02-28 15:59 ` Andreas Bießmann 2011-02-28 16:29 ` Sergey Senozhatsky [not found] ` <AANLkTimARkrtmBBgtXmNA=MOD94FWQ8x-qcmLJ8mdQ6o@mail.gmail.com> 2011-03-02 8:35 ` Andreas Bießmann 2011-03-03 13:58 ` Andreas Bießmann 2011-03-17 21:04 ` Andrew Morton 2011-03-17 21:06 ` George Spelvin 2011-03-17 22:19 ` Sergey Senozhatsky [not found] <fa.8ND6HoGS9dKYUuniFCAoNq+1TFY@ifi.uio.no> [not found] ` <4D78AA72.3060202@secunet.com> [not found] ` <4D7F3CD5.9030105@secunet.com> 2011-03-15 12:11 ` [PATCH] fs-writeback: fix NULL pointer dereference in, __mark_inode_dirty Anton Altaparmakov 2011-03-15 12:35 ` Jens Axboe 2011-03-15 12:40 ` Torsten Hilbrich
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).