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 PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Fri, 21 Oct 2022 21:08:15 -0700 From: Jakub Kicinski <kuba@...nel.org> To: Johannes Berg <johannes@...solutions.net> Cc: davem@...emloft.net, netdev@...r.kernel.org, edumazet@...gle.com, pabeni@...hat.com, jacob.e.keller@...el.com, fw@...len.de, jiri@...dia.com Subject: Re: [PATCH net] genetlink: piggy back on resv_op to default to a reject policy On Fri, 21 Oct 2022 21:57:53 +0200 Johannes Berg wrote: > It feels it might've been easier to implement as simply, apart from the > doc changes: > > --- a/net/netlink/genetlink.c > +++ b/net/netlink/genetlink.c > @@ -529,6 +529,10 @@ genl_family_rcv_msg_attrs_parse(const struct genl_family *family, > struct nlattr **attrbuf; > int err; > > + if (ops->cmd >= family->resv_start_op && !ops->maxattr && > + nlmsg_attrlen(nlh, hdrlen)) > + return ERR_PTR(-EINVAL); > + > if (!ops->maxattr) > return NULL; > > But maybe I'm missing something in the relation with the new split ops > etc. The reason was that payload length check is... "unintrospectable"? The reject all policy shows up in GETPOLICY. Dunno how much it matters in practice but that was the motivation. LMK which way you prefer. > Also, technically, you could now have an op that is >= resv_start_op, > but sets one of GENL_DONT_VALIDATE{_DUMP,}_STRICT and then gets the old > behaviour except that attributes 0 and 1 are rejected? > > Any particular reason you chose this implementation here? I can > understand having chosen it with the yaml things since then you can be > sure you're not setting GENL_DONT_VALIDATE{_DUMP,}_STRICT and you don't > have another choice anyway, but here? > > Hmm. > > Then again, maybe anyway we should make sure that > GENL_DONT_VALIDATE{_DUMP,}_STRICT aren't set for ops >= resv_start_op? > > > Anyway, for the intended use it works, and I guess it'd be a stupid > family that makes sure to set this but then still uses non-strict > validation, though I've seen people (try to) copy/paste non-strict > validation into new ops ... Hm, yeah, adding DONT*_STRICT for new commands would be pretty odd as you say. Someone may copy & paste an existing command, tho, without understanding what this flag does. I can add a check separately I reckon. It's more of a "no new command should set this flag" thing rather than inherently related to the reject-all policy, right?
Powered by blists - more mailing lists