Re: [PATCH][CFT] (updated) ext2 directories in pagecache

Daniel Phillips
2001-05-11T20:20:38Z

On Friday 11 May 2001 18:34, Andreas Dilger wrote:
> Al writes:
> > On Fri, 11 May 2001, Andreas Dilger wrote:
> > > I've tested again, now with kdb, and the system loops in
> > > ext2_find_entry() or ext2_add_link(), because there is a
> > > directory with a zero rec_len. While the actual cause of this
> > > problem is elsewhere, the fact that ext2_next_entry() will loop
> > > forever with a bad rec_len is a bug not in the old ext2 code.
> >
> > No. Bug is that data ends up in pages without being validated.
> > That's the real thing to watch for - if ext2_get_page() is the only
> > way to get pages in cache you get all checks in one place and done
> > once.
>
> OK, I don't think that Daniel is aware of this, I wasn't either. He
> is using ext2_bread() modified to access the page cache instead of
> the buffer cache.

Oh yes, I'm well aware it, that's what I mean by the "bullet proofing"
item on my to-do list. I don't quite agree with the idea of embedding
the checking of directory entry format inside the ext2_get_page
routine, it should be moved outside ext2_get_page, on basic principal.
Never mind that this routine is only used to get pages of directory
files, it's still not nice. Anyway, I think the thing to do is graft a
similar one-time check onto ext2_bread. We can't use Al's PG_checked
mechanism because we might have read only one buffer out of several on
the page.

> It turns out that in adding the checks for rec_len, I fixed my
> original bug, but added another... Please disregard my previous
> patch.

In any event, I couldn't apply it due to patch giving a 'malformed'
error - did you edit it after diffing? I'll wait for a revised patch -
there's quite a lot of stuff in there including formatting changes,
ext2 warnings, etc.

--
Daniel