netdev - Re: [PATCH] rtnetlink: Mask the rta

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]

Date:	Thu, 14 Mar 2013 21:28:21 +0000
From:	Thomas Graf <tgraf@...g.ch>
To:	Vlad Yasevich <vyasevic@...hat.com>
Cc:	Stephen Hemminger <stephen@...workplumber.org>,
	netdev@...r.kernel.org, davem@...emloft.net
Subject: Re: [PATCH] rtnetlink: Mask the rta_type when range checking

On 03/14/13 at 01:40pm, Vlad Yasevich wrote:
> So let me rebuff this a bit more intelligently.
> 
> 1) NLA_F_NESTED is used by netfilter in a lot of places.  It seems that
> only rtnetlink interface doesn't account for it.

Stephen is not wrong, strictly speaking this is an ABI breaker.

However, we have been applying NLA_TYPE_MASK in nla_type() and
thus in nla_parse(), nlmsg_find_attr(), etc. since the
introduction of the current kernel netlink API.

static inline int nla_type(const struct nlattr *nla)
{
        return nla->nla_type & NLA_TYPE_MASK;
}

I have been attempting to get rid of that ugly attribute parsing
code in rtnetlink_rcv_msg() but it's hard to get rid of. I shall
give this another try.

> 2) The following commit:
> 
> commit 25c71c75ac87508528db053b818944f3650dd7a6
> Author: stephen hemminger <shemminger@...tta.com>
> Date:   Tue Nov 13 07:53:05 2012 +0000
> 
>     bridge: bridge port parameters over netlink
> 
> introduced NLA_F_NESTED usage in rtnetlink for both setlink and
> getlink, so one could argue that was an ABI change.

You are right, user space applications that do not apply
NLA_TYPE_MASK will no longer see the IFLA_PROTINFO attribute
with the above commit.

> prove that without the change I am introducing, one can not use the
> above mentioned bridge API.  Feel free to try it with iproute2
> patches
> I sent earlier ([PATCH iproute2 0/2] Add support for bridge port
> link information).

I am not against your patch but I would love to see the affected
code paths to no longer rely on the attribute array provided by
the rtnetlink doit function but instead call nla_parse() themselves
for the sake of proper validation.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html