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] [thread-next>] [day] [month] [year] [list]
Date:   Sat, 23 Feb 2019 12:34:50 -0800
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     Eric Dumazet <eric.dumazet@...il.com>
Cc:     Alexei Starovoitov <ast@...nel.org>, davem@...emloft.net,
        daniel@...earbox.net, edumazet@...gle.com, netdev@...r.kernel.org,
        bpf@...r.kernel.org, kernel-team@...com
Subject: Re: [PATCH v2 bpf-next 1/4] bpf: enable program stats

On Sat, Feb 23, 2019 at 10:36:48AM -0800, Eric Dumazet wrote:
> 
> 
> On 02/23/2019 09:44 AM, Alexei Starovoitov wrote:
> 
> ...
> 
> >  
> > -#define BPF_PROG_RUN(filter, ctx)  ({ cant_sleep(); (*(filter)->bpf_func)(ctx, (filter)->insnsi); })
> > +DECLARE_STATIC_KEY_FALSE(bpf_stats_enabled_key);
> > +
> > +#define BPF_PROG_RUN(prog, ctx)	({				\
> > +	u32 ret;						\
> > +	cant_sleep();						\
> > +	if (static_branch_unlikely(&bpf_stats_enabled_key)) {	\
> > +		struct bpf_prog_stats *stats;			\
> > +		u64 start = sched_clock();			\
> > +		ret = (*(prog)->bpf_func)(ctx, (prog)->insnsi);	\
> > +		stats = this_cpu_ptr(prog->aux->stats);		\
> > +		u64_stats_update_begin(&stats->syncp);		\
> > +		stats->cnt++;					\
> > +		stats->nsecs += sched_clock() - start;		\
> > +		u64_stats_update_end(&stats->syncp);		\
> > +	} else {						\
> > +		ret = (*(prog)->bpf_func)(ctx, (prog)->insnsi);	\
> > +	}							\
> > +	ret; })
> > 
> 
> It seems a cpu running there could still be interrupted (by an interrupt)
> and re-enter this section ?
> 
> If yes, u64_stats_update_begin() and u64_stats_update_end() are unsafe (on 32bit arches)
> 
> u64_stats_update_{begin|end}() assume proper locking, since they use a simple increment.
> 
> But then, even on 64bit arches, the stats->{cnt|nsecs} updates are unsafe ?

No. It cannot reenter for the same program.
socket filter prog can be interrupted by kprobe prog, but they are different
progs and different stats.

Powered by blists - more mailing lists