Skip to content

Commit

Permalink
Fix multiple file match bugs
Browse files Browse the repository at this point in the history
This commit fixes a number of file match bugs:

* __filter_file_buf() was passing a length value of one less than the
  string length to cmpbytes() and as a result Equals and Prefix matches
  were testing one fewer bytes than required.

* ASM_RCMP (used in rcmpbytes()) was decrementing the string indices
  before testing if they were <1. This resulted in one fewer bytes being
  tested than required.

* ASM_RCMP50 comprisd of 2x ASM_RCMP20 and 1x ASM_RCMP5, totalling 45
  iterations of ASM_RCMP, instead of 50.

* __filter_file_buf() tested failed postfix matches with a forward string
  match. This resulted in files that started with the postfix, but didn't
  end with it, matching when they shouldn't.

* cmpbytes() continued to loop to full number of iterations, even when
  the string length had been exhausted (inefficient).

* Added descriptions to cmpbytes() and rcmpbytes() to aid in providing
  the correct parameters.

Signed-off-by: Kevin Sheldrake <[email protected]>
  • Loading branch information
kevsecurity authored and kkourt committed Feb 28, 2023
1 parent 7d7c5aa commit f0b12fa
Showing 1 changed file with 10 additions and 4 deletions.
14 changes: 10 additions & 4 deletions bpf/process/types/basic.h
Original file line number Diff line number Diff line change
Expand Up @@ -306,10 +306,10 @@ parse_iovec_array(long off, unsigned long arg, int i, unsigned long max,
: "c2", "r0"); \
if (c1 != c2) \
goto failed; \
n1--; \
n2--; \
if (n1 < 1 || n2 < 1) \
goto accept; \
n1--; \
n2--; \
}

#define ASM_RCMP5 \
Expand All @@ -334,6 +334,7 @@ parse_iovec_array(long off, unsigned long arg, int i, unsigned long max,
ASM_RCMP20 \
ASM_RCMP20 \
ASM_RCMP5 \
ASM_RCMP5 \
}

#define ASM_RCMP100 \
Expand All @@ -342,6 +343,7 @@ parse_iovec_array(long off, unsigned long arg, int i, unsigned long max,
ASM_RCMP50 \
}

/* reverse compare bytes. n1 is index of last byte in s1. Ditto n2 of s2. */
static inline __attribute__((always_inline)) int rcmpbytes(char *s1, char *s2,
u64 n1, u64 n2)
{
Expand All @@ -359,13 +361,16 @@ static inline __attribute__((always_inline)) int rcmpbytes(char *s1, char *s2,
return -1;
}

/* compare bytes. n is number of bytes to compare. */
static inline __attribute__((always_inline)) int cmpbytes(char *s1, char *s2,
size_t n)
{
int i;
#pragma unroll
for (i = 0; i < MAX_STRING_FILTER; i++) {
if (i < n && s1[i] != s2[i])
if (i >= n)
return 0;
if (s1[i] != s2[i])
return -1;
}
return 0;
Expand Down Expand Up @@ -617,8 +622,9 @@ __filter_file_buf(char *value, char *args, __u32 op)
err = rcmpbytes(&value[4], &args[4], v - 1, a - 1);
if (!err)
return 0;
goto skip_string;
}
err = cmpbytes(&value[4], &args[4], v - 1);
err = cmpbytes(&value[4], &args[4], v);
if (!err)
return 0;
skip_string:
Expand Down

0 comments on commit f0b12fa

Please sign in to comment.