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

[Anders Roxell]

On Mon, 23 Oct 2023 at 11:34, Ian Kent <raven@xxxxxxxxxx> 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@xxxxxxxxxx>
> Cc: Bill O'Donnell <bodonnel@xxxxxxxxxx>
> Cc: Christian Brauner <brauner@xxxxxxxxxx>
> Cc: Arnd Bergmann <arnd@xxxxxxxx>
> Cc: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
> Cc: Anders Roxell <anders.roxell@xxxxxxxxxx>
> Cc: Naresh Kamboju <naresh.kamboju@xxxxxxxxxx>
> Cc: Stephen Rothwell <sfr@xxxxxxxxxxxxxxxx>
> Reported-by: Linux Kernel Functional Testing <lkft@xxxxxxxxxx>
> Reported-by: Anders Roxell <anders.roxell@xxxxxxxxxx>

Thank you Ian for finding the issue so quickly.

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

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
>