netdev - Re: [PATCH iproute2] genl: print caps for all families

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: <20230223175708.51e593f0@kernel.org>
Date:   Thu, 23 Feb 2023 17:57:08 -0800
From:   Jakub Kicinski <kuba@...nel.org>
To:     stephen@...workplumber.org
Cc:     dsahern@...il.com, jhs@...atatu.com, netdev@...r.kernel.org,
        Johannes Berg <johannes@...solutions.net>
Subject: Re: [PATCH iproute2] genl: print caps for all families

On Thu, 23 Feb 2023 17:52:34 -0800 Jakub Kicinski wrote:
> Back in 2006 kernel commit 334c29a64507 ("[GENETLINK]: Move
> command capabilities to flags.") removed some attributes and
> moved the capabilities to flags. Corresponding iproute2
> commit 26328fc3933f ("Add controller support for new features
> exposed") added the ability to print those caps.
> 
> Printing is gated on version of the family, but we're checking
> the version of each individual family rather than the control
> family. The format of attributes in the control family
> is dictated by the version of the control family alone.
> 
> Families can't use flags for random things, anyway,
> because kernel core has a fixed interpretation.
> 
> Thanks to this change caps will be shown for all families
> (assuming kernel newer than 2.6.19), not just those which
> by coincidence have their local version >= 2.
> 
> For instance devlink, before:
> 
>   $ genl ctrl get name devlink
>   Name: devlink
> 	ID: 0x15  Version: 0x1  header size: 0  max attribs: 179
> 	commands supported:
> 		#1:  ID-0x1
> 		#2:  ID-0x5
> 		#3:  ID-0x6
> 		...
> 
> after:
> 
>   $ genl ctrl get name devlink
>   Name: devlink
> 	ID: 0x15  Version: 0x1  header size: 0  max attribs: 179
> 	commands supported:
> 		#1:  ID-0x1
> 		Capabilities (0xe):
>  		  can doit; can dumpit; has policy
> 
> 		#2:  ID-0x5
> 		Capabilities (0xe):
>  		  can doit; can dumpit; has policy
> 
> 		#3:  ID-0x6
> 		Capabilities (0xb):
>  		  requires admin permission; can doit; has policy
> 
> Leave ctrl_v as 0 if we fail to read the version. Old code used 1
> as the default, but 0 or 1 - does not matter, checks are for >= 2.
> 
> Signed-off-by: Jakub Kicinski <kuba@...nel.org>
> ---
> Not really sure if this is a fix or not..

Adding Johannes, that's probably everyone who ever used this 
command on CC? ;)

> diff --git a/genl/ctrl.c b/genl/ctrl.c
> index a2d87af0ad07..1fcb3848f137 100644
> --- a/genl/ctrl.c
> +++ b/genl/ctrl.c
> @@ -21,6 +21,8 @@
>  #define GENL_MAX_FAM_OPS	256
>  #define GENL_MAX_FAM_GRPS	256
>  
> +static unsigned int ctrl_v;
> +
>  static int usage(void)
>  {
>  	fprintf(stderr,"Usage: ctrl <CMD>\n" \
> @@ -109,7 +111,6 @@ static int print_ctrl(struct rtnl_ctrl_data *ctrl,
>  	int len = n->nlmsg_len;
>  	struct rtattr *attrs;
>  	FILE *fp = (FILE *) arg;
> -	__u32 ctrl_v = 0x1;
>  
>  	if (n->nlmsg_type !=  GENL_ID_CTRL) {
>  		fprintf(stderr, "Not a controller message, nlmsg_len=%d "
> @@ -148,7 +149,6 @@ static int print_ctrl(struct rtnl_ctrl_data *ctrl,
>  	if (tb[CTRL_ATTR_VERSION]) {
>  		__u32 *v = RTA_DATA(tb[CTRL_ATTR_VERSION]);
>  		fprintf(fp, " Version: 0x%x ",*v);
> -		ctrl_v = *v;
>  	}
>  	if (tb[CTRL_ATTR_HDRSIZE]) {
>  		__u32 *h = RTA_DATA(tb[CTRL_ATTR_HDRSIZE]);
> @@ -241,6 +241,55 @@ static int print_ctrl2(struct nlmsghdr *n, void *arg)
>  	return print_ctrl(NULL, n, arg);
>  }
>  
> +static void ctrl_load_ctrl_version(struct rtnl_handle *rth)
> +{
> +	struct rtattr *tb[CTRL_ATTR_MAX + 1];
> +	struct genlmsghdr *ghdr;
> +	struct rtattr *attrs;
> +	struct {
> +		struct nlmsghdr         n;
> +		struct genlmsghdr	g;
> +		char                    buf[128];
> +	} req = {
> +		.n.nlmsg_len = NLMSG_LENGTH(GENL_HDRLEN),
> +		.n.nlmsg_flags = NLM_F_REQUEST,
> +		.n.nlmsg_type = GENL_ID_CTRL,
> +		.g.cmd = CTRL_CMD_GETFAMILY,
> +	};
> +	struct nlmsghdr *nlh = &req.n;
> +	struct nlmsghdr *answer = NULL;
> +	__u16 id = GENL_ID_CTRL;
> +	int len;
> +
> +	addattr_l(nlh, 128, CTRL_ATTR_FAMILY_ID, &id, 2);
> +
> +	if (rtnl_talk(rth, nlh, &answer) < 0) {
> +		fprintf(stderr, "Error talking to the kernel\n");
> +		return;
> +	}
> +
> +	ghdr = NLMSG_DATA(answer);
> +	if (answer->nlmsg_type != GENL_ID_CTRL ||
> +	    ghdr->cmd != CTRL_CMD_NEWFAMILY) {
> +		fprintf(stderr, "Unexpected reply nlmsg_type=%u cmd=%u\n",
> +			answer->nlmsg_type, ghdr->cmd);
> +		return;
> +	}
> +
> +	len = answer->nlmsg_len;
> +	len -= NLMSG_LENGTH(GENL_HDRLEN);
> +	if (len < 0) {
> +		fprintf(stderr, "wrong controller message len %d\n", len);
> +		return;
> +	}
> +
> +	attrs = (struct rtattr *) ((char *) ghdr + GENL_HDRLEN);
> +	parse_rtattr_flags(tb, CTRL_ATTR_MAX, attrs, len, NLA_F_NESTED);
> +
> +	if (tb[CTRL_ATTR_VERSION])
> +		ctrl_v = rta_getattr_u16(tb[CTRL_ATTR_VERSION]);
> +}
> +
>  static int ctrl_list(int cmd, int argc, char **argv)
>  {
>  	struct rtnl_handle rth;
> @@ -264,6 +313,9 @@ static int ctrl_list(int cmd, int argc, char **argv)
>  		exit(1);
>  	}
>  
> +	if (!ctrl_v)
> +		ctrl_load_ctrl_version(&rth);
> +
>  	if (cmd == CTRL_CMD_GETFAMILY || cmd == CTRL_CMD_GETPOLICY) {
>  		req.g.cmd = cmd;
>