linux-kernel - [PATCH v2 2/6] xfrm/compat: Attach xfrm dumps to 64=>32 bit translator

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20200826014949.644441-3-dima@arista.com>
Date:   Wed, 26 Aug 2020 02:49:45 +0100
From:   Dmitry Safonov <dima@...sta.com>
To:     linux-kernel@...r.kernel.org
Cc:     Dmitry Safonov <0x7f454c46@...il.com>,
        Dmitry Safonov <dima@...sta.com>,
        "David S. Miller" <davem@...emloft.net>,
        Florian Westphal <fw@...len.de>,
        Herbert Xu <herbert@...dor.apana.org.au>,
        Jakub Kicinski <kuba@...nel.org>,
        Steffen Klassert <steffen.klassert@...unet.com>,
        Stephen Suryaputra <ssuryaextr@...il.com>,
        netdev@...r.kernel.org
Subject: [PATCH v2 2/6] xfrm/compat: Attach xfrm dumps to 64=>32 bit 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.

Currently nlmsg_unicast() is used by functions that dump structures that
can be different in size for compat tasks, see dump_one_state() and
dump_one_policy().

The following nlmsg_unicast() users exist today in xfrm:

         Function                          |    Message can be different
                                           |       in size on compat
-------------------------------------------|------------------------------
    xfrm_get_spdinfo()                     |               N
    xfrm_get_sadinfo()                     |               N
    xfrm_get_sa()                          |               Y
    xfrm_alloc_userspi()                   |               Y
    xfrm_get_policy()                      |               Y
    xfrm_get_ae()                          |               N

Besides, dump_one_state() and dump_one_policy() can be used by filtered
netlink dump for XFRM_MSG_GETSA, XFRM_MSG_GETPOLICY.

Just as for xfrm multicast, allocate frag_list for compat skb journey
down to recvmsg() which will give user the desired skb according to
syscall bitness.

[1]: https://lkml.kernel.org/r/20180726023144.31066-1-dima@arista.com
Signed-off-by: Dmitry Safonov <dima@...sta.com>
---
 include/net/xfrm.h     |  6 ++++++
 net/xfrm/xfrm_compat.c | 10 ++++++++--
 net/xfrm/xfrm_user.c   | 20 ++++++++++++++++++++
 3 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index 9810b5090338..9febf4f5d2ea 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -2002,12 +2002,18 @@ static inline int xfrm_tunnel_check(struct sk_buff *skb, struct xfrm_state *x,
 
 #ifdef CONFIG_XFRM_USER_COMPAT
 extern int xfrm_alloc_compat(struct sk_buff *skb);
+extern int __xfrm_alloc_compat(struct sk_buff *skb, const struct nlmsghdr *nlh);
 extern const int xfrm_msg_min[XFRM_NR_MSGTYPES];
 #else
 static inline int xfrm_alloc_compat(struct sk_buff *skb)
 {
 	return 0;
 }
+static inline int __xfrm_alloc_compat(struct sk_buff *skb,
+				      const struct nlmsghdr *nlh)
+{
+	return 0;
+}
 #endif
 
 #if IS_ENABLED(CONFIG_IPV6)
diff --git a/net/xfrm/xfrm_compat.c b/net/xfrm/xfrm_compat.c
index b9eb65dde0db..b34c8b56a571 100644
--- a/net/xfrm/xfrm_compat.c
+++ b/net/xfrm/xfrm_compat.c
@@ -272,9 +272,8 @@ static int xfrm_xlate64(struct sk_buff *dst, const struct nlmsghdr *nlh_src)
 	return 0;
 }
 
-int xfrm_alloc_compat(struct sk_buff *skb)
+int __xfrm_alloc_compat(struct sk_buff *skb, const struct nlmsghdr *nlh_src)
 {
-	const struct nlmsghdr *nlh_src = nlmsg_hdr(skb);
 	u16 type = nlh_src->nlmsg_type - XFRM_MSG_BASE;
 	struct sk_buff *new = NULL;
 	int err;
@@ -300,3 +299,10 @@ int xfrm_alloc_compat(struct sk_buff *skb)
 
 	return 0;
 }
+
+int xfrm_alloc_compat(struct sk_buff *skb)
+{
+	const struct nlmsghdr *nlh_src = nlmsg_hdr(skb);
+
+	return __xfrm_alloc_compat(skb, nlh_src);
+}
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index 90c57d4a0b47..d135c6949336 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -992,6 +992,13 @@ static int dump_one_state(struct xfrm_state *x, int count, void *ptr)
 		return err;
 	}
 	nlmsg_end(skb, nlh);
+
+	err = __xfrm_alloc_compat(skb, nlh);
+	if (err) {
+		nlmsg_cancel(skb, nlh);
+		return err;
+	}
+
 	return 0;
 }
 
@@ -1365,6 +1372,12 @@ static int xfrm_alloc_userspi(struct sk_buff *skb, struct nlmsghdr *nlh,
 		goto out;
 	}
 
+	err = xfrm_alloc_compat(skb);
+	if (err) {
+		kfree_skb(resp_skb);
+		goto out;
+	}
+
 	err = nlmsg_unicast(net->xfrm.nlsk, resp_skb, NETLINK_CB(skb).portid);
 
 out:
@@ -1795,6 +1808,13 @@ static int dump_one_policy(struct xfrm_policy *xp, int dir, int count, void *ptr
 		return err;
 	}
 	nlmsg_end(skb, nlh);
+
+	err = __xfrm_alloc_compat(skb, nlh);
+	if (err) {
+		nlmsg_cancel(skb, nlh);
+		return err;
+	}
+
 	return 0;
 }
 
-- 
2.27.0