Re: [PATCH v2 1/6] xfrm/compat: Add 64=>32-bit messages translator
On 9/7/20 12:24 PM, Steffen Klassert wrote: [..] > One comment on this. Looks like the above is the same in all > commit messages. Please provide that generic information > with the patch 0/n and remove it from the other patches. Yeah, I think I've used to that from x86/core submissions - they prefer having general information copied from cover-letter to every patch, that way commits in `git log` or `git show` preserve it. Probably, one of small differences in style between contributions to different subsystems. Will do, no problem. Thanks, Dmitry
Re: [PATCH v2 1/6] xfrm/compat: Add 64=>32-bit messages translator
On Wed, Aug 26, 2020 at 02:49:44AM +0100, Dmitry Safonov wrote:
> XFRM is disabled for compatible users because of the UABI difference.
> The difference is in structures paddings and in the result the size
> of netlink messages differ.
>
> Possibility for compatible application to manage xfrm tunnels was
> disabled by: the commmit 19d7df69fdb2 ("xfrm: Refuse to insert 32 bit
> userspace socket policies on 64 bit systems") and the commit 74005991b78a
> ("xfrm: Do not parse 32bits compiled xfrm netlink msg on 64bits host").
>
> This is my second attempt to resolve the xfrm/compat problem by adding
> the 64=>32 and 32=>64 bit translators those non-visibly to a user
> provide translation between compatible user and kernel.
> Previous attempt was to interrupt the message ABI according to a syscall
> by xfrm_user, which resulted in over-complicated code [1].
>
> Florian Westphal provided the idea of translator and some draft patches
> in the discussion. In these patches, his idea is reused and some of his
> initial code is also present.
One comment on this. Looks like the above is the same in all
commit messages. Please provide that generic information
with the patch 0/n and remove it from the other patches.
Re: [PATCH v2 1/6] xfrm/compat: Add 64=>32-bit messages translator
Hi Dmitry,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on ipsec/master]
[also build test ERROR on kselftest/next linus/master v5.9-rc2 next-20200825]
[cannot apply to ipsec-next/master]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url:
https://github.com/0day-ci/linux/commits/Dmitry-Safonov/xfrm-Add-compat-layer/20200826-095240
base: https://git.kernel.org/pub/scm/linux/kernel/git/klassert/ipsec.git
master
config: x86_64-allmodconfig (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce (this is a W=1 build):
# save the attached .config to linux build tree
make W=1 ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All errors (new ones prefixed by >>):
net/xfrm/xfrm_compat.c: In function 'xfrm_nlmsg_put_compat':
>> net/xfrm/xfrm_compat.c:103:16: error: 'xfrm_msg_min' undeclared (first use
>> in this function); did you mean 'xfrm_alg_len'?
103 | int src_len = xfrm_msg_min[type];
|^~~~
|xfrm_alg_len
net/xfrm/xfrm_compat.c:103:16: note: each undeclared identifier is reported
only once for each function it appears in
net/xfrm/xfrm_compat.c: In function 'xfrm_xlate64':
net/xfrm/xfrm_compat.c:260:34: error: 'xfrm_msg_min' undeclared (first use
in this function); did you mean 'xfrm_alg_len'?
260 | attrs = nlmsg_attrdata(nlh_src, xfrm_msg_min[type]);
| ^~~~
| xfrm_alg_len
net/xfrm/xfrm_compat.c: At top level:
>> net/xfrm/xfrm_compat.c:275:5: error: redefinition of 'xfrm_alloc_compat'
275 | int xfrm_alloc_compat(struct sk_buff *skb)
| ^
In file included from net/xfrm/xfrm_compat.c:9:
include/net/xfrm.h:2007:19: note: previous definition of 'xfrm_alloc_compat'
was here
2007 | static inline int xfrm_alloc_compat(struct sk_buff *skb)
| ^
In file included from arch/x86/include/asm/bug.h:93,
from include/linux/bug.h:5,
from include/linux/thread_info.h:12,
from arch/x86/include/asm/preempt.h:7,
from include/linux/preempt.h:78,
from include/linux/spinlock.h:51,
from include/linux/seqlock.h:15,
from include/linux/time.h:6,
from include/linux/compat.h:10,
from net/xfrm/xfrm_compat.c:7:
net/xfrm/xfrm_compat.c: In function 'xfrm_alloc_compat':
net/xfrm/xfrm_compat.c:282:38: error: 'xfrm_msg_min' undeclared (first use
in this function); did you mean 'xfrm_alg_len'?
282 | if (WARN_ON_ONCE(type >= ARRAY_SIZE(xfrm_msg_min)))
| ^~~~
include/asm-generic/bug.h:102:25: note: in definition of macro 'WARN_ON_ONCE'
102 | int __ret_warn_on = !!(condition); \
| ^
net/xfrm/xfrm_compat.c:282:27: note: in expansion of macro 'ARRAY_SIZE'
282 | if (WARN_ON_ONCE(type >= ARRAY_SIZE(xfrm_msg_min)))
| ^~
>> include/linux/build_bug.h:16:51: error: bit-field '' width not an
>> integer constant
16 | #define BUILD_BUG_ON_ZERO(e) ((int)(sizeof(struct { int:(-!!(e));
})))
| ^
include/asm-generic/bug.h:102:25: note: in definition of macro 'WARN_ON_ONCE'
102 | int __ret_warn_on = !!(condition); \
| ^
include/linux/compiler.h:224:28: note: in expansion of macro
'BUILD_BUG_ON_ZERO'
224 | #define __must_be_array(a) BUILD_BUG_ON_ZERO(__same_type((a),
&(a)[0]))
|^
include/linux/kernel.h:47:59: note: in expansion of macro '__must_be_array'
47 | #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) +
__must_be_array(arr))
|
^~~
net/xfrm/xfrm_compat.c:282:27: note: in expansion of macro 'ARRAY_SIZE'
282 | if (WARN_ON_ONCE(type >= ARRAY_SIZE(xfrm_msg_min)))
| ^~
#
https://github.com/0day-ci/linux/commit/fa198f6763bf103396e06e12549e1dc00941a3d0
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review
Dmitry-Safonov/xfrm-Add-compat-layer/20200826-095240
git checkout fa198f6763bf103396e06e12549e1dc00941a3d0
vim +103 net/xfrm/xfrm_compat.c
98
99 static struct nlmsghdr *xfrm_nlmsg_put_compat(struct sk_buff *skb,
100 const struct nlmsghdr *nlh_src, u16 type)
101 {
102 int
[PATCH v2 1/6] xfrm/compat: Add 64=>32-bit messages translator
XFRM is disabled for compatible users because of the UABI difference.
The difference is in structures paddings and in the result the size
of netlink messages differ.
Possibility for compatible application to manage xfrm tunnels was
disabled by: the commmit 19d7df69fdb2 ("xfrm: Refuse to insert 32 bit
userspace socket policies on 64 bit systems") and the commit 74005991b78a
("xfrm: Do not parse 32bits compiled xfrm netlink msg on 64bits host").
This is my second attempt to resolve the xfrm/compat problem by adding
the 64=>32 and 32=>64 bit translators those non-visibly to a user
provide translation between compatible user and kernel.
Previous attempt was to interrupt the message ABI according to a syscall
by xfrm_user, which resulted in over-complicated code [1].
Florian Westphal provided the idea of translator and some draft patches
in the discussion. In these patches, his idea is reused and some of his
initial code is also present.
Provide the kernel-to-user translator under XFRM_USER_COMPAT, that
creates for 64-bit xfrm-user message a 32-bit translation and puts it
in skb's frag_list. net/compat.c layer provides MSG_CMSG_COMPAT to
decide if the message should be taken from skb or frag_list.
(used by wext-core which has also an ABI difference)
Kernel sends 64-bit xfrm messages to the userspace for:
- multicast (monitor events)
- netlink dumps
Wire up the translator to xfrm_nlmsg_multicast().
[1]: https://lkml.kernel.org/r/20180726023144.31066-1-d...@arista.com
Signed-off-by: Dmitry Safonov
---
include/net/xfrm.h | 10 ++
net/xfrm/Kconfig | 10 ++
net/xfrm/Makefile | 1 +
net/xfrm/xfrm_compat.c | 302 +
net/xfrm/xfrm_user.c | 9 +-
5 files changed, 331 insertions(+), 1 deletion(-)
create mode 100644 net/xfrm/xfrm_compat.c
diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index 2737d24ec244..9810b5090338 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -2000,6 +2000,16 @@ static inline int xfrm_tunnel_check(struct sk_buff *skb,
struct xfrm_state *x,
return 0;
}
+#ifdef CONFIG_XFRM_USER_COMPAT
+extern int xfrm_alloc_compat(struct sk_buff *skb);
+extern const int xfrm_msg_min[XFRM_NR_MSGTYPES];
+#else
+static inline int xfrm_alloc_compat(struct sk_buff *skb)
+{
+ return 0;
+}
+#endif
+
#if IS_ENABLED(CONFIG_IPV6)
static inline bool xfrm6_local_dontfrag(const struct sock *sk)
{
diff --git a/net/xfrm/Kconfig b/net/xfrm/Kconfig
index 5b9a5ab48111..e79b48dab61b 100644
--- a/net/xfrm/Kconfig
+++ b/net/xfrm/Kconfig
@@ -28,6 +28,16 @@ config XFRM_USER
If unsure, say Y.
+config XFRM_USER_COMPAT
+ tristate "Compatible ABI support"
+ depends on XFRM_USER && COMPAT_FOR_U64_ALIGNMENT
+ select WANT_COMPAT_NETLINK_MESSAGES
+ help
+ Transformation(XFRM) user configuration interface like IPsec
+ used by compatible Linux applications.
+
+ If unsure, say N.
+
config XFRM_INTERFACE
tristate "Transformation virtual interface"
depends on XFRM && IPV6
diff --git a/net/xfrm/Makefile b/net/xfrm/Makefile
index 2d4bb4b9f75e..494aa744bfb9 100644
--- a/net/xfrm/Makefile
+++ b/net/xfrm/Makefile
@@ -9,6 +9,7 @@ obj-$(CONFIG_XFRM) := xfrm_policy.o xfrm_state.o xfrm_hash.o \
obj-$(CONFIG_XFRM_STATISTICS) += xfrm_proc.o
obj-$(CONFIG_XFRM_ALGO) += xfrm_algo.o
obj-$(CONFIG_XFRM_USER) += xfrm_user.o
+obj-$(CONFIG_XFRM_USER_COMPAT) += xfrm_compat.o
obj-$(CONFIG_XFRM_IPCOMP) += xfrm_ipcomp.o
obj-$(CONFIG_XFRM_INTERFACE) += xfrm_interface.o
obj-$(CONFIG_XFRM_ESPINTCP) += espintcp.o
diff --git a/net/xfrm/xfrm_compat.c b/net/xfrm/xfrm_compat.c
new file mode 100644
index ..b9eb65dde0db
--- /dev/null
+++ b/net/xfrm/xfrm_compat.c
@@ -0,0 +1,302 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * XFRM compat layer
+ * Author: Dmitry Safonov
+ * Based on code and translator idea by: Florian Westphal
+ */
+#include
+#include
+#include
+
+struct compat_xfrm_lifetime_cfg {
+ compat_u64 soft_byte_limit, hard_byte_limit;
+ compat_u64 soft_packet_limit, hard_packet_limit;
+ compat_u64 soft_add_expires_seconds, hard_add_expires_seconds;
+ compat_u64 soft_use_expires_seconds, hard_use_expires_seconds;
+}; /* same size on 32bit, but only 4 byte alignment required */
+
+struct compat_xfrm_lifetime_cur {
+ compat_u64 bytes, packets, add_time, use_time;
+}; /* same size on 32bit, but only 4 byte alignment required */
+
+struct compat_xfrm_userpolicy_info {
+ struct xfrm_selector sel;
+ struct compat_xfrm_lifetime_cfg lft;
+ struct compat_xfrm_lifetime_cur curlft;
+ __u32 priority, index;
+ u8 dir, action, flags, share;
+ /* 4 bytes additional padding on 64bit */
+};
+
+struct compat_xfrm_usersa_info {
+ struct xfrm_selector sel;
+ struct xfrm_id id;
+ xfrm_address_t saddr;
+ struct compat_xfrm_lifetime_cfg lft;
+ struct compat_xfrm_lifetime_cur curlft;
+ struct
