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: <CAHpGcM+92O7Ab-hbFfnt1zO7BAPZPL7zmLyPxi4RsjNhjL1drA@mail.gmail.com>
Date:   Wed, 12 Apr 2017 17:03:37 +0200
From:   Andreas Grünbacher <andreas.gruenbacher@...il.com>
To:     Brian Ashworth <bosrsf04@...il.com>
Cc:     Alexander Viro <viro@...iv.linux.org.uk>,
        Steve French <sfrench@...ba.org>, Jan Kara <jack@...e.com>,
        Jaegeuk Kim <jaegeuk@...nel.org>, Chao Yu <yuchao0@...wei.com>,
        David Woodhouse <dwmw2@...radead.org>,
        Phillip Lougher <phillip@...ashfs.org.uk>,
        Mimi Zohar <zohar@...ux.vnet.ibm.com>,
        James Morris <james.l.morris@...cle.com>,
        "Serge E . Hallyn" <serge@...lyn.com>,
        Andreas Gruenbacher <agruenba@...hat.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Arnd Bergmann <arnd@...db.de>,
        "David S . Miller" <davem@...emloft.net>,
        Ingo Molnar <mingo@...nel.org>,
        Andy Lutomirski <luto@...nel.org>,
        Thomas Garnier <thgarnie@...gle.com>,
        "Paul E . McKenney" <paulmck@...ux.vnet.ibm.com>,
        Petr Mladek <pmladek@...e.com>,
        Nicolas Pitre <nicolas.pitre@...aro.org>,
        Tejun Heo <tj@...nel.org>,
        Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
        Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
        Helge Deller <deller@....de>, Rik van Riel <riel@...hat.com>,
        "seokhoon . yoon" <iamyooon@...il.com>,
        Dave Hansen <dave.hansen@...el.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Linux FS-devel Mailing List <linux-fsdevel@...r.kernel.org>,
        Josh Triplett <josh@...htriplett.org>,
        Conor Curry <conorcurry@...il.com>,
        William Kurek <brkkurek192@...il.com>,
        Zach Romitz <fanofbond138@...il.com>
Subject: Re: [PATCH 1/1] fs: Allows for xattr support to be compiled out

 2017-04-12 0:47 GMT+02:00 Brian Ashworth <bosrsf04@...il.com>:
> Allows for xattr syscalls and related functions to be compiled out.
> These are not needed on machines that do not utilize filesystems that
> support xattrs or userspaces that require extended attributes. This will
> aid in the tinification efforts.

How much does this shrink things in the end?

> The xattr_full_name function was moved from fs/xattr.c to
> include/linux/xattr.h and made static inline. This function only deals
> with interaction of the xattr_handler struct and does not require any
> of the xattr functions to be implemented.

This seems unnecessary.: when there are no xattr syscalls,
xattr_full_name shouldn't be called anymore.

> The rest of the functions in fs/xattr.c have had stubs created in
> include/linux/xattr.h to return the appropriate value to indicate that
> it is not supported or that there is no data.
>
> add/remove: 0/39 grow/shrink: 0/5 up/down: 0/-4020 (-4020)
> function                                     old     new   delta
> xattr_getsecurity                              6       -      -6
> simple_xattr_list_add                         13       -     -13
> fdget                                         56      42     -14
> sys_lremovexattr                              15       -     -15
> sys_removexattr                               18       -     -18
> cap_inode_killpriv                            22       3     -19
> xattr_full_name                               25       -     -25
> sys_llistxattr                                25       -     -25
> sys_listxattr                                 25       -     -25
> cap_inode_need_killpriv                       28       3     -25
> sys_lgetxattr                                 33       -     -33
> sys_getxattr                                  33       -     -33
> vfs_listxattr                                 34       -     -34
> sys_setxattr                                  43       -     -43
> sys_lsetxattr                                 43       -     -43
> removexattr                                   56       -     -56
> simple_xattr_alloc                            59       -     -59
> sys_flistxattr                                61       -     -61
> sys_fgetxattr                                 66       -     -66
> vfs_getxattr                                  69       -     -69
> __vfs_removexattr                             70       -     -70
> __vfs_getxattr                                71       -     -71
> sys_fremovexattr                              75       -     -75
> simple_xattr_get                              75       -     -75
> vfs_removexattr                               79       -     -79
> simple_xattr_list                             79       -     -79
> sys_fsetxattr                                 89       -     -89
> __vfs_setxattr                                91       -     -91
> path_listxattr                                98       -     -98
> path_getxattr                                103       -    -103
> __vfs_setxattr_noperm                        105       -    -105
> vfs_setxattr                                 113       -    -113
> path_removexattr                             114       -    -114
> path_setxattr                                132       -    -132
> listxattr                                    149       -    -149
> xattr_resolve_name                           155       -    -155
> get_vfs_caps_from_disk                       184      19    -165
> generic_listxattr                            194       -    -194
> xattr_permission                             208       -    -208
> vfs_getxattr_alloc                           216       -    -216
> simple_xattr_set                             231       -    -231
> setxattr                                     239       -    -239
> getxattr                                     240       -    -240
> cap_bprm_set_creds                           613     366    -247
> Total: Before=1900297, After=1896277, chg -0.21%
>
> Signed-off-by: Brian Ashworth <bosrsf04@...il.com>
> ---
>  fs/Kconfig                     |   1 +
>  fs/Makefile                    |   3 +-
>  fs/cifs/Kconfig                |   1 +
>  fs/ext2/Kconfig                |   1 +
>  fs/f2fs/Kconfig                |   1 +
>  fs/jffs2/Kconfig               |   1 +
>  fs/reiserfs/Kconfig            |   1 +
>  fs/squashfs/Kconfig            |   1 +
>  fs/xattr.c                     |  24 --------
>  include/linux/xattr.h          | 126 ++++++++++++++++++++++++++++++++++++++++-
>  init/Kconfig                   |  11 ++++
>  kernel/sys_ni.c                |  12 ++++
>  security/integrity/evm/Kconfig |   1 +
>  13 files changed, 157 insertions(+), 27 deletions(-)
>
> diff --git a/fs/Kconfig b/fs/Kconfig
> index 83eab52fb3f6..18ce4032e177 100644
> --- a/fs/Kconfig
> +++ b/fs/Kconfig
> @@ -173,6 +173,7 @@ config TMPFS_POSIX_ACL
>  config TMPFS_XATTR
>         bool "Tmpfs extended attributes"
>         depends on TMPFS
> +       depends on XATTR_SYSCALLS
>         default n
>         help
>           Extended attributes are name:value pairs associated with inodes by
> diff --git a/fs/Makefile b/fs/Makefile
> index 7bbaca9c67b1..1645965883bc 100644
> --- a/fs/Makefile
> +++ b/fs/Makefile
> @@ -9,7 +9,7 @@ obj-y :=        open.o read_write.o file_table.o super.o \
>                 char_dev.o stat.o exec.o pipe.o namei.o fcntl.o \
>                 ioctl.o readdir.o select.o dcache.o inode.o \
>                 attr.o bad_inode.o file.o filesystems.o namespace.o \
> -               seq_file.o xattr.o libfs.o fs-writeback.o \
> +               seq_file.o libfs.o fs-writeback.o \
>                 pnode.o splice.o sync.o utimes.o \
>                 stack.o fs_struct.o statfs.o fs_pin.o nsfs.o
>
> @@ -32,6 +32,7 @@ obj-$(CONFIG_AIO)               += aio.o
>  obj-$(CONFIG_FS_DAX)           += dax.o
>  obj-$(CONFIG_FS_ENCRYPTION)    += crypto/
>  obj-$(CONFIG_FILE_LOCKING)      += locks.o
> +obj-$(CONFIG_XATTR_SYSCALLS)   += xattr.o
>  obj-$(CONFIG_COMPAT)           += compat.o compat_ioctl.o
>  obj-$(CONFIG_BINFMT_AOUT)      += binfmt_aout.o
>  obj-$(CONFIG_BINFMT_EM86)      += binfmt_em86.o
> diff --git a/fs/cifs/Kconfig b/fs/cifs/Kconfig
> index 034f00f21390..3352f6b3efed 100644
> --- a/fs/cifs/Kconfig
> +++ b/fs/cifs/Kconfig
> @@ -94,6 +94,7 @@ config CIFS_UPCALL
>  config CIFS_XATTR
>          bool "CIFS extended attributes"
>          depends on CIFS
> +       depends on XATTR_SYSCALLS
>          help
>            Extended attributes are name:value pairs associated with inodes by
>            the kernel or by users (see the attr(5) manual page, or visit
> diff --git a/fs/ext2/Kconfig b/fs/ext2/Kconfig
> index c634874e12d9..9b16a31785a1 100644
> --- a/fs/ext2/Kconfig
> +++ b/fs/ext2/Kconfig
> @@ -11,6 +11,7 @@ config EXT2_FS
>  config EXT2_FS_XATTR
>         bool "Ext2 extended attributes"
>         depends on EXT2_FS
> +       depends on XATTR_SYSCALLS
>         help
>           Extended attributes are name:value pairs associated with inodes by
>           the kernel or by users (see the attr(5) manual page, or visit
> diff --git a/fs/f2fs/Kconfig b/fs/f2fs/Kconfig
> index 378c221d68a9..3594db075cce 100644
> --- a/fs/f2fs/Kconfig
> +++ b/fs/f2fs/Kconfig
> @@ -32,6 +32,7 @@ config F2FS_STAT_FS
>  config F2FS_FS_XATTR
>         bool "F2FS extended attributes"
>         depends on F2FS_FS
> +       depends on XATTR_SYSCALLS
>         default y
>         help
>           Extended attributes are name:value pairs associated with inodes by
> diff --git a/fs/jffs2/Kconfig b/fs/jffs2/Kconfig
> index d8bb6c411e96..c1646416c94f 100644
> --- a/fs/jffs2/Kconfig
> +++ b/fs/jffs2/Kconfig
> @@ -65,6 +65,7 @@ config JFFS2_SUMMARY
>  config JFFS2_FS_XATTR
>         bool "JFFS2 XATTR support"
>         depends on JFFS2_FS
> +       depends on XATTR_SYSCALLS
>         default n
>         help
>           Extended attributes are name:value pairs associated with inodes by
> diff --git a/fs/reiserfs/Kconfig b/fs/reiserfs/Kconfig
> index 7cd46666ba2c..98adeaa2b312 100644
> --- a/fs/reiserfs/Kconfig
> +++ b/fs/reiserfs/Kconfig
> @@ -55,6 +55,7 @@ config REISERFS_PROC_INFO
>  config REISERFS_FS_XATTR
>         bool "ReiserFS extended attributes"
>         depends on REISERFS_FS
> +       depends on XATTR_SYSCALLS
>         help
>           Extended attributes are name:value pairs associated with inodes by
>           the kernel or by users (see the attr(5) manual page, or visit
> diff --git a/fs/squashfs/Kconfig b/fs/squashfs/Kconfig
> index ffb093e72b6c..083de02a2ab5 100644
> --- a/fs/squashfs/Kconfig
> +++ b/fs/squashfs/Kconfig
> @@ -100,6 +100,7 @@ endchoice
>  config SQUASHFS_XATTR
>         bool "Squashfs XATTR support"
>         depends on SQUASHFS
> +       depends on XATTR_SYSCALLS
>         help
>           Saying Y here includes support for extended attributes (xattrs).
>           Xattrs are name:value pairs associated with inodes by
> diff --git a/fs/xattr.c b/fs/xattr.c
> index 7e3317cf4045..c97ae8365775 100644
> --- a/fs/xattr.c
> +++ b/fs/xattr.c
> @@ -784,30 +784,6 @@ generic_listxattr(struct dentry *dentry, char *buffer, size_t buffer_size)
>  }
>  EXPORT_SYMBOL(generic_listxattr);
>
> -/**
> - * xattr_full_name  -  Compute full attribute name from suffix
> - *
> - * @handler:   handler of the xattr_handler operation
> - * @name:      name passed to the xattr_handler operation
> - *
> - * The get and set xattr handler operations are called with the remainder of
> - * the attribute name after skipping the handler's prefix: for example, "foo"
> - * is passed to the get operation of a handler with prefix "user." to get
> - * attribute "user.foo".  The full name is still "there" in the name though.
> - *
> - * Note: the list xattr handler operation when called from the vfs is passed a
> - * NULL name; some file systems use this operation internally, with varying
> - * semantics.
> - */
> -const char *xattr_full_name(const struct xattr_handler *handler,
> -                           const char *name)
> -{
> -       size_t prefix_len = strlen(xattr_prefix(handler));
> -
> -       return name - prefix_len;
> -}
> -EXPORT_SYMBOL(xattr_full_name);
> -
>  /*
>   * Allocate new xattr and copy in the value; but leave the name to callers.
>   */
> diff --git a/include/linux/xattr.h b/include/linux/xattr.h
> index e77605a0c8da..b1d8ec5b2003 100644
> --- a/include/linux/xattr.h
> +++ b/include/linux/xattr.h
> @@ -37,8 +37,6 @@ struct xattr_handler {
>                    size_t size, int flags);
>  };
>
> -const char *xattr_full_name(const struct xattr_handler *, const char *);
> -
>  struct xattr {
>         const char *name;
>         void *value;
> @@ -46,6 +44,9 @@ struct xattr {
>  };
>
>  ssize_t xattr_getsecurity(struct inode *, const char *, void *, size_t);
> +
> +#ifdef CONFIG_XATTR_SYSCALLS
> +
>  ssize_t __vfs_getxattr(struct dentry *, struct inode *, const char *, void *, size_t);
>  ssize_t vfs_getxattr(struct dentry *, const char *, void *, size_t);
>  ssize_t vfs_listxattr(struct dentry *d, char *list, size_t size);
> @@ -59,11 +60,97 @@ ssize_t generic_listxattr(struct dentry *dentry, char *buffer, size_t buffer_siz
>  ssize_t vfs_getxattr_alloc(struct dentry *dentry, const char *name,
>                            char **xattr_value, size_t size, gfp_t flags);
>
> +#else
> +
> +static inline ssize_t __vfs_getxattr(struct dentry *dentry, struct inode *inode,
> +               const char *name, void *value, size_t size)
> +{
> +       return -EOPNOTSUPP;
> +}
> +
> +static inline ssize_t vfs_getxattr(struct dentry *dentry, const char *name,
> +               void *value, size_t size)
> +{
> +       return -EOPNOTSUPP;
> +}
> +
> +static inline ssize_t vfs_listxattr(struct dentry *dentry, char *list, size_t size)
> +{
> +       return -EOPNOTSUPP;
> +}
> +
> +static inline int __vfs_setxattr(struct dentry *dentry, struct inode *inode,
> +               const char *name, const void *value, size_t size, int flags)
> +{
> +       return -EOPNOTSUPP;
> +}
> +
> +static inline int __vfs_setxattr_noperm(struct dentry *dentry, const char *name,
> +               const void *value, size_t size, int flags)
> +{
> +       return -EOPNOTSUPP;
> +}
> +
> +static inline ssize_t vfs_setxattr(struct dentry *dentry, const char *name,
> +               const void *value, size_t size, int flags)
> +{
> +       return -EOPNOTSUPP;
> +}
> +
> +static inline int __vfs_removexattr(struct dentry *dentry, const char *name)
> +{
> +       return -EOPNOTSUPP;
> +}
> +
> +static inline int vfs_removexattr(struct dentry *dentry, const char *name)
> +{
> +       return -EOPNOTSUPP;
> +}
> +
> +static inline ssize_t generic_listxattr(struct dentry *dentry, char *buffer,
> +               size_t buffer_size)
> +{
> +       return -EOPNOTSUPP;
> +}
> +
> +static inline ssize_t vfs_getxattr_alloc(struct dentry *dentry,
> +               const char *name, char **xattr_value, size_t xattr_size,
> +               gfp_t flags)
> +{
> +       return -EOPNOTSUPP;
> +}
> +
> +#endif  /* CONFIG_XATTR_SYSCALLS */
> +
> +
>  static inline const char *xattr_prefix(const struct xattr_handler *handler)
>  {
>         return handler->prefix ?: handler->name;
>  }
>
> +/**
> + * xattr_full_name  -  Compute full attribute name from suffix
> + *
> + * @handler:   handler of the xattr_handler operation
> + * @name:      name passed to the xattr_handler operation
> + *
> + * The get and set xattr handler operations are called with the remainder of
> + * the attribute name after skipping the handler's prefix: for example, "foo"
> + * is passed to the get operation of a handler with prefix "user." to get
> + * attribute "user.foo".  The full name is still "there" in the name though.
> + *
> + * Note: the list xattr handler operation when called from the vfs is passed a
> + * NULL name; some file systems use this operation internally, with varying
> + * semantics.
> + */
> +static inline const char *xattr_full_name(const struct xattr_handler *handler,
> +               const char *name)
> +{
> +       size_t prefix_len = strlen(xattr_prefix(handler));
> +
> +       return name - prefix_len;
> +}
> +
>  struct simple_xattrs {
>         struct list_head head;
>         spinlock_t lock;
> @@ -98,6 +185,8 @@ static inline void simple_xattrs_free(struct simple_xattrs *xattrs)
>         }
>  }
>
> +#ifdef CONFIG_XATTR_SYSCALLS
> +
>  struct simple_xattr *simple_xattr_alloc(const void *value, size_t size);
>  int simple_xattr_get(struct simple_xattrs *xattrs, const char *name,
>                      void *buffer, size_t size);
> @@ -108,4 +197,37 @@ ssize_t simple_xattr_list(struct inode *inode, struct simple_xattrs *xattrs, cha
>  void simple_xattr_list_add(struct simple_xattrs *xattrs,
>                            struct simple_xattr *new_xattr);
>
> +#else
> +
> +static inline struct simple_xattr *simple_xattr_alloc(const void *value,
> +               size_t size)
> +{
> +       return NULL;
> +}
> +
> +static inline int simple_xattr_get(struct simple_xattrs *xattrs,
> +               const char *name, void *buffer, size_t size)
> +{
> +       return -ENODATA;should be compiled out instead.
> +}
>
> +static inline int simple_xattr_set(struct simple_xattrs *xattrs,
> +               const char *name, const void *value, size_t size, int flags)
> +{
> +       return -ENODATA;
> +}
> +
> +static inline ssize_t simple_xattr_list(struct inode *inode,
> +               struct simple_xattrs *xattrs, char *buffer, size_t size)
> +{
> +       return -ERANGE;
> +}
> +
> +static inline void simple_xattr_list_add(struct simple_xattrs *xattrs,
> +               struct simple_xattr *new_xattr)
> +{
> +}

As with xattr_full_name, please configure out the code calling those
shims instead.

> +#endif  /* CONFIG_XATTR_SYSCALLS */
> +
>  #endif /* _LINUX_XATTR_H */
> diff --git a/init/Kconfig b/init/Kconfig
> index a92f27da4a27..bfb954adc1e6 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -1460,6 +1460,17 @@ config SYSCTL_SYSCALL
>
>           If unsure say N here.
>
> +config XATTR_SYSCALLS
> +       bool "Support for the xattr family of syscalls" if EXPERT
> +       default y
> +       help
> +         The xattr family of syscalls are used to access extended file
> +         attributes from userspace. If you are using a filesystem or
> +         userspace that does not use or require the system calls, they
> +         can be compiled out.
> +
> +         If unsure say Y here.
> +

Shouldn't this be an implied config that the various filesystems
select when needed?

(The xattr code in kernfs is unconditional right now, but that's not
so hard to change.)

>  config POSIX_TIMERS
>         bool "Posix Clocks & timers" if EXPERT
>         default y
> diff --git a/kernel/sys_ni.c b/kernel/sys_ni.c
> index 8acef8576ce9..6f6b8105545d 100644
> --- a/kernel/sys_ni.c
> +++ b/kernel/sys_ni.c
> @@ -178,6 +178,18 @@ cond_syscall(sys_setfsgid);
>  cond_syscall(sys_capget);
>  cond_syscall(sys_capset);
>  cond_syscall(sys_copy_file_range);
> +cond_syscall(sys_setxattr);
> +cond_syscall(sys_lsetxattr);
> +cond_syscall(sys_fsetxattr);
> +cond_syscall(sys_getxattr);
> +cond_syscall(sys_lgetxattr);
> +cond_syscall(sys_fgetxattr);
> +cond_syscall(sys_listxattr);
> +cond_syscall(sys_llistxattr);
> +cond_syscall(sys_flistxattr);
> +cond_syscall(sys_removexattr);
> +cond_syscall(sys_lremovexattr);
> +cond_syscall(sys_fremovexattr);
>
>  /* arch-specific weak syscall entries */
>  cond_syscall(sys_pciconfig_read);
> diff --git a/security/integrity/evm/Kconfig b/security/integrity/evm/Kconfig
> index e825e0ae78e7..56f76bfd5cfb 100644
> --- a/security/integrity/evm/Kconfig
> +++ b/security/integrity/evm/Kconfig
> @@ -28,6 +28,7 @@ config EVM_ATTR_FSUUID
>  config EVM_EXTRA_SMACK_XATTRS
>         bool "Additional SMACK xattrs"
>         depends on EVM && SECURITY_SMACK
> +       depends on XATTR_SYSCALLS
>         default n
>         help
>           Include additional SMACK xattrs for HMAC calculation.
> --config
> 2.12.2

Thanks,
Andreas

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ