netdev - Re: [PATCH][next] bpf: fix swapped arguments in calls to check_buffer

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] [day] [month] [year] [list]

Date:   Tue, 28 Jul 2020 12:12:47 -0700
From:   Yonghong Song <yhs@...com>
To:     Daniel Borkmann <daniel@...earbox.net>,
        Colin King <colin.king@...onical.com>,
        Alexei Starovoitov <ast@...nel.org>,
        Martin KaFai Lau <kafai@...com>,
        Song Liu <songliubraving@...com>,
        Andrii Nakryiko <andriin@...com>,
        John Fastabend <john.fastabend@...il.com>,
        KP Singh <kpsingh@...omium.org>, <netdev@...r.kernel.org>,
        <bpf@...r.kernel.org>
CC:     <kernel-janitors@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH][next] bpf: fix swapped arguments in calls to
 check_buffer_access



On 7/28/20 3:43 AM, Daniel Borkmann wrote:
> On 7/27/20 11:39 PM, Yonghong Song wrote:
>> On 7/27/20 10:54 AM, Colin King wrote:
>>> From: Colin Ian King <colin.king@...onical.com>
>>>
>>> There are a couple of arguments of the boolean flag zero_size_allowed
>>> and the char pointer buf_info when calling to function 
>>> check_buffer_access
>>> that are swapped by mistake. Fix these by swapping them to correct
>>> the argument ordering.
>>>
>>> Addresses-Coverity: ("Array compared to 0")
>>> Fixes: afbf21dce668 ("bpf: Support readonly/readwrite buffers in 
>>> verifier")
>>> Signed-off-by: Colin Ian King <colin.king@...onical.com>
>>
>> Thanks for the fix!
>> Acked-by: Yonghong Song <yhs@...com>
> 
> Sigh, thanks for the fix Colin, applied! Yonghong, could you follow-up with
> BPF selftest test cases that exercise these paths? Thx

This will be triggered with a verifier rejection path, e.g., negative 
offset from the base. I will send a follow-up patch soon.

BTW, using llvm to build the kernel (without this change), the compiler
actually issues a warning:

-bash-4.4$ make -j100 LLVM=1 && make LLVM=1 vmlinux
   GEN     Makefile
...
   CC      kernel/bpf/verifier.o
/data/users/yhs/work/net-next/kernel/bpf/verifier.c:3481:18: warning: 
expression which evaluates to zero treate$
  as a null pointer constant of type 'const char *' 
[-Wnon-literal-null-conversion]
                                           "rdonly", false,
                                                     ^~~~~
/data/users/yhs/work/net-next/kernel/bpf/verifier.c:3487:16: warning: 
expression which evaluates to zero treate$
  as a null pointer constant of type 'const char *' 
[-Wnon-literal-null-conversion]
                                           "rdwr", false,
                                                   ^~~~~
2 warnings generated.
   AR      kernel/bpf/built-in.a

Looks like I need to use LLVM compiler more often...