Hi Willy No problem, I don't recall submitting this particular patch before so as far as I can tell you didn't miss anything! I've attached the patch file here, hopefully that's maintained the spacing better. I'm happy to try more things if there are still issues applying the patch.
Thanks Jacques From: [email protected] At: 11/21/25 06:34:21 UTCTo: Jacques Heunis (BLOOMBERG/ LONDON ) Cc: [email protected] Subject: Re: [PATCH] BUG/MINOR: freq_ctr: Prevent possible signed overflow in freq_ctr Hello, first, sorry, I thought I had already taken your patch but apparently not. On Wed, Nov 12, 2025 at 05:47:34PM -0000, Jacques Heunis (BLOOMBERG/ LONDON) wrote: > All of the other bandwidth-limiting code stores limits and intermediate > (byte) counters as unsigned integers. > (...) > This is a niche possibility, because it requires that a bandwidth limit is > defined in the range [2^31, 2^32). In this case, the raw limit value would > not fit into a signed integer, and close to the end of the period, the > `(elapsed * freq)/period` calculation could produce a value which also > doesn't fit into a signed integer. (...) Yes, you're totally right. I'm just having one (minor) issue, your mailer has completely mangled spaces in your patch as you can see below. I can try to reproduce it manually since it's not very long, but if you still have it, could you please send it as an attachment ? Generally mails that are broken for inline don't modify attachments. thank you! Willy > include/haproxy/freq_ctr.h | 2 +- > src/flt_bwlim.c | 4 ++-- > src/freq_ctr.c | 19 ++++++++++++------- > 3 files changed, 15 insertions(+), 10 deletions(-) > > diff --git a/include/haproxy/freq_ctr.h b/include/haproxy/freq_ctr.h > index 6ddcde842..dfb187483 100644 > --- a/include/haproxy/freq_ctr.h > +++ b/include/haproxy/freq_ctr.h > @@ -31,7 +31,7 @@ > ullong _freq_ctr_total_from_values(uint period, int pend, uint tick, ullong past, ullong curr); > ullong freq_ctr_total(const struct freq_ctr *ctr, uint period, int pend); > ullong freq_ctr_total_estimate(const struct freq_ctr *ctr, uint period, int pend); > -int freq_ctr_overshoot_period(const struct freq_ctr *ctr, uint period, uint freq); > +uint freq_ctr_overshoot_period(const struct freq_ctr *ctr, uint period, uint freq); > uint update_freq_ctr_period_slow(struct freq_ctr *ctr, uint period, uint inc); > > /* Only usable during single threaded startup phase. */ > diff --git a/src/flt_bwlim.c b/src/flt_bwlim.c > index 3c75b9bb0..40a876790 100644 > --- a/src/flt_bwlim.c > +++ b/src/flt_bwlim.c > @@ -79,9 +79,9 @@ static int bwlim_apply_limit(struct filter *filter, struct channel *chn, unsigne > struct bwlim_state *st = filter->ctx; > struct freq_ctr *bytes_rate; > uint64_t remain; > - unsigned int period, limit, tokens, users, factor; > + unsigned int period, limit, tokens, users, factor, overshoot; > unsigned int wait = 0; > - int overshoot, ret = 0; > + int ret = 0; > > /* Don't forward anything if there is nothing to forward or the waiting > * time is not expired > diff --git a/src/freq_ctr.c b/src/freq_ctr.c > index 7f9e346de..571798297 100644 > --- a/src/freq_ctr.c > +++ b/src/freq_ctr.c > @@ -184,17 +184,17 @@ ullong freq_ctr_total_estimate(const struct freq_ctr *ctr, uint period, int pend > return _freq_ctr_total_from_values(period, pend, tick, past, curr); > } > > -/* Returns the excess of events (may be negative) over the current period for > - * target frequency <freq>. It returns 0 if the counter is in the future or if > - * the counter is empty. The result considers the position of the current time > - * within the current period. > +/* Returns the excess of events over the current period for target frequency > + * <freq>. It returns 0 if the counter is in the future or if the counter is > + * empty. The result considers the position of the current time within the > + * current period. > * > * The caller may safely add new events if result is negative or null. > */ > -int freq_ctr_overshoot_period(const struct freq_ctr *ctr, uint period, uint freq) > +uint freq_ctr_overshoot_period(const struct freq_ctr *ctr, uint period, uint freq) > { > ullong curr, old_curr; > - uint tick, old_tick; > + uint tick, old_tick, linear_usage; > int elapsed; > > tick = HA_ATOMIC_LOAD(&ctr->curr_tick); > @@ -245,7 +245,12 @@ int freq_ctr_overshoot_period(const struct freq_ctr *ctr, uint period, uint freq > return 0; > } > > - return curr - div64_32((uint64_t)elapsed * freq, period); > + linear_usage = div64_32((uint64_t)elapsed * freq, period); > + if (curr < linear_usage) { > + /* The counter is below a uniform linear increase across the period, no overshoot */ > + return 0; > + } > + return curr - linear_usage; > } > > /* > -- > 2.43.0 >
BUG-MINOR-freq_ctr-Prevent-possible-signed-overflow.patch
Description: Binary data
