From mboxrd@z Thu Jan  1 00:00:00 1970
From: David Ahern <dsahern@gmail.com>
Subject: Re: [PATCH net-next 08/20] rtnetlink: Update rtnl_dump_ifinfo for
 strict data checking
Date: Fri, 5 Oct 2018 13:22:24 -0600
Message-ID: <cc1e134e-7d02-2f46-2a6d-5c1e823f3e97@gmail.com>
References: <20181004213355.14899-1-dsahern@kernel.org>
 <20181004213355.14899-9-dsahern@kernel.org>
 <20181005175922.4oiwihofhsvcu4ok@brauner.io>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
Cc: netdev@vger.kernel.org, davem@davemloft.net, jbenc@redhat.com,
        stephen@networkplumber.org
To: Christian Brauner <christian@brauner.io>,
        David Ahern <dsahern@kernel.org>
Return-path: <netdev-owner@vger.kernel.org>
Received: from mail-pg1-f193.google.com ([209.85.215.193]:38558 "EHLO
        mail-pg1-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1728139AbeJFCWf (ORCPT
        <rfc822;netdev@vger.kernel.org>); Fri, 5 Oct 2018 22:22:35 -0400
Received: by mail-pg1-f193.google.com with SMTP id r77-v6so5143536pgr.5
        for <netdev@vger.kernel.org>; Fri, 05 Oct 2018 12:22:27 -0700 (PDT)
In-Reply-To: <20181005175922.4oiwihofhsvcu4ok@brauner.io>
Content-Language: en-US
Sender: netdev-owner@vger.kernel.org
List-ID: <netdev.vger.kernel.org>

On 10/5/18 11:59 AM, Christian Brauner wrote:
>> +	err = nlmsg_parse(nlh, hdrlen, tb, IFLA_MAX, ifla_policy, extack);
>> +	if (err < 0) {
>> +		if (cb->strict_check)
>> +			return -EINVAL;
>> +		goto walk_entries;
>> +	}
>>  
>> -		if (master_idx || kind_ops)
>> -			flags |= NLM_F_DUMP_FILTERED;
>> +	for (i = 0; i <= IFLA_MAX; ++i) {
>> +		if (!tb[i])
>> +			continue;
>> +		switch (i) {
>> +		case IFLA_TARGET_NETNSID:
>> +			netnsid = nla_get_s32(tb[i]);
>> +			tgt_net = rtnl_get_net_ns_capable(skb->sk, netnsid);
>> +			if (IS_ERR(tgt_net)) {
>> +				NL_SET_ERR_MSG(extack, "Invalid target namespace id");
>> +				return PTR_ERR(tgt_net);
>> +			}
>> +			break;
>> +		case IFLA_EXT_MASK:
>> +			ext_filter_mask = nla_get_u32(tb[i]);
>> +			break;
>> +		case IFLA_MASTER:
>> +			master_idx = nla_get_u32(tb[i]);
>> +			break;
>> +		case IFLA_LINKINFO:
>> +			kind_ops = linkinfo_to_kind_ops(tb[i]);
>> +			break;
>> +		default:
>> +			if (cb->strict_check) {
>> +				NL_SET_ERR_MSG(extack, "Unsupported attribute in dump request");
>> +				return -EINVAL;
>> +			}
>> +		}
> 
> This might make sense to be split into two helpers for parsing:
> <blablabla>_strict() and <blablabla>_lenient(). :)

I thought about that, but there is so much overlap - they are mostly
common. Besides, ifinfomsg is the header for link dumps, and ifinfomsg
is the one that has been (ab)used for other message types, so strict
versus lenient does not really have a differentiator for this message
type - other than checking the elements of the struct.