[ovs-dev] [PATCH V3 7/9] compat: Use nla_parse deprecated functions

Yi-Hung Wei yihung.wei at gmail.com
Fri Mar 6 18:56:08 UTC 2020


On Wed, Mar 4, 2020 at 3:04 PM Greg Rose <gvrose8192 at gmail.com> wrote:
>
> From: Johannes Berg <johannes.berg at intel.com>
>
> Upstream commit:
>     commit 8cb081746c031fb164089322e2336a0bf5b3070c
>     Author: Johannes Berg <johannes.berg at intel.com>
>     Date:   Fri Apr 26 14:07:28 2019 +0200
>
>     netlink: make validation more configurable for future strictness
>
>     We currently have two levels of strict validation:
>
>      1) liberal (default)
>          - undefined (type >= max) & NLA_UNSPEC attributes accepted
>          - attribute length >= expected accepted
>          - garbage at end of message accepted
>      2) strict (opt-in)
>          - NLA_UNSPEC attributes accepted
>          - attribute length >= expected accepted
>
>     Split out parsing strictness into four different options:
>      * TRAILING     - check that there's no trailing data after parsing
>                       attributes (in message or nested)
>      * MAXTYPE      - reject attrs > max known type
>      * UNSPEC       - reject attributes with NLA_UNSPEC policy entries
>      * STRICT_ATTRS - strictly validate attribute size
>
>     The default for future things should be *everything*.
>     The current *_strict() is a combination of TRAILING and MAXTYPE,
>     and is renamed to _deprecated_strict().
>     The current regular parsing has none of this, and is renamed to
>     *_parse_deprecated().
>
>     Additionally it allows us to selectively set one of the new flags
>     even on old policies. Notably, the UNSPEC flag could be useful in
>     this case, since it can be arranged (by filling in the policy) to
>     not be an incompatible userspace ABI change, but would then going
>     forward prevent forgetting attribute entries. Similar can apply
>     to the POLICY flag.
>
>     We end up with the following renames:
>      * nla_parse           -> nla_parse_deprecated
>      * nla_parse_strict    -> nla_parse_deprecated_strict
>      * nlmsg_parse         -> nlmsg_parse_deprecated
>      * nlmsg_parse_strict  -> nlmsg_parse_deprecated_strict
>      * nla_parse_nested    -> nla_parse_nested_deprecated
>      * nla_validate_nested -> nla_validate_nested_deprecated
>
>     Using spatch, of course:
>         @@
>         expression TB, MAX, HEAD, LEN, POL, EXT;
>         @@
>         -nla_parse(TB, MAX, HEAD, LEN, POL, EXT)
>         +nla_parse_deprecated(TB, MAX, HEAD, LEN, POL, EXT)
>
>         @@
>         expression NLH, HDRLEN, TB, MAX, POL, EXT;
>         @@
>         -nlmsg_parse(NLH, HDRLEN, TB, MAX, POL, EXT)
>         +nlmsg_parse_deprecated(NLH, HDRLEN, TB, MAX, POL, EXT)
>
>         @@
>         expression NLH, HDRLEN, TB, MAX, POL, EXT;
>         @@
>         -nlmsg_parse_strict(NLH, HDRLEN, TB, MAX, POL, EXT)
>         +nlmsg_parse_deprecated_strict(NLH, HDRLEN, TB, MAX, POL, EXT)
>
>         @@
>         expression TB, MAX, NLA, POL, EXT;
>         @@
>         -nla_parse_nested(TB, MAX, NLA, POL, EXT)
>         +nla_parse_nested_deprecated(TB, MAX, NLA, POL, EXT)
>
>         @@
>         expression START, MAX, POL, EXT;
>         @@
>         -nla_validate_nested(START, MAX, POL, EXT)
>         +nla_validate_nested_deprecated(START, MAX, POL, EXT)
>
>         @@
>         expression NLH, HDRLEN, MAX, POL, EXT;
>         @@
>         -nlmsg_validate(NLH, HDRLEN, MAX, POL, EXT)
>         +nlmsg_validate_deprecated(NLH, HDRLEN, MAX, POL, EXT)
>
>     For this patch, don't actually add the strict, non-renamed versions
>     yet so that it breaks compile if I get it wrong.
>
>     Also, while at it, make nla_validate and nla_parse go down to a
>     common __nla_validate_parse() function to avoid code duplication.
>
>     Ultimately, this allows us to have very strict validation for every
>     new caller of nla_parse()/nlmsg_parse() etc as re-introduced in the
>     next patch, while existing things will continue to work as is.
>
>     In effect then, this adds fully strict validation for any new command.
>
>     Signed-off-by: Johannes Berg <johannes.berg at intel.com>
>     Signed-off-by: David S. Miller <davem at davemloft.net>
>
> Backport portions of this commit applicable to openvswitch and
> added necessary compatibility layer changes to support older
> kernels.
>
> Signed-off-by: Greg Rose <gvrose8192 at gmail.com>
> ---
> V3 - As per Yi-Hung's suggestion just backport the upstream patch
>      to stay in sync with upstream kernel code.
> ---

Acked-by: Yi-Hung Wei <yihung.wei at gmail.com>


More information about the dev mailing list