On Fri, Mar 10, 2017 at 11:19:24AM -0500, Chris Mason wrote: > On 03/09/2017 11:41 PM, Zygo Blaxell wrote: > >On Thu, Mar 09, 2017 at 10:39:49AM -0500, Chris Mason wrote: > >> > >> > >>On 03/08/2017 09:12 PM, Zygo Blaxell wrote: > >>>This is a story about 4 distinct (and very old) btrfs bugs. > >>> > >> > >>Really great write up. > >> > >>[ ... ] > >> > >>> > >>>diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c > >>>index 25ac2cf..4d41a31 100644 > >>>--- a/fs/btrfs/inode.c > >>>+++ b/fs/btrfs/inode.c > >>>@@ -6805,6 +6805,12 @@ static noinline int uncompress_inline(struct > >>>btrfs_path *path, > >>> max_size = min_t(unsigned long, PAGE_SIZE, max_size); > >>> ret = btrfs_decompress(compress_type, tmp, page, > >>> extent_offset, inline_size, max_size); > >>>+ WARN_ON(max_size + pg_offset > PAGE_SIZE); > >> > >>Can you please drop this WARN_ON and make the math reflect any possible > >>pg_offset? I do agree it shouldn't be happening, but its easy to correct > >>for and the WARN is likely to get lost. > > > >I'm not sure how to do that. It looks like I'd have to pass pg_offset > >through btrfs_decompress to the decompress functions? > > > > ret = btrfs_decompress(compress_type, tmp, page, > > extent_offset, inline_size, max_size, pg_offset); > > > >and in the compression functions get pg_offset from the argument list > >instead of hardcoding zero. > > Yeah, it's a good point. Both zlib and lzo are assuming a zero pg_offset > right now, but just like there are wacky corners allowing inline extents > followed by more data, there are a few wacky corners allowing inline extents > at the end of the file. > > Lets not mix that change in with this one though. For now, just get the > memset right and we can pass pg_offset down in a later patch.
Are you saying "fix the memset in the patch" (and if so, what's wrong with it?), or are you saying "let's take the patch with its memset as is, and fix the pg_offset > 0 issues later"? > -chris >
signature.asc
Description: Digital signature
