diff mbox series

[ovs-dev,v4,3/3] ofp-util: Fix memory leaks when parsing OF1.5 group properties.

Message ID 20170921165958.3218-4-blp@ovn.org
State Accepted
Headers show
Series Fix memory leaks and overreads in ofp-util | expand

Commit Message

Ben Pfaff Sept. 21, 2017, 4:59 p.m. UTC
Found by libFuzzer.

Reported-by: Bhargava Shastry <bshastry@sec.t-labs.tu-berlin.de>
Signed-off-by: Ben Pfaff <blp@ovn.org>
---
 lib/ofp-util.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

Comments

Justin Pettit Sept. 22, 2017, 9:28 p.m. UTC | #1
> On Sep 21, 2017, at 9:59 AM, Ben Pfaff <blp@ovn.org> wrote:
> 
> Found by libFuzzer.
> 
> Reported-by: Bhargava Shastry <bshastry@sec.t-labs.tu-berlin.de>
> Signed-off-by: Ben Pfaff <blp@ovn.org>

Acked-by: Justin Pettit <jpettit@ovn.org>

--Justin
Ben Pfaff Sept. 22, 2017, 10:01 p.m. UTC | #2
On Fri, Sep 22, 2017 at 02:28:16PM -0700, Justin Pettit wrote:
> 
> > On Sep 21, 2017, at 9:59 AM, Ben Pfaff <blp@ovn.org> wrote:
> > 
> > Found by libFuzzer.
> > 
> > Reported-by: Bhargava Shastry <bshastry@sec.t-labs.tu-berlin.de>
> > Signed-off-by: Ben Pfaff <blp@ovn.org>
> 
> Acked-by: Justin Pettit <jpettit@ovn.org>

Thanks, applied to master, backported as far as 2.4.
Bhargava Shastry Sept. 23, 2017, 2:32 a.m. UTC | #3
If I may, the v4 series of patches are holding up quite well to
medium-term fuzzing (roughly 50 million iterations).

On 09/23/2017 12:01 AM, Ben Pfaff wrote:
> On Fri, Sep 22, 2017 at 02:28:16PM -0700, Justin Pettit wrote:
>>
>>> On Sep 21, 2017, at 9:59 AM, Ben Pfaff <blp@ovn.org> wrote:
>>>
>>> Found by libFuzzer.
>>>
>>> Reported-by: Bhargava Shastry <bshastry@sec.t-labs.tu-berlin.de>
>>> Signed-off-by: Ben Pfaff <blp@ovn.org>
>>
>> Acked-by: Justin Pettit <jpettit@ovn.org>
> 
> Thanks, applied to master, backported as far as 2.4.
>
Justin Pettit Sept. 23, 2017, 3:45 a.m. UTC | #4
That's great!  Thank you for running these tests.

--Justin


> On Sep 22, 2017, at 7:32 PM, Bhargava Shastry <bshastry@sec.t-labs.tu-berlin.de> wrote:
> 
> If I may, the v4 series of patches are holding up quite well to
> medium-term fuzzing (roughly 50 million iterations).
> 
> On 09/23/2017 12:01 AM, Ben Pfaff wrote:
>> On Fri, Sep 22, 2017 at 02:28:16PM -0700, Justin Pettit wrote:
>>> 
>>>> On Sep 21, 2017, at 9:59 AM, Ben Pfaff <blp@ovn.org> wrote:
>>>> 
>>>> Found by libFuzzer.
>>>> 
>>>> Reported-by: Bhargava Shastry <bshastry@sec.t-labs.tu-berlin.de>
>>>> Signed-off-by: Ben Pfaff <blp@ovn.org>
>>> 
>>> Acked-by: Justin Pettit <jpettit@ovn.org>
>> 
>> Thanks, applied to master, backported as far as 2.4.
>> 
> 
> -- 
> Bhargava Shastry <bshastry@sec.t-labs.tu-berlin.de>
> Security in Telecommunications
> TU Berlin / Telekom Innovation Laboratories
> Ernst-Reuter-Platz 7, Sekr TEL 17 / D - 10587 Berlin, Germany
> phone: +49 30 8353 58235
> Keybase: https://keybase.io/bshastry
diff mbox series

Patch

diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 2309a2ad2515..16c4f191a340 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -9581,8 +9581,13 @@  ofputil_decode_ofp15_group_desc_reply(struct ofputil_group_desc *gd,
      * Such properties are valid for group desc replies so
      * claim that the group mod command is OFPGC15_ADD to
      * satisfy the check in parse_group_prop_ntr_selection_method() */
-    return parse_ofp15_group_properties(msg, gd->type, OFPGC15_ADD, &gd->props,
-                                        length - sizeof *ogds - bucket_list_len);
+    error = parse_ofp15_group_properties(
+        msg, gd->type, OFPGC15_ADD, &gd->props,
+        length - sizeof *ogds - bucket_list_len);
+    if (error) {
+        ofputil_bucket_list_destroy(&gd->buckets);
+    }
+    return error;
 }
 
 /* Converts a group description reply in 'msg' into an abstract
@@ -9881,8 +9886,12 @@  ofputil_pull_ofp15_group_mod(struct ofpbuf *msg, enum ofp_version ofp_version,
         return error;
     }
 
-    return parse_ofp15_group_properties(msg, gm->type, gm->command, &gm->props,
-                                        msg->size);
+    error = parse_ofp15_group_properties(msg, gm->type, gm->command,
+                                         &gm->props, msg->size);
+    if (error) {
+        ofputil_bucket_list_destroy(&gm->buckets);
+    }
+    return error;
 }
 
 static enum ofperr