2020-07-26 16:04:42

by Christoph Hellwig

[permalink] [raw]
Subject: improve compat handling for the i386 u64 alignment quirk

Hi all,

the i386 ABI is a little special in that it uses less than natural
alignment for 64-bit integer types (u64 and s64), and a significant
amount of our compat handlers deals with just that. Unfortunately
there is no good way to check for this specific quirk at runtime,
similar how in_compat_syscall() checks for a compat syscall. This
series adds such a check, and then uses the quota code as an example
of how this improves the compat handling. I have a few other places
in mind where this will also be useful going forward.

Diffstat:
b/arch/arm64/include/asm/compat.h | 2
b/arch/arm64/include/asm/stat.h | 2
b/arch/arm64/kernel/process.c | 1
b/arch/arm64/kernel/ptrace.c | 1
b/arch/mips/include/asm/compat.h | 2
b/arch/parisc/include/asm/compat.h | 2
b/arch/powerpc/include/asm/compat.h | 2
b/arch/s390/include/asm/compat.h | 2
b/arch/sparc/include/asm/compat.h | 3
b/arch/x86/entry/syscalls/syscall_32.tbl | 2
b/arch/x86/include/asm/compat.h | 3
b/fs/quota/Kconfig | 5 -
b/fs/quota/Makefile | 1
b/fs/quota/compat.h | 34 ++++++++
b/fs/quota/quota.c | 73 +++++++++++++++---
b/include/linux/compat.h | 17 ++++
b/include/linux/quotaops.h | 3
b/kernel/sys_ni.c | 1
fs/quota/compat.c | 120 -------------------------------
19 files changed, 114 insertions(+), 162 deletions(-)


2020-07-26 16:04:47

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 1/4] arm64: stop using <asm/compat.h> directly

Always use <linux/compat.h> so that we can move more declarations to
common code. In two of the three cases the asm include was in addition
to an existing one for <linux/compat.h> anyway.

Signed-off-by: Christoph Hellwig <[email protected]>
---
arch/arm64/include/asm/stat.h | 2 +-
arch/arm64/kernel/process.c | 1 -
arch/arm64/kernel/ptrace.c | 1 -
3 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/arch/arm64/include/asm/stat.h b/arch/arm64/include/asm/stat.h
index 3b4a62f5aeb0c3..1b5ac1ef5d04cc 100644
--- a/arch/arm64/include/asm/stat.h
+++ b/arch/arm64/include/asm/stat.h
@@ -10,7 +10,7 @@
#ifdef CONFIG_COMPAT

#include <linux/time.h>
-#include <asm/compat.h>
+#include <linux/compat.h>

/*
* struct stat64 is needed for compat tasks only. Its definition is different
diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
index 6089638c7d43f4..70381900ef9b8a 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -46,7 +46,6 @@

#include <asm/alternative.h>
#include <asm/arch_gicv3.h>
-#include <asm/compat.h>
#include <asm/cpufeature.h>
#include <asm/cacheflush.h>
#include <asm/exec.h>
diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
index 1e02e98e68dd37..0497aaea782451 100644
--- a/arch/arm64/kernel/ptrace.c
+++ b/arch/arm64/kernel/ptrace.c
@@ -30,7 +30,6 @@
#include <linux/tracehook.h>
#include <linux/elf.h>

-#include <asm/compat.h>
#include <asm/cpufeature.h>
#include <asm/debug-monitors.h>
#include <asm/fpsimd.h>
--
2.27.0

2020-07-26 16:04:51

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 2/4] compat: lift compat_s64 and compat_u64 to <linux/compat.h>

lift the compat_s64 and compat_u64 definitions into common code using the
COMPAT_FOR_U64_ALIGNMENT symbol for the x86 special case.

Signed-off-by: Christoph Hellwig <[email protected]>
---
arch/arm64/include/asm/compat.h | 2 --
arch/mips/include/asm/compat.h | 2 --
arch/parisc/include/asm/compat.h | 2 --
arch/powerpc/include/asm/compat.h | 2 --
arch/s390/include/asm/compat.h | 2 --
arch/sparc/include/asm/compat.h | 3 +--
arch/x86/include/asm/compat.h | 2 --
include/linux/compat.h | 8 ++++++++
8 files changed, 9 insertions(+), 14 deletions(-)

diff --git a/arch/arm64/include/asm/compat.h b/arch/arm64/include/asm/compat.h
index 935d2aa231bf06..23a9fb73c04ff8 100644
--- a/arch/arm64/include/asm/compat.h
+++ b/arch/arm64/include/asm/compat.h
@@ -35,8 +35,6 @@ typedef s32 compat_nlink_t;
typedef u16 compat_ipc_pid_t;
typedef u32 compat_caddr_t;
typedef __kernel_fsid_t compat_fsid_t;
-typedef s64 compat_s64;
-typedef u64 compat_u64;

struct compat_stat {
#ifdef __AARCH64EB__
diff --git a/arch/mips/include/asm/compat.h b/arch/mips/include/asm/compat.h
index 255afcdd79c94b..65975712a22dcf 100644
--- a/arch/mips/include/asm/compat.h
+++ b/arch/mips/include/asm/compat.h
@@ -26,8 +26,6 @@ typedef s32 compat_caddr_t;
typedef struct {
s32 val[2];
} compat_fsid_t;
-typedef s64 compat_s64;
-typedef u64 compat_u64;

struct compat_stat {
compat_dev_t st_dev;
diff --git a/arch/parisc/include/asm/compat.h b/arch/parisc/include/asm/compat.h
index 2f4f66a3bac079..8f33085ff1bd88 100644
--- a/arch/parisc/include/asm/compat.h
+++ b/arch/parisc/include/asm/compat.h
@@ -22,8 +22,6 @@ typedef u32 compat_dev_t;
typedef u16 compat_nlink_t;
typedef u16 compat_ipc_pid_t;
typedef u32 compat_caddr_t;
-typedef s64 compat_s64;
-typedef u64 compat_u64;

struct compat_stat {
compat_dev_t st_dev; /* dev_t is 32 bits on parisc */
diff --git a/arch/powerpc/include/asm/compat.h b/arch/powerpc/include/asm/compat.h
index 3e3cdfaa76c6a5..9191fc29e6ed11 100644
--- a/arch/powerpc/include/asm/compat.h
+++ b/arch/powerpc/include/asm/compat.h
@@ -27,8 +27,6 @@ typedef s16 compat_nlink_t;
typedef u16 compat_ipc_pid_t;
typedef u32 compat_caddr_t;
typedef __kernel_fsid_t compat_fsid_t;
-typedef s64 compat_s64;
-typedef u64 compat_u64;

struct compat_stat {
compat_dev_t st_dev;
diff --git a/arch/s390/include/asm/compat.h b/arch/s390/include/asm/compat.h
index 9547cd5d6cdc21..ea5b9c34b7be5b 100644
--- a/arch/s390/include/asm/compat.h
+++ b/arch/s390/include/asm/compat.h
@@ -63,8 +63,6 @@ typedef u16 compat_nlink_t;
typedef u16 compat_ipc_pid_t;
typedef u32 compat_caddr_t;
typedef __kernel_fsid_t compat_fsid_t;
-typedef s64 compat_s64;
-typedef u64 compat_u64;

typedef struct {
u32 mask;
diff --git a/arch/sparc/include/asm/compat.h b/arch/sparc/include/asm/compat.h
index 40a267b3bd5208..b85842cda99fe0 100644
--- a/arch/sparc/include/asm/compat.h
+++ b/arch/sparc/include/asm/compat.h
@@ -21,8 +21,7 @@ typedef s16 compat_nlink_t;
typedef u16 compat_ipc_pid_t;
typedef u32 compat_caddr_t;
typedef __kernel_fsid_t compat_fsid_t;
-typedef s64 compat_s64;
-typedef u64 compat_u64;
+
struct compat_stat {
compat_dev_t st_dev;
compat_ino_t st_ino;
diff --git a/arch/x86/include/asm/compat.h b/arch/x86/include/asm/compat.h
index d4edf281fff49d..bf547701f41f87 100644
--- a/arch/x86/include/asm/compat.h
+++ b/arch/x86/include/asm/compat.h
@@ -27,8 +27,6 @@ typedef u16 compat_nlink_t;
typedef u16 compat_ipc_pid_t;
typedef u32 compat_caddr_t;
typedef __kernel_fsid_t compat_fsid_t;
-typedef s64 __attribute__((aligned(4))) compat_s64;
-typedef u64 __attribute__((aligned(4))) compat_u64;

struct compat_stat {
compat_dev_t st_dev;
diff --git a/include/linux/compat.h b/include/linux/compat.h
index e90100c0de72e4..c22a7f1d253b87 100644
--- a/include/linux/compat.h
+++ b/include/linux/compat.h
@@ -91,6 +91,14 @@
static inline long __do_compat_sys##name(__MAP(x,__SC_DECL,__VA_ARGS__))
#endif /* COMPAT_SYSCALL_DEFINEx */

+#ifdef COMPAT_FOR_U64_ALIGNMENT
+typedef s64 __attribute__((aligned(4))) compat_s64;
+typedef u64 __attribute__((aligned(4))) compat_u64;
+#else
+typedef s64 compat_s64;
+typedef u64 compat_u64;
+#endif
+
#ifdef CONFIG_COMPAT

#ifndef compat_user_stack_pointer
--
2.27.0

2020-07-26 16:05:04

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 3/4] compat: add a compat_need_64bit_alignment_fixup() helper

Add a helper to check if the calling syscall needs a fixup for
non-natural 64-bit type alignment in the compat ABI. This will only
return true for i386 syscalls on x86_64.

Signed-off-by: Christoph Hellwig <[email protected]>
---
arch/x86/include/asm/compat.h | 1 +
include/linux/compat.h | 9 +++++++++
2 files changed, 10 insertions(+)

diff --git a/arch/x86/include/asm/compat.h b/arch/x86/include/asm/compat.h
index bf547701f41f87..0e327a01f50fbb 100644
--- a/arch/x86/include/asm/compat.h
+++ b/arch/x86/include/asm/compat.h
@@ -209,6 +209,7 @@ static inline bool in_compat_syscall(void)
return in_32bit_syscall();
}
#define in_compat_syscall in_compat_syscall /* override the generic impl */
+#define compat_need_64bit_alignment_fixup in_ia32_syscall
#endif

struct compat_siginfo;
diff --git a/include/linux/compat.h b/include/linux/compat.h
index c22a7f1d253b87..afdd44ba3a8869 100644
--- a/include/linux/compat.h
+++ b/include/linux/compat.h
@@ -945,6 +945,15 @@ static inline bool in_compat_syscall(void) { return false; }

#endif /* CONFIG_COMPAT */

+/*
+ * Some legacy ABIs like the i386 one use less than natural alignment for 64-bit
+ * types, and will need special compat treatment for that. Most architectures
+ * don't need that special handling even for compat syscalls.
+ */
+#ifndef compat_need_64bit_alignment_fixup
+#define compat_need_64bit_alignment_fixup() false
+#endif
+
/*
* A pointer passed in from user mode. This should not
* be used for syscall parameters, just declare them
--
2.27.0

2020-07-26 16:05:40

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 4/4] quota: simplify the quotactl compat handling

Fold the misaligned u64 workarounds into the main quotactl flow instead
of implementing a separate compat syscall handler.

Signed-off-by: Christoph Hellwig <[email protected]>
---
arch/x86/entry/syscalls/syscall_32.tbl | 2 +-
fs/quota/Kconfig | 5 --
fs/quota/Makefile | 1 -
fs/quota/compat.c | 120 -------------------------
fs/quota/compat.h | 34 +++++++
fs/quota/quota.c | 73 ++++++++++++---
include/linux/quotaops.h | 3 -
kernel/sys_ni.c | 1 -
8 files changed, 94 insertions(+), 145 deletions(-)
delete mode 100644 fs/quota/compat.c
create mode 100644 fs/quota/compat.h

diff --git a/arch/x86/entry/syscalls/syscall_32.tbl b/arch/x86/entry/syscalls/syscall_32.tbl
index d8f8a1a69ed11f..41d442d7c2db67 100644
--- a/arch/x86/entry/syscalls/syscall_32.tbl
+++ b/arch/x86/entry/syscalls/syscall_32.tbl
@@ -142,7 +142,7 @@
128 i386 init_module sys_init_module
129 i386 delete_module sys_delete_module
130 i386 get_kernel_syms
-131 i386 quotactl sys_quotactl compat_sys_quotactl32
+131 i386 quotactl sys_quotactl
132 i386 getpgid sys_getpgid
133 i386 fchdir sys_fchdir
134 i386 bdflush sys_bdflush
diff --git a/fs/quota/Kconfig b/fs/quota/Kconfig
index 7218314ca13f00..4f5bb85099a904 100644
--- a/fs/quota/Kconfig
+++ b/fs/quota/Kconfig
@@ -70,8 +70,3 @@ config QFMT_V2
config QUOTACTL
bool
default n
-
-config QUOTACTL_COMPAT
- bool
- depends on QUOTACTL && COMPAT_FOR_U64_ALIGNMENT
- default y
diff --git a/fs/quota/Makefile b/fs/quota/Makefile
index f2b49d0f0287c9..9160639daffa75 100644
--- a/fs/quota/Makefile
+++ b/fs/quota/Makefile
@@ -4,5 +4,4 @@ obj-$(CONFIG_QFMT_V1) += quota_v1.o
obj-$(CONFIG_QFMT_V2) += quota_v2.o
obj-$(CONFIG_QUOTA_TREE) += quota_tree.o
obj-$(CONFIG_QUOTACTL) += quota.o kqid.o
-obj-$(CONFIG_QUOTACTL_COMPAT) += compat.o
obj-$(CONFIG_QUOTA_NETLINK_INTERFACE) += netlink.o
diff --git a/fs/quota/compat.c b/fs/quota/compat.c
deleted file mode 100644
index c305728576193d..00000000000000
--- a/fs/quota/compat.c
+++ /dev/null
@@ -1,120 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-
-#include <linux/syscalls.h>
-#include <linux/compat.h>
-#include <linux/quotaops.h>
-
-/*
- * This code works only for 32 bit quota tools over 64 bit OS (x86_64, ia64)
- * and is necessary due to alignment problems.
- */
-struct compat_if_dqblk {
- compat_u64 dqb_bhardlimit;
- compat_u64 dqb_bsoftlimit;
- compat_u64 dqb_curspace;
- compat_u64 dqb_ihardlimit;
- compat_u64 dqb_isoftlimit;
- compat_u64 dqb_curinodes;
- compat_u64 dqb_btime;
- compat_u64 dqb_itime;
- compat_uint_t dqb_valid;
-};
-
-/* XFS structures */
-struct compat_fs_qfilestat {
- compat_u64 dqb_bhardlimit;
- compat_u64 qfs_nblks;
- compat_uint_t qfs_nextents;
-};
-
-struct compat_fs_quota_stat {
- __s8 qs_version;
- __u16 qs_flags;
- __s8 qs_pad;
- struct compat_fs_qfilestat qs_uquota;
- struct compat_fs_qfilestat qs_gquota;
- compat_uint_t qs_incoredqs;
- compat_int_t qs_btimelimit;
- compat_int_t qs_itimelimit;
- compat_int_t qs_rtbtimelimit;
- __u16 qs_bwarnlimit;
- __u16 qs_iwarnlimit;
-};
-
-COMPAT_SYSCALL_DEFINE4(quotactl32, unsigned int, cmd,
- const char __user *, special, qid_t, id,
- void __user *, addr)
-{
- unsigned int cmds;
- struct if_dqblk __user *dqblk;
- struct compat_if_dqblk __user *compat_dqblk;
- struct fs_quota_stat __user *fsqstat;
- struct compat_fs_quota_stat __user *compat_fsqstat;
- compat_uint_t data;
- u16 xdata;
- long ret;
-
- cmds = cmd >> SUBCMDSHIFT;
-
- switch (cmds) {
- case Q_GETQUOTA:
- dqblk = compat_alloc_user_space(sizeof(struct if_dqblk));
- compat_dqblk = addr;
- ret = kernel_quotactl(cmd, special, id, dqblk);
- if (ret)
- break;
- if (copy_in_user(compat_dqblk, dqblk, sizeof(*compat_dqblk)) ||
- get_user(data, &dqblk->dqb_valid) ||
- put_user(data, &compat_dqblk->dqb_valid))
- ret = -EFAULT;
- break;
- case Q_SETQUOTA:
- dqblk = compat_alloc_user_space(sizeof(struct if_dqblk));
- compat_dqblk = addr;
- ret = -EFAULT;
- if (copy_in_user(dqblk, compat_dqblk, sizeof(*compat_dqblk)) ||
- get_user(data, &compat_dqblk->dqb_valid) ||
- put_user(data, &dqblk->dqb_valid))
- break;
- ret = kernel_quotactl(cmd, special, id, dqblk);
- break;
- case Q_XGETQSTAT:
- fsqstat = compat_alloc_user_space(sizeof(struct fs_quota_stat));
- compat_fsqstat = addr;
- ret = kernel_quotactl(cmd, special, id, fsqstat);
- if (ret)
- break;
- ret = -EFAULT;
- /* Copying qs_version, qs_flags, qs_pad */
- if (copy_in_user(compat_fsqstat, fsqstat,
- offsetof(struct compat_fs_quota_stat, qs_uquota)))
- break;
- /* Copying qs_uquota */
- if (copy_in_user(&compat_fsqstat->qs_uquota,
- &fsqstat->qs_uquota,
- sizeof(compat_fsqstat->qs_uquota)) ||
- get_user(data, &fsqstat->qs_uquota.qfs_nextents) ||
- put_user(data, &compat_fsqstat->qs_uquota.qfs_nextents))
- break;
- /* Copying qs_gquota */
- if (copy_in_user(&compat_fsqstat->qs_gquota,
- &fsqstat->qs_gquota,
- sizeof(compat_fsqstat->qs_gquota)) ||
- get_user(data, &fsqstat->qs_gquota.qfs_nextents) ||
- put_user(data, &compat_fsqstat->qs_gquota.qfs_nextents))
- break;
- /* Copying the rest */
- if (copy_in_user(&compat_fsqstat->qs_incoredqs,
- &fsqstat->qs_incoredqs,
- sizeof(struct compat_fs_quota_stat) -
- offsetof(struct compat_fs_quota_stat, qs_incoredqs)) ||
- get_user(xdata, &fsqstat->qs_iwarnlimit) ||
- put_user(xdata, &compat_fsqstat->qs_iwarnlimit))
- break;
- ret = 0;
- break;
- default:
- ret = kernel_quotactl(cmd, special, id, addr);
- }
- return ret;
-}
diff --git a/fs/quota/compat.h b/fs/quota/compat.h
new file mode 100644
index 00000000000000..ef7d1e12d650b3
--- /dev/null
+++ b/fs/quota/compat.h
@@ -0,0 +1,34 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/compat.h>
+
+struct compat_if_dqblk {
+ compat_u64 dqb_bhardlimit;
+ compat_u64 dqb_bsoftlimit;
+ compat_u64 dqb_curspace;
+ compat_u64 dqb_ihardlimit;
+ compat_u64 dqb_isoftlimit;
+ compat_u64 dqb_curinodes;
+ compat_u64 dqb_btime;
+ compat_u64 dqb_itime;
+ compat_uint_t dqb_valid;
+};
+
+struct compat_fs_qfilestat {
+ compat_u64 dqb_bhardlimit;
+ compat_u64 qfs_nblks;
+ compat_uint_t qfs_nextents;
+};
+
+struct compat_fs_quota_stat {
+ __s8 qs_version;
+ __u16 qs_flags;
+ __s8 qs_pad;
+ struct compat_fs_qfilestat qs_uquota;
+ struct compat_fs_qfilestat qs_gquota;
+ compat_uint_t qs_incoredqs;
+ compat_int_t qs_btimelimit;
+ compat_int_t qs_itimelimit;
+ compat_int_t qs_rtbtimelimit;
+ __u16 qs_bwarnlimit;
+ __u16 qs_iwarnlimit;
+};
diff --git a/fs/quota/quota.c b/fs/quota/quota.c
index 5444d3c4d93f37..e1e9d05a14c3e4 100644
--- a/fs/quota/quota.c
+++ b/fs/quota/quota.c
@@ -19,6 +19,7 @@
#include <linux/types.h>
#include <linux/writeback.h>
#include <linux/nospec.h>
+#include "compat.h"

static int check_quotactl_permission(struct super_block *sb, int type, int cmd,
qid_t id)
@@ -211,8 +212,18 @@ static int quota_getquota(struct super_block *sb, int type, qid_t id,
if (ret)
return ret;
copy_to_if_dqblk(&idq, &fdq);
- if (copy_to_user(addr, &idq, sizeof(idq)))
- return -EFAULT;
+
+ if (compat_need_64bit_alignment_fixup()) {
+ struct compat_if_dqblk __user *compat_dqblk = addr;
+
+ if (copy_to_user(compat_dqblk, &idq, sizeof(*compat_dqblk)))
+ return -EFAULT;
+ if (put_user(idq.dqb_valid, &compat_dqblk->dqb_valid))
+ return -EFAULT;
+ } else {
+ if (copy_to_user(addr, &idq, sizeof(idq)))
+ return -EFAULT;
+ }
return 0;
}

@@ -277,8 +288,16 @@ static int quota_setquota(struct super_block *sb, int type, qid_t id,
struct if_dqblk idq;
struct kqid qid;

- if (copy_from_user(&idq, addr, sizeof(idq)))
- return -EFAULT;
+ if (compat_need_64bit_alignment_fixup()) {
+ struct compat_if_dqblk __user *compat_dqblk = addr;
+
+ if (copy_from_user(&idq, compat_dqblk, sizeof(*compat_dqblk)) ||
+ get_user(idq.dqb_valid, &compat_dqblk->dqb_valid))
+ return -EFAULT;
+ } else {
+ if (copy_from_user(&idq, addr, sizeof(idq)))
+ return -EFAULT;
+ }
if (!sb->s_qcop->set_dqblk)
return -ENOSYS;
qid = make_kqid(current_user_ns(), type, id);
@@ -382,6 +401,33 @@ static int quota_getstate(struct super_block *sb, int type,
return 0;
}

+static int compat_copy_fs_qfilestat(struct compat_fs_qfilestat __user *to,
+ struct fs_qfilestat *from)
+{
+ if (copy_to_user(to, from, sizeof(*to)) ||
+ put_user(from->qfs_nextents, &to->qfs_nextents))
+ return -EFAULT;
+ return 0;
+}
+
+static int compat_copy_fs_quota_stat(struct compat_fs_quota_stat __user *to,
+ struct fs_quota_stat *from)
+{
+ if (put_user(from->qs_version, &to->qs_version) ||
+ put_user(from->qs_flags, &to->qs_flags) ||
+ put_user(from->qs_pad, &to->qs_pad) ||
+ compat_copy_fs_qfilestat(&to->qs_uquota, &from->qs_uquota) ||
+ compat_copy_fs_qfilestat(&to->qs_gquota, &from->qs_gquota) ||
+ put_user(from->qs_incoredqs, &to->qs_incoredqs) ||
+ put_user(from->qs_btimelimit, &to->qs_btimelimit) ||
+ put_user(from->qs_itimelimit, &to->qs_itimelimit) ||
+ put_user(from->qs_rtbtimelimit, &to->qs_rtbtimelimit) ||
+ put_user(from->qs_bwarnlimit, &to->qs_bwarnlimit) ||
+ put_user(from->qs_iwarnlimit, &to->qs_iwarnlimit))
+ return -EFAULT;
+ return 0;
+}
+
static int quota_getxstate(struct super_block *sb, int type, void __user *addr)
{
struct fs_quota_stat fqs;
@@ -390,9 +436,14 @@ static int quota_getxstate(struct super_block *sb, int type, void __user *addr)
if (!sb->s_qcop->get_state)
return -ENOSYS;
ret = quota_getstate(sb, type, &fqs);
- if (!ret && copy_to_user(addr, &fqs, sizeof(fqs)))
+ if (ret)
+ return ret;
+
+ if (compat_need_64bit_alignment_fixup())
+ return compat_copy_fs_quota_stat(addr, &fqs);
+ if (copy_to_user(addr, &fqs, sizeof(fqs)))
return -EFAULT;
- return ret;
+ return 0;
}

static int quota_getstatev(struct super_block *sb, int type,
@@ -816,8 +867,8 @@ static struct super_block *quotactl_block(const char __user *special, int cmd)
* calls. Maybe we need to add the process quotas etc. in the future,
* but we probably should use rlimits for that.
*/
-int kernel_quotactl(unsigned int cmd, const char __user *special,
- qid_t id, void __user *addr)
+SYSCALL_DEFINE4(quotactl, unsigned int, cmd, const char __user *, special,
+ qid_t, id, void __user *, addr)
{
uint cmds, type;
struct super_block *sb = NULL;
@@ -871,9 +922,3 @@ int kernel_quotactl(unsigned int cmd, const char __user *special,
path_put(pathp);
return ret;
}
-
-SYSCALL_DEFINE4(quotactl, unsigned int, cmd, const char __user *, special,
- qid_t, id, void __user *, addr)
-{
- return kernel_quotactl(cmd, special, id, addr);
-}
diff --git a/include/linux/quotaops.h b/include/linux/quotaops.h
index 9cf0cd3dc88c68..a0f6668924d3ef 100644
--- a/include/linux/quotaops.h
+++ b/include/linux/quotaops.h
@@ -27,9 +27,6 @@ static inline bool is_quota_modification(struct inode *inode, struct iattr *ia)
(ia->ia_valid & ATTR_GID && !gid_eq(ia->ia_gid, inode->i_gid));
}

-int kernel_quotactl(unsigned int cmd, const char __user *special,
- qid_t id, void __user *addr);
-
#if defined(CONFIG_QUOTA)

#define quota_error(sb, fmt, args...) \
diff --git a/kernel/sys_ni.c b/kernel/sys_ni.c
index 3b69a560a7ac56..f01b91cc57fa00 100644
--- a/kernel/sys_ni.c
+++ b/kernel/sys_ni.c
@@ -370,7 +370,6 @@ COND_SYSCALL_COMPAT(fanotify_mark);
/* x86 */
COND_SYSCALL(vm86old);
COND_SYSCALL(modify_ldt);
-COND_SYSCALL_COMPAT(quotactl32);
COND_SYSCALL(vm86);
COND_SYSCALL(kexec_file_load);

--
2.27.0

2020-07-26 16:33:28

by Al Viro

[permalink] [raw]
Subject: Re: [PATCH 4/4] quota: simplify the quotactl compat handling

On Sun, Jul 26, 2020 at 06:04:01PM +0200, Christoph Hellwig wrote:
> Fold the misaligned u64 workarounds into the main quotactl flow instead
> of implementing a separate compat syscall handler.

I can live with that (and drop the local quota-related stuff from
copy_in_user/compat_alloc_user_space elimination series). One question,
though:

> +static int compat_copy_fs_qfilestat(struct compat_fs_qfilestat __user *to,
> + struct fs_qfilestat *from)
> +{
> + if (copy_to_user(to, from, sizeof(*to)) ||
> + put_user(from->qfs_nextents, &to->qfs_nextents))
> + return -EFAULT;
> + return 0;
> +}

do we have any need of that put_user()? Note that you don't even call
that thing unless compat_need_64bit_alignment_fixup() is true. And AFAICS
all such cases are little-endian...

2020-07-26 16:35:12

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 4/4] quota: simplify the quotactl compat handling

On Sun, Jul 26, 2020 at 05:32:14PM +0100, Al Viro wrote:
> > +static int compat_copy_fs_qfilestat(struct compat_fs_qfilestat __user *to,
> > + struct fs_qfilestat *from)
> > +{
> > + if (copy_to_user(to, from, sizeof(*to)) ||
> > + put_user(from->qfs_nextents, &to->qfs_nextents))
> > + return -EFAULT;
> > + return 0;
> > +}
>
> do we have any need of that put_user()? Note that you don't even call
> that thing unless compat_need_64bit_alignment_fixup() is true. And AFAICS
> all such cases are little-endian...

The main reason it is there is to preserve the previous semantics.
And no, I don't think we actually need it on x86. But what if some
poor souls adds a BE version that needs this? E.g. arm oabi has similar
weird alignment, and now imagine someone adding arm64 compat code for
that..

2020-07-26 16:39:56

by Al Viro

[permalink] [raw]
Subject: Re: [PATCH 4/4] quota: simplify the quotactl compat handling

On Sun, Jul 26, 2020 at 06:34:22PM +0200, Christoph Hellwig wrote:
> On Sun, Jul 26, 2020 at 05:32:14PM +0100, Al Viro wrote:
> > > +static int compat_copy_fs_qfilestat(struct compat_fs_qfilestat __user *to,
> > > + struct fs_qfilestat *from)
> > > +{
> > > + if (copy_to_user(to, from, sizeof(*to)) ||
> > > + put_user(from->qfs_nextents, &to->qfs_nextents))
> > > + return -EFAULT;
> > > + return 0;
> > > +}
> >
> > do we have any need of that put_user()? Note that you don't even call
> > that thing unless compat_need_64bit_alignment_fixup() is true. And AFAICS
> > all such cases are little-endian...
>
> The main reason it is there is to preserve the previous semantics.
> And no, I don't think we actually need it on x86. But what if some
> poor souls adds a BE version that needs this? E.g. arm oabi has similar
> weird alignment, and now imagine someone adding arm64 compat code for
> that..

I'd probably add /* just in case some poor sod fucks up the same way for big-endian biarch */
next to that put_user(), then ;-)

2020-07-27 12:43:11

by Jan Kara

[permalink] [raw]
Subject: Re: [PATCH 4/4] quota: simplify the quotactl compat handling

On Sun 26-07-20 18:04:01, Christoph Hellwig wrote:
> Fold the misaligned u64 workarounds into the main quotactl flow instead
> of implementing a separate compat syscall handler.
>
> Signed-off-by: Christoph Hellwig <[email protected]>

The patch looks good to me and it saves a lot of boiler-plate code so feel
free to add:

Acked-by: Jan Kara <[email protected]>

Honza

> ---
> arch/x86/entry/syscalls/syscall_32.tbl | 2 +-
> fs/quota/Kconfig | 5 --
> fs/quota/Makefile | 1 -
> fs/quota/compat.c | 120 -------------------------
> fs/quota/compat.h | 34 +++++++
> fs/quota/quota.c | 73 ++++++++++++---
> include/linux/quotaops.h | 3 -
> kernel/sys_ni.c | 1 -
> 8 files changed, 94 insertions(+), 145 deletions(-)
> delete mode 100644 fs/quota/compat.c
> create mode 100644 fs/quota/compat.h
>
> diff --git a/arch/x86/entry/syscalls/syscall_32.tbl b/arch/x86/entry/syscalls/syscall_32.tbl
> index d8f8a1a69ed11f..41d442d7c2db67 100644
> --- a/arch/x86/entry/syscalls/syscall_32.tbl
> +++ b/arch/x86/entry/syscalls/syscall_32.tbl
> @@ -142,7 +142,7 @@
> 128 i386 init_module sys_init_module
> 129 i386 delete_module sys_delete_module
> 130 i386 get_kernel_syms
> -131 i386 quotactl sys_quotactl compat_sys_quotactl32
> +131 i386 quotactl sys_quotactl
> 132 i386 getpgid sys_getpgid
> 133 i386 fchdir sys_fchdir
> 134 i386 bdflush sys_bdflush
> diff --git a/fs/quota/Kconfig b/fs/quota/Kconfig
> index 7218314ca13f00..4f5bb85099a904 100644
> --- a/fs/quota/Kconfig
> +++ b/fs/quota/Kconfig
> @@ -70,8 +70,3 @@ config QFMT_V2
> config QUOTACTL
> bool
> default n
> -
> -config QUOTACTL_COMPAT
> - bool
> - depends on QUOTACTL && COMPAT_FOR_U64_ALIGNMENT
> - default y
> diff --git a/fs/quota/Makefile b/fs/quota/Makefile
> index f2b49d0f0287c9..9160639daffa75 100644
> --- a/fs/quota/Makefile
> +++ b/fs/quota/Makefile
> @@ -4,5 +4,4 @@ obj-$(CONFIG_QFMT_V1) += quota_v1.o
> obj-$(CONFIG_QFMT_V2) += quota_v2.o
> obj-$(CONFIG_QUOTA_TREE) += quota_tree.o
> obj-$(CONFIG_QUOTACTL) += quota.o kqid.o
> -obj-$(CONFIG_QUOTACTL_COMPAT) += compat.o
> obj-$(CONFIG_QUOTA_NETLINK_INTERFACE) += netlink.o
> diff --git a/fs/quota/compat.c b/fs/quota/compat.c
> deleted file mode 100644
> index c305728576193d..00000000000000
> --- a/fs/quota/compat.c
> +++ /dev/null
> @@ -1,120 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0
> -
> -#include <linux/syscalls.h>
> -#include <linux/compat.h>
> -#include <linux/quotaops.h>
> -
> -/*
> - * This code works only for 32 bit quota tools over 64 bit OS (x86_64, ia64)
> - * and is necessary due to alignment problems.
> - */
> -struct compat_if_dqblk {
> - compat_u64 dqb_bhardlimit;
> - compat_u64 dqb_bsoftlimit;
> - compat_u64 dqb_curspace;
> - compat_u64 dqb_ihardlimit;
> - compat_u64 dqb_isoftlimit;
> - compat_u64 dqb_curinodes;
> - compat_u64 dqb_btime;
> - compat_u64 dqb_itime;
> - compat_uint_t dqb_valid;
> -};
> -
> -/* XFS structures */
> -struct compat_fs_qfilestat {
> - compat_u64 dqb_bhardlimit;
> - compat_u64 qfs_nblks;
> - compat_uint_t qfs_nextents;
> -};
> -
> -struct compat_fs_quota_stat {
> - __s8 qs_version;
> - __u16 qs_flags;
> - __s8 qs_pad;
> - struct compat_fs_qfilestat qs_uquota;
> - struct compat_fs_qfilestat qs_gquota;
> - compat_uint_t qs_incoredqs;
> - compat_int_t qs_btimelimit;
> - compat_int_t qs_itimelimit;
> - compat_int_t qs_rtbtimelimit;
> - __u16 qs_bwarnlimit;
> - __u16 qs_iwarnlimit;
> -};
> -
> -COMPAT_SYSCALL_DEFINE4(quotactl32, unsigned int, cmd,
> - const char __user *, special, qid_t, id,
> - void __user *, addr)
> -{
> - unsigned int cmds;
> - struct if_dqblk __user *dqblk;
> - struct compat_if_dqblk __user *compat_dqblk;
> - struct fs_quota_stat __user *fsqstat;
> - struct compat_fs_quota_stat __user *compat_fsqstat;
> - compat_uint_t data;
> - u16 xdata;
> - long ret;
> -
> - cmds = cmd >> SUBCMDSHIFT;
> -
> - switch (cmds) {
> - case Q_GETQUOTA:
> - dqblk = compat_alloc_user_space(sizeof(struct if_dqblk));
> - compat_dqblk = addr;
> - ret = kernel_quotactl(cmd, special, id, dqblk);
> - if (ret)
> - break;
> - if (copy_in_user(compat_dqblk, dqblk, sizeof(*compat_dqblk)) ||
> - get_user(data, &dqblk->dqb_valid) ||
> - put_user(data, &compat_dqblk->dqb_valid))
> - ret = -EFAULT;
> - break;
> - case Q_SETQUOTA:
> - dqblk = compat_alloc_user_space(sizeof(struct if_dqblk));
> - compat_dqblk = addr;
> - ret = -EFAULT;
> - if (copy_in_user(dqblk, compat_dqblk, sizeof(*compat_dqblk)) ||
> - get_user(data, &compat_dqblk->dqb_valid) ||
> - put_user(data, &dqblk->dqb_valid))
> - break;
> - ret = kernel_quotactl(cmd, special, id, dqblk);
> - break;
> - case Q_XGETQSTAT:
> - fsqstat = compat_alloc_user_space(sizeof(struct fs_quota_stat));
> - compat_fsqstat = addr;
> - ret = kernel_quotactl(cmd, special, id, fsqstat);
> - if (ret)
> - break;
> - ret = -EFAULT;
> - /* Copying qs_version, qs_flags, qs_pad */
> - if (copy_in_user(compat_fsqstat, fsqstat,
> - offsetof(struct compat_fs_quota_stat, qs_uquota)))
> - break;
> - /* Copying qs_uquota */
> - if (copy_in_user(&compat_fsqstat->qs_uquota,
> - &fsqstat->qs_uquota,
> - sizeof(compat_fsqstat->qs_uquota)) ||
> - get_user(data, &fsqstat->qs_uquota.qfs_nextents) ||
> - put_user(data, &compat_fsqstat->qs_uquota.qfs_nextents))
> - break;
> - /* Copying qs_gquota */
> - if (copy_in_user(&compat_fsqstat->qs_gquota,
> - &fsqstat->qs_gquota,
> - sizeof(compat_fsqstat->qs_gquota)) ||
> - get_user(data, &fsqstat->qs_gquota.qfs_nextents) ||
> - put_user(data, &compat_fsqstat->qs_gquota.qfs_nextents))
> - break;
> - /* Copying the rest */
> - if (copy_in_user(&compat_fsqstat->qs_incoredqs,
> - &fsqstat->qs_incoredqs,
> - sizeof(struct compat_fs_quota_stat) -
> - offsetof(struct compat_fs_quota_stat, qs_incoredqs)) ||
> - get_user(xdata, &fsqstat->qs_iwarnlimit) ||
> - put_user(xdata, &compat_fsqstat->qs_iwarnlimit))
> - break;
> - ret = 0;
> - break;
> - default:
> - ret = kernel_quotactl(cmd, special, id, addr);
> - }
> - return ret;
> -}
> diff --git a/fs/quota/compat.h b/fs/quota/compat.h
> new file mode 100644
> index 00000000000000..ef7d1e12d650b3
> --- /dev/null
> +++ b/fs/quota/compat.h
> @@ -0,0 +1,34 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <linux/compat.h>
> +
> +struct compat_if_dqblk {
> + compat_u64 dqb_bhardlimit;
> + compat_u64 dqb_bsoftlimit;
> + compat_u64 dqb_curspace;
> + compat_u64 dqb_ihardlimit;
> + compat_u64 dqb_isoftlimit;
> + compat_u64 dqb_curinodes;
> + compat_u64 dqb_btime;
> + compat_u64 dqb_itime;
> + compat_uint_t dqb_valid;
> +};
> +
> +struct compat_fs_qfilestat {
> + compat_u64 dqb_bhardlimit;
> + compat_u64 qfs_nblks;
> + compat_uint_t qfs_nextents;
> +};
> +
> +struct compat_fs_quota_stat {
> + __s8 qs_version;
> + __u16 qs_flags;
> + __s8 qs_pad;
> + struct compat_fs_qfilestat qs_uquota;
> + struct compat_fs_qfilestat qs_gquota;
> + compat_uint_t qs_incoredqs;
> + compat_int_t qs_btimelimit;
> + compat_int_t qs_itimelimit;
> + compat_int_t qs_rtbtimelimit;
> + __u16 qs_bwarnlimit;
> + __u16 qs_iwarnlimit;
> +};
> diff --git a/fs/quota/quota.c b/fs/quota/quota.c
> index 5444d3c4d93f37..e1e9d05a14c3e4 100644
> --- a/fs/quota/quota.c
> +++ b/fs/quota/quota.c
> @@ -19,6 +19,7 @@
> #include <linux/types.h>
> #include <linux/writeback.h>
> #include <linux/nospec.h>
> +#include "compat.h"
>
> static int check_quotactl_permission(struct super_block *sb, int type, int cmd,
> qid_t id)
> @@ -211,8 +212,18 @@ static int quota_getquota(struct super_block *sb, int type, qid_t id,
> if (ret)
> return ret;
> copy_to_if_dqblk(&idq, &fdq);
> - if (copy_to_user(addr, &idq, sizeof(idq)))
> - return -EFAULT;
> +
> + if (compat_need_64bit_alignment_fixup()) {
> + struct compat_if_dqblk __user *compat_dqblk = addr;
> +
> + if (copy_to_user(compat_dqblk, &idq, sizeof(*compat_dqblk)))
> + return -EFAULT;
> + if (put_user(idq.dqb_valid, &compat_dqblk->dqb_valid))
> + return -EFAULT;
> + } else {
> + if (copy_to_user(addr, &idq, sizeof(idq)))
> + return -EFAULT;
> + }
> return 0;
> }
>
> @@ -277,8 +288,16 @@ static int quota_setquota(struct super_block *sb, int type, qid_t id,
> struct if_dqblk idq;
> struct kqid qid;
>
> - if (copy_from_user(&idq, addr, sizeof(idq)))
> - return -EFAULT;
> + if (compat_need_64bit_alignment_fixup()) {
> + struct compat_if_dqblk __user *compat_dqblk = addr;
> +
> + if (copy_from_user(&idq, compat_dqblk, sizeof(*compat_dqblk)) ||
> + get_user(idq.dqb_valid, &compat_dqblk->dqb_valid))
> + return -EFAULT;
> + } else {
> + if (copy_from_user(&idq, addr, sizeof(idq)))
> + return -EFAULT;
> + }
> if (!sb->s_qcop->set_dqblk)
> return -ENOSYS;
> qid = make_kqid(current_user_ns(), type, id);
> @@ -382,6 +401,33 @@ static int quota_getstate(struct super_block *sb, int type,
> return 0;
> }
>
> +static int compat_copy_fs_qfilestat(struct compat_fs_qfilestat __user *to,
> + struct fs_qfilestat *from)
> +{
> + if (copy_to_user(to, from, sizeof(*to)) ||
> + put_user(from->qfs_nextents, &to->qfs_nextents))
> + return -EFAULT;
> + return 0;
> +}
> +
> +static int compat_copy_fs_quota_stat(struct compat_fs_quota_stat __user *to,
> + struct fs_quota_stat *from)
> +{
> + if (put_user(from->qs_version, &to->qs_version) ||
> + put_user(from->qs_flags, &to->qs_flags) ||
> + put_user(from->qs_pad, &to->qs_pad) ||
> + compat_copy_fs_qfilestat(&to->qs_uquota, &from->qs_uquota) ||
> + compat_copy_fs_qfilestat(&to->qs_gquota, &from->qs_gquota) ||
> + put_user(from->qs_incoredqs, &to->qs_incoredqs) ||
> + put_user(from->qs_btimelimit, &to->qs_btimelimit) ||
> + put_user(from->qs_itimelimit, &to->qs_itimelimit) ||
> + put_user(from->qs_rtbtimelimit, &to->qs_rtbtimelimit) ||
> + put_user(from->qs_bwarnlimit, &to->qs_bwarnlimit) ||
> + put_user(from->qs_iwarnlimit, &to->qs_iwarnlimit))
> + return -EFAULT;
> + return 0;
> +}
> +
> static int quota_getxstate(struct super_block *sb, int type, void __user *addr)
> {
> struct fs_quota_stat fqs;
> @@ -390,9 +436,14 @@ static int quota_getxstate(struct super_block *sb, int type, void __user *addr)
> if (!sb->s_qcop->get_state)
> return -ENOSYS;
> ret = quota_getstate(sb, type, &fqs);
> - if (!ret && copy_to_user(addr, &fqs, sizeof(fqs)))
> + if (ret)
> + return ret;
> +
> + if (compat_need_64bit_alignment_fixup())
> + return compat_copy_fs_quota_stat(addr, &fqs);
> + if (copy_to_user(addr, &fqs, sizeof(fqs)))
> return -EFAULT;
> - return ret;
> + return 0;
> }
>
> static int quota_getstatev(struct super_block *sb, int type,
> @@ -816,8 +867,8 @@ static struct super_block *quotactl_block(const char __user *special, int cmd)
> * calls. Maybe we need to add the process quotas etc. in the future,
> * but we probably should use rlimits for that.
> */
> -int kernel_quotactl(unsigned int cmd, const char __user *special,
> - qid_t id, void __user *addr)
> +SYSCALL_DEFINE4(quotactl, unsigned int, cmd, const char __user *, special,
> + qid_t, id, void __user *, addr)
> {
> uint cmds, type;
> struct super_block *sb = NULL;
> @@ -871,9 +922,3 @@ int kernel_quotactl(unsigned int cmd, const char __user *special,
> path_put(pathp);
> return ret;
> }
> -
> -SYSCALL_DEFINE4(quotactl, unsigned int, cmd, const char __user *, special,
> - qid_t, id, void __user *, addr)
> -{
> - return kernel_quotactl(cmd, special, id, addr);
> -}
> diff --git a/include/linux/quotaops.h b/include/linux/quotaops.h
> index 9cf0cd3dc88c68..a0f6668924d3ef 100644
> --- a/include/linux/quotaops.h
> +++ b/include/linux/quotaops.h
> @@ -27,9 +27,6 @@ static inline bool is_quota_modification(struct inode *inode, struct iattr *ia)
> (ia->ia_valid & ATTR_GID && !gid_eq(ia->ia_gid, inode->i_gid));
> }
>
> -int kernel_quotactl(unsigned int cmd, const char __user *special,
> - qid_t id, void __user *addr);
> -
> #if defined(CONFIG_QUOTA)
>
> #define quota_error(sb, fmt, args...) \
> diff --git a/kernel/sys_ni.c b/kernel/sys_ni.c
> index 3b69a560a7ac56..f01b91cc57fa00 100644
> --- a/kernel/sys_ni.c
> +++ b/kernel/sys_ni.c
> @@ -370,7 +370,6 @@ COND_SYSCALL_COMPAT(fanotify_mark);
> /* x86 */
> COND_SYSCALL(vm86old);
> COND_SYSCALL(modify_ldt);
> -COND_SYSCALL_COMPAT(quotactl32);
> COND_SYSCALL(vm86);
> COND_SYSCALL(kexec_file_load);
>
> --
> 2.27.0
>
--
Jan Kara <[email protected]>
SUSE Labs, CR

2020-07-27 18:48:07

by Al Viro

[permalink] [raw]
Subject: Re: [PATCH 4/4] quota: simplify the quotactl compat handling

On Mon, Jul 27, 2020 at 02:41:27PM +0200, Jan Kara wrote:
> On Sun 26-07-20 18:04:01, Christoph Hellwig wrote:
> > Fold the misaligned u64 workarounds into the main quotactl flow instead
> > of implementing a separate compat syscall handler.
> >
> > Signed-off-by: Christoph Hellwig <[email protected]>
>
> The patch looks good to me and it saves a lot of boiler-plate code so feel
> free to add:
>
> Acked-by: Jan Kara <[email protected]>

Which tree do we put that through? I can grab it into vfs.git, but IIRC usually
you put quota-related stuff through yours...

2020-07-27 21:32:46

by Jan Kara

[permalink] [raw]
Subject: Re: [PATCH 4/4] quota: simplify the quotactl compat handling

On Mon 27-07-20 16:56:16, Al Viro wrote:
> On Mon, Jul 27, 2020 at 02:41:27PM +0200, Jan Kara wrote:
> > On Sun 26-07-20 18:04:01, Christoph Hellwig wrote:
> > > Fold the misaligned u64 workarounds into the main quotactl flow instead
> > > of implementing a separate compat syscall handler.
> > >
> > > Signed-off-by: Christoph Hellwig <[email protected]>
> >
> > The patch looks good to me and it saves a lot of boiler-plate code so feel
> > free to add:
> >
> > Acked-by: Jan Kara <[email protected]>
>
> Which tree do we put that through? I can grab it into vfs.git, but IIRC usually
> you put quota-related stuff through yours...

I can take it through quota tree as well but I thought it's not a great fit
given the generic arch bits which this last patch depends on...

Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR

2020-07-28 01:50:35

by Al Viro

[permalink] [raw]
Subject: Re: [PATCH 4/4] quota: simplify the quotactl compat handling

On Mon, Jul 27, 2020 at 11:31:58PM +0200, Jan Kara wrote:
> On Mon 27-07-20 16:56:16, Al Viro wrote:
> > On Mon, Jul 27, 2020 at 02:41:27PM +0200, Jan Kara wrote:
> > > On Sun 26-07-20 18:04:01, Christoph Hellwig wrote:
> > > > Fold the misaligned u64 workarounds into the main quotactl flow instead
> > > > of implementing a separate compat syscall handler.
> > > >
> > > > Signed-off-by: Christoph Hellwig <[email protected]>
> > >
> > > The patch looks good to me and it saves a lot of boiler-plate code so feel
> > > free to add:
> > >
> > > Acked-by: Jan Kara <[email protected]>
> >
> > Which tree do we put that through? I can grab it into vfs.git, but IIRC usually
> > you put quota-related stuff through yours...
>
> I can take it through quota tree as well but I thought it's not a great fit
> given the generic arch bits which this last patch depends on...

OK, into vfs.git it goes, then...

2020-07-29 06:09:06

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 2/4 v2] compat: lift compat_s64 and compat_u64 to <linux/compat.h>

lift the compat_s64 and compat_u64 definitions into common code using the
COMPAT_FOR_U64_ALIGNMENT symbol for the x86 special case.

Signed-off-by: Christoph Hellwig <[email protected]>
---

Changes since v1: fix a typo

arch/arm64/include/asm/compat.h | 2 --
arch/mips/include/asm/compat.h | 2 --
arch/parisc/include/asm/compat.h | 2 --
arch/powerpc/include/asm/compat.h | 2 --
arch/s390/include/asm/compat.h | 2 --
arch/sparc/include/asm/compat.h | 3 +--
arch/x86/include/asm/compat.h | 2 --
include/linux/compat.h | 8 ++++++++
8 files changed, 9 insertions(+), 14 deletions(-)

diff --git a/arch/arm64/include/asm/compat.h b/arch/arm64/include/asm/compat.h
index 935d2aa231bf06..23a9fb73c04ff8 100644
--- a/arch/arm64/include/asm/compat.h
+++ b/arch/arm64/include/asm/compat.h
@@ -35,8 +35,6 @@ typedef s32 compat_nlink_t;
typedef u16 compat_ipc_pid_t;
typedef u32 compat_caddr_t;
typedef __kernel_fsid_t compat_fsid_t;
-typedef s64 compat_s64;
-typedef u64 compat_u64;

struct compat_stat {
#ifdef __AARCH64EB__
diff --git a/arch/mips/include/asm/compat.h b/arch/mips/include/asm/compat.h
index 255afcdd79c94b..65975712a22dcf 100644
--- a/arch/mips/include/asm/compat.h
+++ b/arch/mips/include/asm/compat.h
@@ -26,8 +26,6 @@ typedef s32 compat_caddr_t;
typedef struct {
s32 val[2];
} compat_fsid_t;
-typedef s64 compat_s64;
-typedef u64 compat_u64;

struct compat_stat {
compat_dev_t st_dev;
diff --git a/arch/parisc/include/asm/compat.h b/arch/parisc/include/asm/compat.h
index 2f4f66a3bac079..8f33085ff1bd88 100644
--- a/arch/parisc/include/asm/compat.h
+++ b/arch/parisc/include/asm/compat.h
@@ -22,8 +22,6 @@ typedef u32 compat_dev_t;
typedef u16 compat_nlink_t;
typedef u16 compat_ipc_pid_t;
typedef u32 compat_caddr_t;
-typedef s64 compat_s64;
-typedef u64 compat_u64;

struct compat_stat {
compat_dev_t st_dev; /* dev_t is 32 bits on parisc */
diff --git a/arch/powerpc/include/asm/compat.h b/arch/powerpc/include/asm/compat.h
index 3e3cdfaa76c6a5..9191fc29e6ed11 100644
--- a/arch/powerpc/include/asm/compat.h
+++ b/arch/powerpc/include/asm/compat.h
@@ -27,8 +27,6 @@ typedef s16 compat_nlink_t;
typedef u16 compat_ipc_pid_t;
typedef u32 compat_caddr_t;
typedef __kernel_fsid_t compat_fsid_t;
-typedef s64 compat_s64;
-typedef u64 compat_u64;

struct compat_stat {
compat_dev_t st_dev;
diff --git a/arch/s390/include/asm/compat.h b/arch/s390/include/asm/compat.h
index 9547cd5d6cdc21..ea5b9c34b7be5b 100644
--- a/arch/s390/include/asm/compat.h
+++ b/arch/s390/include/asm/compat.h
@@ -63,8 +63,6 @@ typedef u16 compat_nlink_t;
typedef u16 compat_ipc_pid_t;
typedef u32 compat_caddr_t;
typedef __kernel_fsid_t compat_fsid_t;
-typedef s64 compat_s64;
-typedef u64 compat_u64;

typedef struct {
u32 mask;
diff --git a/arch/sparc/include/asm/compat.h b/arch/sparc/include/asm/compat.h
index 40a267b3bd5208..b85842cda99fe0 100644
--- a/arch/sparc/include/asm/compat.h
+++ b/arch/sparc/include/asm/compat.h
@@ -21,8 +21,7 @@ typedef s16 compat_nlink_t;
typedef u16 compat_ipc_pid_t;
typedef u32 compat_caddr_t;
typedef __kernel_fsid_t compat_fsid_t;
-typedef s64 compat_s64;
-typedef u64 compat_u64;
+
struct compat_stat {
compat_dev_t st_dev;
compat_ino_t st_ino;
diff --git a/arch/x86/include/asm/compat.h b/arch/x86/include/asm/compat.h
index d4edf281fff49d..bf547701f41f87 100644
--- a/arch/x86/include/asm/compat.h
+++ b/arch/x86/include/asm/compat.h
@@ -27,8 +27,6 @@ typedef u16 compat_nlink_t;
typedef u16 compat_ipc_pid_t;
typedef u32 compat_caddr_t;
typedef __kernel_fsid_t compat_fsid_t;
-typedef s64 __attribute__((aligned(4))) compat_s64;
-typedef u64 __attribute__((aligned(4))) compat_u64;

struct compat_stat {
compat_dev_t st_dev;
diff --git a/include/linux/compat.h b/include/linux/compat.h
index e90100c0de72e4..ffb641f77bb7af 100644
--- a/include/linux/compat.h
+++ b/include/linux/compat.h
@@ -91,6 +91,14 @@
static inline long __do_compat_sys##name(__MAP(x,__SC_DECL,__VA_ARGS__))
#endif /* COMPAT_SYSCALL_DEFINEx */

+#ifdef CONFIG_COMPAT_FOR_U64_ALIGNMENT
+typedef s64 __attribute__((aligned(4))) compat_s64;
+typedef u64 __attribute__((aligned(4))) compat_u64;
+#else
+typedef s64 compat_s64;
+typedef u64 compat_u64;
+#endif
+
#ifdef CONFIG_COMPAT

#ifndef compat_user_stack_pointer
--
2.27.0

2020-07-30 17:37:41

by Nathan Chancellor

[permalink] [raw]
Subject: Re: [PATCH 1/4] arm64: stop using <asm/compat.h> directly

On Sun, Jul 26, 2020 at 06:03:58PM +0200, Christoph Hellwig wrote:
> Always use <linux/compat.h> so that we can move more declarations to
> common code. In two of the three cases the asm include was in addition
> to an existing one for <linux/compat.h> anyway.
>
> Signed-off-by: Christoph Hellwig <[email protected]>
> ---
> arch/arm64/include/asm/stat.h | 2 +-
> arch/arm64/kernel/process.c | 1 -
> arch/arm64/kernel/ptrace.c | 1 -
> 3 files changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/arch/arm64/include/asm/stat.h b/arch/arm64/include/asm/stat.h
> index 3b4a62f5aeb0c3..1b5ac1ef5d04cc 100644
> --- a/arch/arm64/include/asm/stat.h
> +++ b/arch/arm64/include/asm/stat.h
> @@ -10,7 +10,7 @@
> #ifdef CONFIG_COMPAT
>
> #include <linux/time.h>
> -#include <asm/compat.h>
> +#include <linux/compat.h>

This breaks arm64 defconfig:

$ make -skj"$(nproc)" ARCH=arm64 CROSS_COMPILE=aarch64-linux- distclean defconfig init/main.o
In file included from ./include/linux/compat.h:17,
from ./arch/arm64/include/asm/stat.h:13,
from ./include/linux/stat.h:6,
from ./include/linux/sysfs.h:22,
from ./include/linux/kobject.h:20,
from ./include/linux/of.h:17,
from ./include/linux/irqdomain.h:35,
from ./include/linux/acpi.h:13,
from ./include/acpi/apei.h:9,
from ./include/acpi/ghes.h:5,
from ./include/linux/arm_sdei.h:8,
from arch/arm64/kernel/asm-offsets.c:10:
./include/linux/fs.h: In function 'vfs_whiteout':
./include/linux/fs.h:1736:32: error: 'S_IFCHR' undeclared (first use in this function)
1736 | return vfs_mknod(dir, dentry, S_IFCHR | WHITEOUT_MODE, WHITEOUT_DEV);
| ^~~~~~~
./include/linux/fs.h:1736:32: note: each undeclared identifier is reported only once for each function it appears in
./include/linux/fs.h: At top level:
./include/linux/fs.h:1886:46: warning: 'struct kstat' declared inside parameter list will not be visible outside of this definition or declaration
1886 | int (*getattr) (const struct path *, struct kstat *, u32, unsigned int);
| ^~~~~
./include/linux/fs.h: In function '__mandatory_lock':
./include/linux/fs.h:2372:25: error: 'S_ISGID' undeclared (first use in this function); did you mean 'SIGIO'?
2372 | return (ino->i_mode & (S_ISGID | S_IXGRP)) == S_ISGID;
| ^~~~~~~
| SIGIO
./include/linux/fs.h:2372:35: error: 'S_IXGRP' undeclared (first use in this function)
2372 | return (ino->i_mode & (S_ISGID | S_IXGRP)) == S_ISGID;
| ^~~~~~~
...

$ git bisect log
# bad: [7b287a5c6ac518c415a258f2aa7b1ebb25c263d2] Add linux-next specific files for 20200730
# good: [d3590ebf6f91350192737dd1d1b219c05277f067] Merge tag 'audit-pr-20200729' of git://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/audit
git bisect start '7b287a5c6ac518c415a258f2aa7b1ebb25c263d2' 'd3590ebf6f91350192737dd1d1b219c05277f067'
# bad: [1f1ed12be70e9eb4e05ac206c6ad6a5a31f5b921] Merge remote-tracking branch 'crypto/master'
git bisect bad 1f1ed12be70e9eb4e05ac206c6ad6a5a31f5b921
# bad: [07fad673c2f1a02440c879c34f8182b12786a735] Merge remote-tracking branch 'hid/for-next'
git bisect bad 07fad673c2f1a02440c879c34f8182b12786a735
# good: [7a77c92312546a74d3507484b256ae17bfb2cfe2] Merge remote-tracking branch 'm68knommu/for-next'
git bisect good 7a77c92312546a74d3507484b256ae17bfb2cfe2
# good: [40dd62f180e38317e744d7f82c98af31a24fd2c9] Merge remote-tracking branch 'f2fs/dev'
git bisect good 40dd62f180e38317e744d7f82c98af31a24fd2c9
# bad: [52138dfdd2192bcfc7d3bc2e79475966ee4b20c4] Merge remote-tracking branch 'printk/for-next'
git bisect bad 52138dfdd2192bcfc7d3bc2e79475966ee4b20c4
# good: [a37c3e37fa3fa1381e03d918d708f82927ddd160] Merge remote-tracking branch 'xfs/for-next'
git bisect good a37c3e37fa3fa1381e03d918d708f82927ddd160
# good: [4e523547e2bf755d40cb10e85795c2f9620ff3fb] nvme-pci: add a blank line after declarations
git bisect good 4e523547e2bf755d40cb10e85795c2f9620ff3fb
# bad: [5066741180729f7bad9401de34efda3766c3274a] Merge branches 'fixes' and 'work.quota-compat' into for-next
git bisect bad 5066741180729f7bad9401de34efda3766c3274a
# good: [4ff8a356daafaafbf90141ee7a3b8fdc18e560a8] ia64: switch to ->regset_get()
git bisect good 4ff8a356daafaafbf90141ee7a3b8fdc18e560a8
# good: [ce327e1c54119179066d6f3573a28001febc9265] regset: kill user_regset_copyout{,_zero}()
git bisect good ce327e1c54119179066d6f3573a28001febc9265
# good: [1697a322e28ba96d35953c5d824540d172546d36] [elf-fdpic] switch coredump to regsets
git bisect good 1697a322e28ba96d35953c5d824540d172546d36
# good: [259bf01c1bd1f049958496a089c4f334fe0c8a48] Merge branches 'work.misc', 'work.regset' and 'work.fdpic' into for-next
git bisect good 259bf01c1bd1f049958496a089c4f334fe0c8a48
# bad: [0a3a4497a1de8e68e809a693b549c7ec2f195301] compat: lift compat_s64 and compat_u64 to <linux/compat.h>
git bisect bad 0a3a4497a1de8e68e809a693b549c7ec2f195301
# bad: [b902bfb3f0e9d07ec9f48256e57e5c5de6108f8c] arm64: stop using <asm/compat.h> directly
git bisect bad b902bfb3f0e9d07ec9f48256e57e5c5de6108f8c
# first bad commit: [b902bfb3f0e9d07ec9f48256e57e5c5de6108f8c] arm64: stop using <asm/compat.h> directly

I assume the stat header order should be messed around with but I am not
sure what exactly that would entail to make sure that nothing else
breaks, hence just the report.

Cheers,
Nathan

2020-07-31 08:45:20

by Naresh Kamboju

[permalink] [raw]
Subject: Re: [PATCH 1/4] arm64: stop using <asm/compat.h> directly

On Thu, 30 Jul 2020 at 23:04, Nathan Chancellor
<[email protected]> wrote:
>
> On Sun, Jul 26, 2020 at 06:03:58PM +0200, Christoph Hellwig wrote:
> > Always use <linux/compat.h> so that we can move more declarations to
> > common code. In two of the three cases the asm include was in addition
> > to an existing one for <linux/compat.h> anyway.
> >
> > Signed-off-by: Christoph Hellwig <[email protected]>
> > ---
> > arch/arm64/include/asm/stat.h | 2 +-
> > arch/arm64/kernel/process.c | 1 -
> > arch/arm64/kernel/ptrace.c | 1 -
> > 3 files changed, 1 insertion(+), 3 deletions(-)
> >
> > diff --git a/arch/arm64/include/asm/stat.h b/arch/arm64/include/asm/stat.h
> > index 3b4a62f5aeb0c3..1b5ac1ef5d04cc 100644
> > --- a/arch/arm64/include/asm/stat.h
> > +++ b/arch/arm64/include/asm/stat.h
> > @@ -10,7 +10,7 @@
> > #ifdef CONFIG_COMPAT
> >
> > #include <linux/time.h>
> > -#include <asm/compat.h>
> > +#include <linux/compat.h>
>
> This breaks arm64 defconfig:
>
> $ make -skj"$(nproc)" ARCH=arm64 CROSS_COMPILE=aarch64-linux- distclean defconfig init/main.o
> In file included from ./include/linux/compat.h:17,
> from ./arch/arm64/include/asm/stat.h:13,
> from ./include/linux/stat.h:6,
> from ./include/linux/sysfs.h:22,
> from ./include/linux/kobject.h:20,
> from ./include/linux/of.h:17,
> from ./include/linux/irqdomain.h:35,
> from ./include/linux/acpi.h:13,
> from ./include/acpi/apei.h:9,
> from ./include/acpi/ghes.h:5,
> from ./include/linux/arm_sdei.h:8,
> from arch/arm64/kernel/asm-offsets.c:10:
> ./include/linux/fs.h: In function 'vfs_whiteout':
> ./include/linux/fs.h:1736:32: error: 'S_IFCHR' undeclared (first use in this function)
> 1736 | return vfs_mknod(dir, dentry, S_IFCHR | WHITEOUT_MODE, WHITEOUT_DEV);
> | ^~~~~~~
> ./include/linux/fs.h:1736:32: note: each undeclared identifier is reported only once for each function it appears in
> ./include/linux/fs.h: At top level:
> ./include/linux/fs.h:1886:46: warning: 'struct kstat' declared inside parameter list will not be visible outside of this definition or declaration
> 1886 | int (*getattr) (const struct path *, struct kstat *, u32, unsigned int);
> | ^~~~~
> ./include/linux/fs.h: In function '__mandatory_lock':
> ./include/linux/fs.h:2372:25: error: 'S_ISGID' undeclared (first use in this function); did you mean 'SIGIO'?
> 2372 | return (ino->i_mode & (S_ISGID | S_IXGRP)) == S_ISGID;
> | ^~~~~~~
> | SIGIO
> ./include/linux/fs.h:2372:35: error: 'S_IXGRP' undeclared (first use in this function)
> 2372 | return (ino->i_mode & (S_ISGID | S_IXGRP)) == S_ISGID;
> | ^~~~~~~
> ...
>
> $ git bisect log
> # bad: [7b287a5c6ac518c415a258f2aa7b1ebb25c263d2] Add linux-next specific files for 20200730
> # good: [d3590ebf6f91350192737dd1d1b219c05277f067] Merge tag 'audit-pr-20200729' of git://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/audit
> git bisect start '7b287a5c6ac518c415a258f2aa7b1ebb25c263d2' 'd3590ebf6f91350192737dd1d1b219c05277f067'
> # bad: [1f1ed12be70e9eb4e05ac206c6ad6a5a31f5b921] Merge remote-tracking branch 'crypto/master'
> git bisect bad 1f1ed12be70e9eb4e05ac206c6ad6a5a31f5b921
> # bad: [07fad673c2f1a02440c879c34f8182b12786a735] Merge remote-tracking branch 'hid/for-next'
> git bisect bad 07fad673c2f1a02440c879c34f8182b12786a735
> # good: [7a77c92312546a74d3507484b256ae17bfb2cfe2] Merge remote-tracking branch 'm68knommu/for-next'
> git bisect good 7a77c92312546a74d3507484b256ae17bfb2cfe2
> # good: [40dd62f180e38317e744d7f82c98af31a24fd2c9] Merge remote-tracking branch 'f2fs/dev'
> git bisect good 40dd62f180e38317e744d7f82c98af31a24fd2c9
> # bad: [52138dfdd2192bcfc7d3bc2e79475966ee4b20c4] Merge remote-tracking branch 'printk/for-next'
> git bisect bad 52138dfdd2192bcfc7d3bc2e79475966ee4b20c4
> # good: [a37c3e37fa3fa1381e03d918d708f82927ddd160] Merge remote-tracking branch 'xfs/for-next'
> git bisect good a37c3e37fa3fa1381e03d918d708f82927ddd160
> # good: [4e523547e2bf755d40cb10e85795c2f9620ff3fb] nvme-pci: add a blank line after declarations
> git bisect good 4e523547e2bf755d40cb10e85795c2f9620ff3fb
> # bad: [5066741180729f7bad9401de34efda3766c3274a] Merge branches 'fixes' and 'work.quota-compat' into for-next
> git bisect bad 5066741180729f7bad9401de34efda3766c3274a
> # good: [4ff8a356daafaafbf90141ee7a3b8fdc18e560a8] ia64: switch to ->regset_get()
> git bisect good 4ff8a356daafaafbf90141ee7a3b8fdc18e560a8
> # good: [ce327e1c54119179066d6f3573a28001febc9265] regset: kill user_regset_copyout{,_zero}()
> git bisect good ce327e1c54119179066d6f3573a28001febc9265
> # good: [1697a322e28ba96d35953c5d824540d172546d36] [elf-fdpic] switch coredump to regsets
> git bisect good 1697a322e28ba96d35953c5d824540d172546d36
> # good: [259bf01c1bd1f049958496a089c4f334fe0c8a48] Merge branches 'work.misc', 'work.regset' and 'work.fdpic' into for-next
> git bisect good 259bf01c1bd1f049958496a089c4f334fe0c8a48
> # bad: [0a3a4497a1de8e68e809a693b549c7ec2f195301] compat: lift compat_s64 and compat_u64 to <linux/compat.h>
> git bisect bad 0a3a4497a1de8e68e809a693b549c7ec2f195301
> # bad: [b902bfb3f0e9d07ec9f48256e57e5c5de6108f8c] arm64: stop using <asm/compat.h> directly
> git bisect bad b902bfb3f0e9d07ec9f48256e57e5c5de6108f8c
> # first bad commit: [b902bfb3f0e9d07ec9f48256e57e5c5de6108f8c] arm64: stop using <asm/compat.h> directly
>
> I assume the stat header order should be messed around with but I am not
> sure what exactly that would entail to make sure that nothing else
> breaks, hence just the report.

FYI,
We have also noticed this arm64 build break on linux next 20200730
with gcc-8.x, gcc-9.x and gcc-10.x

make -sk KBUILD_BUILD_USER=TuxBuild -C/linux -j16 ARCH=arm64
CROSS_COMPILE=aarch64-linux-gnu- HOSTCC=gcc CC="sccache
aarch64-linux-gnu-gcc" O=build Image
#
In file included from ../include/linux/compat.h:17,
from ../arch/arm64/include/asm/stat.h:13,
from ../include/linux/stat.h:6,
from ../include/linux/sysfs.h:22,
from ../include/linux/kobject.h:20,
from ../include/linux/of.h:17,
from ../include/linux/irqdomain.h:35,
from ../include/linux/acpi.h:13,
from ../include/acpi/apei.h:9,
from ../include/acpi/ghes.h:5,
from ../include/linux/arm_sdei.h:8,
from ../arch/arm64/kernel/asm-offsets.c:10:
../include/linux/fs.h: In function ‘vfs_whiteout’:
../include/linux/fs.h:1709:32: error: ‘S_IFCHR’ undeclared (first use
in this function)
1709 | return vfs_mknod(dir, dentry, S_IFCHR | WHITEOUT_MODE, WHITEOUT_DEV);
| ^~~~~~~
../include/linux/fs.h:1709:32: note: each undeclared identifier is
reported only once for each function it appears in
../include/linux/fs.h: At top level:
../include/linux/fs.h:1855:46: warning: ‘struct kstat’ declared inside
parameter list will not be visible outside of this definition or
declaration
1855 | int (*getattr) (const struct path *, struct kstat *, u32,
unsigned int);
| ^~~~~
../include/linux/fs.h: In function ‘__mandatory_lock’:
../include/linux/fs.h:2325:25: error: ‘S_ISGID’ undeclared (first use
in this function); did you mean ‘SIGIO’?
2325 | return (ino->i_mode & (S_ISGID | S_IXGRP)) == S_ISGID;
| ^~~~~~~
| SIGIO
../include/linux/fs.h:2325:35: error: ‘S_IXGRP’ undeclared (first use
in this function)
2325 | return (ino->i_mode & (S_ISGID | S_IXGRP)) == S_ISGID;
| ^~~~~~~
../include/linux/fs.h: In function ‘invalidate_remote_inode’:
../include/linux/fs.h:2588:6: error: implicit declaration of function
‘S_ISREG’ [-Werror=implicit-function-declaration]
2588 | if (S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode) ||
| ^~~~~~~
../include/linux/fs.h:2588:32: error: implicit declaration of function
‘S_ISDIR’; did you mean ‘EISDIR’?
[-Werror=implicit-function-declaration]
2588 | if (S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode) ||
| ^~~~~~~
| EISDIR
../include/linux/fs.h:2589:6: error: implicit declaration of function
‘S_ISLNK’ [-Werror=implicit-function-declaration]
2589 | S_ISLNK(inode->i_mode))
| ^~~~~~~
../include/linux/fs.h: In function ‘execute_ok’:
../include/linux/fs.h:2768:26: error: ‘S_IXUGO’ undeclared (first use
in this function)
2768 | return (inode->i_mode & S_IXUGO) || S_ISDIR(inode->i_mode);
| ^~~~~~~
In file included from ../include/linux/compat.h:17,
from ../arch/arm64/include/asm/stat.h:13,
from ../include/linux/stat.h:6,
from ../include/linux/sysfs.h:22,
from ../include/linux/kobject.h:20,
from ../include/linux/of.h:17,
from ../include/linux/irqdomain.h:35,
from ../include/linux/acpi.h:13,
from ../include/acpi/apei.h:9,
from ../include/acpi/ghes.h:5,
from ../include/linux/arm_sdei.h:8,
from ../arch/arm64/kernel/asm-offsets.c:10:
../include/linux/fs.h: At top level:
../include/linux/fs.h:3141:53: warning: ‘struct kstat’ declared inside
parameter list will not be visible outside of this definition or
declaration
3141 | extern void generic_fillattr(struct inode *, struct kstat *);
| ^~~~~
../include/linux/fs.h:3142:58: warning: ‘struct kstat’ declared inside
parameter list will not be visible outside of this definition or
declaration
3142 | extern int vfs_getattr_nosec(const struct path *, struct kstat
*, u32, unsigned int);
| ^~~~~
../include/linux/fs.h:3143:52: warning: ‘struct kstat’ declared inside
parameter list will not be visible outside of this definition or
declaration
3143 | extern int vfs_getattr(const struct path *, struct kstat *,
u32, unsigned int);
| ^~~~~
../include/linux/fs.h:3160:60: warning: ‘struct kstat’ declared inside
parameter list will not be visible outside of this definition or
declaration
3160 | extern int vfs_statx(int, const char __user *, int, struct
kstat *, u32);
| ^~~~~
../include/linux/fs.h:3161:46: warning: ‘struct kstat’ declared inside
parameter list will not be visible outside of this definition or
declaration
3161 | extern int vfs_statx_fd(unsigned int, struct kstat *, u32,
unsigned int);
| ^~~~~
../include/linux/fs.h:3163:64: warning: ‘struct kstat’ declared inside
parameter list will not be visible outside of this definition or
declaration
3163 | static inline int vfs_stat(const char __user *filename, struct
kstat *stat)
| ^~~~~
../include/linux/fs.h: In function ‘vfs_stat’:
../include/linux/fs.h:3166:11: error: ‘STATX_BASIC_STATS’ undeclared
(first use in this function)
3166 | stat, STATX_BASIC_STATS);
| ^~~~~~~~~~~~~~~~~
../include/linux/fs.h:3166:5: error: passing argument 4 of ‘vfs_statx’
from incompatible pointer type [-Werror=incompatible-pointer-types]
3166 | stat, STATX_BASIC_STATS);
| ^~~~
| |
| struct kstat *
../include/linux/fs.h:3160:53: note: expected ‘struct kstat *’ but
argument is of type ‘struct kstat *’
3160 | extern int vfs_statx(int, const char __user *, int, struct
kstat *, u32);
| ^~~~~~~~~~~~~~
../include/linux/fs.h: At top level:
../include/linux/fs.h:3168:61: warning: ‘struct kstat’ declared inside
parameter list will not be visible outside of this definition or
declaration
3168 | static inline int vfs_lstat(const char __user *name, struct kstat *stat)
| ^~~~~
../include/linux/fs.h: In function ‘vfs_lstat’:
../include/linux/fs.h:3171:11: error: ‘STATX_BASIC_STATS’ undeclared
(first use in this function)
3171 | stat, STATX_BASIC_STATS);
| ^~~~~~~~~~~~~~~~~
../include/linux/fs.h:3171:5: error: passing argument 4 of ‘vfs_statx’
from incompatible pointer type [-Werror=incompatible-pointer-types]
3171 | stat, STATX_BASIC_STATS);
| ^~~~
| |
| struct kstat *
../include/linux/fs.h:3160:53: note: expected ‘struct kstat *’ but
argument is of type ‘struct kstat *’
3160 | extern int vfs_statx(int, const char __user *, int, struct
kstat *, u32);
| ^~~~~~~~~~~~~~
../include/linux/fs.h: At top level:
../include/linux/fs.h:3174:17: warning: ‘struct kstat’ declared inside
parameter list will not be visible outside of this definition or
declaration
3174 | struct kstat *stat, int flags)
| ^~~~~
../include/linux/fs.h: In function ‘vfs_fstatat’:
../include/linux/fs.h:3177:11: error: ‘STATX_BASIC_STATS’ undeclared
(first use in this function)
3177 | stat, STATX_BASIC_STATS);
| ^~~~~~~~~~~~~~~~~
../include/linux/fs.h:3177:5: error: passing argument 4 of ‘vfs_statx’
from incompatible pointer type [-Werror=incompatible-pointer-types]
3177 | stat, STATX_BASIC_STATS);
| ^~~~
| |
| struct kstat *
../include/linux/fs.h:3160:53: note: expected ‘struct kstat *’ but
argument is of type ‘struct kstat *’
3160 | extern int vfs_statx(int, const char __user *, int, struct
kstat *, u32);
| ^~~~~~~~~~~~~~
../include/linux/fs.h: At top level:
../include/linux/fs.h:3179:44: warning: ‘struct kstat’ declared inside
parameter list will not be visible outside of this definition or
declaration
3179 | static inline int vfs_fstat(int fd, struct kstat *stat)
| ^~~~~
../include/linux/fs.h: In function ‘vfs_fstat’:
../include/linux/fs.h:3181:32: error: ‘STATX_BASIC_STATS’ undeclared
(first use in this function)
3181 | return vfs_statx_fd(fd, stat, STATX_BASIC_STATS, 0);
| ^~~~~~~~~~~~~~~~~
../include/linux/fs.h:3181:26: error: passing argument 2 of
‘vfs_statx_fd’ from incompatible pointer type
[-Werror=incompatible-pointer-types]
3181 | return vfs_statx_fd(fd, stat, STATX_BASIC_STATS, 0);
| ^~~~
| |
| struct kstat *
../include/linux/fs.h:3161:39: note: expected ‘struct kstat *’ but
argument is of type ‘struct kstat *’
3161 | extern int vfs_statx_fd(unsigned int, struct kstat *, u32,
unsigned int);
| ^~~~~~~~~~~~~~
../include/linux/fs.h: At top level:
../include/linux/fs.h:3206:55: warning: ‘struct kstat’ declared inside
parameter list will not be visible outside of this definition or
declaration
3206 | extern int simple_getattr(const struct path *, struct kstat *,
u32, unsigned int);
| ^~~~~
../include/linux/fs.h: In function ‘vma_is_fsdax’:
../include/linux/fs.h:3289:6: error: implicit declaration of function
‘S_ISCHR’ [-Werror=implicit-function-declaration]
3289 | if (S_ISCHR(inode->i_mode))
| ^~~~~~~
../include/linux/fs.h: In function ‘is_sxid’:
../include/linux/fs.h:3428:17: error: ‘S_ISUID’ undeclared (first use
in this function)
3428 | return (mode & S_ISUID) || ((mode & S_ISGID) && (mode & S_IXGRP));
| ^~~~~~~
../include/linux/fs.h:3428:38: error: ‘S_ISGID’ undeclared (first use
in this function); did you mean ‘SIGIO’?
3428 | return (mode & S_ISUID) || ((mode & S_ISGID) && (mode & S_IXGRP));
| ^~~~~~~
| SIGIO
../include/linux/fs.h:3428:58: error: ‘S_IXGRP’ undeclared (first use
in this function)
3428 | return (mode & S_ISUID) || ((mode & S_ISGID) && (mode & S_IXGRP));
| ^~~~~~~
../include/linux/fs.h: In function ‘check_sticky’:
../include/linux/fs.h:3433:22: error: ‘S_ISVTX’ undeclared (first use
in this function)
3433 | if (!(dir->i_mode & S_ISVTX))
| ^~~~~~~
cc1: some warnings being treated as errors
make[2]: *** [../scripts/Makefile.build:114:
arch/arm64/kernel/asm-offsets.s] Error 1


kernel config:
https://builds.tuxbuild.com/BsPWKexQQ3JXC2WwDQSoQg/kernel.config

- Naresh
>
> Cheers,
> Nathan