On Fri, Sep 5, 2014 at 10:17 AM, Daniel Borkmann <dbork...@redhat.com> wrote:
> 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 <mpato...@redhat.com>
>>>>>
>>>>>
>>>>> 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.
imo it's cleaner to convert to bool unconditionally instead
of annotating things everywhere.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
