Re: [bug report] selftest: bpf: Test bpf_sk_assign_tcp_reqsk().

[Kuniyuki Iwashima <kuniyu@amazon.com>]

From: Dan Carpenter <dan.carpenter@linaro.org>
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.