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 majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Attachment:
signature.asc
Description: Digital signature
