Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: WARN_ON_ONCE
To:     Michael Ellerman <mpe@ellerman.id.au>,
        Dmitry Vyukov <dvyukov@google.com>
Cc:     syzkaller <syzkaller@googlegroups.com>,
        LKML <linux-kernel@vger.kernel.org>
References: <87f443cf-26c0-6302-edee-556045bca18a@ozlabs.ru>
 <CACT4Y+ZAyhk6CuddQNix0fAupXhOpv1t3iOdcXbDh4VDEPyOJQ@mail.gmail.com>
 <CACT4Y+ZHH5DiDj7KvRKtPqkV1CS0TFOkCH-M5bitfCgd5PWotg@mail.gmail.com>
 <87k0tyuztw.fsf@mpe.ellerman.id.au>
From:   Alexey Kardashevskiy <aik@ozlabs.ru>
Message-ID: <414bc088-9441-70c7-88e2-2c928b97db36@ozlabs.ru>
Date:   Fri, 4 Dec 2020 13:30:06 +1100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:84.0) Gecko/20100101
 Thunderbird/84.0
MIME-Version: 1.0
In-Reply-To: <87k0tyuztw.fsf@mpe.ellerman.id.au>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk


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