* Re: [PATCH 1/1] fs: Allows for xattr support to be compiled out
2017-04-11 22:47 ` [PATCH 1/1] " Brian Ashworth
@ 2017-04-12 15:03 ` Andreas Grünbacher
2017-04-21 14:18 ` kbuild test robot
1 sibling, 0 replies; 4+ messages in thread
From: Andreas Grünbacher @ 2017-04-12 15:03 UTC (permalink / raw)
To: Brian Ashworth
Cc: Alexander Viro, Steve French, Jan Kara, Jaegeuk Kim, Chao Yu,
David Woodhouse, Phillip Lougher, Mimi Zohar, James Morris,
Serge E . Hallyn, Andreas Gruenbacher, Andrew Morton,
Arnd Bergmann, David S . Miller, Ingo Molnar, Andy Lutomirski,
Thomas Garnier, Paul E . McKenney, Petr Mladek, Nicolas Pitre,
Tejun Heo, Sebastian Andrzej Siewior, Sergey Senozhatsky,
Helge Deller, Rik van Riel, seokhoon . yoon, Dave Hansen,
Thomas Gleixner, Linux Kernel Mailing List,
Linux FS-devel Mailing List, Josh Triplett, Conor Curry,
William Kurek, Zach Romitz
2017-04-12 0:47 GMT+02:00 Brian Ashworth <bosrsf04@gmail.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@gmail.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
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH 1/1] fs: Allows for xattr support to be compiled out
2017-04-11 22:47 ` [PATCH 1/1] " Brian Ashworth
2017-04-12 15:03 ` Andreas Grünbacher
@ 2017-04-21 14:18 ` kbuild test robot
1 sibling, 0 replies; 4+ messages in thread
From: kbuild test robot @ 2017-04-21 14:18 UTC (permalink / raw)
To: Brian Ashworth
Cc: kbuild-all, Alexander Viro, Steve French, Jan Kara, Jaegeuk Kim,
Chao Yu, David Woodhouse, Phillip Lougher, Mimi Zohar,
James Morris, Serge E . Hallyn, Andreas Gruenbacher,
Andrew Morton, Arnd Bergmann, David S . Miller, Ingo Molnar,
Andy Lutomirski, Thomas Garnier, Paul E . McKenney, Petr Mladek,
Nicolas Pitre, Tejun Heo, Sebastian Andrzej Siewior,
Sergey Senozhatsky, Helge Deller, Rik van Riel, seokhoon . yoon,
Dave Hansen, Thomas Gleixner, linux-kernel, linux-fsdevel,
Josh Triplett, Conor Curry, William Kurek, Zach Romitz,
Brian Ashworth
[-- Attachment #1: Type: text/plain, Size: 846 bytes --]
Hi Brian,
[auto build test WARNING on linus/master]
[also build test WARNING on v4.11-rc7 next-20170421]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Brian-Ashworth/fs-Allows-for-xattr-support-to-be-compiled-out/20170412-131505
config: x86_64-randconfig-g0-04211438 (attached as .config)
compiler: gcc-5 (Debian 5.4.1-2) 5.4.1 20160904
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All warnings (new ones prefixed by >>):
warning: (TMPFS_POSIX_ACL) selects TMPFS_XATTR which has unmet direct dependencies (TMPFS && XATTR_SYSCALLS)
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 28448 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread