From: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
Date: Mon, 19 Aug 2024 21:57:57 +0300
> On Mon, Aug 19, 2024 at 09:54:00PM +0300, Dan Carpenter wrote:
> > Hello Kuniyuki Iwashima,
> >
> > Commit a74712241b46 ("selftest: bpf: Test
> > bpf_sk_assign_tcp_reqsk().") from Jan 15, 2024 (linux-next), leads to
> > the following Smatch static checker warning:
> >
> > tools/testing/selftests/bpf/progs/test_tcp_custom_syncookie.c:493 tcp_validate_cookie()
> > warn: off by one 'mssind' == ARRAY_SIZE()?
> >
> > ./tools/testing/selftests/bpf/progs/test_tcp_custom_syncookie.c
> > 462 static int tcp_validate_cookie(struct tcp_syncookie *ctx)
> > 463 {
> > 464 u32 cookie = bpf_ntohl(ctx->tcp->ack_seq) - 1;
> > 465 u32 seq = bpf_ntohl(ctx->tcp->seq) - 1;
> > 466 u64 first = 0, second;
> > 467 int mssind;
> > 468 u32 hash;
> > 469
> > 470 if (ctx->ipv4)
> > 471 first = (u64)ctx->ipv4->saddr << 32 | ctx->ipv4->daddr;
> > 472 else if (ctx->ipv6)
> > 473 first = (u64)ctx->ipv6->saddr.in6_u.u6_addr8[0] << 32 |
> > 474 ctx->ipv6->daddr.in6_u.u6_addr32[0];
> > 475
> > 476 second = (u64)seq << 32 | ctx->tcp->source << 16 | ctx->tcp->dest;
> > 477 hash = siphash_2u64(first, second, &test_key_siphash);
> > 478
> > 479 if (ctx->attrs.tstamp_ok)
> > 480 hash -= ctx->attrs.rcv_tsecr & COOKIE_MASK;
> > 481 else
> > 482 hash &= ~COOKIE_MASK;
> > 483
> > 484 hash -= cookie & ~COOKIE_MASK;
> > 485 if (hash)
> > 486 goto err;
> > 487
> > 488 mssind = (cookie & (3 << 6)) >> 6;
> > 489 if (ctx->ipv4) {
> > 490 if (mssind > ARRAY_SIZE(msstab4))
> > ^
> > Should be >= instead of >.
> >
> > 491 goto err;
> > 492
> > --> 493 ctx->attrs.mss = msstab4[mssind];
> > 494 } else {
> > 495 if (mssind > ARRAY_SIZE(msstab6))
> ^
>
> Here too, I guess.
Thanks for reporting.
Will fix it.
But I'm curious why BPF verifier couldn't catch it.
