On Mon, Aug 19, 2024 at 12:14:13PM -0700, Kuniyuki Iwashima wrote:
From: Kuniyuki Iwashima kuniyu@amazon.com Date: Mon, 19 Aug 2024 12:07:04 -0700
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.
Ok, this off-by-one report is false-positive as the test has
mssind = (cookie & (3 << 6)) >> 6;
and the following (mssind > ARRAY_SIZE()) is just to make verifier happy.
In this case, I was testing code that Smatch couldn't parse completely.
But also I have a different check for "> ARRAY_SIZE()" which deliberately ignores the value of mssind since I was missing "false positive" bugs like this.
regards, dan carpenter