[PATCH] autofs: fix add autofs_parse_fd()

[PATCH] autofs: fix add autofs_parse_fd()

[Ian Kent]

We are seeing systemd hang on its autofs direct mount at
/proc/sys/fs/binfmt_misc.

Historically this was due to a mismatch in the communication structure
size between a 64 bit kernel and a 32 bit user space and was fixed by
making the pipe communication record oriented.

During autofs v5 development I decided to stay with the existing usage
instead of changing to a packed structure for autofs <=> user space
communications which turned out to be a mistake on my part.

Problems arose and they were fixed by allowing for the 64 bit to 32
bit size difference in the automount(8) code.

Along the way systemd started to use autofs and eventually encountered
this problem too. systemd refused to compensate for the length
difference insisting it be fixed in the kernel. Fortunately Linus
implemented the packetized pipe which resolved the problem in a
straight forward and simple way.

In the autofs mount api conversion series I inadvertatly dropped the
packet pipe flag settings when adding the autofs_parse_fd() function.
This patch fixes that omission.

Fixes: 546694b8f658 ("autofs: add autofs_parse_fd()")
Signed-off-by: Ian Kent <raven@themaw.net>
Cc: Bill O'Donnell <bodonnel@redhat.com>
Cc: Christian Brauner <brauner@kernel.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Dan Carpenter <dan.carpenter@linaro.org>
Cc: Anders Roxell <anders.roxell@linaro.org>
Cc: Naresh Kamboju <naresh.kamboju@linaro.org>
Cc: Stephen Rothwell <sfr@canb.auug.org.au>
Reported-by: Linux Kernel Functional Testing <lkft@linaro.org>
Reported-by: Anders Roxell <anders.roxell@linaro.org>
---
 fs/autofs/autofs_i.h | 13 +++++++++----
 fs/autofs/inode.c    |  2 ++
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/fs/autofs/autofs_i.h b/fs/autofs/autofs_i.h
index 244f18cdf23c..8c1d587b3eef 100644
--- a/fs/autofs/autofs_i.h
+++ b/fs/autofs/autofs_i.h
@@ -221,15 +221,20 @@ static inline int autofs_check_pipe(struct file *pipe)
 	return 0;
 }
 
-static inline int autofs_prepare_pipe(struct file *pipe)
+static inline void autofs_set_packet_pipe_flags(struct file *pipe)
 {
-	int ret = autofs_check_pipe(pipe);
-	if (ret < 0)
-		return ret;
 	/* We want a packet pipe */
 	pipe->f_flags |= O_DIRECT;
 	/* We don't expect -EAGAIN */
 	pipe->f_flags &= ~O_NONBLOCK;
+}
+
+static inline int autofs_prepare_pipe(struct file *pipe)
+{
+	int ret = autofs_check_pipe(pipe);
+	if (ret < 0)
+		return ret;
+	autofs_set_packet_pipe_flags(pipe);
 	return 0;
 }
 
diff --git a/fs/autofs/inode.c b/fs/autofs/inode.c
index 6d2e01c9057d..a3d62acc293a 100644
--- a/fs/autofs/inode.c
+++ b/fs/autofs/inode.c
@@ -177,6 +177,8 @@ static int autofs_parse_fd(struct fs_context *fc, struct autofs_sb_info *sbi,
 		return -EBADF;
 	}
 
+	autofs_set_packet_pipe_flags(pipe);
+
 	if (sbi->pipe)
 		fput(sbi->pipe);
 
-- 
2.41.0

Re: [PATCH] autofs: fix add autofs_parse_fd()

[Anders Roxell]

On Mon, 23 Oct 2023 at 11:34, Ian Kent <raven@themaw.net> wrote:
>
> We are seeing systemd hang on its autofs direct mount at
> /proc/sys/fs/binfmt_misc.
>
> Historically this was due to a mismatch in the communication structure
> size between a 64 bit kernel and a 32 bit user space and was fixed by
> making the pipe communication record oriented.
>
> During autofs v5 development I decided to stay with the existing usage
> instead of changing to a packed structure for autofs <=> user space
> communications which turned out to be a mistake on my part.
>
> Problems arose and they were fixed by allowing for the 64 bit to 32
> bit size difference in the automount(8) code.
>
> Along the way systemd started to use autofs and eventually encountered
> this problem too. systemd refused to compensate for the length
> difference insisting it be fixed in the kernel. Fortunately Linus
> implemented the packetized pipe which resolved the problem in a
> straight forward and simple way.
>
> In the autofs mount api conversion series I inadvertatly dropped the
> packet pipe flag settings when adding the autofs_parse_fd() function.
> This patch fixes that omission.
>
> Fixes: 546694b8f658 ("autofs: add autofs_parse_fd()")
> Signed-off-by: Ian Kent <raven@themaw.net>
> Cc: Bill O'Donnell <bodonnel@redhat.com>
> Cc: Christian Brauner <brauner@kernel.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Dan Carpenter <dan.carpenter@linaro.org>
> Cc: Anders Roxell <anders.roxell@linaro.org>
> Cc: Naresh Kamboju <naresh.kamboju@linaro.org>
> Cc: Stephen Rothwell <sfr@canb.auug.org.au>
> Reported-by: Linux Kernel Functional Testing <lkft@linaro.org>
> Reported-by: Anders Roxell <anders.roxell@linaro.org>

Thank you Ian for finding the issue so quickly.

Tested-by: Anders Roxell <anders.roxell@linaro.org>

I built todays next-20231023 with this patch and the kernel booted up
fine in qemu.


Cheers,
Anders

> ---
>  fs/autofs/autofs_i.h | 13 +++++++++----
>  fs/autofs/inode.c    |  2 ++
>  2 files changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/fs/autofs/autofs_i.h b/fs/autofs/autofs_i.h
> index 244f18cdf23c..8c1d587b3eef 100644
> --- a/fs/autofs/autofs_i.h
> +++ b/fs/autofs/autofs_i.h
> @@ -221,15 +221,20 @@ static inline int autofs_check_pipe(struct file *pipe)
>         return 0;
>  }
>
> -static inline int autofs_prepare_pipe(struct file *pipe)
> +static inline void autofs_set_packet_pipe_flags(struct file *pipe)
>  {
> -       int ret = autofs_check_pipe(pipe);
> -       if (ret < 0)
> -               return ret;
>         /* We want a packet pipe */
>         pipe->f_flags |= O_DIRECT;
>         /* We don't expect -EAGAIN */
>         pipe->f_flags &= ~O_NONBLOCK;
> +}
> +
> +static inline int autofs_prepare_pipe(struct file *pipe)
> +{
> +       int ret = autofs_check_pipe(pipe);
> +       if (ret < 0)
> +               return ret;
> +       autofs_set_packet_pipe_flags(pipe);
>         return 0;
>  }
>
> diff --git a/fs/autofs/inode.c b/fs/autofs/inode.c
> index 6d2e01c9057d..a3d62acc293a 100644
> --- a/fs/autofs/inode.c
> +++ b/fs/autofs/inode.c
> @@ -177,6 +177,8 @@ static int autofs_parse_fd(struct fs_context *fc, struct autofs_sb_info *sbi,
>                 return -EBADF;
>         }
>
> +       autofs_set_packet_pipe_flags(pipe);
> +
>         if (sbi->pipe)
>                 fput(sbi->pipe);
>
> --
> 2.41.0
>

Re: [PATCH] autofs: fix add autofs_parse_fd()

[Christian Brauner]

On Mon, 23 Oct 2023 17:33:59 +0800, Ian Kent wrote:
> We are seeing systemd hang on its autofs direct mount at
> /proc/sys/fs/binfmt_misc.
> 
> Historically this was due to a mismatch in the communication structure
> size between a 64 bit kernel and a 32 bit user space and was fixed by
> making the pipe communication record oriented.
> 
> [...]

Thanks for the fix!

---

Applied to the vfs.autofs branch of the vfs/vfs.git tree.
Patches in the vfs.autofs branch should appear in linux-next soon.

Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.

It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.

Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs.autofs

[1/1] autofs: fix add autofs_parse_fd()
      https://git.kernel.org/vfs/vfs/c/d3c50061765d