linux-kernel - Re: [PATCH v2 1/5] autofs - improve ioctl sbi checks

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] [day] [month] [year] [list]

Date:   Mon, 26 Nov 2018 06:57:40 +0800
From:   Ian Kent <raven@...maw.net>
To:     Andrew Morton <akpm@...ux-foundation.org>
Cc:     Al Viro <viro@...IV.linux.org.uk>,
        autofs mailing list <autofs@...r.kernel.org>,
        linux-fsdevel <linux-fsdevel@...r.kernel.org>,
        Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 1/5] autofs - improve ioctl sbi checks

On Fri, 2018-11-23 at 15:29 -0800, Andrew Morton wrote:
> On Fri, 23 Nov 2018 18:41:50 +0800 Ian Kent <raven@...maw.net> wrote:
> 
> > Al Viro made some suggestions to improve the implementation
> > of commit 0633da48f0 "fix autofs_sbi() does not check super
> > block type".
> > 
> > The check is unnessesary in all cases except for ioctl usage
> > so placing the check in the super block accessor function
> > adds a small overhead to the common case where it isn't
> > needed.
> > 
> > So it's sufficient to do this in the ioctl code only.
> > 
> > Also the check in the ioctl code is needlessly complex.
> > 
> > ...
> > 
> > --- a/fs/autofs/dev-ioctl.c
> > +++ b/fs/autofs/dev-ioctl.c
> > @@ -14,6 +14,8 @@
> >  
> >  #include "autofs_i.h"
> >  
> > +extern struct file_system_type autofs_fs_type;
> > +
> >  /*
> >   * This module implements an interface for routing autofs ioctl control
> >   * commands via a miscellaneous device file.
> 
> It's naughty to declare externs in C files, for various reasons.  Is
> this OK?

OK, I understand the reasoning I guess.

The change below is fine.

Thanks, Ian.

> 
> --- a/fs/autofs/autofs_i.h~autofs-improve-ioctl-sbi-checks-fix
> +++ a/fs/autofs/autofs_i.h
> @@ -42,6 +42,8 @@
>  #endif
>  #define pr_fmt(fmt) KBUILD_MODNAME ":pid:%d:%s: " fmt, current->pid, __func__
>  
> +extern struct file_system_type autofs_fs_type;
> +
>  /*
>   * Unified info structure.  This is pointed to by both the dentry and
>   * inode structures.  Each file in the filesystem has an instance of this
> --- a/fs/autofs/dev-ioctl.c~autofs-improve-ioctl-sbi-checks-fix
> +++ a/fs/autofs/dev-ioctl.c
> @@ -14,8 +14,6 @@
>  
>  #include "autofs_i.h"
>  
> -extern struct file_system_type autofs_fs_type;
> -
>  /*
>   * This module implements an interface for routing autofs ioctl control
>   * commands via a miscellaneous device file.
> _
>