[ovs-dev] [PATCH] odp-util: Stop key parsing if already oversized.

[Ilya Maximets]

On 6/24/21 1:44 PM, Ilya Maximets wrote:
> We don't need to continue parsing if already oversized.  This is not
> very important, but fuzzer times out while parsing very long flow.
> 
> The check could be written as a single 'if' statement, but I found
> my variant much more readable.
> 
> Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=35519
> Signed-off-by: Ilya Maximets <i.maximets at ovn.org>
> ---

Gentle reminder.
Would be nice to have some review on this patch.

Best regards, Ilya Maximets.

>  lib/odp-util.c |  9 +++++++++
>  tests/odp.at   | 14 ++++++++++++++
>  2 files changed, 23 insertions(+)
> 
> diff --git a/lib/odp-util.c b/lib/odp-util.c
> index 04a183c7c..7729a9060 100644
> --- a/lib/odp-util.c
> +++ b/lib/odp-util.c
> @@ -6077,6 +6077,15 @@ odp_flow_from_string(const char *s, const struct simap *port_names,
>          }
>  
>          retval = parse_odp_key_mask_attr(&context, s, key, mask);
> +
> +        if (retval >= 0) {
> +            if (nl_attr_oversized(key->size - NLA_HDRLEN)) {
> +                retval = -E2BIG;
> +            } else if (mask && nl_attr_oversized(mask->size - NLA_HDRLEN)) {
> +                retval = -E2BIG;
> +            }
> +        }
> +
>          if (retval < 0) {
>              if (errorp) {
>                  *errorp = xasprintf("syntax error at %s", s);
> diff --git a/tests/odp.at b/tests/odp.at
> index dccafd9d3..07a5cfe39 100644
> --- a/tests/odp.at
> +++ b/tests/odp.at
> @@ -449,6 +449,20 @@ odp_actions_from_string: error
>  ])
>  AT_CLEANUP
>  
> +AT_SETUP([OVS datapath keys parsing and formatting - keys too long])
> +dnl Flow keys should fit into a single netlink message.
> +dnl Empty encap() takes 4 bytes.  So, 16384 is too many, but 16383 still fits.
> +dnl We're getting 'duplicate attribute' error since it's not a logically valid
> +dnl sequence of keys.  'syntax error' indicates oversized list of keys.
> +keys=$(printf 'encap(),%.0s' $(seq 16382))
> +echo "${keys}encap()" > keys.txt
> +echo "${keys}encap(),encap()" >> keys.txt
> +AT_CHECK([ovstest test-odp parse-keys < keys.txt | sed 's/encap(),//g'], [0], [dnl
> +odp_flow_key_to_flow: error (duplicate encap attribute in flow key; the flow key in error is: encap())
> +odp_flow_from_string: error (syntax error at encap())
> +])
> +AT_CLEANUP
> +
>  AT_SETUP([OVS datapath keys parsing and formatting - 33 nested encap ])
>  AT_DATA([odp-in.txt], [dnl
>  encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap()))))))))))))))))))))))))))))))))
>