Re: XDP Software Issue - Payload Matching

["Toke Høiland-Jørgensen" <toke@redhat.com>]

Christian Deacon <gamemann@gflclan.com> writes:

> Hey Toke,
>
> Thank you for your response and the information!
>
> I actually found out last night about this as well, but when setting the 
> max payload match length to 1500 bytes (instead of 65535), I receive a 
> new error:
>
> ```
> invalid read from stack off -488+0 size 8
> processed 46277 insns (limit 1000000) max_states_per_insn 4 total_states 
> 617 peak_states 617 mark_read 599
> ```

Hmm, this sounds like the verifier can't prove that your stack variable
(presumably the 'byte' pointer) is actually safe to read from the stack.
Not sure why - maybe because the variable is declared in the inner loop
and the verifier loses track? IDK, really...

> Here's the new code:
>
> ```
> if (filter[i]->payloadLen > 0)
> {
>      uint8_t found = 1;
>
>      // MAX_PAYLOAD_LENGTH = 1500
>      for (uint16_t j = 0; j < MAX_PAYLOAD_LENGTH; j++)
>      {
>          if ((j + 1) > filter[i]->payloadLen)
>          {
>              break;
>          }
>
>          uint8_t *byte = data + sizeof(struct ethhdr) + (iph->ihl * 4) + 
> l4headerLen + j;
>
>          if (byte + 1 > (uint8_t *)data_end)
>          {
>              break;
>          }
>
>          if (*byte == filter[i]->payloadMatch[j])
>          {
>              continue;
>          }
>
>          found = 0;
>
>          break;
>      }
>
>      if (!found)
>      {
>          continue;
>      }
> }
> ```
>
> This error occurs when initializing the `byte` pointer (if I comment out 
> any lines with the `byte` pointer, the XDP program runs without 
> crashing). Though, to my understanding, I am doing things correctly here 
> along with ensuring the `byte` pointer is within the packet's range.

Well, "doing something safely" and "convincing the verifier that what
you're doing is safe" is not always the same thing ;)

> I am also going to look into implementing your second suggestion! I've 
> been trying to think of ways to improve performance with the software 
> and initially, I planned to only have filtering rules activated after a 
> certain global PPS/BPS threshold is met (specified in the config file). 
> However, it sounds like your suggestion would be more efficient.
>
> Thank you for all your help!

You're welcome! :)

-Toke