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
| ||
|
Date: Thu, 01 Oct 2020 09:48:31 +0200 From: Johannes Berg <johannes@...solutions.net> To: Jakub Kicinski <kuba@...nel.org>, netdev@...r.kernel.org Cc: andrew@...n.ch, jiri@...nulli.us, mkubecek@...e.cz, dsahern@...nel.org, pablo@...filter.org Subject: Re: [RFC net-next 5/9] genetlink: add a structure for dump state On Wed, 2020-09-30 at 17:05 -0700, Jakub Kicinski wrote: > Whenever netlink dump uses more than 2 cb->args[] entries > code gets hard to read. We're about to add more state to > ctrl_dumppolicy() so create a structure. > > Since the structure is typed and clearly named we can remove > the local fam_id variable and use ctx->fam_id directly. > > Signed-off-by: Jakub Kicinski <kuba@...nel.org> > --- > net/netlink/genetlink.c | 22 +++++++++++++--------- > 1 file changed, 13 insertions(+), 9 deletions(-) > > diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c > index 38d8f353dba1..a8001044d8cd 100644 > --- a/net/netlink/genetlink.c > +++ b/net/netlink/genetlink.c > @@ -1102,13 +1102,18 @@ static int genl_ctrl_event(int event, const struct genl_family *family, > return 0; > } > > +struct ctrl_dump_policy_ctx { > + unsigned long state; Maybe if we do this, also make a "struct netlink_policy_dump_state" in include/net/netlink.h for the policy dump to use as a state? Right now it just uses an "unsigned long *state" there. I feel that would more clearly show what this "state" actually is. Alternatively, perhaps just rename it to "policy_dump_state"? Yeah, that's longer, but at least would be very obvious? > + unsigned int fam_id; You could make this a u16 I guess, but it doesn't really matter. > static int ctrl_dumppolicy(struct sk_buff *skb, struct netlink_callback *cb) > { > + struct ctrl_dump_policy_ctx *ctx = (void *)cb->args; I'd also prefer if you stuck a BUILD_BUG_ON(sizeof(*ctx) > sizeof(cb->args)); here. It's not likely we'll need so much more state here, but would still be good to check IMHO. But in general looks good :) johannes
Powered by blists - more mailing lists