linux-kernel - Re: WARN_ON

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <414bc088-9441-70c7-88e2-2c928b97db36@ozlabs.ru>
Date:   Fri, 4 Dec 2020 13:30:06 +1100
From:   Alexey Kardashevskiy <aik@...abs.ru>
To:     Michael Ellerman <mpe@...erman.id.au>,
        Dmitry Vyukov <dvyukov@...gle.com>
Cc:     syzkaller <syzkaller@...glegroups.com>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: WARN_ON_ONCE



On 04/12/2020 12:25, Michael Ellerman wrote:
> Dmitry Vyukov <dvyukov@...gle.com> writes:
>> On Thu, Dec 3, 2020 at 10:19 AM Dmitry Vyukov <dvyukov@...gle.com> wrote:
>>> On Thu, Dec 3, 2020 at 10:10 AM Alexey Kardashevskiy <aik@...abs.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