On 02/09/2024 17:02, Joshua Lant wrote:
In order to fix alignment between kernel and userspace structs in UAPI headers, create a new type which will provide space for a user pointer in the struct. This is required in order not to break the existing fix in the event that a fully purecap kernel is created. As it stands, the purecap kernel would break the uapi structs since only 8B is ever given in the field for the kernel pointers in the uapi structs, due to the use of unsigned long in those fields. Make nonPCuABI use this unsigned long, but the PCuABI should use a capability. This means both hybrid mode and a purecap kernel would work in both instances.
This is pretty hard to parse for someone lacking further context. The two most important points to make are: 1. we need a type that is the same size from the perspective of both the kernel and userspace, and 2. we need this type to be able to hold a kernel pointer, regardless of the kernel ABI. 1. justifies not using a pointer type (kernel and userspace won't agree in the hybrid model), and 2. justifies making the type a capability, in case the kernel wants its pointers to be capabilities. Worth spelling out that this is unnecessary in the hybrid model, it is only done to guarantee that the uapi remains stable.
A few more things: there is no "existing fix" (considering this patch is applied first, at which point the netfilter interface is unmodified), and there is no "user pointer" involved here.
Signed-off-by: Joshua Lant joshualant@gmail.com
include/linux/netfilter.h | 6 ++++++ include/uapi/linux/netfilter.h | 8 ++++++++ include/uapi/linux/netfilter/x_tables.h | 8 ++++++++ 3 files changed, 22 insertions(+)
diff --git a/include/linux/netfilter.h b/include/linux/netfilter.h index 80900d910992..58fd97995e23 100644 --- a/include/linux/netfilter.h +++ b/include/linux/netfilter.h @@ -17,6 +17,12 @@ #include <linux/sockptr.h> #include <net/net_namespace.h> +#ifdef CONFIG_CHERI_PURECAP_UABI +typedef __uintcap_t __nf_kptr_t; +#else +typedef unsigned long __nf_kptr_t;
Nit: careful with tabs/spaces, there's currently a strange mix across the typedefs. There's typically no tab after "typedef", while alignment (if desired) of the type name is achieved with tabs only.
+#endif
static inline int NF_DROP_GETERR(int verdict) { return -(verdict >> NF_VERDICT_QBITS); diff --git a/include/uapi/linux/netfilter.h b/include/uapi/linux/netfilter.h index 5a79ccb76701..bd8542cb9a41 100644 --- a/include/uapi/linux/netfilter.h +++ b/include/uapi/linux/netfilter.h @@ -7,6 +7,14 @@ #include <linux/in.h> #include <linux/in6.h> +#ifndef __KERNEL__ +#ifdef __CHERI_PURE_CAPABILITY__ +typedef __uintcap_t __nf_kptr_t; +#else +typedef unsigned long __nf_kptr_t; +#endif +#endif
/* Responses from hook functions. */ #define NF_DROP 0 #define NF_ACCEPT 1 diff --git a/include/uapi/linux/netfilter/x_tables.h b/include/uapi/linux/netfilter/x_tables.h index 796af83a963a..e695b6d34a6f 100644 --- a/include/uapi/linux/netfilter/x_tables.h +++ b/include/uapi/linux/netfilter/x_tables.h @@ -8,6 +8,14 @@ #define XT_EXTENSION_MAXNAMELEN 29 #define XT_TABLE_MAXNAMELEN 32 +#ifndef __KERNEL__ +#ifdef __CHERI_PURE_CAPABILITY__ +typedef __uintcap_t __nf_kptr_t; +#else +typedef unsigned long __nf_kptr_t; +#endif +#endif
Do we really have to duplicate the typedefs here? There are already quite a few headers including <linux/netfilter.h> under include/uapi/linux/netfilter/ so including it in x_tables.h too shouldn't be too problematic.
Similarly each patch adding a reference to __nf_kptr_t to a uapi header should include <linux/netfilter.h> if needed (removing the need for patch 17).
Kevin
struct xt_entry_match { union { struct {