[ovs-dev] [PATCH V2 09/11] compat: Use nla_parse deprecated functions

[David Ahern]

On 2/28/20 10:21 AM, Gregory Rose wrote:
> 
> On 2/27/2020 4:39 PM, Yi-Hung Wei wrote:
>> On Wed, Feb 26, 2020 at 9:42 AM Greg Rose <gvrose8192 at gmail.com> wrote:
>>> Changes for in kernel generated netlink attribute parsing functions
>>> require our out of tree driver to use the deprecated forms of those
>>> functions.  Otherwise the message parsing will return -EINVAL because
>>> NLA_F_NESTED is not set in the nla_type field.
>>>
>>> Signed-off-by: Greg Rose <gvrose8192 at gmail.com>
>>> ---
>>>   acinclude.m4                                | 3 +++
>>>   datapath/linux/compat/include/net/netlink.h | 5 +++++
>>>   2 files changed, 8 insertions(+)
>>>
>>> diff --git a/acinclude.m4 b/acinclude.m4
>>> index a55c905..43d1576 100644
>>> --- a/acinclude.m4
>>> +++ b/acinclude.m4
>>> @@ -1072,6 +1072,9 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [
>>>     OVS_FIND_FIELD_IFELSE([$KSRC/include/net/genetlink.h], [genl_ops],
>>>                           [policy],
>>>                           [OVS_DEFINE([HAVE_GENL_OPS_POLICY])])
>>> +  OVS_GREP_IFELSE([$KSRC/include/net/netlink.h],
>>> +                  [nla_parse_deprecated_strict],
>>> +                  [OVS_DEFINE([HAVE_NLA_PARSE_DEPRECATED_STRICT])])
>>>
>>>     if cmp -s datapath/linux/kcompat.h.new \
>>>               datapath/linux/kcompat.h >/dev/null 2>&1; then
>>> diff --git a/datapath/linux/compat/include/net/netlink.h
>>> b/datapath/linux/compat/include/net/netlink.h
>>> index 34fc346..7c0d993 100644
>>> --- a/datapath/linux/compat/include/net/netlink.h
>>> +++ b/datapath/linux/compat/include/net/netlink.h
>>> @@ -143,6 +143,10 @@ static inline int nla_put_be64(struct sk_buff
>>> *skb, int attrtype, __be64 value,
>>>
>>>   #endif
>>>
>>> +#ifdef HAVE_NLA_PARSE_DEPRECATED_STRICT
>>> +#define nla_parse_nested nla_parse_nested_deprecated
>>> +#define nla_parse nla_parse_deprecated_strict
>> Thanks for this patch.  Looks like this change is related to upstream
>> patch 8cb081746c031 ("netlink: make validation more configurable for
>> future strictness").
>>
>> To be in sync with the upstream kernel, should we use the new upstream
>> netlink parsing APIs (i.e. nla_parse_nested_deprecated(),
>> nla_parse_deprecated_strict(), genlmsg_parse_deprecated()), and
>> backports them to the old APIs if they are not available in the older
>> kernel?
> 
> Hi Yi-Hung,
> 
> I'm not sure that provides much value since we never intend (SFAIK) to
> start using the NLA_F_NESTED nla_type in our out of tree kernel code.
> I think this should be sufficient but I'm CC'ing David Ahern to get his
> opinion since he suggested that I use the deprecated functions for
> our out of tree driver.
> 
> David,
> 
> any suggestion?
> 

The intent is that all new code uses the strict parsing functions. Older
code has to use the deprecated ones to maintain backwards compatibility.

The new parsing functions are very strict and expect all attributes to
be properly validated (e.g., a u32 is 4 bytes, a u8 is 1 byte), with
proper flags (e.g., the NESTED one) and all bytes accounted for.