linux-kernel - Re: [GIT PULL] Btrfs updates for 5.16

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <351dac77-310c-40f5-b9c0-ce7f03f723f9@gmx.com>
Date:   Tue, 2 Nov 2021 08:08:22 +0800
From:   Qu Wenruo <quwenruo.btrfs@....com>
To:     Linus Torvalds <torvalds@...ux-foundation.org>,
        David Sterba <dsterba@...e.com>, Qu Wenruo <wqu@...e.com>
Cc:     linux-btrfs <linux-btrfs@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [GIT PULL] Btrfs updates for 5.16



On 2021/11/2 04:03, Linus Torvalds wrote:
> On Mon, Nov 1, 2021 at 9:46 AM David Sterba <dsterba@...e.com> wrote:
>>
>> There's a merge conflict due to the last minute 5.15 changes (kmap
>> reverts) and the conflict is not trivial.
>
> You don't say.
>
> I ended up just re-doing that resolution entirely, and as I did so, I
> think I found a bug in the original revert that caused the conflict in
> the first place.
>
> And since that revert made it into 5.15, I felt like I had to fix that
> bug first - and separately - so that the fix can be backported to
> stable.
>
> I then re-did my merge on top of that hopefully fixed state, and maybe
> it's correct.
>
> Or maybe I messed up entirely.
>
> I did end up comparing it to your other branch too, but that was
> equally as messy, apart from the "ok, I can mindlessly just take your
> side".
>
> And it was fairly different from what I had done in my merge
> resolution, so who knows.
>
> ANYWAY. What I'm trying to say is that you should look very very
> carefully at commits
>
>    2cf3f8133bda ("btrfs: fix lzo_decompress_bio() kmap leakage")

Since I'm doing the revert manually for lzo part, I double checked the code.

It turns out, your fix is the same as the original version I sent to
David (although not through the mail list).
Full patch attached.

@@ -345,8 +358,9 @@ int lzo_decompress_bio(struct list_head *ws, struct
compressed_bio *cb)
  		       (cur_in + LZO_LEN - 1) / sectorsize);
  		cur_page = cb->compressed_pages[cur_in / PAGE_SIZE];
  		ASSERT(cur_page);
-		seg_len = read_compress_length(page_address(cur_page) +
-					       offset_in_page(cur_in));
+		kaddr = kmap(cur_page);
+		seg_len = read_compress_length(kaddr + offset_in_page(cur_in));
+		kunmap(cur_page);
  		cur_in += LZO_LEN;

Thus it looks like by somehow my version is not applied?

Thanks,
Qu

>    037c50bfbeb3 ("Merge tag 'for-5.16-tag' of git://git.../linux") >
> because I marked that first one for stable, and the second is
> obviously my entirely untested merge.
>
> It makes sense to me, but apart from "it builds", I've not actually
> tested any of it. This is all purely from looking at the code and
> trying to figure out what the RightThing(tm) is.
>
> Obviously the kmap thing tends to only be noticeable on 32-bit
> platforms, and that lzo_decompress_bio() bug also needs just the
> proper filesystem settings to trigger in the first place.
>
> Again - please take a careful look. Both at my merge and at that
> alleged kmap fix.
>
>                            Linus
>

View attachment "0001-Revert-btrfs-compression-drop-kmap-kunmap-from-lzo.patch" of type "text/x-patch" (4997 bytes)