| Message ID | 20171128182032.22981-1-blp@ovn.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev] odp-util: Fix parsing corner case for encap_nsh() actions. | expand |
Thanks for spotting this.
The proposed correction is correct, but the other error cases in the function use the pattern
ret = -EINVAL;
goto out;
Can you please align or use
ret = -EINVAL;
break;
BR, Jan
Acked-by: Jan Scheurich <jan.scheurich@ericsson.com>
> -----Original Message-----
> From: ovs-dev-bounces@openvswitch.org [mailto:ovs-dev-bounces@openvswitch.org] On Behalf Of Ben Pfaff
> Sent: Tuesday, 28 November, 2017 19:21
> To: dev@openvswitch.org
> Cc: Ben Pfaff <blp@ovn.org>; Bhargava Shastry <bshastry@sec.t-labs.tu-berlin.de>
> Subject: [ovs-dev] [PATCH] odp-util: Fix parsing corner case for encap_nsh() actions.
>
> When nothing matched, the code would loop forever.
>
> Found with libfuzzer.
>
> Reported-by: Bhargava Shastry <bshastry@sec.t-labs.tu-berlin.de>
> Signed-off-by: Ben Pfaff <blp@ovn.org>
> ---
> lib/odp-util.c | 2 ++
> tests/odp.at | 8 ++++++++
> 2 files changed, 10 insertions(+)
>
> diff --git a/lib/odp-util.c b/lib/odp-util.c
> index 45a890c46aa0..3e30b9ae7719 100644
> --- a/lib/odp-util.c
> +++ b/lib/odp-util.c
> @@ -1870,6 +1870,8 @@ parse_odp_encap_nsh_action(const char *s, struct ofpbuf *actions)
> }
> continue;
> }
> +
> + return -EINVAL;
> }
> out:
> if (ret < 0) {
> diff --git a/tests/odp.at b/tests/odp.at
> index cd01b32d72ef..1a80322890eb 100644
> --- a/tests/odp.at
> +++ b/tests/odp.at
> @@ -362,3 +362,11 @@ AT_CHECK_UNQUOTED([ovstest test-odp parse-actions < actions.txt], [0],
> [`cat actions.txt`
> ])
> AT_CLEANUP
> +
> +AT_SETUP([OVS datapath actions parsing and formatting - invalid forms])
> +dnl This caused a hang in older versions.
> +AT_CHECK([echo 'encap_nsh@:{@' | ovstest test-odp parse-actions
> +], [0], [dnl
> +odp_actions_from_string: error
> +])
> +AT_CLEANUP
> --
> 2.10.2
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Thanks for the review. Sure. I made that change and applied this to master and branch-2.8. On Fri, Dec 01, 2017 at 02:25:40PM +0000, Jan Scheurich wrote: > Thanks for spotting this. > > The proposed correction is correct, but the other error cases in the function use the pattern > ret = -EINVAL; > goto out; > > Can you please align or use > ret = -EINVAL; > break; > > BR, Jan > > Acked-by: Jan Scheurich <jan.scheurich@ericsson.com> > > > -----Original Message----- > > From: ovs-dev-bounces@openvswitch.org [mailto:ovs-dev-bounces@openvswitch.org] On Behalf Of Ben Pfaff > > Sent: Tuesday, 28 November, 2017 19:21 > > To: dev@openvswitch.org > > Cc: Ben Pfaff <blp@ovn.org>; Bhargava Shastry <bshastry@sec.t-labs.tu-berlin.de> > > Subject: [ovs-dev] [PATCH] odp-util: Fix parsing corner case for encap_nsh() actions. > > > > When nothing matched, the code would loop forever. > > > > Found with libfuzzer. > > > > Reported-by: Bhargava Shastry <bshastry@sec.t-labs.tu-berlin.de> > > Signed-off-by: Ben Pfaff <blp@ovn.org> > > --- > > lib/odp-util.c | 2 ++ > > tests/odp.at | 8 ++++++++ > > 2 files changed, 10 insertions(+) > > > > diff --git a/lib/odp-util.c b/lib/odp-util.c > > index 45a890c46aa0..3e30b9ae7719 100644 > > --- a/lib/odp-util.c > > +++ b/lib/odp-util.c > > @@ -1870,6 +1870,8 @@ parse_odp_encap_nsh_action(const char *s, struct ofpbuf *actions) > > } > > continue; > > } > > + > > + return -EINVAL; > > } > > out: > > if (ret < 0) { > > diff --git a/tests/odp.at b/tests/odp.at > > index cd01b32d72ef..1a80322890eb 100644 > > --- a/tests/odp.at > > +++ b/tests/odp.at > > @@ -362,3 +362,11 @@ AT_CHECK_UNQUOTED([ovstest test-odp parse-actions < actions.txt], [0], > > [`cat actions.txt` > > ]) > > AT_CLEANUP > > + > > +AT_SETUP([OVS datapath actions parsing and formatting - invalid forms]) > > +dnl This caused a hang in older versions. > > +AT_CHECK([echo 'encap_nsh@:{@' | ovstest test-odp parse-actions > > +], [0], [dnl > > +odp_actions_from_string: error > > +]) > > +AT_CLEANUP > > -- > > 2.10.2 > > > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
diff --git a/lib/odp-util.c b/lib/odp-util.c index 45a890c46aa0..3e30b9ae7719 100644 --- a/lib/odp-util.c +++ b/lib/odp-util.c @@ -1870,6 +1870,8 @@ parse_odp_encap_nsh_action(const char *s, struct ofpbuf *actions) } continue; } + + return -EINVAL; } out: if (ret < 0) { diff --git a/tests/odp.at b/tests/odp.at index cd01b32d72ef..1a80322890eb 100644 --- a/tests/odp.at +++ b/tests/odp.at @@ -362,3 +362,11 @@ AT_CHECK_UNQUOTED([ovstest test-odp parse-actions < actions.txt], [0], [`cat actions.txt` ]) AT_CLEANUP + +AT_SETUP([OVS datapath actions parsing and formatting - invalid forms]) +dnl This caused a hang in older versions. +AT_CHECK([echo 'encap_nsh@:{@' | ovstest test-odp parse-actions +], [0], [dnl +odp_actions_from_string: error +]) +AT_CLEANUP
When nothing matched, the code would loop forever. Found with libfuzzer. Reported-by: Bhargava Shastry <bshastry@sec.t-labs.tu-berlin.de> Signed-off-by: Ben Pfaff <blp@ovn.org> --- lib/odp-util.c | 2 ++ tests/odp.at | 8 ++++++++ 2 files changed, 10 insertions(+)