On Tue, Jul 2, 2024 at 7:25 AM Sebastian Andrzej Siewior
<bigeasy@xxxxxxxxxxxxx> wrote:
>
> sparse complains about the argument type for filter that is passed to
> bpf_check_basics_ok(). There are two users of the function where the
> variable is with __user attribute one without. The pointer is only
> checked against NULL so there is no access to the content and so no need
> for any user-wrapper.
>
> Adding the __user to the declaration doesn't solve anything because
> there is one kernel user so it will be wrong again.
> Splitting the function in two seems an overkill because the function is
> small and simple.
>
> Make a macro based on the function which does not trigger a sparse
> warning. The change to a macro and "unsigned int" -> "u16" for `flen'
> alters gcc's code generation a bit.
>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx>
> ---
> net/core/filter.c | 24 ++++++++++++++----------
> 1 file changed, 14 insertions(+), 10 deletions(-)
>
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 3f14c8019f26d..5747533ed5491 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -1035,16 +1035,20 @@ static bool chk_code_allowed(u16 code_to_probe)
> return codes[code_to_probe];
> }
>
> -static bool bpf_check_basics_ok(const struct sock_filter *filter,
> - unsigned int flen)
> -{
> - if (filter == NULL)
> - return false;
> - if (flen == 0 || flen > BPF_MAXINSNS)
> - return false;
> -
> - return true;
> -}
Why not open-code part of it and then have a function for checking length limit:
if (!filter)
return <error>;
if (!bpf_check_prog_len(flen))
return <another-error>;
It's all local to a single file, no big deal adding a few if
(!pointer) checks explicitly, IMO. "basics" is super generic and not a
great name either way.
> + /* macro instead of a function to avoid woring about _filter which might be a
> + * user or kernel pointer. It does not matter for the NULL check.
> + */
> +#define bpf_check_basics_ok(fprog_filter, fprog_flen) \
> +({ \
> + bool __ret = true; \
> + u16 __flen = fprog_flen; \
> + \
> + if (!(fprog_filter)) \
> + __ret = false; \
> + else if (__flen == 0 || __flen > BPF_MAXINSNS) \
> + __ret = false; \
> + __ret; \
> +})
>
> /**
> * bpf_check_classic - verify socket filter code
> --
> 2.45.2
>
