On 26/05/2023 18:40, Mickaël Salaün wrote:
On 21/05/2023 23:13, Richard Weinberger wrote:
----- Ursprüngliche Mail -----
Von: "Mickaël Salaün" <mic@xxxxxxxxxxx>
hostfs creates a new inode for each opened or created file, which created
useless inode allocations and forbade identifying a host file with a kernel
inode.
Fix this uncommon filesystem behavior by tying kernel inodes to host
file's inode and device IDs. Even if the host filesystem inodes may be
recycled, this cannot happen while a file referencing it is open, which
is the case with hostfs. It should be noted that hostfs inode IDs may
not be unique for the same hostfs superblock because multiple host's
(backed) superblocks may be used.
Delete inodes when dropping them to force backed host's file descriptors
closing.
This enables to entirely remove ARCH_EPHEMERAL_INODES, and then makes
Landlock fully supported by UML. This is very useful for testing
(ongoing and backported) changes.
Removing ARCH_EPHEMERAL_INODES should be a patch on its own, IMHO.
OK, I'll do that in the next series.
Well, I added ARCH_EPHEMERAL_INODES for Landlock specifically because of
this hostfs inconsistency, and it is not used by anything else in the
kernel: https://git.kernel.org/torvalds/c/cb2c7d1a1776
I then think it makes sense to remove this Kconfig option with the
hostfs change. Moreover, this protects against erroneously backporting
the ARCH_EPHEMERAL_INODES change, which would silently introduce a bug
for Landlock.
These changes also factor out and simplify some helpers thanks to the
new hostfs_inode_update() and the hostfs_iget() revamp: read_name(),
hostfs_create(), hostfs_lookup(), hostfs_mknod(), and
hostfs_fill_sb_common().
A following commit with new Landlock tests check this new hostfs inode
consistency.
Cc: Anton Ivanov <anton.ivanov@xxxxxxxxxxxxxxxxxx>
Cc: Johannes Berg <johannes@xxxxxxxxxxxxxxxx>
Cc: Richard Weinberger <richard@xxxxxx>
Cc: <stable@xxxxxxxxxxxxxxx> # 5.15.x: ce72750f04d6: hostfs: Fix writeback of
dirty pages
Cc: <stable@xxxxxxxxxxxxxxx> # 5.15+
I'm not sure whether this patch qualifies as stable material.
While I fully agree that the current behavoir is odd, nothing user visible
is really broken so far.
I added the ARCH_EPHEMERAL_INODES knob to avoid unexpected behavior.
Thanks to that there is no regression for Landlock, but it's unfortunate
that we could not use UML to test old kernel versions. According to this
odd behavior, I guess some user space may not work with hostfs because
of this issue, hence this Cc. I can remove it if you think it is not the
case.
Signed-off-by: Mickaël Salaün <mic@xxxxxxxxxxx>
Link: https://lore.kernel.org/r/20230309165455.175131-2-mic@xxxxxxxxxxx
Other than that, patch looks good to me.
Good, I'll send a new series with your suggestions.
Can I add your Signed-off-by to this patch (without touching
ARCH_EPHEMERAL_INODES changes, but removing the Cc stable)?
Are you OK for me to push this patch (with the whole series) in the
Landlock and next tree?
I'll send a new series splitting the Landlock tests to make a patch
dedicated to Landlock with hostfs tests (not backported), and with
another patch containing backportable and independent new Landlock FS tests.
