On Mon, Feb 11, 2019 at 04:23:57PM -0800, Toms Atteka wrote:
> If enough large input is passed to odp_actions_from_string it can
> cause netlink attribute to overflow.
> ovs_assert was added just before the problematic code so it could
> be debugged faster in similar cases if they would arise. Check
> for buffer size was added to prevent entering this function and
> returning appropriate error code.
>
> Basic manual testing was performed.
>
> Reported-by:
> https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=12231
> Signed-off-by: Toms Atteka
Thank you for the fix!
An nlattr's size is a uint16_t. I am more comfortable using UINT16_MAX
as the maximum value for this type than I am USHRT_MAX, since the latter
can in theory be bigger than 16 bits.
I am not sure why the assertion in nl_msg_end_nested() is sure to be
correct. It seems risky to me. Can you explain?
Thanks,
Ben.
> ---
> lib/netlink.c | 1 +
> lib/odp-util.c | 4
> 2 files changed, 5 insertions(+)
>
> diff --git a/lib/netlink.c b/lib/netlink.c
> index de3ebcd..c91c868 100644
> --- a/lib/netlink.c
> +++ b/lib/netlink.c
> @@ -498,6 +498,7 @@ void
> nl_msg_end_nested(struct ofpbuf *msg, size_t offset)
> {
> struct nlattr *attr = ofpbuf_at_assert(msg, offset, sizeof *attr);
> +ovs_assert(msg->size - offset <= USHRT_MAX);
> attr->nla_len = msg->size - offset;
> }
>
> diff --git a/lib/odp-util.c b/lib/odp-util.c
> index e893f46..9f637ca 100644
> --- a/lib/odp-util.c
> +++ b/lib/odp-util.c
> @@ -2161,6 +2161,10 @@ parse_action_list(const char *s, const struct simap
> *port_names,
> n += retval;
> }
>
> +if (actions->size > USHRT_MAX) {
> +return -EFBIG;
> +}
> +
> return n;
> }
>
> --
> 2.7.4
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
