On Mon, Jul 30, 2018 at 03:46:58PM +0200, U.Mutlu wrote:
> ...
> if (/* offset > sizeof ctrl.c32 || */ offset + len > sizeof ctrl.c32)
But here you commented out offset > sizeof ctrl.c32? The original code
isn't like that. With it commented out, of course you're going to hit
"B" when offset underflows -- I get the same output as you, too.
With that predicate uncommented, your code snippet won't segfault.
> {
> len = 0;
> printf("A: s->addr=%zu off=%zu len=0\n", s->addr, (size_t) offset);
> }
> else
> {
> const uint8_t* p = ((uint8_t *)&ctrl.c32) + offset;
> printf("B: s->addr=%zu off=%zu poff=%ld\n", s->addr, (size_t)
> offset, p - ((uint8_t *)&ctrl.c32));
> memcpy(buf, p, len);
> }
> }
>
> int main()
> {
> kdd_state s;
> s.buf[0] = 0;
> s.length_req = 84;
> for (int i = 0; i <= 1000; i += 10)
> {
> s.addr = i;
> func(&s);
> }
>
> return 0;
> }
>
> Just compiling and executing normal 64bit:
>
> $ gcc -Wall -Wextra -std=gnu99 -O2 t.c
> $ ./a.out
> A: s->addr=0 off=4294967092 len=0
> A: s->addr=10 off=4294967102 len=0
> A: s->addr=20 off=4294967112 len=0
> A: s->addr=30 off=4294967122 len=0
> A: s->addr=40 off=4294967132 len=0
> A: s->addr=50 off=4294967142 len=0
> A: s->addr=60 off=4294967152 len=0
> A: s->addr=70 off=4294967162 len=0
> A: s->addr=80 off=4294967172 len=0
> A: s->addr=90 off=4294967182 len=0
> A: s->addr=100 off=4294967192 len=0
> A: s->addr=110 off=4294967202 len=0
> B: s->addr=120 off=4294967212 poff=4294967212
> Segmentation fault
>
> IMO, the code is buggy as hell.
I don't necessarily disagree, because this code comes from
reverse-engineering of a protocol. :-/
Wei.
