Re: Better overflow check in bgpd
On Wed, Dec 02, 2020 at 12:19:11PM +0100, Claudio Jeker wrote:
> The overflow check for the relative metric adjustments of filtersets
> assumes a certain overflow behaviour of signed integers. I think it is
> better to write this in a way that does not involve an overflow.
>
Here a small follow up. The underflow check can be adjusted to be similar
to the overflow check which makes my OCD happy.
OK?
--
:wq Claudio
Index: rde_filter.c
===
RCS file: /cvs/src/usr.sbin/bgpd/rde_filter.c,v
retrieving revision 1.125
diff -u -p -r1.125 rde_filter.c
--- rde_filter.c3 Dec 2020 11:53:34 - 1.125
+++ rde_filter.c3 Dec 2020 11:58:58 -
@@ -55,8 +55,8 @@ rde_apply_set(struct filter_set_head *sh
state->aspath.lpref +=
set->action.relative;
} else {
- if ((u_int32_t)-set->action.relative >
- state->aspath.lpref)
+ if (state->aspath.lpref <
+ 0U - set->action.relative)
state->aspath.lpref = 0;
else
state->aspath.lpref +=
@@ -77,8 +77,8 @@ rde_apply_set(struct filter_set_head *sh
state->aspath.med +=
set->action.relative;
} else {
- if ((u_int32_t)-set->action.relative >
- state->aspath.med)
+ if (state->aspath.med <
+ 0U - set->action.relative)
state->aspath.med = 0;
else
state->aspath.med +=
@@ -97,8 +97,8 @@ rde_apply_set(struct filter_set_head *sh
state->aspath.weight +=
set->action.relative;
} else {
- if ((u_int32_t)-set->action.relative >
- state->aspath.weight)
+ if (state->aspath.weight <
+ 0U - set->action.relative)
state->aspath.weight = 0;
else
state->aspath.weight +=
Re: Better overflow check in bgpd
On Wed, Dec 02, 2020 at 06:49:38AM -0700, Todd C. Miller wrote: > On Wed, 02 Dec 2020 12:19:11 +0100, Claudio Jeker wrote: > > > The overflow check for the relative metric adjustments of filtersets > > assumes a certain overflow behaviour of signed integers. I think it is > > better to write this in a way that does not involve an overflow. > > OK millert@ though I think > would work just as well as >= here. > Good point. I'll change that before commit -- :wq Claudio
Re: Better overflow check in bgpd
On Wed, 02 Dec 2020 12:19:11 +0100, Claudio Jeker wrote: > The overflow check for the relative metric adjustments of filtersets > assumes a certain overflow behaviour of signed integers. I think it is > better to write this in a way that does not involve an overflow. OK millert@ though I think > would work just as well as >= here. - todd
Better overflow check in bgpd
The overflow check for the relative metric adjustments of filtersets
assumes a certain overflow behaviour of signed integers. I think it is
better to write this in a way that does not involve an overflow.
OK?
--
:wq Claudio
Index: rde_filter.c
===
RCS file: /cvs/src/usr.sbin/bgpd/rde_filter.c,v
retrieving revision 1.124
diff -u -p -r1.124 rde_filter.c
--- rde_filter.c5 Nov 2020 11:51:13 - 1.124
+++ rde_filter.c2 Dec 2020 11:12:24 -
@@ -48,8 +48,8 @@ rde_apply_set(struct filter_set_head *sh
break;
case ACTION_SET_RELATIVE_LOCALPREF:
if (set->action.relative > 0) {
- if (set->action.relative + state->aspath.lpref <
- state->aspath.lpref)
+ if (state->aspath.lpref >=
+ UINT_MAX - set->action.relative)
state->aspath.lpref = UINT_MAX;
else
state->aspath.lpref +=
@@ -70,8 +70,8 @@ rde_apply_set(struct filter_set_head *sh
case ACTION_SET_RELATIVE_MED:
state->aspath.flags |= F_ATTR_MED | F_ATTR_MED_ANNOUNCE;
if (set->action.relative > 0) {
- if (set->action.relative + state->aspath.med <
- state->aspath.med)
+ if (state->aspath.med >=
+ UINT_MAX - set->action.relative)
state->aspath.med = UINT_MAX;
else
state->aspath.med +=
@@ -90,8 +90,8 @@ rde_apply_set(struct filter_set_head *sh
break;
case ACTION_SET_RELATIVE_WEIGHT:
if (set->action.relative > 0) {
- if (set->action.relative + state->aspath.weight
- < state->aspath.weight)
+ if (state->aspath.weight >=
+ UINT_MAX - set->action.relative)
state->aspath.weight = UINT_MAX;
else
state->aspath.weight +=
