src: replace postfix decremental loops

During fuzzing, UBSan discovered that postfix increments/decrements
inside a loop condition are executed after the condition is met.

While in most cases loop counter is dropped right after it's been used
in some cases where it doesn't it may lead to unexpected behaviour.

Yet in both cases loop peforms extra addition/subtraction, which would
be nice to eliminate.

So the solution is to switch to incremental "for" loop or nest such
counters inside loops so they are executed only if loop termination
condition isn't met.

Ticket: #8184

Signed-off-by: Sergey Zhidkih <rx1513@altlinux.org>
pull/14966/head
Sergey Zhidkih 10 months ago
parent 6ec9e5c957
commit ffbc648aa9
No known key found for this signature in database
GPG Key ID: 879C4656EE98A5C4

@ -423,7 +423,7 @@ static int DNP3DecodeObjectG{{object.group}}V{{object.variation}}(const uint8_t
if (*len < count/8) {
goto error;
}
while (count--) {
for (uint32_t i = 0; i < count; i++) {
object = SCCalloc(1, sizeof(*object));
if (unlikely(object == NULL)) {

File diff suppressed because it is too large Load Diff

@ -163,7 +163,7 @@ static uint16_t DNP3ComputeCRC(const uint8_t *buf, uint32_t len)
uint16_t crc = 0;
int idx;
while (len--) {
for (uint32_t i = 0; i < len; i++) {
idx = (crc ^ *byte) & 0xff;
crc = (crc_table[idx] ^ (crc >> 8)) & 0xffff;
byte++;

@ -671,9 +671,8 @@ DefragTracker *DefragLookupTrackerFromHash (Packet *p)
static DefragTracker *DefragTrackerGetUsedDefragTracker(ThreadVars *tv, const DecodeThreadVars *dtv)
{
uint32_t idx = SC_ATOMIC_GET(defragtracker_prune_idx) % defrag_config.hash_size;
uint32_t cnt = defrag_config.hash_size;
while (cnt--) {
for (uint32_t i = 0; i < defrag_config.hash_size; i++) {
if (++idx >= defrag_config.hash_size)
idx = 0;
@ -720,7 +719,7 @@ static DefragTracker *DefragTrackerGetUsedDefragTracker(ThreadVars *tv, const De
StatsCounterIncr(&tv->stats, dtv->counter_defrag_tracker_soft_reuse);
}
(void) SC_ATOMIC_ADD(defragtracker_prune_idx, (defrag_config.hash_size - cnt));
(void)SC_ATOMIC_ADD(defragtracker_prune_idx, i);
return dt;
}

@ -330,11 +330,10 @@ static inline void DetectPrefilterCopyDeDup(
const DetectEngineCtx *de_ctx, DetectEngineThreadCtx *det_ctx)
{
SigIntId *pf_ptr = det_ctx->pmq.rule_id_array;
uint32_t final_cnt = det_ctx->pmq.rule_id_array_cnt;
Signature **sig_array = de_ctx->sig_array;
Signature **match_array = det_ctx->match_array;
SigIntId previous_id = (SigIntId)-1;
while (final_cnt-- > 0) {
for (uint32_t i = 0; i < det_ctx->pmq.rule_id_array_cnt; i++) {
SigIntId id = *pf_ptr++;
Signature *s = sig_array[id];
@ -682,7 +681,8 @@ static inline uint8_t DetectRulePacketRules(ThreadVars *const tv,
next_s = *match_array++;
next_sflags = next_s->flags;
}
while (match_cnt--) {
while (match_cnt) {
--match_cnt;
RULE_PROFILING_START(p);
bool break_out_of_packet_filter = false;
uint8_t alert_flags = 0;

@ -659,9 +659,8 @@ Host *HostLookupHostFromHash (Address *a)
static Host *HostGetUsedHost(void)
{
uint32_t idx = SC_ATOMIC_GET(host_prune_idx) % host_config.hash_size;
uint32_t cnt = host_config.hash_size;
while (cnt--) {
for (uint32_t i = 0; i < host_config.hash_size; i++) {
if (++idx >= host_config.hash_size)
idx = 0;
@ -707,7 +706,7 @@ static Host *HostGetUsedHost(void)
SCMutexUnlock(&h->m);
(void) SC_ATOMIC_ADD(host_prune_idx, (host_config.hash_size - cnt));
(void)SC_ATOMIC_ADD(host_prune_idx, i);
return h;
}

@ -694,9 +694,8 @@ IPPair *IPPairLookupIPPairFromHash (Address *a, Address *b)
static IPPair *IPPairGetUsedIPPair(void)
{
uint32_t idx = SC_ATOMIC_GET(ippair_prune_idx) % ippair_config.hash_size;
uint32_t cnt = ippair_config.hash_size;
while (cnt--) {
for (uint32_t i = 0; i < ippair_config.hash_size; i++) {
if (++idx >= ippair_config.hash_size)
idx = 0;
@ -742,7 +741,7 @@ static IPPair *IPPairGetUsedIPPair(void)
SCMutexUnlock(&h->m);
(void) SC_ATOMIC_ADD(ippair_prune_idx, (ippair_config.hash_size - cnt));
(void)SC_ATOMIC_ADD(ippair_prune_idx, i);
return h;
}

@ -181,7 +181,7 @@ int TmThreadTimeoutLoop(ThreadVars *tv, TmSlot *s)
uint32_t len = tv->stream_pq->len;
SCMutexUnlock(&tv->stream_pq->mutex_q);
if (len > 0) {
while (len--) {
for (uint32_t i = 0; i < len; i++) {
SCMutexLock(&tv->stream_pq->mutex_q);
Packet *p = PacketDequeue(tv->stream_pq);
SCMutexUnlock(&tv->stream_pq->mutex_q);

@ -821,9 +821,8 @@ THashData *THashLookupFromHash (THashTableContext *ctx, void *data)
static THashData *THashGetUsed(THashTableContext *ctx, uint32_t data_size)
{
uint32_t idx = SC_ATOMIC_GET(ctx->prune_idx) % ctx->config.hash_size;
uint32_t cnt = ctx->config.hash_size;
while (cnt--) {
for (uint32_t i = 0; i < ctx->config.hash_size; i++) {
if (++idx >= ctx->config.hash_size)
idx = 0;
@ -874,7 +873,7 @@ static THashData *THashGetUsed(THashTableContext *ctx, uint32_t data_size)
}
SCMutexUnlock(&h->m);
(void) SC_ATOMIC_ADD(ctx->prune_idx, (ctx->config.hash_size - cnt));
(void)SC_ATOMIC_ADD(ctx->prune_idx, i);
if (data_size > 0)
(void)SC_ATOMIC_ADD(ctx->memuse, data_size);
return h;

Loading…
Cancel
Save