Messages in this thread | | | Subject | Re: WARN_ON_ONCE | From | Alexey Kardashevskiy <> | Date | Fri, 4 Dec 2020 13:30:06 +1100 |
| |
On 04/12/2020 12:25, Michael Ellerman wrote: > Dmitry Vyukov <dvyukov@google.com> writes: >> On Thu, Dec 3, 2020 at 10:19 AM Dmitry Vyukov <dvyukov@google.com> wrote: >>> On Thu, Dec 3, 2020 at 10:10 AM Alexey Kardashevskiy <aik@ozlabs.ru> wrote: >>>> >>>> Hi! >>>> >>>> Syzkaller triggered WARN_ON_ONCE at >>>> >>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/kernel/tracepoint.c?h=v5.10-rc6#n266 >>>> >>>> >>>> === >>>> static int tracepoint_add_func(struct tracepoint *tp, >>>> struct tracepoint_func *func, int prio) >>>> { >>>> struct tracepoint_func *old, *tp_funcs; >>>> int ret; >>>> >>>> if (tp->regfunc && !static_key_enabled(&tp->key)) { >>>> ret = tp->regfunc(); >>>> if (ret < 0) >>>> return ret; >>>> } >>>> >>>> tp_funcs = rcu_dereference_protected(tp->funcs, >>>> lockdep_is_held(&tracepoints_mutex)); >>>> old = func_add(&tp_funcs, func, prio); >>>> if (IS_ERR(old)) { >>>> WARN_ON_ONCE(PTR_ERR(old) != -ENOMEM); >>>> return PTR_ERR(old); >>>> } >>>> >>>> === >>>> >>>> What is the common approach here? Syzkaller reacts on this as if it was >>>> a bug but WARN_ON_ONCE here seems intentional. Do we still push for >>>> removing such warnings? > > AFAICS it is a bug if that fires. > > See the commit that added it: > d66a270be331 ("tracepoint: Do not warn on ENOMEM") > > Which says: > Tracepoint should only warn when a kernel API user does not respect the > required preconditions (e.g. same tracepoint enabled twice,
This says that the userspace can trigger the warning if it does not use the API right.
> or called > to remove a tracepoint that does not exist). > > Silence warning in out-of-memory conditions, given that the error is > returned to the caller. > > > So if you're seeing it then you've someone caused it to return something > other than ENOMEM, and that is a bug.
This is an userspace bug which registers the same thing twice, the kernel returns a correct error. The question is should it warn by WARN_ON or pr_err(). The comment in bug.h suggests pr_err() is the right way, is not it?
-- Alexey
| |