* [PATCH][next] bpf: fix swapped arguments in calls to check_buffer_access
@ 2020-07-27 17:54 ` Colin King
0 siblings, 0 replies; 10+ messages in thread
From: Colin King @ 2020-07-27 17:54 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu,
Yonghong Song, Andrii Nakryiko, John Fastabend, KP Singh, netdev,
bpf
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.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@canonical.com>
---
kernel/bpf/verifier.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index cd14e70f2d07..88bb25d08bf8 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -3477,14 +3477,14 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn
regno, reg_type_str[reg->type]);
return -EACCES;
}
- err = check_buffer_access(env, reg, regno, off, size, "rdonly",
- false,
+ err = check_buffer_access(env, reg, regno, off, size, false,
+ "rdonly",
&env->prog->aux->max_rdonly_access);
if (!err && value_regno >= 0)
mark_reg_unknown(env, regs, value_regno);
} else if (reg->type == PTR_TO_RDWR_BUF) {
- err = check_buffer_access(env, reg, regno, off, size, "rdwr",
- false,
+ err = check_buffer_access(env, reg, regno, off, size, false,
+ "rdwr",
&env->prog->aux->max_rdwr_access);
if (!err && t == BPF_READ && value_regno >= 0)
mark_reg_unknown(env, regs, value_regno);
--
2.27.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH][next] bpf: fix swapped arguments in calls to check_buffer_access
@ 2020-07-27 17:54 ` Colin King
0 siblings, 0 replies; 10+ messages in thread
From: Colin King @ 2020-07-27 17:54 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu,
Yonghong Song, Andrii Nakryiko, John Fastabend, KP Singh, netdev,
bpf
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.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@canonical.com>
---
kernel/bpf/verifier.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index cd14e70f2d07..88bb25d08bf8 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -3477,14 +3477,14 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn
regno, reg_type_str[reg->type]);
return -EACCES;
}
- err = check_buffer_access(env, reg, regno, off, size, "rdonly",
- false,
+ err = check_buffer_access(env, reg, regno, off, size, false,
+ "rdonly",
&env->prog->aux->max_rdonly_access);
if (!err && value_regno >= 0)
mark_reg_unknown(env, regs, value_regno);
} else if (reg->type = PTR_TO_RDWR_BUF) {
- err = check_buffer_access(env, reg, regno, off, size, "rdwr",
- false,
+ err = check_buffer_access(env, reg, regno, off, size, false,
+ "rdwr",
&env->prog->aux->max_rdwr_access);
if (!err && t = BPF_READ && value_regno >= 0)
mark_reg_unknown(env, regs, value_regno);
--
2.27.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH][next] bpf: fix swapped arguments in calls to check_buffer_access
2020-07-27 17:54 ` Colin King
@ 2020-07-27 21:39 ` Yonghong Song
-1 siblings, 0 replies; 10+ messages in thread
From: Yonghong Song @ 2020-07-27 21:39 UTC (permalink / raw)
To: Colin King, Alexei Starovoitov, Daniel Borkmann,
Martin KaFai Lau, Song Liu, Andrii Nakryiko, John Fastabend,
KP Singh, netdev, bpf
Cc: kernel-janitors, linux-kernel
On 7/27/20 10:54 AM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.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@canonical.com>
Thanks for the fix!
Acked-by: Yonghong Song <yhs@fb.com>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH][next] bpf: fix swapped arguments in calls to check_buffer_access
@ 2020-07-27 21:39 ` Yonghong Song
0 siblings, 0 replies; 10+ messages in thread
From: Yonghong Song @ 2020-07-27 21:39 UTC (permalink / raw)
To: Colin King, Alexei Starovoitov, Daniel Borkmann,
Martin KaFai Lau, Song Liu, Andrii Nakryiko, John Fastabend,
KP Singh, netdev, bpf
Cc: kernel-janitors, linux-kernel
On 7/27/20 10:54 AM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.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@canonical.com>
Thanks for the fix!
Acked-by: Yonghong Song <yhs@fb.com>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH][next] bpf: fix swapped arguments in calls to check_buffer_access
2020-07-27 21:39 ` Yonghong Song
@ 2020-07-28 10:43 ` Daniel Borkmann
-1 siblings, 0 replies; 10+ messages in thread
From: Daniel Borkmann @ 2020-07-28 10:43 UTC (permalink / raw)
To: Yonghong Song, Colin King, Alexei Starovoitov, Martin KaFai Lau,
Song Liu, Andrii Nakryiko, John Fastabend, KP Singh, netdev, bpf
Cc: kernel-janitors, linux-kernel
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@canonical.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@canonical.com>
>
> Thanks for the fix!
> Acked-by: Yonghong Song <yhs@fb.com>
Sigh, thanks for the fix Colin, applied! Yonghong, could you follow-up with
BPF selftest test cases that exercise these paths? Thx
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH][next] bpf: fix swapped arguments in calls to check_buffer_access
@ 2020-07-28 10:43 ` Daniel Borkmann
0 siblings, 0 replies; 10+ messages in thread
From: Daniel Borkmann @ 2020-07-28 10:43 UTC (permalink / raw)
To: Yonghong Song, Colin King, Alexei Starovoitov, Martin KaFai Lau,
Song Liu, Andrii Nakryiko, John Fastabend, KP Singh, netdev, bpf
Cc: kernel-janitors, linux-kernel
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@canonical.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@canonical.com>
>
> Thanks for the fix!
> Acked-by: Yonghong Song <yhs@fb.com>
Sigh, thanks for the fix Colin, applied! Yonghong, could you follow-up with
BPF selftest test cases that exercise these paths? Thx
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH][next] bpf: fix swapped arguments in calls to check_buffer_access
2020-07-28 10:43 ` Daniel Borkmann
@ 2020-07-28 10:45 ` Colin Ian King
-1 siblings, 0 replies; 10+ messages in thread
From: Colin Ian King @ 2020-07-28 10:45 UTC (permalink / raw)
To: Daniel Borkmann, Yonghong Song, Alexei Starovoitov,
Martin KaFai Lau, Song Liu, Andrii Nakryiko, John Fastabend,
KP Singh, netdev, bpf
Cc: kernel-janitors, linux-kernel
On 28/07/2020 11:43, 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@canonical.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@canonical.com>
>>
>> Thanks for the fix!
>> Acked-by: Yonghong Song <yhs@fb.com>
>
> Sigh, thanks for the fix Colin, applied! Yonghong, could you follow-up with
> BPF selftest test cases that exercise these paths? Thx
No problem. Thanks to static analysis, it catches a lot of subtle bugs
like this.
Colin
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH][next] bpf: fix swapped arguments in calls to check_buffer_access
@ 2020-07-28 10:45 ` Colin Ian King
0 siblings, 0 replies; 10+ messages in thread
From: Colin Ian King @ 2020-07-28 10:45 UTC (permalink / raw)
To: Daniel Borkmann, Yonghong Song, Alexei Starovoitov,
Martin KaFai Lau, Song Liu, Andrii Nakryiko, John Fastabend,
KP Singh, netdev, bpf
Cc: kernel-janitors, linux-kernel
On 28/07/2020 11:43, 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@canonical.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@canonical.com>
>>
>> Thanks for the fix!
>> Acked-by: Yonghong Song <yhs@fb.com>
>
> Sigh, thanks for the fix Colin, applied! Yonghong, could you follow-up with
> BPF selftest test cases that exercise these paths? Thx
No problem. Thanks to static analysis, it catches a lot of subtle bugs
like this.
Colin
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH][next] bpf: fix swapped arguments in calls to check_buffer_access
2020-07-28 10:43 ` Daniel Borkmann
@ 2020-07-28 19:12 ` Yonghong Song
-1 siblings, 0 replies; 10+ messages in thread
From: Yonghong Song @ 2020-07-28 19:12 UTC (permalink / raw)
To: Daniel Borkmann, Colin King, Alexei Starovoitov,
Martin KaFai Lau, Song Liu, Andrii Nakryiko, John Fastabend,
KP Singh, netdev, bpf
Cc: kernel-janitors, linux-kernel
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@canonical.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@canonical.com>
>>
>> Thanks for the fix!
>> Acked-by: Yonghong Song <yhs@fb.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...
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH][next] bpf: fix swapped arguments in calls to check_buffer_access
@ 2020-07-28 19:12 ` Yonghong Song
0 siblings, 0 replies; 10+ messages in thread
From: Yonghong Song @ 2020-07-28 19:12 UTC (permalink / raw)
To: Daniel Borkmann, Colin King, Alexei Starovoitov,
Martin KaFai Lau, Song Liu, Andrii Nakryiko, John Fastabend,
KP Singh, netdev, bpf
Cc: kernel-janitors, linux-kernel
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@canonical.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@canonical.com>
>>
>> Thanks for the fix!
>> Acked-by: Yonghong Song <yhs@fb.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...
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-07-28 19:13 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-27 17:54 [PATCH][next] bpf: fix swapped arguments in calls to check_buffer_access Colin King
2020-07-27 17:54 ` Colin King
2020-07-27 21:39 ` Yonghong Song
2020-07-27 21:39 ` Yonghong Song
2020-07-28 10:43 ` Daniel Borkmann
2020-07-28 10:43 ` Daniel Borkmann
2020-07-28 10:45 ` Colin Ian King
2020-07-28 10:45 ` Colin Ian King
2020-07-28 19:12 ` Yonghong Song
2020-07-28 19:12 ` Yonghong Song
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.