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: <7cbc3044.688a7.18cd32c5f14.Coremail.linma@zju.edu.cn>
Date: Thu, 4 Jan 2024 14:32:29 +0800 (GMT+08:00)
From: "Lin Ma" <linma@....edu.cn>
To: "Jakub Kicinski" <kuba@...nel.org>
Cc: jk@...econstruct.com.au, matt@...econstruct.com.au, davem@...emloft.net, 
	edumazet@...gle.com, pabeni@...hat.com, netdev@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next v1] net: mctp: use deprecated parser in
 mctp_set_link_af

Hello Jakub,

> > 
> > For other set_link_af users IPV4 and IPV6 both make a trick here by
> > using nla_parse_nested_deprecated, which will check the NLA_F_NESTED
> > then able to use this type field as family value. This patch simply port
> > the MCTP code also to deprecated parser to make it work.
> 
> Did you test this? It's a suspiciously detailed and yet seemingly
> incorrect explanation..

Yes, I hacked the iproute tool and add below code

https://github.com/iproute2/iproute2/blob/main/ip/iplink.c#L801

853a854,861
> 		} else if (matches(*argv, "mctptest") == 0) {
> 			struct rtattr *afs, *afsmctp;
> 			afs = addattr_nest(&req->n, sizeof(*req), IFLA_AF_SPEC);
> 			afsmctp = addattr_nest(&req->n, sizeof(*req), AF_MCTP);
> 			addattr32(&req->n, sizeof(*req),
>                                  IFLA_MCTP_NET, 0);
> 			addattr_nest_end(&req->n, afsmctp);
> 			addattr_nest_end(&req->n, afs);

according to the implementation of the `addrgenmode`.

In short, the package format should be like
     
  L         T        L      T  
+---+--------------+---+---------+-------------
|   | IFLA_AF_SPEC |   | AF_MCTP | .....
+---+--------------+---+---------+-------------
                   ^
                   |
                   af

Therefore, the code

  nla_for_each_nested(af, tb[IFLA_AF_SPEC], rem) {
    const struct rtnl_af_ops *af_ops;
    BUG_ON(!(af_ops = rtnl_af_lookup(nla_type(af)))); <= (1)
    err = af_ops->set_link_af(dev, af, extack);       <= (2)

(1) will get AF_MCTP with nla_type(...) call, and (2) will then passing `af` to
`mctp_set_link_af`, which will expect the `NLA_F_NESTED` flag.

With the hacked iproute tools, simple type command like

`ip link set eth0 mtu 1024 mctptest` will hit the function mctp_set_link_af and
with my debugger, I found that the nla_parse_nested always return error.

Another elegant solution is to change how (1) calls rtnl_af_lookup, we can introduce
another family attribute and avoid to use nla_type(..) which is so suspicious.
But this will have to change also other set_link_af implementations (ipv4, ipv6).

> -- 
> pv-bot: s
> pw-bot: cr

Regards
Lin

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ