netdev - Re: [net-next v3 1/6] net: dcb: add new pcp selector to app object

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, 27 Oct 2022 09:00:54 +0000
From:   <Daniel.Machon@...rochip.com>
To:     <petrm@...dia.com>
CC:     <netdev@...r.kernel.org>, <davem@...emloft.net>,
        <maxime.chevallier@...tlin.com>, <thomas.petazzoni@...tlin.com>,
        <edumazet@...gle.com>, <kuba@...nel.org>, <pabeni@...hat.com>,
        <Lars.Povlsen@...rochip.com>, <Steen.Hegelund@...rochip.com>,
        <UNGLinuxDriver@...rochip.com>, <joe@...ches.com>,
        <linux@...linux.org.uk>, <Horatiu.Vultur@...rochip.com>,
        <Julia.Lawall@...ia.fr>, <vladimir.oltean@....com>,
        <linux-kernel@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [net-next v3 1/6] net: dcb: add new pcp selector to app object

> >> I'm missing a validation that DCB_APP_SEL_PCP is always sent in
> >> DCB_ATTR_DCB_APP encapsulation. Wouldn't the current code permit
> >> sending it in the IEEE encap? This should be forbidden.
> >
> > Right. Current impl. does not check that the non-std selectors received, are
> > sent with a DCB_ATTR_DCB_APP type.  We could introduce a new check
> > dcbnl_app_attr_selector_validate() that checks combination of type and
> > selector, after the type and nla_len(attr) has been checked, so that:
> >
> >  validate type -> validate nla_len(attr) -> validate selector
> 
> This needs to be validated, otherwise there's no point in having a
> dedicated attribute for the non-standard stuff.

Agree.

> 
> >> And vice versa: I'm not sure we want to permit sending the standard
> >> attributes in the DCB encap.
> >
> > dcbnl_app_attr_type_get() in dcbnl_ieee_fill() takes care of this. IEEE are
> > always sent in DCB_ATTR_IEEE and non-std are sent in DCB_ATTR_DCB.
> 
> By "sending" I meant userspace sending this to the kernel. So bounce
> extended opcodes that are wrapped in IEEE and bounce IEEE opcodes
> wrapped in DCB as well.

Right. Then we only need to decide what to do with any opcode in-between
(not defined in uapi, neither ieee or extension opcode, 7-254). If they are 
sent in DCB_ATTR_DCB they should be bounced, because we agreed that we can 
interpret data in the new attr), _but_ if they are sent in DCB_ATTR_IEEE I 
guess we should accept them, to not break userspace that is already sending
them.

Here is what that could look like:

	/* Make sure any non-std selectors is always encapsulated in the non-std
	* DCB_ATTR_DCB_APP attribute.
	*/
	static bool dcbnl_app_attr_selector_validate(enum ieee_attrs_app type, u32 selector)
	{
		switch (selector) {
		case DCB_APP_SEL_PCP:
			/* Non-std selector in non-std attr? */
			if (type == DCB_ATTR_DCB_APP)
				return true;
		default:
			/* Std selector in std attr? */
			if (type == DCB_ATTR_IEEE_APP)
				return true;
		}

		return false;
	}

/ Daniel