From 0cafd77dcd032d1687efaba5598cf07bce85997f Mon Sep 17 00:00:00 2001
From: Eric Dumazet <edumazet@google.com>
Date: Thu, 20 Oct 2022 23:20:18 +0000
Subject: net: add a refcount tracker for kernel sockets

Commit ffa84b5ffb37 ("net: add netns refcount tracker to struct sock")
added a tracker to sockets, but did not track kernel sockets.

We still have syzbot reports hinting about netns being destroyed
while some kernel TCP sockets had not been dismantled.

This patch tracks kernel sockets, and adds a ref_tracker_dir_print()
call to net_free() right before the netns is freed.

Normally, each layer is responsible for properly releasing its
kernel sockets before last call to net_free().

This debugging facility is enabled with CONFIG_NET_NS_REFCNT_TRACKER=y

Signed-off-by: Eric Dumazet <edumazet@google.com>
Reviewed-by: Kuniyuki Iwashima <kuniyu@amazon.com>
Tested-by: Kuniyuki Iwashima <kuniyu@amazon.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
---
 net/netlink/af_netlink.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

(limited to 'net/netlink')

diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index a662e8a5ff84..f0c94d394ab1 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -812,6 +812,17 @@ static int netlink_release(struct socket *sock)
 	}
 
 	sock_prot_inuse_add(sock_net(sk), &netlink_proto, -1);
+
+	/* Because struct net might disappear soon, do not keep a pointer. */
+	if (!sk->sk_net_refcnt && sock_net(sk) != &init_net) {
+		__netns_tracker_free(sock_net(sk), &sk->ns_tracker, false);
+		/* Because of deferred_put_nlk_sk and use of work queue,
+		 * it is possible  netns will be freed before this socket.
+		 */
+		sock_net_set(sk, &init_net);
+		__netns_tracker_alloc(&init_net, &sk->ns_tracker,
+				      false, GFP_KERNEL);
+	}
 	call_rcu(&nlk->rcu, deferred_put_nlk_sk);
 	return 0;
 }
-- 
cgit v1.2.3


From 738136a0e3757a8534df3ad97d6ff6d7f429f6c1 Mon Sep 17 00:00:00 2001
From: Jakub Kicinski <kuba@kernel.org>
Date: Thu, 27 Oct 2022 14:25:53 -0700
Subject: netlink: split up copies in the ack construction

Clean up the use of unsafe_memcpy() by adding a flexible array
at the end of netlink message header and splitting up the header
and data copies.

Reviewed-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
---
 include/net/netlink.h        | 21 +++++++++++++++++++++
 include/uapi/linux/netlink.h |  2 ++
 net/netlink/af_netlink.c     | 29 ++++++++++++++++++++---------
 3 files changed, 43 insertions(+), 9 deletions(-)

(limited to 'net/netlink')

diff --git a/include/net/netlink.h b/include/net/netlink.h
index 4418b1981e31..784b4688fc6f 100644
--- a/include/net/netlink.h
+++ b/include/net/netlink.h
@@ -931,6 +931,27 @@ static inline struct nlmsghdr *nlmsg_put(struct sk_buff *skb, u32 portid, u32 se
 	return __nlmsg_put(skb, portid, seq, type, payload, flags);
 }
 
+/**
+ * nlmsg_append - Add more data to a nlmsg in a skb
+ * @skb: socket buffer to store message in
+ * @size: length of message payload
+ *
+ * Append data to an existing nlmsg, used when constructing a message
+ * with multiple fixed-format headers (which is rare).
+ * Returns NULL if the tailroom of the skb is insufficient to store
+ * the extra payload.
+ */
+static inline void *nlmsg_append(struct sk_buff *skb, u32 size)
+{
+	if (unlikely(skb_tailroom(skb) < NLMSG_ALIGN(size)))
+		return NULL;
+
+	if (NLMSG_ALIGN(size) - size)
+		memset(skb_tail_pointer(skb) + size, 0,
+		       NLMSG_ALIGN(size) - size);
+	return __skb_put(skb, NLMSG_ALIGN(size));
+}
+
 /**
  * nlmsg_put_answer - Add a new callback based netlink message to an skb
  * @skb: socket buffer to store message in
diff --git a/include/uapi/linux/netlink.h b/include/uapi/linux/netlink.h
index e2ae82e3f9f7..5da0da59bf01 100644
--- a/include/uapi/linux/netlink.h
+++ b/include/uapi/linux/netlink.h
@@ -48,6 +48,7 @@ struct sockaddr_nl {
  * @nlmsg_flags: Additional flags
  * @nlmsg_seq:   Sequence number
  * @nlmsg_pid:   Sending process port ID
+ * @nlmsg_data:  Message payload
  */
 struct nlmsghdr {
 	__u32		nlmsg_len;
@@ -55,6 +56,7 @@ struct nlmsghdr {
 	__u16		nlmsg_flags;
 	__u32		nlmsg_seq;
 	__u32		nlmsg_pid;
+	__u8		nlmsg_data[];
 };
 
 /* Flags values */
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index f0c94d394ab1..b10d5e50b99d 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -2499,19 +2499,24 @@ void netlink_ack(struct sk_buff *in_skb, struct nlmsghdr *nlh, int err,
 		flags |= NLM_F_ACK_TLVS;
 
 	skb = nlmsg_new(payload + tlvlen, GFP_KERNEL);
-	if (!skb) {
-		NETLINK_CB(in_skb).sk->sk_err = ENOBUFS;
-		sk_error_report(NETLINK_CB(in_skb).sk);
-		return;
-	}
+	if (!skb)
+		goto err_bad_put;
 
 	rep = nlmsg_put(skb, NETLINK_CB(in_skb).portid, nlh->nlmsg_seq,
-			NLMSG_ERROR, payload, flags);
+			NLMSG_ERROR, sizeof(*errmsg), flags);
+	if (!rep)
+		goto err_bad_put;
 	errmsg = nlmsg_data(rep);
 	errmsg->error = err;
-	unsafe_memcpy(&errmsg->msg, nlh, payload > sizeof(*errmsg)
-					 ? nlh->nlmsg_len : sizeof(*nlh),
-		      /* Bounds checked by the skb layer. */);
+	errmsg->msg = *nlh;
+
+	if (!(flags & NLM_F_CAPPED)) {
+		if (!nlmsg_append(skb, nlmsg_len(nlh)))
+			goto err_bad_put;
+
+		memcpy(errmsg->msg.nlmsg_data, nlh->nlmsg_data,
+		       nlmsg_len(nlh));
+	}
 
 	if (tlvlen)
 		netlink_ack_tlv_fill(in_skb, skb, nlh, err, extack);
@@ -2519,6 +2524,12 @@ void netlink_ack(struct sk_buff *in_skb, struct nlmsghdr *nlh, int err,
 	nlmsg_end(skb, rep);
 
 	nlmsg_unicast(in_skb->sk, skb, NETLINK_CB(in_skb).portid);
+
+	return;
+
+err_bad_put:
+	NETLINK_CB(in_skb).sk->sk_err = ENOBUFS;
+	sk_error_report(NETLINK_CB(in_skb).sk);
 }
 EXPORT_SYMBOL(netlink_ack);
 
-- 
cgit v1.2.3


From ff14adbd8779f098576ffdb98381809ed8e40dfe Mon Sep 17 00:00:00 2001
From: Jakub Kicinski <kuba@kernel.org>
Date: Fri, 4 Nov 2022 12:13:31 -0700
Subject: genetlink: refactor the cmd <> policy mapping dump

The code at the top of ctrl_dumppolicy() dumps mappings between
ops and policies. It supports dumping both the entire family and
single op if dump is filtered. But both of those cases are handled
inside a loop, which makes the logic harder to follow and change.
Refactor to split the two cases more clearly.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
---
 net/netlink/genetlink.c | 27 +++++++++++++++------------
 1 file changed, 15 insertions(+), 12 deletions(-)

(limited to 'net/netlink')

diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c
index 3e16527beb91..0a7a856e9ce0 100644
--- a/net/netlink/genetlink.c
+++ b/net/netlink/genetlink.c
@@ -1319,21 +1319,24 @@ static int ctrl_dumppolicy(struct sk_buff *skb, struct netlink_callback *cb)
 	void *hdr;
 
 	if (!ctx->policies) {
-		while (ctx->opidx < genl_get_cmd_cnt(ctx->rt)) {
-			struct genl_ops op;
+		struct genl_ops op;
 
-			if (ctx->single_op) {
-				int err;
+		if (ctx->single_op) {
+			int err;
 
-				err = genl_get_cmd(ctx->op, ctx->rt, &op);
-				if (WARN_ON(err))
-					return skb->len;
+			err = genl_get_cmd(ctx->op, ctx->rt, &op);
+			if (WARN_ON(err))
+				return err;
 
-				/* break out of the loop after this one */
-				ctx->opidx = genl_get_cmd_cnt(ctx->rt);
-			} else {
-				genl_get_cmd_by_index(ctx->opidx, ctx->rt, &op);
-			}
+			if (ctrl_dumppolicy_put_op(skb, cb, &op))
+				return skb->len;
+
+			/* don't enter the loop below */
+			ctx->opidx = genl_get_cmd_cnt(ctx->rt);
+		}
+
+		while (ctx->opidx < genl_get_cmd_cnt(ctx->rt)) {
+			genl_get_cmd_by_index(ctx->opidx, ctx->rt, &op);
 
 			if (ctrl_dumppolicy_put_op(skb, cb, &op))
 				return skb->len;
-- 
cgit v1.2.3


From 20b0b53aca436af9fece9428ca3ab7c7b9cf4583 Mon Sep 17 00:00:00 2001
From: Jakub Kicinski <kuba@kernel.org>
Date: Fri, 4 Nov 2022 12:13:33 -0700
Subject: genetlink: introduce split op representation

We currently have two forms of operations - small ops and "full" ops
(or just ops). The former does not have pointers for some of the less
commonly used features (namely dump start/done and policy).

The "full" ops, however, still don't contain all the necessary
information. In particular the policy is per command ID, while
do and dump often accept different attributes. It's also not
possible to define different pre_doit and post_doit callbacks
for different commands within the family.

At the same time a lot of commands do not support dumping and
therefore all the dump-related information is wasted space.

Create a new command representation which can hold info about
a do implementation or a dump implementation, but not both at
the same time.

Use this new representation on the command execution path
(genl_family_rcv_msg) as we either run a do or a dump and
don't have to create a "full" op there.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
---
 include/net/genetlink.h   | 60 ++++++++++++++++++++++++++++++++---
 net/batman-adv/netlink.c  |  6 ++--
 net/core/devlink.c        |  4 +--
 net/core/drop_monitor.c   |  4 +--
 net/ieee802154/nl802154.c |  6 ++--
 net/netlink/genetlink.c   | 79 ++++++++++++++++++++++++++++++++++++++---------
 net/wireless/nl80211.c    |  6 ++--
 7 files changed, 136 insertions(+), 29 deletions(-)

(limited to 'net/netlink')

diff --git a/include/net/genetlink.h b/include/net/genetlink.h
index 81180fc6526a..4be7989c451b 100644
--- a/include/net/genetlink.h
+++ b/include/net/genetlink.h
@@ -18,7 +18,7 @@ struct genl_multicast_group {
 	u8			flags;
 };
 
-struct genl_ops;
+struct genl_split_ops;
 struct genl_info;
 
 /**
@@ -66,10 +66,10 @@ struct genl_family {
 	u8			n_mcgrps;
 	u8			resv_start_op;
 	const struct nla_policy *policy;
-	int			(*pre_doit)(const struct genl_ops *ops,
+	int			(*pre_doit)(const struct genl_split_ops *ops,
 					    struct sk_buff *skb,
 					    struct genl_info *info);
-	void			(*post_doit)(const struct genl_ops *ops,
+	void			(*post_doit)(const struct genl_split_ops *ops,
 					     struct sk_buff *skb,
 					     struct genl_info *info);
 	const struct genl_ops *	ops;
@@ -182,6 +182,58 @@ struct genl_ops {
 	u8			validate;
 };
 
+/**
+ * struct genl_split_ops - generic netlink operations (do/dump split version)
+ * @cmd: command identifier
+ * @internal_flags: flags used by the family
+ * @flags: GENL_* flags (%GENL_ADMIN_PERM or %GENL_UNS_ADMIN_PERM)
+ * @validate: validation flags from enum genl_validate_flags
+ * @policy: netlink policy (takes precedence over family policy)
+ * @maxattr: maximum number of attributes supported
+ *
+ * Do callbacks:
+ * @pre_doit: called before an operation's @doit callback, it may
+ *	do additional, common, filtering and return an error
+ * @doit: standard command callback
+ * @post_doit: called after an operation's @doit callback, it may
+ *	undo operations done by pre_doit, for example release locks
+ *
+ * Dump callbacks:
+ * @start: start callback for dumps
+ * @dumpit: callback for dumpers
+ * @done: completion callback for dumps
+ *
+ * Do callbacks can be used if %GENL_CMD_CAP_DO is set in @flags.
+ * Dump callbacks can be used if %GENL_CMD_CAP_DUMP is set in @flags.
+ * Exactly one of those flags must be set.
+ */
+struct genl_split_ops {
+	union {
+		struct {
+			int (*pre_doit)(const struct genl_split_ops *ops,
+					struct sk_buff *skb,
+					struct genl_info *info);
+			int (*doit)(struct sk_buff *skb,
+				    struct genl_info *info);
+			void (*post_doit)(const struct genl_split_ops *ops,
+					  struct sk_buff *skb,
+					  struct genl_info *info);
+		};
+		struct {
+			int (*start)(struct netlink_callback *cb);
+			int (*dumpit)(struct sk_buff *skb,
+				      struct netlink_callback *cb);
+			int (*done)(struct netlink_callback *cb);
+		};
+	};
+	const struct nla_policy *policy;
+	unsigned int		maxattr;
+	u8			cmd;
+	u8			internal_flags;
+	u8			flags;
+	u8			validate;
+};
+
 /**
  * struct genl_dumpit_info - info that is available during dumpit op call
  * @family: generic netlink family - for internal genl code usage
@@ -190,7 +242,7 @@ struct genl_ops {
  */
 struct genl_dumpit_info {
 	const struct genl_family *family;
-	struct genl_ops op;
+	struct genl_split_ops op;
 	struct nlattr **attrs;
 };
 
diff --git a/net/batman-adv/netlink.c b/net/batman-adv/netlink.c
index a5e4a4e976cf..ad5714f737be 100644
--- a/net/batman-adv/netlink.c
+++ b/net/batman-adv/netlink.c
@@ -1267,7 +1267,8 @@ batadv_get_vlan_from_info(struct batadv_priv *bat_priv, struct net *net,
  *
  * Return: 0 on success or negative error number in case of failure
  */
-static int batadv_pre_doit(const struct genl_ops *ops, struct sk_buff *skb,
+static int batadv_pre_doit(const struct genl_split_ops *ops,
+			   struct sk_buff *skb,
 			   struct genl_info *info)
 {
 	struct net *net = genl_info_net(info);
@@ -1332,7 +1333,8 @@ err_put_softif:
  * @skb: Netlink message with request data
  * @info: receiver information
  */
-static void batadv_post_doit(const struct genl_ops *ops, struct sk_buff *skb,
+static void batadv_post_doit(const struct genl_split_ops *ops,
+			     struct sk_buff *skb,
 			     struct genl_info *info)
 {
 	struct batadv_hard_iface *hard_iface;
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 2dcf2bcc3527..40fcdded57e6 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -770,7 +770,7 @@ devlink_region_snapshot_get_by_id(struct devlink_region *region, u32 id)
 #define DEVLINK_NL_FLAG_NEED_RATE_NODE		BIT(3)
 #define DEVLINK_NL_FLAG_NEED_LINECARD		BIT(4)
 
-static int devlink_nl_pre_doit(const struct genl_ops *ops,
+static int devlink_nl_pre_doit(const struct genl_split_ops *ops,
 			       struct sk_buff *skb, struct genl_info *info)
 {
 	struct devlink_linecard *linecard;
@@ -828,7 +828,7 @@ unlock:
 	return err;
 }
 
-static void devlink_nl_post_doit(const struct genl_ops *ops,
+static void devlink_nl_post_doit(const struct genl_split_ops *ops,
 				 struct sk_buff *skb, struct genl_info *info)
 {
 	struct devlink_linecard *linecard;
diff --git a/net/core/drop_monitor.c b/net/core/drop_monitor.c
index 11aa6e8a3098..5a782d1d8fd3 100644
--- a/net/core/drop_monitor.c
+++ b/net/core/drop_monitor.c
@@ -1620,7 +1620,7 @@ static const struct genl_small_ops dropmon_ops[] = {
 	},
 };
 
-static int net_dm_nl_pre_doit(const struct genl_ops *ops,
+static int net_dm_nl_pre_doit(const struct genl_split_ops *ops,
 			      struct sk_buff *skb, struct genl_info *info)
 {
 	mutex_lock(&net_dm_mutex);
@@ -1628,7 +1628,7 @@ static int net_dm_nl_pre_doit(const struct genl_ops *ops,
 	return 0;
 }
 
-static void net_dm_nl_post_doit(const struct genl_ops *ops,
+static void net_dm_nl_post_doit(const struct genl_split_ops *ops,
 				struct sk_buff *skb, struct genl_info *info)
 {
 	mutex_unlock(&net_dm_mutex);
diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c
index 38c4f3cb010e..b33d1b5eda87 100644
--- a/net/ieee802154/nl802154.c
+++ b/net/ieee802154/nl802154.c
@@ -2157,7 +2157,8 @@ static int nl802154_del_llsec_seclevel(struct sk_buff *skb,
 #define NL802154_FLAG_CHECK_NETDEV_UP	0x08
 #define NL802154_FLAG_NEED_WPAN_DEV	0x10
 
-static int nl802154_pre_doit(const struct genl_ops *ops, struct sk_buff *skb,
+static int nl802154_pre_doit(const struct genl_split_ops *ops,
+			     struct sk_buff *skb,
 			     struct genl_info *info)
 {
 	struct cfg802154_registered_device *rdev;
@@ -2219,7 +2220,8 @@ static int nl802154_pre_doit(const struct genl_ops *ops, struct sk_buff *skb,
 	return 0;
 }
 
-static void nl802154_post_doit(const struct genl_ops *ops, struct sk_buff *skb,
+static void nl802154_post_doit(const struct genl_split_ops *ops,
+			       struct sk_buff *skb,
 			       struct genl_info *info)
 {
 	if (info->user_ptr[1]) {
diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c
index 0a7a856e9ce0..c66299740c05 100644
--- a/net/netlink/genetlink.c
+++ b/net/netlink/genetlink.c
@@ -189,6 +189,51 @@ static int genl_get_cmd(u32 cmd, const struct genl_family *family,
 	return genl_get_cmd_small(cmd, family, op);
 }
 
+static void
+genl_cmd_full_to_split(struct genl_split_ops *op,
+		       const struct genl_family *family,
+		       const struct genl_ops *full, u8 flags)
+{
+	if (flags & GENL_CMD_CAP_DUMP) {
+		op->start	= full->start;
+		op->dumpit	= full->dumpit;
+		op->done	= full->done;
+	} else {
+		op->pre_doit	= family->pre_doit;
+		op->doit	= full->doit;
+		op->post_doit	= family->post_doit;
+	}
+
+	op->policy		= full->policy;
+	op->maxattr		= full->maxattr;
+
+	op->cmd			= full->cmd;
+	op->internal_flags	= full->internal_flags;
+	op->flags		= full->flags;
+	op->validate		= full->validate;
+
+	/* Make sure flags include the GENL_CMD_CAP_DO / GENL_CMD_CAP_DUMP */
+	op->flags		|= flags;
+}
+
+static int
+genl_get_cmd_split(u32 cmd, u8 flags, const struct genl_family *family,
+		   struct genl_split_ops *op)
+{
+	struct genl_ops full;
+	int err;
+
+	err = genl_get_cmd(cmd, family, &full);
+	if (err) {
+		memset(op, 0, sizeof(*op));
+		return err;
+	}
+
+	genl_cmd_full_to_split(op, family, &full, flags);
+
+	return 0;
+}
+
 static void genl_get_cmd_by_index(unsigned int i,
 				  const struct genl_family *family,
 				  struct genl_ops *op)
@@ -544,7 +589,7 @@ static struct nlattr **
 genl_family_rcv_msg_attrs_parse(const struct genl_family *family,
 				struct nlmsghdr *nlh,
 				struct netlink_ext_ack *extack,
-				const struct genl_ops *ops,
+				const struct genl_split_ops *ops,
 				int hdrlen,
 				enum genl_validate_flags no_strict_flag)
 {
@@ -580,18 +625,19 @@ struct genl_start_context {
 	const struct genl_family *family;
 	struct nlmsghdr *nlh;
 	struct netlink_ext_ack *extack;
-	const struct genl_ops *ops;
+	const struct genl_split_ops *ops;
 	int hdrlen;
 };
 
 static int genl_start(struct netlink_callback *cb)
 {
 	struct genl_start_context *ctx = cb->data;
-	const struct genl_ops *ops = ctx->ops;
+	const struct genl_split_ops *ops;
 	struct genl_dumpit_info *info;
 	struct nlattr **attrs = NULL;
 	int rc = 0;
 
+	ops = ctx->ops;
 	if (ops->validate & GENL_DONT_VALIDATE_DUMP)
 		goto no_attrs;
 
@@ -633,7 +679,7 @@ no_attrs:
 
 static int genl_lock_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
 {
-	const struct genl_ops *ops = &genl_dumpit_info(cb)->op;
+	const struct genl_split_ops *ops = &genl_dumpit_info(cb)->op;
 	int rc;
 
 	genl_lock();
@@ -645,7 +691,7 @@ static int genl_lock_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
 static int genl_lock_done(struct netlink_callback *cb)
 {
 	const struct genl_dumpit_info *info = genl_dumpit_info(cb);
-	const struct genl_ops *ops = &info->op;
+	const struct genl_split_ops *ops = &info->op;
 	int rc = 0;
 
 	if (ops->done) {
@@ -661,7 +707,7 @@ static int genl_lock_done(struct netlink_callback *cb)
 static int genl_parallel_done(struct netlink_callback *cb)
 {
 	const struct genl_dumpit_info *info = genl_dumpit_info(cb);
-	const struct genl_ops *ops = &info->op;
+	const struct genl_split_ops *ops = &info->op;
 	int rc = 0;
 
 	if (ops->done)
@@ -675,7 +721,7 @@ static int genl_family_rcv_msg_dumpit(const struct genl_family *family,
 				      struct sk_buff *skb,
 				      struct nlmsghdr *nlh,
 				      struct netlink_ext_ack *extack,
-				      const struct genl_ops *ops,
+				      const struct genl_split_ops *ops,
 				      int hdrlen, struct net *net)
 {
 	struct genl_start_context ctx;
@@ -721,7 +767,7 @@ static int genl_family_rcv_msg_doit(const struct genl_family *family,
 				    struct sk_buff *skb,
 				    struct nlmsghdr *nlh,
 				    struct netlink_ext_ack *extack,
-				    const struct genl_ops *ops,
+				    const struct genl_split_ops *ops,
 				    int hdrlen, struct net *net)
 {
 	struct nlattr **attrbuf;
@@ -747,16 +793,16 @@ static int genl_family_rcv_msg_doit(const struct genl_family *family,
 	genl_info_net_set(&info, net);
 	memset(&info.user_ptr, 0, sizeof(info.user_ptr));
 
-	if (family->pre_doit) {
-		err = family->pre_doit(ops, skb, &info);
+	if (ops->pre_doit) {
+		err = ops->pre_doit(ops, skb, &info);
 		if (err)
 			goto out;
 	}
 
 	err = ops->doit(skb, &info);
 
-	if (family->post_doit)
-		family->post_doit(ops, skb, &info);
+	if (ops->post_doit)
+		ops->post_doit(ops, skb, &info);
 
 out:
 	genl_family_rcv_msg_attrs_free(attrbuf);
@@ -801,8 +847,9 @@ static int genl_family_rcv_msg(const struct genl_family *family,
 {
 	struct net *net = sock_net(skb->sk);
 	struct genlmsghdr *hdr = nlmsg_data(nlh);
-	struct genl_ops op;
+	struct genl_split_ops op;
 	int hdrlen;
+	u8 flags;
 
 	/* this family doesn't exist in this netns */
 	if (!family->netnsok && !net_eq(net, &init_net))
@@ -815,7 +862,9 @@ static int genl_family_rcv_msg(const struct genl_family *family,
 	if (genl_header_check(family, nlh, hdr, extack))
 		return -EINVAL;
 
-	if (genl_get_cmd(hdr->cmd, family, &op))
+	flags = (nlh->nlmsg_flags & NLM_F_DUMP) == NLM_F_DUMP ?
+		GENL_CMD_CAP_DUMP : GENL_CMD_CAP_DO;
+	if (genl_get_cmd_split(hdr->cmd, flags, family, &op))
 		return -EOPNOTSUPP;
 
 	if ((op.flags & GENL_ADMIN_PERM) &&
@@ -826,7 +875,7 @@ static int genl_family_rcv_msg(const struct genl_family *family,
 	    !netlink_ns_capable(skb, net->user_ns, CAP_NET_ADMIN))
 		return -EPERM;
 
-	if ((nlh->nlmsg_flags & NLM_F_DUMP) == NLM_F_DUMP)
+	if (flags & GENL_CMD_CAP_DUMP)
 		return genl_family_rcv_msg_dumpit(family, skb, nlh, extack,
 						  &op, hdrlen, net);
 	else
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 148f66edb015..1ad0326ff4dc 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -16140,7 +16140,8 @@ static u32 nl80211_internal_flags[] = {
 #undef SELECTOR
 };
 
-static int nl80211_pre_doit(const struct genl_ops *ops, struct sk_buff *skb,
+static int nl80211_pre_doit(const struct genl_split_ops *ops,
+			    struct sk_buff *skb,
 			    struct genl_info *info)
 {
 	struct cfg80211_registered_device *rdev = NULL;
@@ -16241,7 +16242,8 @@ out_unlock:
 	return err;
 }
 
-static void nl80211_post_doit(const struct genl_ops *ops, struct sk_buff *skb,
+static void nl80211_post_doit(const struct genl_split_ops *ops,
+			      struct sk_buff *skb,
 			      struct genl_info *info)
 {
 	u32 internal_flags = nl80211_internal_flags[ops->internal_flags];
-- 
cgit v1.2.3


From 7747eb75f6185a92db2e4a2643fdf6e4b6d87153 Mon Sep 17 00:00:00 2001
From: Jakub Kicinski <kuba@kernel.org>
Date: Fri, 4 Nov 2022 12:13:34 -0700
Subject: genetlink: load policy based on validation flags

Set the policy and maxattr pointers based on validation flags.
genl_family_rcv_msg_attrs_parse() will do nothing and return NULL
if maxattrs is zero, so no behavior change is expected.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
---
 net/netlink/genetlink.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

(limited to 'net/netlink')

diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c
index c66299740c05..770726ac491e 100644
--- a/net/netlink/genetlink.c
+++ b/net/netlink/genetlink.c
@@ -204,8 +204,14 @@ genl_cmd_full_to_split(struct genl_split_ops *op,
 		op->post_doit	= family->post_doit;
 	}
 
-	op->policy		= full->policy;
-	op->maxattr		= full->maxattr;
+	if (flags & GENL_CMD_CAP_DUMP &&
+	    full->validate & GENL_DONT_VALIDATE_DUMP) {
+		op->policy	= NULL;
+		op->maxattr	= 0;
+	} else {
+		op->policy	= full->policy;
+		op->maxattr	= full->maxattr;
+	}
 
 	op->cmd			= full->cmd;
 	op->internal_flags	= full->internal_flags;
@@ -638,10 +644,8 @@ static int genl_start(struct netlink_callback *cb)
 	int rc = 0;
 
 	ops = ctx->ops;
-	if (ops->validate & GENL_DONT_VALIDATE_DUMP)
-		goto no_attrs;
-
-	if (ctx->nlh->nlmsg_len < nlmsg_msg_size(ctx->hdrlen))
+	if (!(ops->validate & GENL_DONT_VALIDATE_DUMP) &&
+	    ctx->nlh->nlmsg_len < nlmsg_msg_size(ctx->hdrlen))
 		return -EINVAL;
 
 	attrs = genl_family_rcv_msg_attrs_parse(ctx->family, ctx->nlh, ctx->extack,
@@ -650,7 +654,6 @@ static int genl_start(struct netlink_callback *cb)
 	if (IS_ERR(attrs))
 		return PTR_ERR(attrs);
 
-no_attrs:
 	info = genl_dumpit_info_alloc();
 	if (!info) {
 		genl_family_rcv_msg_attrs_free(attrs);
-- 
cgit v1.2.3


From e1a248911d0694c8f95510fcaa920c0b8e99c26c Mon Sep 17 00:00:00 2001
From: Jakub Kicinski <kuba@kernel.org>
Date: Fri, 4 Nov 2022 12:13:35 -0700
Subject: genetlink: check for callback type at op load time

Now that genl_get_cmd_split() is informed what type of callback
user is trying to access (do or dump) we can check that this
callback is indeed available and return an error early.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
---
 net/netlink/genetlink.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

(limited to 'net/netlink')

diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c
index 770726ac491e..7c04df1bee2b 100644
--- a/net/netlink/genetlink.c
+++ b/net/netlink/genetlink.c
@@ -189,11 +189,17 @@ static int genl_get_cmd(u32 cmd, const struct genl_family *family,
 	return genl_get_cmd_small(cmd, family, op);
 }
 
-static void
+static int
 genl_cmd_full_to_split(struct genl_split_ops *op,
 		       const struct genl_family *family,
 		       const struct genl_ops *full, u8 flags)
 {
+	if ((flags & GENL_CMD_CAP_DO && !full->doit) ||
+	    (flags & GENL_CMD_CAP_DUMP && !full->dumpit)) {
+		memset(op, 0, sizeof(*op));
+		return -ENOENT;
+	}
+
 	if (flags & GENL_CMD_CAP_DUMP) {
 		op->start	= full->start;
 		op->dumpit	= full->dumpit;
@@ -220,6 +226,8 @@ genl_cmd_full_to_split(struct genl_split_ops *op,
 
 	/* Make sure flags include the GENL_CMD_CAP_DO / GENL_CMD_CAP_DUMP */
 	op->flags		|= flags;
+
+	return 0;
 }
 
 static int
@@ -235,9 +243,7 @@ genl_get_cmd_split(u32 cmd, u8 flags, const struct genl_family *family,
 		return err;
 	}
 
-	genl_cmd_full_to_split(op, family, &full, flags);
-
-	return 0;
+	return genl_cmd_full_to_split(op, family, &full, flags);
 }
 
 static void genl_get_cmd_by_index(unsigned int i,
@@ -730,9 +736,6 @@ static int genl_family_rcv_msg_dumpit(const struct genl_family *family,
 	struct genl_start_context ctx;
 	int err;
 
-	if (!ops->dumpit)
-		return -EOPNOTSUPP;
-
 	ctx.family = family;
 	ctx.nlh = nlh;
 	ctx.extack = extack;
@@ -777,9 +780,6 @@ static int genl_family_rcv_msg_doit(const struct genl_family *family,
 	struct genl_info info;
 	int err;
 
-	if (!ops->doit)
-		return -EOPNOTSUPP;
-
 	attrbuf = genl_family_rcv_msg_attrs_parse(family, nlh, extack,
 						  ops, hdrlen,
 						  GENL_DONT_VALIDATE_STRICT);
-- 
cgit v1.2.3


From 92d3d9ba9bb3ab17b5bd4fd46032ced0264872c9 Mon Sep 17 00:00:00 2001
From: Jakub Kicinski <kuba@kernel.org>
Date: Fri, 4 Nov 2022 12:13:36 -0700
Subject: genetlink: add policies for both doit and dumpit in
 ctrl_dumppolicy_start()

Separate adding doit and dumpit policies for CTRL_CMD_GETPOLICY.
This has no effect until we actually allow do and dump to come
from different sources as netlink_policy_dump_add_policy()
does deduplication.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
---
 net/netlink/genetlink.c | 48 ++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 38 insertions(+), 10 deletions(-)

(limited to 'net/netlink')

diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c
index 7c04df1bee2b..d0c35738839b 100644
--- a/net/netlink/genetlink.c
+++ b/net/netlink/genetlink.c
@@ -1260,29 +1260,57 @@ static int ctrl_dumppolicy_start(struct netlink_callback *cb)
 	ctx->rt = rt;
 
 	if (tb[CTRL_ATTR_OP]) {
+		struct genl_split_ops doit, dump;
+
 		ctx->single_op = true;
 		ctx->op = nla_get_u32(tb[CTRL_ATTR_OP]);
 
-		err = genl_get_cmd(ctx->op, rt, &op);
-		if (err) {
+		if (genl_get_cmd_split(ctx->op, GENL_CMD_CAP_DO, rt, &doit) &&
+		    genl_get_cmd_split(ctx->op, GENL_CMD_CAP_DUMP, rt, &dump)) {
 			NL_SET_BAD_ATTR(cb->extack, tb[CTRL_ATTR_OP]);
-			return err;
+			return -ENOENT;
 		}
 
-		if (!op.policy)
-			return -ENODATA;
+		if (doit.policy) {
+			err = netlink_policy_dump_add_policy(&ctx->state,
+							     doit.policy,
+							     doit.maxattr);
+			if (err)
+				goto err_free_state;
+		}
+		if (dump.policy) {
+			err = netlink_policy_dump_add_policy(&ctx->state,
+							     dump.policy,
+							     dump.maxattr);
+			if (err)
+				goto err_free_state;
+		}
 
-		return netlink_policy_dump_add_policy(&ctx->state, op.policy,
-						      op.maxattr);
+		if (!ctx->state)
+			return -ENODATA;
+		return 0;
 	}
 
 	for (i = 0; i < genl_get_cmd_cnt(rt); i++) {
+		struct genl_split_ops doit, dumpit;
+
 		genl_get_cmd_by_index(i, rt, &op);
 
-		if (op.policy) {
+		genl_cmd_full_to_split(&doit, ctx->rt, &op, GENL_CMD_CAP_DO);
+		genl_cmd_full_to_split(&dumpit, ctx->rt,
+				       &op, GENL_CMD_CAP_DUMP);
+
+		if (doit.policy) {
+			err = netlink_policy_dump_add_policy(&ctx->state,
+							     doit.policy,
+							     doit.maxattr);
+			if (err)
+				goto err_free_state;
+		}
+		if (dumpit.policy) {
 			err = netlink_policy_dump_add_policy(&ctx->state,
-							     op.policy,
-							     op.maxattr);
+							     dumpit.policy,
+							     dumpit.maxattr);
 			if (err)
 				goto err_free_state;
 		}
-- 
cgit v1.2.3


From 26588edbef6094c41734b4443c2d7b4c4bd0085f Mon Sep 17 00:00:00 2001
From: Jakub Kicinski <kuba@kernel.org>
Date: Fri, 4 Nov 2022 12:13:37 -0700
Subject: genetlink: support split policies in ctrl_dumppolicy_put_op()

Pass do and dump versions of the op to ctrl_dumppolicy_put_op()
so that it can provide a different policy index for the two.

Since we now look at policies, and those are set appropriately
there's no need to look at the GENL_DONT_VALIDATE_DUMP flag.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
---
 net/netlink/genetlink.c | 55 +++++++++++++++++++++++++++++--------------------
 1 file changed, 33 insertions(+), 22 deletions(-)

(limited to 'net/netlink')

diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c
index d0c35738839b..93e33e20a0e8 100644
--- a/net/netlink/genetlink.c
+++ b/net/netlink/genetlink.c
@@ -1345,7 +1345,8 @@ static void *ctrl_dumppolicy_prep(struct sk_buff *skb,
 
 static int ctrl_dumppolicy_put_op(struct sk_buff *skb,
 				  struct netlink_callback *cb,
-			          struct genl_ops *op)
+				  struct genl_split_ops *doit,
+				  struct genl_split_ops *dumpit)
 {
 	struct ctrl_dump_policy_ctx *ctx = (void *)cb->ctx;
 	struct nlattr *nest_pol, *nest_op;
@@ -1353,10 +1354,7 @@ static int ctrl_dumppolicy_put_op(struct sk_buff *skb,
 	int idx;
 
 	/* skip if we have nothing to show */
-	if (!op->policy)
-		return 0;
-	if (!op->doit &&
-	    (!op->dumpit || op->validate & GENL_DONT_VALIDATE_DUMP))
+	if (!doit->policy && !dumpit->policy)
 		return 0;
 
 	hdr = ctrl_dumppolicy_prep(skb, cb);
@@ -1367,21 +1365,26 @@ static int ctrl_dumppolicy_put_op(struct sk_buff *skb,
 	if (!nest_pol)
 		goto err;
 
-	nest_op = nla_nest_start(skb, op->cmd);
+	nest_op = nla_nest_start(skb, doit->cmd);
 	if (!nest_op)
 		goto err;
 
-	/* for now both do/dump are always the same */
-	idx = netlink_policy_dump_get_policy_idx(ctx->state,
-						 op->policy,
-						 op->maxattr);
+	if (doit->policy) {
+		idx = netlink_policy_dump_get_policy_idx(ctx->state,
+							 doit->policy,
+							 doit->maxattr);
 
-	if (op->doit && nla_put_u32(skb, CTRL_ATTR_POLICY_DO, idx))
-		goto err;
+		if (nla_put_u32(skb, CTRL_ATTR_POLICY_DO, idx))
+			goto err;
+	}
+	if (dumpit->policy) {
+		idx = netlink_policy_dump_get_policy_idx(ctx->state,
+							 dumpit->policy,
+							 dumpit->maxattr);
 
-	if (op->dumpit && !(op->validate & GENL_DONT_VALIDATE_DUMP) &&
-	    nla_put_u32(skb, CTRL_ATTR_POLICY_DUMP, idx))
-		goto err;
+		if (nla_put_u32(skb, CTRL_ATTR_POLICY_DUMP, idx))
+			goto err;
+	}
 
 	nla_nest_end(skb, nest_op);
 	nla_nest_end(skb, nest_pol);
@@ -1399,16 +1402,19 @@ static int ctrl_dumppolicy(struct sk_buff *skb, struct netlink_callback *cb)
 	void *hdr;
 
 	if (!ctx->policies) {
+		struct genl_split_ops doit, dumpit;
 		struct genl_ops op;
 
 		if (ctx->single_op) {
-			int err;
-
-			err = genl_get_cmd(ctx->op, ctx->rt, &op);
-			if (WARN_ON(err))
-				return err;
+			if (genl_get_cmd_split(ctx->op, GENL_CMD_CAP_DO,
+					       ctx->rt, &doit) &&
+			    genl_get_cmd_split(ctx->op, GENL_CMD_CAP_DUMP,
+					       ctx->rt, &dumpit)) {
+				WARN_ON(1);
+				return -ENOENT;
+			}
 
-			if (ctrl_dumppolicy_put_op(skb, cb, &op))
+			if (ctrl_dumppolicy_put_op(skb, cb, &doit, &dumpit))
 				return skb->len;
 
 			/* don't enter the loop below */
@@ -1418,7 +1424,12 @@ static int ctrl_dumppolicy(struct sk_buff *skb, struct netlink_callback *cb)
 		while (ctx->opidx < genl_get_cmd_cnt(ctx->rt)) {
 			genl_get_cmd_by_index(ctx->opidx, ctx->rt, &op);
 
-			if (ctrl_dumppolicy_put_op(skb, cb, &op))
+			genl_cmd_full_to_split(&doit, ctx->rt,
+					       &op, GENL_CMD_CAP_DO);
+			genl_cmd_full_to_split(&dumpit, ctx->rt,
+					       &op, GENL_CMD_CAP_DUMP);
+
+			if (ctrl_dumppolicy_put_op(skb, cb, &doit, &dumpit))
 				return skb->len;
 
 			ctx->opidx++;
-- 
cgit v1.2.3


From 8d84322ae6d7921c461b6a5ef39be0b48c177b8a Mon Sep 17 00:00:00 2001
From: Jakub Kicinski <kuba@kernel.org>
Date: Fri, 4 Nov 2022 12:13:38 -0700
Subject: genetlink: inline genl_get_cmd()

All callers go via genl_get_cmd_split() now, so rename it
to genl_get_cmd() remove the original.

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
---
 net/netlink/genetlink.c | 30 ++++++++++++------------------
 1 file changed, 12 insertions(+), 18 deletions(-)

(limited to 'net/netlink')

diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c
index 93e33e20a0e8..ec32b6063a3f 100644
--- a/net/netlink/genetlink.c
+++ b/net/netlink/genetlink.c
@@ -181,14 +181,6 @@ static int genl_get_cmd_small(u32 cmd, const struct genl_family *family,
 	return -ENOENT;
 }
 
-static int genl_get_cmd(u32 cmd, const struct genl_family *family,
-			struct genl_ops *op)
-{
-	if (!genl_get_cmd_full(cmd, family, op))
-		return 0;
-	return genl_get_cmd_small(cmd, family, op);
-}
-
 static int
 genl_cmd_full_to_split(struct genl_split_ops *op,
 		       const struct genl_family *family,
@@ -231,13 +223,15 @@ genl_cmd_full_to_split(struct genl_split_ops *op,
 }
 
 static int
-genl_get_cmd_split(u32 cmd, u8 flags, const struct genl_family *family,
-		   struct genl_split_ops *op)
+genl_get_cmd(u32 cmd, u8 flags, const struct genl_family *family,
+	     struct genl_split_ops *op)
 {
 	struct genl_ops full;
 	int err;
 
-	err = genl_get_cmd(cmd, family, &full);
+	err = genl_get_cmd_full(cmd, family, &full);
+	if (err == -ENOENT)
+		err = genl_get_cmd_small(cmd, family, &full);
 	if (err) {
 		memset(op, 0, sizeof(*op));
 		return err;
@@ -867,7 +861,7 @@ static int genl_family_rcv_msg(const struct genl_family *family,
 
 	flags = (nlh->nlmsg_flags & NLM_F_DUMP) == NLM_F_DUMP ?
 		GENL_CMD_CAP_DUMP : GENL_CMD_CAP_DO;
-	if (genl_get_cmd_split(hdr->cmd, flags, family, &op))
+	if (genl_get_cmd(hdr->cmd, flags, family, &op))
 		return -EOPNOTSUPP;
 
 	if ((op.flags & GENL_ADMIN_PERM) &&
@@ -1265,8 +1259,8 @@ static int ctrl_dumppolicy_start(struct netlink_callback *cb)
 		ctx->single_op = true;
 		ctx->op = nla_get_u32(tb[CTRL_ATTR_OP]);
 
-		if (genl_get_cmd_split(ctx->op, GENL_CMD_CAP_DO, rt, &doit) &&
-		    genl_get_cmd_split(ctx->op, GENL_CMD_CAP_DUMP, rt, &dump)) {
+		if (genl_get_cmd(ctx->op, GENL_CMD_CAP_DO, rt, &doit) &&
+		    genl_get_cmd(ctx->op, GENL_CMD_CAP_DUMP, rt, &dump)) {
 			NL_SET_BAD_ATTR(cb->extack, tb[CTRL_ATTR_OP]);
 			return -ENOENT;
 		}
@@ -1406,10 +1400,10 @@ static int ctrl_dumppolicy(struct sk_buff *skb, struct netlink_callback *cb)
 		struct genl_ops op;
 
 		if (ctx->single_op) {
-			if (genl_get_cmd_split(ctx->op, GENL_CMD_CAP_DO,
-					       ctx->rt, &doit) &&
-			    genl_get_cmd_split(ctx->op, GENL_CMD_CAP_DUMP,
-					       ctx->rt, &dumpit)) {
+			if (genl_get_cmd(ctx->op, GENL_CMD_CAP_DO,
+					 ctx->rt, &doit) &&
+			    genl_get_cmd(ctx->op, GENL_CMD_CAP_DUMP,
+					 ctx->rt, &dumpit)) {
 				WARN_ON(1);
 				return -ENOENT;
 			}
-- 
cgit v1.2.3


From 6557461cd27843213ca19a7918bbaa8c3b7c5f5c Mon Sep 17 00:00:00 2001
From: Jakub Kicinski <kuba@kernel.org>
Date: Fri, 4 Nov 2022 12:13:39 -0700
Subject: genetlink: add iterator for walking family ops

Subsequent changes will expose split op structures to users,
so walking the family ops with just an index will get harder.
Add a structured iterator, convert the simple cases.
Policy dumping needs more careful conversion.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
---
 net/netlink/genetlink.c | 117 +++++++++++++++++++++++++++++++-----------------
 1 file changed, 76 insertions(+), 41 deletions(-)

(limited to 'net/netlink')

diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c
index ec32b6063a3f..06bf091c1b8a 100644
--- a/net/netlink/genetlink.c
+++ b/net/netlink/genetlink.c
@@ -252,6 +252,57 @@ static void genl_get_cmd_by_index(unsigned int i,
 		WARN_ON_ONCE(1);
 }
 
+struct genl_op_iter {
+	const struct genl_family *family;
+	struct genl_split_ops doit;
+	struct genl_split_ops dumpit;
+	int i;
+	u32 cmd;
+	u8 flags;
+};
+
+static bool
+genl_op_iter_init(const struct genl_family *family, struct genl_op_iter *iter)
+{
+	iter->family = family;
+	iter->i = 0;
+
+	iter->flags = 0;
+
+	return genl_get_cmd_cnt(iter->family);
+}
+
+static bool genl_op_iter_next(struct genl_op_iter *iter)
+{
+	struct genl_ops op;
+
+	if (iter->i >= genl_get_cmd_cnt(iter->family))
+		return false;
+
+	genl_get_cmd_by_index(iter->i, iter->family, &op);
+	iter->i++;
+
+	genl_cmd_full_to_split(&iter->doit, iter->family, &op, GENL_CMD_CAP_DO);
+	genl_cmd_full_to_split(&iter->dumpit, iter->family,
+			       &op, GENL_CMD_CAP_DUMP);
+
+	iter->cmd = iter->doit.cmd | iter->dumpit.cmd;
+	iter->flags = iter->doit.flags | iter->dumpit.flags;
+
+	return true;
+}
+
+static void
+genl_op_iter_copy(struct genl_op_iter *dst, struct genl_op_iter *src)
+{
+	*dst = *src;
+}
+
+static unsigned int genl_op_iter_idx(struct genl_op_iter *iter)
+{
+	return iter->i;
+}
+
 static int genl_allocate_reserve_groups(int n_groups, int *first_id)
 {
 	unsigned long *new_groups;
@@ -419,25 +470,23 @@ static void genl_unregister_mc_groups(const struct genl_family *family)
 
 static int genl_validate_ops(const struct genl_family *family)
 {
-	int i, j;
+	struct genl_op_iter i, j;
 
 	if (WARN_ON(family->n_ops && !family->ops) ||
 	    WARN_ON(family->n_small_ops && !family->small_ops))
 		return -EINVAL;
 
-	for (i = 0; i < genl_get_cmd_cnt(family); i++) {
-		struct genl_ops op;
-
-		genl_get_cmd_by_index(i, family, &op);
-		if (op.dumpit == NULL && op.doit == NULL)
+	for (genl_op_iter_init(family, &i); genl_op_iter_next(&i); ) {
+		if (!(i.flags & (GENL_CMD_CAP_DO | GENL_CMD_CAP_DUMP)))
 			return -EINVAL;
-		if (WARN_ON(op.cmd >= family->resv_start_op && op.validate))
+
+		if (WARN_ON(i.cmd >= family->resv_start_op &&
+			    (i.doit.validate || i.dumpit.validate)))
 			return -EINVAL;
-		for (j = i + 1; j < genl_get_cmd_cnt(family); j++) {
-			struct genl_ops op2;
 
-			genl_get_cmd_by_index(j, family, &op2);
-			if (op.cmd == op2.cmd)
+		genl_op_iter_copy(&j, &i);
+		while (genl_op_iter_next(&j)) {
+			if (i.cmd == j.cmd)
 				return -EINVAL;
 		}
 	}
@@ -917,6 +966,7 @@ static struct genl_family genl_ctrl;
 static int ctrl_fill_info(const struct genl_family *family, u32 portid, u32 seq,
 			  u32 flags, struct sk_buff *skb, u8 cmd)
 {
+	struct genl_op_iter i;
 	void *hdr;
 
 	hdr = genlmsg_put(skb, portid, seq, &genl_ctrl, flags, cmd);
@@ -930,33 +980,26 @@ static int ctrl_fill_info(const struct genl_family *family, u32 portid, u32 seq,
 	    nla_put_u32(skb, CTRL_ATTR_MAXATTR, family->maxattr))
 		goto nla_put_failure;
 
-	if (genl_get_cmd_cnt(family)) {
+	if (genl_op_iter_init(family, &i)) {
 		struct nlattr *nla_ops;
-		int i;
 
 		nla_ops = nla_nest_start_noflag(skb, CTRL_ATTR_OPS);
 		if (nla_ops == NULL)
 			goto nla_put_failure;
 
-		for (i = 0; i < genl_get_cmd_cnt(family); i++) {
+		while (genl_op_iter_next(&i)) {
 			struct nlattr *nest;
-			struct genl_ops op;
 			u32 op_flags;
 
-			genl_get_cmd_by_index(i, family, &op);
-			op_flags = op.flags;
-			if (op.dumpit)
-				op_flags |= GENL_CMD_CAP_DUMP;
-			if (op.doit)
-				op_flags |= GENL_CMD_CAP_DO;
-			if (op.policy)
+			op_flags = i.flags;
+			if (i.doit.policy || i.dumpit.policy)
 				op_flags |= GENL_CMD_CAP_HASPOL;
 
-			nest = nla_nest_start_noflag(skb, i + 1);
+			nest = nla_nest_start_noflag(skb, genl_op_iter_idx(&i));
 			if (nest == NULL)
 				goto nla_put_failure;
 
-			if (nla_put_u32(skb, CTRL_ATTR_OP_ID, op.cmd) ||
+			if (nla_put_u32(skb, CTRL_ATTR_OP_ID, i.cmd) ||
 			    nla_put_u32(skb, CTRL_ATTR_OP_FLAGS, op_flags))
 				goto nla_put_failure;
 
@@ -1229,8 +1272,8 @@ static int ctrl_dumppolicy_start(struct netlink_callback *cb)
 	struct ctrl_dump_policy_ctx *ctx = (void *)cb->ctx;
 	struct nlattr **tb = info->attrs;
 	const struct genl_family *rt;
-	struct genl_ops op;
-	int err, i;
+	struct genl_op_iter i;
+	int err;
 
 	BUILD_BUG_ON(sizeof(*ctx) > sizeof(cb->ctx));
 
@@ -1285,26 +1328,18 @@ static int ctrl_dumppolicy_start(struct netlink_callback *cb)
 		return 0;
 	}
 
-	for (i = 0; i < genl_get_cmd_cnt(rt); i++) {
-		struct genl_split_ops doit, dumpit;
-
-		genl_get_cmd_by_index(i, rt, &op);
-
-		genl_cmd_full_to_split(&doit, ctx->rt, &op, GENL_CMD_CAP_DO);
-		genl_cmd_full_to_split(&dumpit, ctx->rt,
-				       &op, GENL_CMD_CAP_DUMP);
-
-		if (doit.policy) {
+	for (genl_op_iter_init(rt, &i); genl_op_iter_next(&i); ) {
+		if (i.doit.policy) {
 			err = netlink_policy_dump_add_policy(&ctx->state,
-							     doit.policy,
-							     doit.maxattr);
+							     i.doit.policy,
+							     i.doit.maxattr);
 			if (err)
 				goto err_free_state;
 		}
-		if (dumpit.policy) {
+		if (i.dumpit.policy) {
 			err = netlink_policy_dump_add_policy(&ctx->state,
-							     dumpit.policy,
-							     dumpit.maxattr);
+							     i.dumpit.policy,
+							     i.dumpit.maxattr);
 			if (err)
 				goto err_free_state;
 		}
-- 
cgit v1.2.3


From b502b3185cd68f24510f8a253bf31a1b34605bb6 Mon Sep 17 00:00:00 2001
From: Jakub Kicinski <kuba@kernel.org>
Date: Fri, 4 Nov 2022 12:13:40 -0700
Subject: genetlink: use iterator in the op to policy map dumping

We can't put the full iterator in the struct ctrl_dump_policy_ctx
because dump context is statically sized by netlink core.
Allocate it dynamically.

Rename policy to dump_map to make the logic a little easier to follow.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
---
 net/netlink/genetlink.c | 49 ++++++++++++++++++++++++++-----------------------
 1 file changed, 26 insertions(+), 23 deletions(-)

(limited to 'net/netlink')

diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c
index 06bf091c1b8a..4b8c65d9e9d3 100644
--- a/net/netlink/genetlink.c
+++ b/net/netlink/genetlink.c
@@ -1252,10 +1252,10 @@ static int genl_ctrl_event(int event, const struct genl_family *family,
 struct ctrl_dump_policy_ctx {
 	struct netlink_policy_dump_state *state;
 	const struct genl_family *rt;
-	unsigned int opidx;
+	struct genl_op_iter *op_iter;
 	u32 op;
 	u16 fam_id;
-	u8 policies:1,
+	u8 dump_map:1,
 	   single_op:1;
 };
 
@@ -1325,9 +1325,16 @@ static int ctrl_dumppolicy_start(struct netlink_callback *cb)
 
 		if (!ctx->state)
 			return -ENODATA;
+
+		ctx->dump_map = 1;
 		return 0;
 	}
 
+	ctx->op_iter = kmalloc(sizeof(*ctx->op_iter), GFP_KERNEL);
+	if (!ctx->op_iter)
+		return -ENOMEM;
+	ctx->dump_map = genl_op_iter_init(rt, ctx->op_iter);
+
 	for (genl_op_iter_init(rt, &i); genl_op_iter_next(&i); ) {
 		if (i.doit.policy) {
 			err = netlink_policy_dump_add_policy(&ctx->state,
@@ -1345,12 +1352,16 @@ static int ctrl_dumppolicy_start(struct netlink_callback *cb)
 		}
 	}
 
-	if (!ctx->state)
-		return -ENODATA;
+	if (!ctx->state) {
+		err = -ENODATA;
+		goto err_free_op_iter;
+	}
 	return 0;
 
 err_free_state:
 	netlink_policy_dump_free(ctx->state);
+err_free_op_iter:
+	kfree(ctx->op_iter);
 	return err;
 }
 
@@ -1430,11 +1441,10 @@ static int ctrl_dumppolicy(struct sk_buff *skb, struct netlink_callback *cb)
 	struct ctrl_dump_policy_ctx *ctx = (void *)cb->ctx;
 	void *hdr;
 
-	if (!ctx->policies) {
-		struct genl_split_ops doit, dumpit;
-		struct genl_ops op;
-
+	if (ctx->dump_map) {
 		if (ctx->single_op) {
+			struct genl_split_ops doit, dumpit;
+
 			if (genl_get_cmd(ctx->op, GENL_CMD_CAP_DO,
 					 ctx->rt, &doit) &&
 			    genl_get_cmd(ctx->op, GENL_CMD_CAP_DUMP,
@@ -1446,26 +1456,18 @@ static int ctrl_dumppolicy(struct sk_buff *skb, struct netlink_callback *cb)
 			if (ctrl_dumppolicy_put_op(skb, cb, &doit, &dumpit))
 				return skb->len;
 
-			/* don't enter the loop below */
-			ctx->opidx = genl_get_cmd_cnt(ctx->rt);
+			/* done with the per-op policy index list */
+			ctx->dump_map = 0;
 		}
 
-		while (ctx->opidx < genl_get_cmd_cnt(ctx->rt)) {
-			genl_get_cmd_by_index(ctx->opidx, ctx->rt, &op);
-
-			genl_cmd_full_to_split(&doit, ctx->rt,
-					       &op, GENL_CMD_CAP_DO);
-			genl_cmd_full_to_split(&dumpit, ctx->rt,
-					       &op, GENL_CMD_CAP_DUMP);
-
-			if (ctrl_dumppolicy_put_op(skb, cb, &doit, &dumpit))
+		while (ctx->dump_map) {
+			if (ctrl_dumppolicy_put_op(skb, cb,
+						   &ctx->op_iter->doit,
+						   &ctx->op_iter->dumpit))
 				return skb->len;
 
-			ctx->opidx++;
+			ctx->dump_map = genl_op_iter_next(ctx->op_iter);
 		}
-
-		/* completed with the per-op policy index list */
-		ctx->policies = true;
 	}
 
 	while (netlink_policy_dump_loop(ctx->state)) {
@@ -1498,6 +1500,7 @@ static int ctrl_dumppolicy_done(struct netlink_callback *cb)
 {
 	struct ctrl_dump_policy_ctx *ctx = (void *)cb->ctx;
 
+	kfree(ctx->op_iter);
 	netlink_policy_dump_free(ctx->state);
 	return 0;
 }
-- 
cgit v1.2.3


From 7acfbbe17c18554960b812a95c26d7ef9b7b28ac Mon Sep 17 00:00:00 2001
From: Jakub Kicinski <kuba@kernel.org>
Date: Fri, 4 Nov 2022 12:13:41 -0700
Subject: genetlink: inline old iteration helpers

All dumpers use the iterators now, inline the cmd by index
stuff into iterator code.

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
---
 net/netlink/genetlink.c | 32 +++++++++-----------------------
 1 file changed, 9 insertions(+), 23 deletions(-)

(limited to 'net/netlink')

diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c
index 4b8c65d9e9d3..0a4f1470f442 100644
--- a/net/netlink/genetlink.c
+++ b/net/netlink/genetlink.c
@@ -118,11 +118,6 @@ static const struct genl_family *genl_family_find_byname(char *name)
 	return NULL;
 }
 
-static int genl_get_cmd_cnt(const struct genl_family *family)
-{
-	return family->n_ops + family->n_small_ops;
-}
-
 static void genl_op_from_full(const struct genl_family *family,
 			      unsigned int i, struct genl_ops *op)
 {
@@ -240,18 +235,6 @@ genl_get_cmd(u32 cmd, u8 flags, const struct genl_family *family,
 	return genl_cmd_full_to_split(op, family, &full, flags);
 }
 
-static void genl_get_cmd_by_index(unsigned int i,
-				  const struct genl_family *family,
-				  struct genl_ops *op)
-{
-	if (i < family->n_ops)
-		genl_op_from_full(family, i, op);
-	else if (i < family->n_ops + family->n_small_ops)
-		genl_op_from_small(family, i - family->n_ops, op);
-	else
-		WARN_ON_ONCE(1);
-}
-
 struct genl_op_iter {
 	const struct genl_family *family;
 	struct genl_split_ops doit;
@@ -269,22 +252,25 @@ genl_op_iter_init(const struct genl_family *family, struct genl_op_iter *iter)
 
 	iter->flags = 0;
 
-	return genl_get_cmd_cnt(iter->family);
+	return iter->family->n_ops + iter->family->n_small_ops;
 }
 
 static bool genl_op_iter_next(struct genl_op_iter *iter)
 {
+	const struct genl_family *family = iter->family;
 	struct genl_ops op;
 
-	if (iter->i >= genl_get_cmd_cnt(iter->family))
+	if (iter->i < family->n_ops)
+		genl_op_from_full(family, iter->i, &op);
+	else if (iter->i < family->n_ops + family->n_small_ops)
+		genl_op_from_small(family, iter->i - family->n_ops, &op);
+	else
 		return false;
 
-	genl_get_cmd_by_index(iter->i, iter->family, &op);
 	iter->i++;
 
-	genl_cmd_full_to_split(&iter->doit, iter->family, &op, GENL_CMD_CAP_DO);
-	genl_cmd_full_to_split(&iter->dumpit, iter->family,
-			       &op, GENL_CMD_CAP_DUMP);
+	genl_cmd_full_to_split(&iter->doit, family, &op, GENL_CMD_CAP_DO);
+	genl_cmd_full_to_split(&iter->dumpit, family, &op, GENL_CMD_CAP_DUMP);
 
 	iter->cmd = iter->doit.cmd | iter->dumpit.cmd;
 	iter->flags = iter->doit.flags | iter->dumpit.flags;
-- 
cgit v1.2.3


From b8fd60c36a44351f773432e24efd8bb92f8ba0c6 Mon Sep 17 00:00:00 2001
From: Jakub Kicinski <kuba@kernel.org>
Date: Fri, 4 Nov 2022 12:13:42 -0700
Subject: genetlink: allow families to use split ops directly

Let families to hook in the new split ops.

They are more flexible and should not be much larger than
full ops. Each split op is 40B while full op is 48B.
Devlink for example has 54 dos and 19 dumps, 2 of the dumps
do not have a do -> 56 full commands = 2688B.
Split ops would have taken 2920B, so 9% more space while
allowing individual per/post doit and per-type policies.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
---
 include/net/genetlink.h |   5 ++
 net/netlink/genetlink.c | 170 ++++++++++++++++++++++++++++++++++++++++--------
 2 files changed, 149 insertions(+), 26 deletions(-)

(limited to 'net/netlink')

diff --git a/include/net/genetlink.h b/include/net/genetlink.h
index 4be7989c451b..d21210709f84 100644
--- a/include/net/genetlink.h
+++ b/include/net/genetlink.h
@@ -46,6 +46,9 @@ struct genl_info;
  * @n_ops: number of operations supported by this family
  * @small_ops: the small-struct operations supported by this family
  * @n_small_ops: number of small-struct operations supported by this family
+ * @split_ops: the split do/dump form of operation definition
+ * @n_split_ops: number of entries in @split_ops, not that with split do/dump
+ *	ops the number of entries is not the same as number of commands
  *
  * Attribute policies (the combination of @policy and @maxattr fields)
  * can be attached at the family level or at the operation level.
@@ -63,6 +66,7 @@ struct genl_family {
 	u8			parallel_ops:1;
 	u8			n_ops;
 	u8			n_small_ops;
+	u8			n_split_ops;
 	u8			n_mcgrps;
 	u8			resv_start_op;
 	const struct nla_policy *policy;
@@ -74,6 +78,7 @@ struct genl_family {
 					     struct genl_info *info);
 	const struct genl_ops *	ops;
 	const struct genl_small_ops *small_ops;
+	const struct genl_split_ops *split_ops;
 	const struct genl_multicast_group *mcgrps;
 	struct module		*module;
 
diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c
index 0a4f1470f442..90b0feb5eb73 100644
--- a/net/netlink/genetlink.c
+++ b/net/netlink/genetlink.c
@@ -101,6 +101,17 @@ genl_op_fill_in_reject_policy(const struct genl_family *family,
 	op->maxattr = 1;
 }
 
+static void
+genl_op_fill_in_reject_policy_split(const struct genl_family *family,
+				    struct genl_split_ops *op)
+{
+	if (op->policy)
+		return;
+
+	op->policy = genl_policy_reject_all;
+	op->maxattr = 1;
+}
+
 static const struct genl_family *genl_family_find_byid(unsigned int id)
 {
 	return idr_find(&genl_fam_idr, id);
@@ -118,6 +129,16 @@ static const struct genl_family *genl_family_find_byname(char *name)
 	return NULL;
 }
 
+struct genl_op_iter {
+	const struct genl_family *family;
+	struct genl_split_ops doit;
+	struct genl_split_ops dumpit;
+	int cmd_idx;
+	int entry_idx;
+	u32 cmd;
+	u8 flags;
+};
+
 static void genl_op_from_full(const struct genl_family *family,
 			      unsigned int i, struct genl_ops *op)
 {
@@ -176,6 +197,50 @@ static int genl_get_cmd_small(u32 cmd, const struct genl_family *family,
 	return -ENOENT;
 }
 
+static void genl_op_from_split(struct genl_op_iter *iter)
+{
+	const struct genl_family *family = iter->family;
+	int i, cnt = 0;
+
+	i = iter->entry_idx - family->n_ops - family->n_small_ops;
+
+	if (family->split_ops[i + cnt].flags & GENL_CMD_CAP_DO) {
+		iter->doit = family->split_ops[i + cnt];
+		genl_op_fill_in_reject_policy_split(family, &iter->doit);
+		cnt++;
+	} else {
+		memset(&iter->doit, 0, sizeof(iter->doit));
+	}
+
+	if (i + cnt < family->n_split_ops &&
+	    family->split_ops[i + cnt].flags & GENL_CMD_CAP_DUMP) {
+		iter->dumpit = family->split_ops[i + cnt];
+		genl_op_fill_in_reject_policy_split(family, &iter->dumpit);
+		cnt++;
+	} else {
+		memset(&iter->dumpit, 0, sizeof(iter->dumpit));
+	}
+
+	WARN_ON(!cnt);
+	iter->entry_idx += cnt;
+}
+
+static int
+genl_get_cmd_split(u32 cmd, u8 flag, const struct genl_family *family,
+		   struct genl_split_ops *op)
+{
+	int i;
+
+	for (i = 0; i < family->n_split_ops; i++)
+		if (family->split_ops[i].cmd == cmd &&
+		    family->split_ops[i].flags & flag) {
+			*op = family->split_ops[i];
+			return 0;
+		}
+
+	return -ENOENT;
+}
+
 static int
 genl_cmd_full_to_split(struct genl_split_ops *op,
 		       const struct genl_family *family,
@@ -227,50 +292,60 @@ genl_get_cmd(u32 cmd, u8 flags, const struct genl_family *family,
 	err = genl_get_cmd_full(cmd, family, &full);
 	if (err == -ENOENT)
 		err = genl_get_cmd_small(cmd, family, &full);
-	if (err) {
-		memset(op, 0, sizeof(*op));
-		return err;
-	}
+	/* Found one of legacy forms */
+	if (err == 0)
+		return genl_cmd_full_to_split(op, family, &full, flags);
 
-	return genl_cmd_full_to_split(op, family, &full, flags);
+	err = genl_get_cmd_split(cmd, flags, family, op);
+	if (err)
+		memset(op, 0, sizeof(*op));
+	return err;
 }
 
-struct genl_op_iter {
-	const struct genl_family *family;
-	struct genl_split_ops doit;
-	struct genl_split_ops dumpit;
-	int i;
-	u32 cmd;
-	u8 flags;
-};
-
 static bool
 genl_op_iter_init(const struct genl_family *family, struct genl_op_iter *iter)
 {
 	iter->family = family;
-	iter->i = 0;
+	iter->cmd_idx = 0;
+	iter->entry_idx = 0;
 
 	iter->flags = 0;
 
-	return iter->family->n_ops + iter->family->n_small_ops;
+	return iter->family->n_ops +
+		iter->family->n_small_ops +
+		iter->family->n_split_ops;
 }
 
 static bool genl_op_iter_next(struct genl_op_iter *iter)
 {
 	const struct genl_family *family = iter->family;
+	bool legacy_op = true;
 	struct genl_ops op;
 
-	if (iter->i < family->n_ops)
-		genl_op_from_full(family, iter->i, &op);
-	else if (iter->i < family->n_ops + family->n_small_ops)
-		genl_op_from_small(family, iter->i - family->n_ops, &op);
-	else
+	if (iter->entry_idx < family->n_ops) {
+		genl_op_from_full(family, iter->entry_idx, &op);
+	} else if (iter->entry_idx < family->n_ops + family->n_small_ops) {
+		genl_op_from_small(family, iter->entry_idx - family->n_ops,
+				   &op);
+	} else if (iter->entry_idx <
+		   family->n_ops + family->n_small_ops + family->n_split_ops) {
+		legacy_op = false;
+		/* updates entry_idx */
+		genl_op_from_split(iter);
+	} else {
 		return false;
+	}
 
-	iter->i++;
+	iter->cmd_idx++;
 
-	genl_cmd_full_to_split(&iter->doit, family, &op, GENL_CMD_CAP_DO);
-	genl_cmd_full_to_split(&iter->dumpit, family, &op, GENL_CMD_CAP_DUMP);
+	if (legacy_op) {
+		iter->entry_idx++;
+
+		genl_cmd_full_to_split(&iter->doit, family,
+				       &op, GENL_CMD_CAP_DO);
+		genl_cmd_full_to_split(&iter->dumpit, family,
+				       &op, GENL_CMD_CAP_DUMP);
+	}
 
 	iter->cmd = iter->doit.cmd | iter->dumpit.cmd;
 	iter->flags = iter->doit.flags | iter->dumpit.flags;
@@ -286,7 +361,7 @@ genl_op_iter_copy(struct genl_op_iter *dst, struct genl_op_iter *src)
 
 static unsigned int genl_op_iter_idx(struct genl_op_iter *iter)
 {
-	return iter->i;
+	return iter->cmd_idx;
 }
 
 static int genl_allocate_reserve_groups(int n_groups, int *first_id)
@@ -454,12 +529,22 @@ static void genl_unregister_mc_groups(const struct genl_family *family)
 	}
 }
 
+static bool genl_split_op_check(const struct genl_split_ops *op)
+{
+	if (WARN_ON(hweight8(op->flags & (GENL_CMD_CAP_DO |
+					  GENL_CMD_CAP_DUMP)) != 1))
+		return true;
+	return false;
+}
+
 static int genl_validate_ops(const struct genl_family *family)
 {
 	struct genl_op_iter i, j;
+	unsigned int s;
 
 	if (WARN_ON(family->n_ops && !family->ops) ||
-	    WARN_ON(family->n_small_ops && !family->small_ops))
+	    WARN_ON(family->n_small_ops && !family->small_ops) ||
+	    WARN_ON(family->n_split_ops && !family->split_ops))
 		return -EINVAL;
 
 	for (genl_op_iter_init(family, &i); genl_op_iter_next(&i); ) {
@@ -477,6 +562,39 @@ static int genl_validate_ops(const struct genl_family *family)
 		}
 	}
 
+	if (family->n_split_ops) {
+		if (genl_split_op_check(&family->split_ops[0]))
+			return -EINVAL;
+	}
+
+	for (s = 1; s < family->n_split_ops; s++) {
+		const struct genl_split_ops *a, *b;
+
+		a = &family->split_ops[s - 1];
+		b = &family->split_ops[s];
+
+		if (genl_split_op_check(b))
+			return -EINVAL;
+
+		/* Check sort order */
+		if (a->cmd < b->cmd)
+			continue;
+
+		if (a->internal_flags != b->internal_flags ||
+		    ((a->flags ^ b->flags) & ~(GENL_CMD_CAP_DO |
+					       GENL_CMD_CAP_DUMP))) {
+			WARN_ON(1);
+			return -EINVAL;
+		}
+
+		if ((a->flags & GENL_CMD_CAP_DO) &&
+		    (b->flags & GENL_CMD_CAP_DUMP))
+			continue;
+
+		WARN_ON(1);
+		return -EINVAL;
+	}
+
 	return 0;
 }
 
-- 
cgit v1.2.3


From aba22ca8ccd6954ffbb1a7a671f34300887b4727 Mon Sep 17 00:00:00 2001
From: Jakub Kicinski <kuba@kernel.org>
Date: Fri, 4 Nov 2022 12:13:43 -0700
Subject: genetlink: convert control family to split ops

Prove that the split ops work.
Sadly we need to keep bug-wards compatibility and specify
the same policy for dump as do, even tho we don't parse
inputs for the dump.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
---
 net/netlink/genetlink.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

(limited to 'net/netlink')

diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c
index 90b0feb5eb73..362a61179036 100644
--- a/net/netlink/genetlink.c
+++ b/net/netlink/genetlink.c
@@ -1609,14 +1609,22 @@ static int ctrl_dumppolicy_done(struct netlink_callback *cb)
 	return 0;
 }
 
-static const struct genl_ops genl_ctrl_ops[] = {
+static const struct genl_split_ops genl_ctrl_ops[] = {
 	{
 		.cmd		= CTRL_CMD_GETFAMILY,
-		.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
+		.validate	= GENL_DONT_VALIDATE_STRICT,
 		.policy		= ctrl_policy_family,
 		.maxattr	= ARRAY_SIZE(ctrl_policy_family) - 1,
 		.doit		= ctrl_getfamily,
+		.flags		= GENL_CMD_CAP_DO,
+	},
+	{
+		.cmd		= CTRL_CMD_GETFAMILY,
+		.validate	= GENL_DONT_VALIDATE_DUMP,
+		.policy		= ctrl_policy_family,
+		.maxattr	= ARRAY_SIZE(ctrl_policy_family) - 1,
 		.dumpit		= ctrl_dumpfamily,
+		.flags		= GENL_CMD_CAP_DUMP,
 	},
 	{
 		.cmd		= CTRL_CMD_GETPOLICY,
@@ -1625,6 +1633,7 @@ static const struct genl_ops genl_ctrl_ops[] = {
 		.start		= ctrl_dumppolicy_start,
 		.dumpit		= ctrl_dumppolicy,
 		.done		= ctrl_dumppolicy_done,
+		.flags		= GENL_CMD_CAP_DUMP,
 	},
 };
 
@@ -1634,8 +1643,8 @@ static const struct genl_multicast_group genl_ctrl_groups[] = {
 
 static struct genl_family genl_ctrl __ro_after_init = {
 	.module = THIS_MODULE,
-	.ops = genl_ctrl_ops,
-	.n_ops = ARRAY_SIZE(genl_ctrl_ops),
+	.split_ops = genl_ctrl_ops,
+	.n_split_ops = ARRAY_SIZE(genl_ctrl_ops),
 	.resv_start_op = CTRL_CMD_GETPOLICY + 1,
 	.mcgrps = genl_ctrl_groups,
 	.n_mcgrps = ARRAY_SIZE(genl_ctrl_groups),
-- 
cgit v1.2.3


From e69761483361f3df455bc493c99af0ef1744a14f Mon Sep 17 00:00:00 2001
From: Tao Chen <chentao.kernel@linux.alibaba.com>
Date: Sat, 5 Nov 2022 17:05:04 +0800
Subject: netlink: Fix potential skb memleak in netlink_ack

Fix coverity issue 'Resource leak'.

We should clean the skb resource if nlmsg_put/append failed.

Fixes: 738136a0e375 ("netlink: split up copies in the ack construction")
Signed-off-by: Tao Chen <chentao.kernel@linux.alibaba.com>
Link: https://lore.kernel.org/r/bff442d62c87de6299817fe1897cc5a5694ba9cc.1667638204.git.chentao.kernel@linux.alibaba.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 net/netlink/af_netlink.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

(limited to 'net/netlink')

diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index b10d5e50b99d..9ebdf3262015 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -2500,7 +2500,7 @@ void netlink_ack(struct sk_buff *in_skb, struct nlmsghdr *nlh, int err,
 
 	skb = nlmsg_new(payload + tlvlen, GFP_KERNEL);
 	if (!skb)
-		goto err_bad_put;
+		goto err_skb;
 
 	rep = nlmsg_put(skb, NETLINK_CB(in_skb).portid, nlh->nlmsg_seq,
 			NLMSG_ERROR, sizeof(*errmsg), flags);
@@ -2528,6 +2528,8 @@ void netlink_ack(struct sk_buff *in_skb, struct nlmsghdr *nlh, int err,
 	return;
 
 err_bad_put:
+	nlmsg_free(skb);
+err_skb:
 	NETLINK_CB(in_skb).sk->sk_err = ENOBUFS;
 	sk_error_report(NETLINK_CB(in_skb).sk);
 }
-- 
cgit v1.2.3


From 154ba79c9f160e652a2c9c46435b928b3bfae11f Mon Sep 17 00:00:00 2001
From: Jakub Kicinski <kuba@kernel.org>
Date: Tue, 8 Nov 2022 12:41:28 -0800
Subject: genetlink: correctly begin the iteration over policies

The return value from genl_op_iter_init() only tells us if
there are any policies but to begin the iteration (and therefore
load the first entry) we need to call genl_op_iter_next().
Note that it's safe to call genl_op_iter_next() on a family
with no ops, it will just return false.

This may lead to various crashes, a warning in
netlink_policy_dump_get_policy_idx() when policy is not found
or.. no problem at all if the kmalloc'ed memory happens to be
zeroed.

Fixes: b502b3185cd6 ("genetlink: use iterator in the op to policy map dumping")
Link: https://lore.kernel.org/r/20221108204128.330287-1-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 net/netlink/genetlink.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

(limited to 'net/netlink')

diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c
index 362a61179036..9b7dfc45dd67 100644
--- a/net/netlink/genetlink.c
+++ b/net/netlink/genetlink.c
@@ -1437,7 +1437,9 @@ static int ctrl_dumppolicy_start(struct netlink_callback *cb)
 	ctx->op_iter = kmalloc(sizeof(*ctx->op_iter), GFP_KERNEL);
 	if (!ctx->op_iter)
 		return -ENOMEM;
-	ctx->dump_map = genl_op_iter_init(rt, ctx->op_iter);
+
+	genl_op_iter_init(rt, ctx->op_iter);
+	ctx->dump_map = genl_op_iter_next(ctx->op_iter);
 
 	for (genl_op_iter_init(rt, &i); genl_op_iter_next(&i); ) {
 		if (i.doit.policy) {
-- 
cgit v1.2.3


From c1b05105573b2cd5845921eb0d2caa26e2144a34 Mon Sep 17 00:00:00 2001
From: Jakub Kicinski <kuba@kernel.org>
Date: Wed, 9 Nov 2022 10:32:54 -0800
Subject: genetlink: fix single op policy dump when do is present

Jonathan reports crashes when running net-next in Meta's fleet.
Stats collection uses ethtool -I which does a per-op policy dump
to check if stats are supported. We don't initialize the dumpit
information if doit succeeds due to evaluation short-circuiting.

The crash may look like this:

   BUG: kernel NULL pointer dereference, address: 0000000000000cc0
   RIP: 0010:netlink_policy_dump_add_policy+0x174/0x2a0
     ctrl_dumppolicy_start+0x19f/0x2f0
     genl_start+0xe7/0x140

Or we may trigger a warning:

   WARNING: CPU: 1 PID: 785 at net/netlink/policy.c:87 netlink_policy_dump_get_policy_idx+0x79/0x80
   RIP: 0010:netlink_policy_dump_get_policy_idx+0x79/0x80
     ctrl_dumppolicy_put_op+0x214/0x360

depending on what garbage we pick up from the stack.

Reported-by: Jonathan Lemon <bsd@meta.com>
Fixes: 26588edbef60 ("genetlink: support split policies in ctrl_dumppolicy_put_op()")
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Tested-by: Leon Romanovsky <leonro@nvidia.com>
Link: https://lore.kernel.org/r/20221109183254.554051-1-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 net/netlink/genetlink.c | 30 +++++++++++++++++++++---------
 1 file changed, 21 insertions(+), 9 deletions(-)

(limited to 'net/netlink')

diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c
index 9b7dfc45dd67..600993c80050 100644
--- a/net/netlink/genetlink.c
+++ b/net/netlink/genetlink.c
@@ -282,6 +282,7 @@ genl_cmd_full_to_split(struct genl_split_ops *op,
 	return 0;
 }
 
+/* Must make sure that op is initialized to 0 on failure */
 static int
 genl_get_cmd(u32 cmd, u8 flags, const struct genl_family *family,
 	     struct genl_split_ops *op)
@@ -302,6 +303,21 @@ genl_get_cmd(u32 cmd, u8 flags, const struct genl_family *family,
 	return err;
 }
 
+/* For policy dumping only, get ops of both do and dump.
+ * Fail if both are missing, genl_get_cmd() will zero-init in case of failure.
+ */
+static int
+genl_get_cmd_both(u32 cmd, const struct genl_family *family,
+		  struct genl_split_ops *doit, struct genl_split_ops *dumpit)
+{
+	int err1, err2;
+
+	err1 = genl_get_cmd(cmd, GENL_CMD_CAP_DO, family, doit);
+	err2 = genl_get_cmd(cmd, GENL_CMD_CAP_DUMP, family, dumpit);
+
+	return err1 && err2 ? -ENOENT : 0;
+}
+
 static bool
 genl_op_iter_init(const struct genl_family *family, struct genl_op_iter *iter)
 {
@@ -1406,10 +1422,10 @@ static int ctrl_dumppolicy_start(struct netlink_callback *cb)
 		ctx->single_op = true;
 		ctx->op = nla_get_u32(tb[CTRL_ATTR_OP]);
 
-		if (genl_get_cmd(ctx->op, GENL_CMD_CAP_DO, rt, &doit) &&
-		    genl_get_cmd(ctx->op, GENL_CMD_CAP_DUMP, rt, &dump)) {
+		err = genl_get_cmd_both(ctx->op, rt, &doit, &dump);
+		if (err) {
 			NL_SET_BAD_ATTR(cb->extack, tb[CTRL_ATTR_OP]);
-			return -ENOENT;
+			return err;
 		}
 
 		if (doit.policy) {
@@ -1551,13 +1567,9 @@ static int ctrl_dumppolicy(struct sk_buff *skb, struct netlink_callback *cb)
 		if (ctx->single_op) {
 			struct genl_split_ops doit, dumpit;
 
-			if (genl_get_cmd(ctx->op, GENL_CMD_CAP_DO,
-					 ctx->rt, &doit) &&
-			    genl_get_cmd(ctx->op, GENL_CMD_CAP_DUMP,
-					 ctx->rt, &dumpit)) {
-				WARN_ON(1);
+			if (WARN_ON(genl_get_cmd_both(ctx->op, ctx->rt,
+						      &doit, &dumpit)))
 				return -ENOENT;
-			}
 
 			if (ctrl_dumppolicy_put_op(skb, cb, &doit, &dumpit))
 				return skb->len;
-- 
cgit v1.2.3


From c73a72f4cbb47672c8cc7f7d7aba52f1cb15baca Mon Sep 17 00:00:00 2001
From: Jakub Kicinski <kuba@kernel.org>
Date: Thu, 17 Nov 2022 19:39:03 -0800
Subject: netlink: remove the flex array from struct nlmsghdr

I've added a flex array to struct nlmsghdr in
commit 738136a0e375 ("netlink: split up copies in the ack construction")
to allow accessing the data easily. It leads to warnings with clang,
if user space wraps this structure into another struct and the flex
array is not at the end of the container.

Reviewed-by: Kees Cook <keescook@chromium.org>
Reviewed-by: David Ahern <dsahern@kernel.org>
Link: https://lore.kernel.org/all/20221114023927.GA685@u2004-local/
Link: https://lore.kernel.org/r/20221118033903.1651026-1-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 include/uapi/linux/netlink.h | 2 --
 net/netlink/af_netlink.c     | 2 +-
 2 files changed, 1 insertion(+), 3 deletions(-)

(limited to 'net/netlink')

diff --git a/include/uapi/linux/netlink.h b/include/uapi/linux/netlink.h
index 5da0da59bf01..e2ae82e3f9f7 100644
--- a/include/uapi/linux/netlink.h
+++ b/include/uapi/linux/netlink.h
@@ -48,7 +48,6 @@ struct sockaddr_nl {
  * @nlmsg_flags: Additional flags
  * @nlmsg_seq:   Sequence number
  * @nlmsg_pid:   Sending process port ID
- * @nlmsg_data:  Message payload
  */
 struct nlmsghdr {
 	__u32		nlmsg_len;
@@ -56,7 +55,6 @@ struct nlmsghdr {
 	__u16		nlmsg_flags;
 	__u32		nlmsg_seq;
 	__u32		nlmsg_pid;
-	__u8		nlmsg_data[];
 };
 
 /* Flags values */
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 9ebdf3262015..d73091f6bb0f 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -2514,7 +2514,7 @@ void netlink_ack(struct sk_buff *in_skb, struct nlmsghdr *nlh, int err,
 		if (!nlmsg_append(skb, nlmsg_len(nlh)))
 			goto err_bad_put;
 
-		memcpy(errmsg->msg.nlmsg_data, nlh->nlmsg_data,
+		memcpy(nlmsg_data(&errmsg->msg), nlmsg_data(nlh),
 		       nlmsg_len(nlh));
 	}
 
-- 
cgit v1.2.3