diff mbox series

[ovs-dev] ovsdb-data: Fix overflow in ovsdb_datum_sort_unique()'s return value.

Message ID 70f86507a727a76e18ce8be8786368d18533bc03.1733327334.git.echaudro@redhat.com
State Superseded
Headers show
Series [ovs-dev] ovsdb-data: Fix overflow in ovsdb_datum_sort_unique()'s return value. | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed

Commit Message

Eelco Chaudron Dec. 4, 2024, 3:48 p.m. UTC
This change prevents returning an overflowing value by simply
returning zero in such cases. The return value is currently
unused in all its use cases.

Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
---
 lib/ovsdb-data.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Ilya Maximets Dec. 11, 2024, 1:46 p.m. UTC | #1
On 12/4/24 16:48, Eelco Chaudron wrote:
> This change prevents returning an overflowing value by simply
> returning zero in such cases. The return value is currently
> unused in all its use cases.
> 
> Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
> ---
>  lib/ovsdb-data.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/ovsdb-data.c b/lib/ovsdb-data.c
> index abb923ad8..adb4714ee 100644
> --- a/lib/ovsdb-data.c
> +++ b/lib/ovsdb-data.c
> @@ -1230,7 +1230,7 @@ ovsdb_datum_sort_unique(struct ovsdb_datum *datum,
>          }
>      }
>      datum->n = dst;
> -    return datum->n - src;
> +    return datum->n > src ? datum->n - src : 0;

This function sorts the elements of the datum and removes duplicates.
'src' is the old number of elements, and datum->n is the new number.
New number is always less or equal to the old one, so (datum->n > src)
is always false and the function will always return zero.

I think, the original subtraction should just be reversed to not be
always negative.

Will that solve the issue?

Best regards, Ilya Maximets.
Eelco Chaudron Dec. 11, 2024, 4:34 p.m. UTC | #2
On 11 Dec 2024, at 14:46, Ilya Maximets wrote:

> On 12/4/24 16:48, Eelco Chaudron wrote:
>> This change prevents returning an overflowing value by simply
>> returning zero in such cases. The return value is currently
>> unused in all its use cases.
>>
>> Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
>> ---
>>  lib/ovsdb-data.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/lib/ovsdb-data.c b/lib/ovsdb-data.c
>> index abb923ad8..adb4714ee 100644
>> --- a/lib/ovsdb-data.c
>> +++ b/lib/ovsdb-data.c
>> @@ -1230,7 +1230,7 @@ ovsdb_datum_sort_unique(struct ovsdb_datum *datum,
>>          }
>>      }
>>      datum->n = dst;
>> -    return datum->n - src;
>> +    return datum->n > src ? datum->n - src : 0;
>
> This function sorts the elements of the datum and removes duplicates.
> 'src' is the old number of elements, and datum->n is the new number.
> New number is always less or equal to the old one, so (datum->n > src)
> is always false and the function will always return zero.
>
> I think, the original subtraction should just be reversed to not be
> always negative.
>
> Will that solve the issue?

Yes, it will, thanks for reviewing this! I’ve sent a v2 for this.

//Eelco
diff mbox series

Patch

diff --git a/lib/ovsdb-data.c b/lib/ovsdb-data.c
index abb923ad8..adb4714ee 100644
--- a/lib/ovsdb-data.c
+++ b/lib/ovsdb-data.c
@@ -1230,7 +1230,7 @@  ovsdb_datum_sort_unique(struct ovsdb_datum *datum,
         }
     }
     datum->n = dst;
-    return datum->n - src;
+    return datum->n > src ? datum->n - src : 0;
 }
 
 /* Checks that each of the atoms in 'datum' conforms to the constraints