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: Tue, 14 Jan 2020 09:49:11 -0700 From: David Ahern <dsahern@...il.com> To: Nikolay Aleksandrov <nikolay@...ulusnetworks.com>, Jakub Kicinski <kuba@...nel.org> Cc: netdev@...r.kernel.org, roopa@...ulusnetworks.com, davem@...emloft.net, bridge@...ts.linux-foundation.org Subject: Re: [PATCH net-next 3/8] net: bridge: vlan: add rtm definitions and dump support On 1/14/20 9:45 AM, Nikolay Aleksandrov wrote: > On 14/01/2020 18:36, Nikolay Aleksandrov wrote: >> On 14/01/2020 17:34, David Ahern wrote: >>> On 1/14/20 6:55 AM, Jakub Kicinski wrote: >>>> On Mon, 13 Jan 2020 17:52:28 +0200, Nikolay Aleksandrov wrote: >>>>> +static int br_vlan_rtm_dump(struct sk_buff *skb, struct netlink_callback *cb) >>>>> +{ >>>>> + int idx = 0, err = 0, s_idx = cb->args[0]; >>>>> + struct net *net = sock_net(skb->sk); >>>>> + struct br_vlan_msg *bvm; >>>>> + struct net_device *dev; >>>>> + >>>>> + if (cb->nlh->nlmsg_len < nlmsg_msg_size(sizeof(*bvm))) { >>>> >>>> I wonder if it'd be useful to make this a strict != check? At least >>>> when strict validation is on? Perhaps we'll one day want to extend >>>> the request? >>>> >>> >>> +1. All new code should be using the strict checks. >>> >> >> IIRC, I did it to be able to add filter attributes later, but it should just use nlmsg_parse() >> instead and all will be taken care of. >> I'll respin v2 with that change. >> >> Thanks, >> Nik >> > > Actually nlmsg_parse() uses the same "<" check for the size before parsing. :) > If I change to it and with no attributes to parse would be essentially equal to the > current situation, but if I make it strict "!=" then we won't be able to add > filter attributes later as we won't be backwards compatible. I'll continue looking > into it, but IMO we should leave it as it is in order to be able to add the filtering later. > > Thoughts ? > > > > If the header is > sizeof(*bvm) I expect this part of __nla_validate_parse() to kick in: if (unlikely(rem > 0)) { pr_warn_ratelimited("netlink: %d bytes leftover after parsing attributes in process `%s'.\n", rem, current->comm); NL_SET_ERR_MSG(extack, "bytes leftover after parsing attributes"); if (validate & NL_VALIDATE_TRAILING) return -EINVAL; }
Powered by blists - more mailing lists