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.
But how does pg_offset become non-zero for an inline extent? A micro-hole
before the first byte? If the offset was >= 4096, the data wouldn't
be in the first block so there would never be an inline extent in the
first place.
> >+ if (max_size + pg_offset < PAGE_SIZE) {
> >+ char *map = kmap(page);
> >+ memset(map + pg_offset + max_size, 0, PAGE_SIZE - max_size -
> >pg_offset);
> >+ kunmap(page);
> >+ }
>
> Both lzo and zlib have a memset to cover the gap between what they actually
> decompress and the max_size that we pass here. That's important because
> ram_bytes may not be 100% accurate.
>
> Can you also please toss in a comment about how the decompression code is
> responsible for the memset up to max_bytes?
>
> -chris
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
signature.asc
Description: Digital signature
