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-next>] [day] [month] [year] [list]
Message-Id: <20231026151659.1676037-1-razor@blackwall.org>
Date: Thu, 26 Oct 2023 18:16:59 +0300
From: Nikolay Aleksandrov <razor@...ckwall.org>
To: bpf@...r.kernel.org
Cc: jiri@...nulli.us,
	netdev@...r.kernel.org,
	martin.lau@...ux.dev,
	ast@...nel.org,
	andrii@...nel.org,
	john.fastabend@...il.com,
	kuba@...nel.org,
	andrew@...n.ch,
	toke@...nel.org,
	toke@...hat.com,
	sdf@...gle.com,
	daniel@...earbox.net,
	idosch@...sch.org,
	Nikolay Aleksandrov <razor@...ckwall.org>
Subject: [PATCH bpf-next v2] netkit: use netlink policy for mode and policy attributes validation

Use netlink's NLA_POLICY_VALIDATE_FN() type for mode and primary/peer
policy with custom validation functions to return better errors. This
simplifies the logic a bit and relies on netlink's policy validation.
We have to use NLA_BINARY and validate the length inside the callbacks.

Suggested-by: Jiri Pirko <jiri@...nulli.us>
Signed-off-by: Nikolay Aleksandrov <razor@...ckwall.org>
---
v2: use NLA_BINARY instead of NLA_U32 (thanks Ido!), validate attribute
    length inside the callbacks, run tests again
    the patch is sent out of the set as only the first one was applied
    before, see:
    https://lore.kernel.org/bpf/8533255d-9b73-cdbe-fbbd-28a275313229@iogearbox.net/

 drivers/net/netkit.c | 79 ++++++++++++++++++++------------------------
 1 file changed, 35 insertions(+), 44 deletions(-)

diff --git a/drivers/net/netkit.c b/drivers/net/netkit.c
index 5a0f86f38f09..df819df86944 100644
--- a/drivers/net/netkit.c
+++ b/drivers/net/netkit.c
@@ -247,29 +247,39 @@ static struct net *netkit_get_link_net(const struct net_device *dev)
 	return peer ? dev_net(peer) : dev_net(dev);
 }
 
-static int netkit_check_policy(int policy, struct nlattr *tb,
+static int netkit_check_policy(const struct nlattr *attr,
 			       struct netlink_ext_ack *extack)
 {
-	switch (policy) {
+	if (nla_len(attr) != sizeof(u32)) {
+		NL_SET_ERR_MSG_ATTR(extack, attr, "Invalid policy attribute length");
+		return -EINVAL;
+	}
+
+	switch (nla_get_u32(attr)) {
 	case NETKIT_PASS:
 	case NETKIT_DROP:
 		return 0;
 	default:
-		NL_SET_ERR_MSG_ATTR(extack, tb,
+		NL_SET_ERR_MSG_ATTR(extack, attr,
 				    "Provided default xmit policy not supported");
 		return -EINVAL;
 	}
 }
 
-static int netkit_check_mode(int mode, struct nlattr *tb,
+static int netkit_check_mode(const struct nlattr *attr,
 			     struct netlink_ext_ack *extack)
 {
-	switch (mode) {
+	if (nla_len(attr) != sizeof(u32)) {
+		NL_SET_ERR_MSG_ATTR(extack, attr, "Invalid mode attribute length");
+		return -EINVAL;
+	}
+
+	switch (nla_get_u32(attr)) {
 	case NETKIT_L2:
 	case NETKIT_L3:
 		return 0;
 	default:
-		NL_SET_ERR_MSG_ATTR(extack, tb,
+		NL_SET_ERR_MSG_ATTR(extack, attr,
 				    "Provided device mode can only be L2 or L3");
 		return -EINVAL;
 	}
@@ -306,13 +316,8 @@ static int netkit_new_link(struct net *src_net, struct net_device *dev,
 	int err;
 
 	if (data) {
-		if (data[IFLA_NETKIT_MODE]) {
-			attr = data[IFLA_NETKIT_MODE];
-			mode = nla_get_u32(attr);
-			err = netkit_check_mode(mode, attr, extack);
-			if (err < 0)
-				return err;
-		}
+		if (data[IFLA_NETKIT_MODE])
+			mode = nla_get_u32(data[IFLA_NETKIT_MODE]);
 		if (data[IFLA_NETKIT_PEER_INFO]) {
 			attr = data[IFLA_NETKIT_PEER_INFO];
 			ifmp = nla_data(attr);
@@ -324,20 +329,10 @@ static int netkit_new_link(struct net *src_net, struct net_device *dev,
 				return err;
 			tbp = peer_tb;
 		}
-		if (data[IFLA_NETKIT_POLICY]) {
-			attr = data[IFLA_NETKIT_POLICY];
-			default_prim = nla_get_u32(attr);
-			err = netkit_check_policy(default_prim, attr, extack);
-			if (err < 0)
-				return err;
-		}
-		if (data[IFLA_NETKIT_PEER_POLICY]) {
-			attr = data[IFLA_NETKIT_PEER_POLICY];
-			default_peer = nla_get_u32(attr);
-			err = netkit_check_policy(default_peer, attr, extack);
-			if (err < 0)
-				return err;
-		}
+		if (data[IFLA_NETKIT_POLICY])
+			default_prim = nla_get_u32(data[IFLA_NETKIT_POLICY]);
+		if (data[IFLA_NETKIT_PEER_POLICY])
+			default_peer = nla_get_u32(data[IFLA_NETKIT_PEER_POLICY]);
 	}
 
 	if (ifmp && tbp[IFLA_IFNAME]) {
@@ -818,8 +813,6 @@ static int netkit_change_link(struct net_device *dev, struct nlattr *tb[],
 	struct netkit *nk = netkit_priv(dev);
 	struct net_device *peer = rtnl_dereference(nk->peer);
 	enum netkit_action policy;
-	struct nlattr *attr;
-	int err;
 
 	if (!nk->primary) {
 		NL_SET_ERR_MSG(extack,
@@ -834,22 +827,14 @@ static int netkit_change_link(struct net_device *dev, struct nlattr *tb[],
 	}
 
 	if (data[IFLA_NETKIT_POLICY]) {
-		attr = data[IFLA_NETKIT_POLICY];
-		policy = nla_get_u32(attr);
-		err = netkit_check_policy(policy, attr, extack);
-		if (err)
-			return err;
+		policy = nla_get_u32(data[IFLA_NETKIT_POLICY]);
 		WRITE_ONCE(nk->policy, policy);
 	}
 
 	if (data[IFLA_NETKIT_PEER_POLICY]) {
-		err = -EOPNOTSUPP;
-		attr = data[IFLA_NETKIT_PEER_POLICY];
-		policy = nla_get_u32(attr);
-		if (peer)
-			err = netkit_check_policy(policy, attr, extack);
-		if (err)
-			return err;
+		if (!peer)
+			return -EOPNOTSUPP;
+		policy = nla_get_u32(data[IFLA_NETKIT_PEER_POLICY]);
 		nk = netkit_priv(peer);
 		WRITE_ONCE(nk->policy, policy);
 	}
@@ -889,9 +874,15 @@ static int netkit_fill_info(struct sk_buff *skb, const struct net_device *dev)
 
 static const struct nla_policy netkit_policy[IFLA_NETKIT_MAX + 1] = {
 	[IFLA_NETKIT_PEER_INFO]		= { .len = sizeof(struct ifinfomsg) },
-	[IFLA_NETKIT_POLICY]		= { .type = NLA_U32 },
-	[IFLA_NETKIT_MODE]		= { .type = NLA_U32 },
-	[IFLA_NETKIT_PEER_POLICY]	= { .type = NLA_U32 },
+	[IFLA_NETKIT_POLICY]		= NLA_POLICY_VALIDATE_FN(NLA_BINARY,
+								 netkit_check_policy,
+								 sizeof(u32)),
+	[IFLA_NETKIT_MODE]		= NLA_POLICY_VALIDATE_FN(NLA_BINARY,
+								 netkit_check_mode,
+								 sizeof(u32)),
+	[IFLA_NETKIT_PEER_POLICY]	= NLA_POLICY_VALIDATE_FN(NLA_BINARY,
+								 netkit_check_policy,
+								 sizeof(u32)),
 	[IFLA_NETKIT_PRIMARY]		= { .type = NLA_REJECT,
 					    .reject_message = "Primary attribute is read-only" },
 };
-- 
2.38.1


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ