On 2024-07-02 18:27:31 [+0200], Jiri Olsa wrote:
> On Tue, Jul 02, 2024 at 04:21:43PM +0200, Sebastian Andrzej Siewior 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.
>
> could we just retype the __user argument? like
>
> bpf_check_basics_ok((const struct sock_filter *) fprog->filter, ...)
If we keep the function and add a cast here then cast the __user part
away and it would be wrong if we do something else with the pointer.
If it is understood that that will never happen…
> > --- a/net/core/filter.c
> > +++ b/net/core/filter.c
> > @@ -1035,16 +1035,20 @@ static bool chk_code_allowed(u16 code_to_probe)
…
> > + /* 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; \
>
> why not use fprog_flen directly? I'm not sure I get the changelog
> explanation
This was to avoid expanding `fprog_flen' twice. But looking at the
actual output, the code generation seems to be unaffected.
> thanks,
> jirka
Sebastian
