linux-kernel - Re: UML/hostfs - mount failure at tip of tree

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: <b84a6ef8-7c3b-4c04-81d3-859692d91137@huawei.com>
Date: Wed, 24 Jul 2024 09:34:47 +0800
From: Hongbo Li <lihongbo22@...wei.com>
To: Linus Torvalds <torvalds@...ux-foundation.org>,
	Maciej Żenczykowski <maze@...gle.com>, Matthew Wilcox
	<willy@...radead.org>
CC: Kernel hackers <linux-kernel@...r.kernel.org>, Patrick Rohr
	<prohr@...gle.com>, Christian Brauner <brauner@...nel.org>
Subject: Re: UML/hostfs - mount failure at tip of tree



On 2024/7/24 9:14, Linus Torvalds wrote:
> On Tue, 23 Jul 2024 at 15:33, Maciej Żenczykowski <maze@...gle.com> wrote:
>>
>> Reverting the following 3 patches:
>> - 104eef133fd9 hostfs: Add const qualifier to host_root in hostfs_fill_super()
>> - cd140ce9f611 hostfs: convert hostfs to use the new mount API
>> - e3ec0fe944d2 hostfs: Convert hostfs_read_folio() to use a folio
>>
>> appears to be necessary to get the Android net test framework to boot
>> with tip of tree,
>> *without* the reverts we get:
>>    mount: /host: special device hostfs does not exist.
>> (if I don't revert the folio change then it mounts, but appears to not
>> actually work)
> 
> Interesting. That folio change was clearly supposed to be a no-op, but
> isn't. Which makes a revert the right thing to do regardless.
> 
> That code was odd before too, but clearly that commit is completely broken.
> 
> I think this part is buggy:
> 
>                  buffer = folio_zero_tail(folio, bytes_read, buffer);
> 
> because while the documentation for folio_zero_tail() does imply that
> usage, the third argument is supposed really looks like it should be
> "buffer + bytes_read".
> 
> So  instead of reverting that commit, does it help to just do that instead:
> 
> -               buffer = folio_zero_tail(folio, bytes_read, buffer);
> +               buffer = folio_zero_tail(folio, bytes_read, buffer +
> bytes_read);
> 
> Willy, that function is really bad. It's not helpful when it
> apparently confused even you, and the calling convention really is
> broken. I think that folio_zero_tail() needs to be rewritten to have
> sane calling conventions (like matching the docs!) or just die.
> 
> The mount API change is somethign else. Again, it wasn't supposed to
> break anything, but clearly does, and so reverting it sounds sane
> unless somebody sees what the problem is.
> 
I apologize for causing this issue. I am currently tracking it down.  If 
reverting this can solve the problem, you can revert it first.

Thanks,
Hongbo

> I'm not even guessing at what might have been wrong in that mount API
> conversion.
> 
>             Linus