[ovs-dev] [PATCHv3] netlink: added check to prevent netlink attribute overflow
Gregory Rose
gvrose8192 at gmail.com
Wed Feb 20 23:44:34 UTC 2019
On 2/20/2019 1:53 PM, Cpp Code wrote:
>
>> On 20 Feb 2019, at 02:27, Gregory Rose <gvrose8192 at gmail.com
>> <mailto:gvrose8192 at gmail.com>> wrote:
>>
>>
>> On 2/19/2019 10:55 AM, Toms Atteka wrote:
>>> If enough large input is passed to odp_actions_from_string it can
>>> cause netlink attribute to overflow.
>>> 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<cpp.code.lv at gmail.com>
>>> ---
>>> lib/odp-util.c | 4 ++++
>>> 1 file changed, 4 insertions(+)
>>>
>>> diff --git a/lib/odp-util.c b/lib/odp-util.c
>>> index e893f46..e288ae8 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 > UINT16_MAX) {
>>> + return -EFBIG;
>>> + }
>>> +
>>> return n;
>>> }
>>>
>> Hi Toms,
>>
>> Thanks for the patch. Question though, why EFBIG instead of E2BIG?
>> This would appear to be a situation in
>> which too many arguments are sent (E2BIG) but then maybe it is from a
>> file (EFBIG)?
>>
>> Also, I see this is a version 3 of this patch? What changed from
>> version 1 to version 3? Commonly the
>> changes from each version of a patch are posted beneath the git
>> separator '---'. Like below...
>>
>> Thanks,
>>
>> - Greg
>>
>> Signed-off-by: Martin Xu <martinxu9.ovs at gmail.com>
>> CC: Flavio Leitner <fbl at sysclose.org>
>> CC: Yi-Hung Wei <yihung.wei at gmail.com> CC: Yifeng Sun
>> <pkusunyifeng at gmail.com> CC: Zak Whittington
>> <zwhitt.vmware at gmail.com> CC: Ben Pfaff <blp at ovn.org> --- v1->v2:
>> adds "Obsoletes" tag needed for upgrade after renaming adds more
>> reviewers
>>
>
> Hi Greg,
>
>
> Thats not a case of too many arguments provided, but the the size of a
> single argument is too large, so I believe EFBIG is more appropriate.
>
> I guess its not worth creating v4 but ill keep in my mind off adding
> change log next time.
>
> Thanks,
> Tom
>
OK, thanks for the explanation. You can add my Reviewed-by tag.
Reviewed-by: Greg Rose <gvrose8192 at gmail.com>
More information about the dev
mailing list