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 |
Context | Check | Description |
---|---|---|
ovsrobot/apply-robot | success | apply and check: success |
ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
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.
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 --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
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(-)