3diy
/
klipper
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

stepcompress: Rework addfactor integer overflow check 

Revert 4a16053c and avoid integer overflows in the addfactor
calculation by exiting the loop early if count > 0x200.  This provides
stronger protection against overflows.

Signed-off-by: Kevin O'Connor <kevin@koconnor.net>
This commit is contained in:
Kevin O'Connor 2016-12-26 12:47:17 -05:00
parent a9444d3399
commit 800d53db6a
1 changed files with 7 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
klippy/stepcompress.c
View File

@ -95,13 +95,6 @@ idiv_down(int32_t n, int32_t d)
return (n>=0) ? (n/d) : (n - d + 1) / d; return (n>=0) ? (n/d) : (n - d + 1) / d;
} }
// Calculate the impact of each 'add' for a sequence with 'count' steps
static inline int32_t
calc_addfactor(int32_t count)
{
return (uint32_t)count*(count-1)/2;
}
struct points { struct points {
int32_t minp, maxp; int32_t minp, maxp;
}; };
@ -157,7 +150,7 @@ compress_bisect_add(struct stepcompress *sc)
return (struct step_move){ interval, count, add }; return (struct step_move){ interval, count, add };
} }
nextpoint = minmax_point(sc, sc->queue_pos + nextcount - 1); nextpoint = minmax_point(sc, sc->queue_pos + nextcount - 1);
int32_t nextaddfactor = calc_addfactor(nextcount); int32_t nextaddfactor = nextcount*(nextcount-1)/2;
int32_t c = add*nextaddfactor; int32_t c = add*nextaddfactor;
if (nextmininterval*nextcount < nextpoint.minp - c) if (nextmininterval*nextcount < nextpoint.minp - c)
nextmininterval = DIV_UP(nextpoint.minp - c, nextcount); nextmininterval = DIV_UP(nextpoint.minp - c, nextcount);
@ -169,7 +162,7 @@ compress_bisect_add(struct stepcompress *sc)
} }
// Check if this is the best sequence found so far // Check if this is the best sequence found so far
int32_t count = nextcount - 1, addfactor = calc_addfactor(count); int32_t count = nextcount - 1, addfactor = count*(count-1)/2;
int32_t reach = add*addfactor + interval*count; int32_t reach = add*addfactor + interval*count;
if (reach > bestreach if (reach > bestreach
|| (reach == bestreach && interval > bestinterval)) { || (reach == bestreach && interval > bestinterval)) {
@ -181,10 +174,13 @@ compress_bisect_add(struct stepcompress *sc)
zerointerval = interval; zerointerval = interval;
zerocount = count; zerocount = count;
} }
if (count > 0x200)
// No 'add' will improve sequence; avoid integer overflow
break;
} }
// Check if a greater or lesser add could extend the sequence // Check if a greater or lesser add could extend the sequence
int32_t nextaddfactor = calc_addfactor(nextcount); int32_t nextaddfactor = nextcount*(nextcount-1)/2;
int32_t nextreach = add*nextaddfactor + interval*nextcount; int32_t nextreach = add*nextaddfactor + interval*nextcount;
if (nextreach < nextpoint.minp) { if (nextreach < nextpoint.minp) {
minadd = add + 1; minadd = add + 1;
@ -339,7 +335,7 @@ stepcompress_flush(struct stepcompress *sc, uint64_t move_clock)
// Be careful with 32bit overflow // Be careful with 32bit overflow
sc->last_step_clock = qm->req_clock = *sc->queue_pos; sc->last_step_clock = qm->req_clock = *sc->queue_pos;
} else { } else {
int32_t addfactor = calc_addfactor(move.count); int32_t addfactor = move.count*(move.count-1)/2;
uint32_t ticks = move.add*addfactor + move.interval*move.count; uint32_t ticks = move.add*addfactor + move.interval*move.count;
sc->last_step_clock += ticks; sc->last_step_clock += ticks;
} }