| Message ID | 1582738882-15842-10-git-send-email-gvrose8192@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add Linux kernel datapath support up to 5.5 | expand |
On Wed, Feb 26, 2020 at 9:42 AM Greg Rose <gvrose8192@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@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? Thanks, -Yi-Hung > +#else > #ifndef HAVE_NETLINK_EXT_ACK > struct netlink_ext_ack; > > @@ -164,6 +168,7 @@ static inline int rpl_nla_parse(struct nlattr **tb, int maxtype, > } > #define nla_parse rpl_nla_parse > #endif > +#endif /* HAVE_NLA_PARSE_DEPRECATED_STRICT */ > > #ifndef HAVE_NLA_NEST_START_NOFLAG > static inline struct nlattr *rpl_nla_nest_start_noflag(struct sk_buff *skb, > --
On 2/27/2020 4:39 PM, Yi-Hung Wei wrote: > On Wed, Feb 26, 2020 at 9:42 AM Greg Rose <gvrose8192@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@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? Thanks, - Greg > > Thanks, > > -Yi-Hung > > >> +#else >> #ifndef HAVE_NETLINK_EXT_ACK >> struct netlink_ext_ack; >> >> @@ -164,6 +168,7 @@ static inline int rpl_nla_parse(struct nlattr **tb, int maxtype, >> } >> #define nla_parse rpl_nla_parse >> #endif >> +#endif /* HAVE_NLA_PARSE_DEPRECATED_STRICT */ >> >> #ifndef HAVE_NLA_NEST_START_NOFLAG >> static inline struct nlattr *rpl_nla_nest_start_noflag(struct sk_buff *skb, >> --
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@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@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.
On 2/28/2020 9:42 AM, David Ahern wrote: > 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@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@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. Thanks for the clarification David. Yi-Hung, let me take this offline with you and we can discuss the correct solution. - Greg
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 +#else #ifndef HAVE_NETLINK_EXT_ACK struct netlink_ext_ack; @@ -164,6 +168,7 @@ static inline int rpl_nla_parse(struct nlattr **tb, int maxtype, } #define nla_parse rpl_nla_parse #endif +#endif /* HAVE_NLA_PARSE_DEPRECATED_STRICT */ #ifndef HAVE_NLA_NEST_START_NOFLAG static inline struct nlattr *rpl_nla_nest_start_noflag(struct sk_buff *skb,
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@gmail.com> --- acinclude.m4 | 3 +++ datapath/linux/compat/include/net/netlink.h | 5 +++++ 2 files changed, 8 insertions(+)