netdev - Re: [RFC PATCH v2 net-next 1/2] 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:   Wed, 28 Sep 2022 13:52:39 +0000
From:   <Daniel.Machon@...rochip.com>
To:     <petrm@...dia.com>
CC:     <netdev@...r.kernel.org>, <Allan.Nielsen@...rochip.com>,
        <UNGLinuxDriver@...rochip.com>, <maxime.chevallier@...tlin.com>,
        <vladimir.oltean@....com>, <kuba@...nel.org>,
        <vinicius.gomes@...el.com>, <thomas.petazzoni@...tlin.com>
Subject: Re: [RFC PATCH v2 net-next 1/2] net: dcb: add new pcp selector to app
 object

Den Mon, Sep 19, 2022 at 11:45:41AM +0200 skrev Petr Machata:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Daniel Machon <daniel.machon@...rochip.com> writes:
> 
> > diff --git a/include/uapi/linux/dcbnl.h b/include/uapi/linux/dcbnl.h
> > index a791a94013a6..8eab16e5bc13 100644
> > --- a/include/uapi/linux/dcbnl.h
> > +++ b/include/uapi/linux/dcbnl.h
> > @@ -217,6 +217,7 @@ struct cee_pfc {
> >  #define IEEE_8021QAZ_APP_SEL_DGRAM   3
> >  #define IEEE_8021QAZ_APP_SEL_ANY     4
> >  #define IEEE_8021QAZ_APP_SEL_DSCP       5
> > +#define IEEE_8021QAZ_APP_SEL_PCP     255
> >
> >  /* This structure contains the IEEE 802.1Qaz APP managed object. This
> >   * object is also used for the CEE std as well.
> 
> One more thought: please verify how this behaves with openlldpad.
> It's a fairly major user of this API.
> 
> I guess it is OK if it refuses to run or bails out in face of the PCP
> APP entries. On its own it will never introduce them, so this clear and
> noisy diagnostic when a user messes with the system through a different
> channels is OK IMHO.
> 
> But it shouldn't silently reinterpret the 255 to mean something else.

Hi Petr,

Looks like we are in trouble here:

https://github.com/openSUSE/lldpad/blob/master/lldp_8021qaz.c#L911

protocol is shifted and masked with selector to fit in u8. Same u8
value is being transmitted in the APP TLVs.

A dscp mapping of 10:7 will become (7 << 5) | 5 = e5
A pcp mapping of 1:1 will become (1 << 5) | ff = ff (always)

Looks like the loop does not even check for DCB_ATTR_IEEE_APP, so putting
the pcp stuff in a non-standard attribute in the DCB_ATTR_IEEE_APP_TABLE
wont work either.

The pcp selector will have to fit in 5 bits (0x1f instead of 0xff) to not
interfere with the priority in lldapd.

Thoughts?

/ Daniel