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: <CAHk-=wijWMpPk7feEZ8DzdLi7WLp_BhRpm+qgs6Tew1Bb2CmyQ@mail.gmail.com>
Date: Tue, 23 Jul 2024 18:14:54 -0700
From: Linus Torvalds <torvalds@...ux-foundation.org>
To: Maciej Żenczykowski <maze@...gle.com>, 
	Matthew Wilcox <willy@...radead.org>, Hongbo Li <lihongbo22@...wei.com>
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 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'm not even guessing at what might have been wrong in that mount API
conversion.

           Linus