linux-kernel - Re: [PATCH v2] net: make unregister netdev warning timeout configurable

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Thu, 25 Mar 2021 11:32:27 +0100
From:   Dmitry Vyukov <dvyukov@...gle.com>
To:     Eric Dumazet <edumazet@...gle.com>
Cc:     David Miller <davem@...emloft.net>,
        Leon Romanovsky <leon@...nel.org>,
        netdev <netdev@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] net: make unregister netdev warning timeout configurable

On Thu, Mar 25, 2021 at 9:46 AM Eric Dumazet <edumazet@...gle.com> wrote:
>
> On Thu, Mar 25, 2021 at 8:39 AM Dmitry Vyukov <dvyukov@...gle.com> wrote:
> >
> > On Wed, Mar 24, 2021 at 10:40 AM Eric Dumazet <edumazet@...gle.com> wrote:
> > >
> > > On Tue, Mar 23, 2021 at 7:49 AM Dmitry Vyukov <dvyukov@...gle.com> wrote:
> > > >
> > > > netdev_wait_allrefs() issues a warning if refcount does not drop to 0
> > > > after 10 seconds. While 10 second wait generally should not happen
> > > > under normal workload in normal environment, it seems to fire falsely
> > > > very often during fuzzing and/or in qemu emulation (~10x slower).
> > > > At least it's not possible to understand if it's really a false
> > > > positive or not. Automated testing generally bumps all timeouts
> > > > to very high values to avoid flake failures.
> > > > Add net.core.netdev_unregister_timeout_secs sysctl to make
> > > > the timeout configurable for automated testing systems.
> > > > Lowering the timeout may also be useful for e.g. manual bisection.
> > > > The default value matches the current behavior.
> > > >
> > > > Signed-off-by: Dmitry Vyukov <dvyukov@...gle.com>
> > > > Fixes: https://bugzilla.kernel.org/show_bug.cgi?id=211877
> > > > Cc: netdev@...r.kernel.org
> > > > Cc: linux-kernel@...r.kernel.org
> > > >
> > > > ---
> > > > Changes since v1:
> > > >  - use sysctl instead of a config
> > > > ---
> > >
> > > >         },
> > > > +       {
> > > > +               .procname       = "netdev_unregister_timeout_secs",
> > > > +               .data           = &netdev_unregister_timeout_secs,
> > > > +               .maxlen         = sizeof(unsigned int),
> > > > +               .mode           = 0644,
> > > > +               .proc_handler   = proc_dointvec_minmax,
> > > > +               .extra1         = SYSCTL_ZERO,
> > > > +               .extra2         = &int_3600,
> > > > +       },
> > > >         { }
> > > >  };
> > > >
> > >
> > > If we allow the sysctl to be 0, then we risk a flood of pr_emerg()
> > > (one per jiffy ?)
> >
> > My reasoning was that it's up to the user. Some spammy output on the
> > console for rare events is probably not the worst way how root can
> > misconfigure the kernel :)
> > It allows one to check (more or less) if we are reaching
> > unregister_netdevice with non-zero refcount, which may be useful for
> > some debugging maybe.
> > But I don't mind changing it to 1 (or 5) if you prefer. On syzbot we
> > only want to increase it.
> >
>
> Yes, please use a lower limit of one to avoid spurious reports.

This commit is already in net-next and net-next is not rebased, right?
I sent a separate fix as "net: change netdev_unregister_timeout_secs
min value to 1".