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
| ||
|
Message-ID: <YyDajuBHfAP/dIF6@DEN-LT-70577> Date: Tue, 13 Sep 2022 19:22:10 +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 net-next 2/2] net: dcb: add new apptrust attribute Hi Petr, > <Daniel.Machon@...rochip.com> writes: > > > > Petr Machata <petrm@...dia.com> writes: > >> > >> But of course this will never get anywhere close to that. We will end up > >> passing maybe one, two entries. So the UAPI seems excessive in how it > >> hands around this large array. > >> > >> I wonder if it would be better to make the DCB_ATTR_IEEE_APP_TABLE > >> payload be an array of bytes, each byte a selector? Or something similar > >> to DCB_ATTR_IEEE_APP_TABLE / DCB_ATTR_IEEE_APP, a nest and an array of > >> payload attributes? > > > > Hmm. It might seem excessive, but a quick few thoughts on your proposed solution: > > - We need more code to define and parse the new DCB_ATTR_IEEE_APP_TRUST_TABLE / > > DCB_ATTR_IEEE_APP_TRUST attributes. > > Yes, a bit. But it's not too bad IMHO. Am I forgetting something here? > > u8 selectors[256]; > int nselectors; > int rem; > > nla_for_each_nested(attr, ieee[DCB_ATTR_DCB_APP_TRUST_TABLE], rem) { > if (nla_type(attr) != DCB_ATTR_DCB_APP_TRUST || > nla_len(attr) != 1 || > nselectors >= sizeof(selectors)) { > err = -EINVAL; > goto err; > } > > selectors[nselectors++] = nla_get_u8(attr); > } > > ... and you have reconstructed the array. LGTM. > > > - If the selectors are passed individually to the driver, we need a > > dcbnl_delapptrust(), because now, the driver have to add and del from the > > driver maintained array. You could of course accumulate selectors in an array > > before passing them to the driver, but then why go away from the array in the > > first place. > > I have no problem with using an array for the in-kernel API. There it's > easy to change. UAPI can't ever change. > > >> > + struct ieee_apptrust *trust = > >> > + nla_data(ieee[DCB_ATTR_IEEE_APP_TRUST]); > >> > >> Besides invoking the OP, this should validate the payload. E.g. no > >> driver is supposed to accept trust policies that contain invalid > >> selectors. Pretty sure there's no value in repeated entries either. > > > > Validation (bogus input and unique selectors) is done in userspace (dcb-apptrust). > > Using iproute2 dcb is not mandatory, the UAPI is client-agnostic. The > kernel needs to bounce bogons as well. Otherwise they will become part > of the UAPI with the meaning "this doesn't do anything". > > And yeah, drivers will validate supported configurations. But still the > requests that go to the driver should already be sanitized, so that the > driver code doesn't need to worry about this. Good point. Will prepare a v2 with suggested changes.
Powered by blists - more mailing lists