Re: [PATCH] bpf: fix a false positive kmemcheck warning
From: Daniel Borkmann
Date: Fri Sep 05 2014 - 13:17:52 EST
On 09/05/2014 07:13 PM, Mikulas Patocka wrote:
On Fri, 5 Sep 2014, Daniel Borkmann wrote:
On 09/05/2014 07:00 PM, Hannes Frederic Sowa wrote:
On Fr, 2014-09-05 at 18:20 +0200, Daniel Borkmann wrote:
Hi Mikulas,
On 09/05/2014 06:01 PM, Mikulas Patocka wrote:
This patch fixes false positive kmemcheck warning in bpf.
When we try to write the variable len, the compiler generates a code
that
reads the 32-bit word, modifies the bits belonging to "len" and writes
the
32-bit word back. The reading of the word results in kmemcheck warning
due
to reading uninitialized memory. This patch fixes it by avoiding using
bit
fields when kmemcheck is enabled.
Signed-off-by: Mikulas Patocka <mpatocka@xxxxxxxxxx>
You need to submit this patch to netdev (Cc'ed).
---
include/linux/filter.h | 5 +++++
1 file changed, 5 insertions(+)
Index: linux-2.6/include/linux/filter.h
===================================================================
--- linux-2.6.orig/include/linux/filter.h 2014-09-04 23:04:26.000000000
+0200
+++ linux-2.6/include/linux/filter.h 2014-09-04 23:43:05.000000000
+0200
@@ -325,8 +325,13 @@ struct sock;
struct seccomp_data;
struct bpf_prog {
+#ifdef CONFIG_KMEMCHECK
+ bool jited;
+ u32 len;
+#else
u32 jited:1, /* Is our filter
JIT'ed? */
len:31; /* Number of filter
blocks */
+#endif
struct sock_fprog_kern *orig_prog; /* Original BPF
program */
unsigned int (*bpf_func)(const struct sk_buff *skb,
const struct bpf_insn
*filter);
I don't really like this if-def. If you really want to fix it, can't
you just use :
kmemcheck_bitfield_begin(bpf_anc_data)
...
kmemcheck_bitfield_end(bpf_anc_data)
you also need to annotate the bitfield after allocation:
struct bpf_prog *prog = kalloc(...);
kmemcheck_annotate_bitfield(prog, bpf_anc_data);
Yes, sure, sorry if that was not clear from my side, that was what I
intended to say with kmemcheck /infrastructure/. :)
So, change it to use these markings. I'm not an expert in this area, so I
don't know all the places where this structure could be allocated. If you
know them all, mark it in this way.
Ok, fine by me. I have some pending items, so I'll put it
on top of them.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/