Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752568AbdHHVUI (ORCPT ); Tue, 8 Aug 2017 17:20:08 -0400 Received: from mail-wr0-f193.google.com ([209.85.128.193]:37155 "EHLO mail-wr0-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752324AbdHHVUG (ORCPT ); Tue, 8 Aug 2017 17:20:06 -0400 Date: Tue, 08 Aug 2017 22:20:05 +0100 In-Reply-To: <20170808.094857.245786887664041622.davem@davemloft.net> References: <20170807222514.24292-1-james.hogan@imgtec.com> <20170807222514.24292-3-james.hogan@imgtec.com> <59897A7C.10009@iogearbox.net> <20170808.094857.245786887664041622.davem@davemloft.net> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Subject: Re: [RFC PATCH 2/2] bpf: Initialise mod[] in bpf_trace_printk To: David Miller , daniel@iogearbox.net CC: ast@kernel.org, linux-kernel@vger.kernel.org, rostedt@goodmis.org, mingo@redhat.com, netdev@vger.kernel.org From: James Hogan Message-ID: <650EB3B0-1101-4624-905A-E68D2EC5920F@imgtec.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from quoted-printable to 8bit by nfs id v78LKL83028700 Content-Length: 2163 Lines: 58 On 8 August 2017 17:48:57 BST, David Miller wrote: >From: Daniel Borkmann >Date: Tue, 08 Aug 2017 10:46:52 +0200 > >> On 08/08/2017 12:25 AM, James Hogan wrote: >>> In bpf_trace_printk(), the elements in mod[] are left uninitialised, >>> but >>> they are then incremented to track the width of the formats. Zero >>> initialise the array just in case the memory contains non-zero >values >>> on >>> entry. >>> >>> Fixes: 9c959c863f82 ("tracing: Allow BPF programs to call >>> bpf_trace_printk()") >>> Signed-off-by: James Hogan >>> Cc: Alexei Starovoitov >>> Cc: Daniel Borkmann >>> Cc: Steven Rostedt >>> Cc: Ingo Molnar >>> Cc: netdev@vger.kernel.org >>> --- >>> When I checked (on MIPS32), the elements tended to have the value >zero >>> anyway (does BPF zero the stack or something clever?), so this is a >>> purely theoretical fix. >>> --- >>> kernel/trace/bpf_trace.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c >>> index 32dcbe1b48f2..86a52857d941 100644 >>> --- a/kernel/trace/bpf_trace.c >>> +++ b/kernel/trace/bpf_trace.c >>> @@ -129,7 +129,7 @@ BPF_CALL_5(bpf_trace_printk, char *, fmt, u32, >>> fmt_size, u64, arg1, >>> u64, arg2, u64, arg3) >>> { >>> bool str_seen = false; >>> - int mod[3] = {}; >>> + int mod[3] = { 0, 0, 0 }; >> >> I'm probably missing something, but is the behavior of gcc wrt >> above initializers different on mips (it zeroes just fine on x86 >> at least)? If yes, we'd probably need a cocci script to also check >> rest of the kernel given this is used in a number of places. Hm, >> could you elaborate? > >This change is not necessary at all. > >An empty initializer must clear the whole object to zero. > >"theoretical" fix indeed... :-( cool, i hadn't realised unmentioned elements in an initialiser are always zeroed, even when non-global/static, so had interpreted the whole array as uninitialised. learn something new every day :-) sorry for the noise. cheers James