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  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:   Tue, 17 May 2022 13:28:28 -0700
From:   "Paul E. McKenney" <paulmck@...nel.org>
To:     Eric Dumazet <edumazet@...gle.com>
Cc:     Marco Elver <elver@...gle.com>, Liu Jian <liujian56@...wei.com>,
        Dmitry Vyukov <dvyukov@...gle.com>,
        LKML <linux-kernel@...r.kernel.org>,
        David Miller <davem@...emloft.net>,
        Hideaki YOSHIFUJI <yoshfuji@...ux-ipv6.org>,
        David Ahern <dsahern@...nel.org>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Neal Cardwell <ncardwell@...gle.com>,
        netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH net] tcp: Add READ_ONCE() to read tcp_orphan_count

On Thu, May 12, 2022 at 04:43:20PM -0700, Eric Dumazet wrote:
> On Thu, May 12, 2022 at 4:10 PM Paul E. McKenney <paulmck@...nel.org> wrote:
> >
> > On Thu, May 12, 2022 at 02:31:48PM -0700, Eric Dumazet wrote:
> > > On Thu, May 12, 2022 at 2:18 PM Marco Elver <elver@...gle.com> wrote:
> > >
> > > >
> > > > I guess the question is, is it the norm that per_cpu() retrieves data
> > > > that can legally be modified concurrently, or not. If not, and in most
> > > > cases it's a bug, the annotations should be here.
> > > >
> > > > Paul, was there any guidance/documentation on this, but I fail to find
> > > > it right now? (access-marking.txt doesn't say much about per-CPU
> > > > data.)
> > >
> > > Normally, whenever we add a READ_ONCE(), we are supposed to add a comment.
> >
> > I am starting to think that comments are even more necessary for unmarked
> > accesses to shared variables, with the comments setting out why the
> > compiler cannot mess things up.  ;-)
> >
> > > We could make an exception for per_cpu_once(), because the comment
> > > would be centralized
> > > at per_cpu_once() definition.
> >
> > This makes a lot of sense to me.
> >
> > > We will be stuck with READ_ONCE() in places we are using
> > > per_cpu_ptr(), for example
> > > in dev_fetch_sw_netstats()
> >
> > If this is strictly statistics, data_race() is another possibility.
> > But it does not constrain the compiler at all.
> 
> Statistics are supposed to be monotonically increasing ;)
> 
> Some SNMP agents would be very confused if they could observe 'garbage' there.
> 
> I sense that we are going to add thousands of READ_ONCE() soon :/

Indeed, adding READ_ONCE() instances can be annoying.  Then again, it
can also be annoying to have to debug the problems that sometimes arise
from omitting them where they are needed.

							Thanx, Paul

Powered by blists - more mailing lists