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: <4a8ae74e-2f90-4da8-9511-325ca6f67aa6@huawei.com>
Date: Wed, 24 Jul 2024 17:49:16 +0800
From: Hongbo Li <lihongbo22@...wei.com>
To: Maciej Żenczykowski <maze@...gle.com>, Christian Brauner
	<brauner@...nel.org>
CC: Matthew Wilcox <willy@...radead.org>, Kernel hackers
	<linux-kernel@...r.kernel.org>, Patrick Rohr <prohr@...gle.com>, Linus
 Torvalds <torvalds@...ux-foundation.org>
Subject: Re: UML/hostfs - mount failure at tip of tree



On 2024/7/24 11:59, Maciej Żenczykowski wrote:
> On Tue, Jul 23, 2024 at 7:55 PM Maciej Żenczykowski <maze@...gle.com> wrote:
>>
>> On Tue, Jul 23, 2024 at 7:22 PM Linus Torvalds
>> <torvalds@...ux-foundation.org> wrote:
>>>
>>> On Tue, 23 Jul 2024 at 18:35, Hongbo Li <lihongbo22@...wei.com> wrote:
>>>>
>>>> I apologize for causing this issue. I am currently tracking it down.  If
>>>> reverting this can solve the problem, you can revert it first.
>>>
>>> I don't get the feeling that this is _so_ urgent that it needs to be
>>> reverted immediately - let's give it at least a few days and see if
>>> you (or somebody else) figures out the bug.
>>>
>>> Maciej - if you can verify that folio conversion fix suggestion of
>>> mine (or alternatively report that it doesn't help and I was barking
>>> up the wrong tree), that would be great.
>>
>> That appears to fix the folio patch indeed (ie. I no longer need to revert it).
>>
>> The tests are still super unhappy, but I've yet to fix our tests very
>> broken netlink parser for changes that released in 6.10, so that may
>> be unrelated ;-)
> 
> +++ fs/hostfs/hostfs_kern.c:
>   static int hostfs_fill_super(struct super_block *sb, struct fs_context *fc)
>   {
>          struct hostfs_fs_info *fsi = sb->s_fs_info;
> -       const char *host_root = fc->source;
> +       const char *host_root = "/";
> 
This doesn't work in case where the host directory is designated (such 
as mount -t hostfs hostfs -o /home /host).

I can fix this by the following patch, the root cause of this issue is 
the incorrect parsing of the host directory. The original mount path 
will use `parse_monolithic` to parse the host directory. For the new 
mount api, it use `parse_param` directly. So we should call 
`fsconfig(fd, FSCONFIG_SET_STRING, "hostfs", "xxx", 0)` to mount the 
hostfs(I think may be we should add hostfs as the key for host 
directory.). This may need Christian's reviews.:

```
 From e7cc3be86a01b8382e9510f6ae1a2764942c7cba Mon Sep 17 00:00:00 2001
From: Hongbo Li <lihongbo22@...wei.com>
Date: Wed, 24 Jul 2024 16:08:32 +0800
Subject: [PATCH] hostfs: fix the host directory parse when mounting.

hostfs not keep the host directory when mounting. When the host
directory is none (default), fc->source is used as the host root
directory, and this is wrong. Here we use `parse_monolithic` to
handle the old mount path for parsing the root directory. For new
mount path, The `parse_param` is used for the host directory parse.

Fixes: cd140ce9f611 ("hostfs: convert hostfs to use the new mount API")
Signed-off-by: Hongbo Li <lihongbo22@...wei.com>
---
  fs/hostfs/hostfs_kern.c | 64 ++++++++++++++++++++++++++++++++++-------
  1 file changed, 54 insertions(+), 10 deletions(-)

diff --git a/fs/hostfs/hostfs_kern.c b/fs/hostfs/hostfs_kern.c
index 3eb747d26924..205c3700a035 100644
--- a/fs/hostfs/hostfs_kern.c
+++ b/fs/hostfs/hostfs_kern.c
@@ -17,6 +17,7 @@
  #include <linux/writeback.h>
  #include <linux/mount.h>
  #include <linux/fs_context.h>
+#include <linux/fs_parser.h>
  #include <linux/namei.h>
  #include "hostfs.h"
  #include <init.h>
@@ -927,7 +928,6 @@ static const struct inode_operations 
hostfs_link_iops = {
  static int hostfs_fill_super(struct super_block *sb, struct fs_context 
*fc)
  {
  	struct hostfs_fs_info *fsi = sb->s_fs_info;
-	const char *host_root = fc->source;
  	struct inode *root_inode;
  	int err;

@@ -941,15 +941,6 @@ static int hostfs_fill_super(struct super_block 
*sb, struct fs_context *fc)
  	if (err)
  		return err;

-	/* NULL is printed as '(null)' by printf(): avoid that. */
-	if (fc->source == NULL)
-		host_root = "";
-
-	fsi->host_root_path =
-		kasprintf(GFP_KERNEL, "%s/%s", root_ino, host_root);
-	if (fsi->host_root_path == NULL)
-		return -ENOMEM;
-
  	root_inode = hostfs_iget(sb, fsi->host_root_path);
  	if (IS_ERR(root_inode))
  		return PTR_ERR(root_inode);
@@ -975,6 +966,57 @@ static int hostfs_fill_super(struct super_block 
*sb, struct fs_context *fc)
  	return 0;
  }

+enum hostfs_parma {
+	Opt_hostfs,
+};
+
+static const struct fs_parameter_spec hostfs_param_specs[] = {
+	fsparam_string_empty("hostfs",		Opt_hostfs),
+	{}
+};
+
+static int hostfs_parse_param(struct fs_context *fc, struct 
fs_parameter *param)
+{
+	struct hostfs_fs_info *fsi = fc->s_fs_info;
+	struct fs_parse_result result;
+	char *host_root;
+	int opt;
+
+	opt = fs_parse(fc, hostfs_param_specs, param, &result);
+	if (opt < 0)
+		return opt;
+
+	switch (opt) {
+	case Opt_hostfs:
+		host_root = param->string;
+		if (!host_root)
+			host_root = "";
+		fsi->host_root_path =
+			kasprintf(GFP_KERNEL, "%s/%s", root_ino, host_root);
+		if (fsi->host_root_path == NULL)
+			return -ENOMEM;
+		break;
+	}
+
+	return 0;
+}
+static int hostfs_parse_monolithic(struct fs_context *fc, void *data)
+{
+	struct hostfs_fs_info *fsi = fc->s_fs_info;
+	char *host_root = (char *)data;
+
+	/* NULL is printed as '(null)' by printf(): avoid that. */
+	if (host_root == NULL)
+		host_root = "";
+
+	fsi->host_root_path =
+		kasprintf(GFP_KERNEL, "%s/%s", root_ino, host_root);
+	if (fsi->host_root_path == NULL)
+		return -ENOMEM;
+
+	return 0;
+}
+
  static int hostfs_fc_get_tree(struct fs_context *fc)
  {
  	return get_tree_nodev(fc, hostfs_fill_super);
@@ -992,6 +1034,8 @@ static void hostfs_fc_free(struct fs_context *fc)
  }

  static const struct fs_context_operations hostfs_context_ops = {
+	.parse_monolithic = hostfs_parse_monolithic,
+	.parse_param	= hostfs_parse_param,
  	.get_tree	= hostfs_fc_get_tree,
  	.free		= hostfs_fc_free,
  };
-- 
2.34.1
```

Thanks,
Hongbo

> appears to fix the problem (when combined with Linus' folio fix).
> 
> I think fc->source is just the 'block device' passed to mount, and
> thus for a virtual filesystem like hostfs, it is just garbage...
> 
> (and with the appropriate netlink fixes all the tests now pass at tip-of-tree:
> 87f3073c2871 (HEAD) hostfs_fill_super(): host_root := "/" (not fc->source)
> 2743a4aabac6 fs/hostfs/hostfs_kern.c:445 buffer =
> folio_zero_tail(folio, bytes_read, buffer + bytes_read);
> a2caf678d7e1 neighbour: add RTNL_FLAG_DUMP_SPLIT_NLM_DONE to RTM_GETNEIGH
> 3bb0c5772acf net: add RTNL_FLAG_DUMP_SPLIT_NLM_DONE to RTM_GET(RULE|ROUTE)
> 786c8248dbd3 (linux/master) Merge tag
> 'perf-tools-fixes-for-v6.11-2024-07-23' of
> git://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools
> )
> 
>>> And perhaps remind me about this mount API thing too if it doesn't
>>> seem to be resolved by the end of the week when I'm starting to get
>>> ready to do the rc1?
>>>
>>>               Linus
>>
>> --
>> Maciej Żenczykowski, Kernel Networking Developer @ Google
> 
> --
> Maciej Żenczykowski, Kernel Networking Developer @ Google
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ