On Apr 24, 2010, at 6:36 PM, Daniel Mentz via RT wrote:
> Robin Seggelmann via RT wrote:
>> #define RSMBLY_BITMASK_IS_COMPLETE(bitmask, msg_len, is_complete) { \
>
>> + if (is_complete) for (ii = (((msg_len) - 1) >> 3) - 1;
>> ii > 0 ; ii--) \
>
> I'm wondering if there are two issues with this for loop:
>
> 1. It fails to check if bitmask[0] equals 0xff.
>
> 2. ii is initialized with ((((msg_len) - 1) >> 3) - 1). If msg_len is 8
> or smaller, this expression is basically equal to (0-1) which wraps
> around the unsigned long i.e. the value becomes something like
> 4294967295 on a 32 bit system.
You're right. The loop was written in the assumption that the control variable
in the for loop can become -1. Since the variable type was changed to unsigned
long, that didn't work anymore and my fix also didn't correct that. Here's an
updated version.
Regards,
Robin
--- ssl/d1_both.c 14 Apr 2010 00:41:01 -0000 1.14.2.20
+++ ssl/d1_both.c 26 Apr 2010 08:24:31 -0000
@@ -127,25 +127,26 @@
#define RSMBLY_BITMASK_MARK(bitmask, start, end) { \
if ((end) - (start) <= 8) { \
- unsigned long ii; \
+ long ii; \
for (ii = (start); ii < (end); ii++)
bitmask[((ii) >> 3)] |= (1 << ((ii) & 7)); \
} else { \
- unsigned long ii; \
+ long ii; \
bitmask[((start) >> 3)] |=
bitmask_start_values[((start) & 7)]; \
- for (ii = (((start) >> 3) + 1); ii < ((end) >>
3); ii++) bitmask[ii] = 0xff; \
- bitmask[((end) >> 3)] |=
bitmask_end_values[((end) & 7)]; \
+ for (ii = (((start) >> 3) + 1); ii < ((((end) -
1)) >> 3); ii++) bitmask[ii] = 0xff; \
+ bitmask[(((end) - 1) >> 3)] |=
bitmask_end_values[((end) & 7)]; \
} }
#define RSMBLY_BITMASK_IS_COMPLETE(bitmask, msg_len, is_complete) { \
- unsigned long ii; \
+ long ii; \
+ OPENSSL_assert((msg_len) > 0); \
is_complete = 1; \
- if (bitmask[((msg_len) >> 3)] !=
bitmask_end_values[((msg_len) & 7)]) is_complete = 0; \
- if (is_complete) for (ii = 0; ii < ((msg_len) >> 3);
ii++) \
- if (bitmask[ii] != 0xff) { is_complete = 0; break; } }
+ if (bitmask[(((msg_len) - 1) >> 3)] !=
bitmask_end_values[((msg_len) & 7)]) is_complete = 0; \
+ if (is_complete) for (ii = (((msg_len) - 1) >> 3) - 1;
ii >= 0 ; ii--) \
+ if (bitmask[ii] != 0xff) { is_complete = 0;
break; } }
#if 0
#define RSMBLY_BITMASK_PRINT(bitmask, msg_len) { \
- int ii; \
+ long ii; \
printf("bitmask: "); for (ii = 0; ii < (msg_len); ii++)
\
printf("%d ", (bitmask[ii >> 3] & (1 << (ii & 7))) >>
(ii & 7)); \
printf("\n"); }
@@ -662,7 +663,7 @@
msg_hdr->frag_off + frag_len);
RSMBLY_BITMASK_IS_COMPLETE(frag->reassembly, msg_hdr->msg_len,
- is_complete)
+ is_complete);
if (is_complete)
{
dtls-bitmask.patch
Description: Binary data
