lkml.org 
[lkml]   [2008]   [Sep]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: __getblk infinite loop
hi,

* Andrew Morton (akpm@linux-foundation.org) wrote:
> On Thu, 4 Sep 2008 23:24:11 -0400 Bob Copeland <me@bobcopeland.com> wrote:
>
> > Hi all,
> >
> > Eric Sesterhenn and I were puzzling over a lockup found by his fsfuzzer.
> >
> > sb_bread() calls __getblk, which says:
> >
> > /*
> > * __getblk will locate (and, if necessary, create) the buffer_head
> > * which corresponds to the passed block_device, block and size. The
> > * returned buffer has its reference count incremented.
> > *
> > * __getblk() cannot fail - it just keeps trying. If you pass it an
> > * illegal block number, __getblk() will happily return a buffer_head
> > * which represents the non-existent block. Very weird.
> > *
> > * __getblk() will lock up the machine if grow_dev_page's try_to_free_buffers()
> > * attempt is failing. FIXME, perhaps?
> > */
> >
> > In fact the following will cause an infinite loop when mounting omfs
> > loopback (on 32 bit x86 at least):
> >
> > diff --git a/fs/omfs/inode.c b/fs/omfs/inode.c
> > index a95fe59..80eacc8 100644
> > --- a/fs/omfs/inode.c
> > +++ b/fs/omfs/inode.c
> > @@ -413,6 +413,15 @@ static int omfs_fill_super(struct super_block *sb, void *data, int silent)
> > sector_t start;
> > int ret = -EINVAL;
> >
> > + if (1) {
> > + sector_t foo = 0x1d4000004ULL;
> > +
> > + sb_set_blocksize(sb, 2048);
> > + bh = sb_bread(sb, foo);
> > + brelse(bh);
> > + goto end;
> > + }
> > +
> > save_mount_options(sb, (char *) data);
> >
> > sbi = kzalloc(sizeof(struct omfs_sb_info), GFP_KERNEL);
> >
> > What's supposed to happen here? I would have thought that sb_bread
> > would realize foo was outside the block dev and bail out, but instead
> > it just gets stuck. Do I need to bounds-check anything passed to
> > sb_bread?
>
> That loop does lock up on people occasionally - last time was in isofs,
> because it had done an insane set_blocksize() earlier on.
>
> Yes, it's always a case of garbage in, garbage out (or nothing out, as
> the case may be).
>
> No, it's not particularly programmer-friendly behaviour.

Wouldnt it make sense to limit the loop in __getblk_slow()?

Like only try five times and drop a warning?
Yesterday I changed free_more_memory() to
return the number of pages freed by try_to_free_pages() and abort
if no pages where freed. But it seems it always frees exactly one
page...

Greetings, Eric


\
 
 \ /
  Last update: 2008-09-05 11:23    [W:0.064 / U:6.228 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site