* [PATCH 43/82] bpf: Refactor intentional wrap-around test
[not found] <20240122235208.work.748-kees@kernel.org>
@ 2024-01-23 0:27 ` Kees Cook
2024-01-23 4:00 ` Yonghong Song
0 siblings, 1 reply; 4+ messages in thread
From: Kees Cook @ 2024-01-23 0:27 UTC (permalink / raw)
To: linux-hardening
Cc: Kees Cook, Alexei Starovoitov, Daniel Borkmann, John Fastabend,
Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, bpf,
Gustavo A. R. Silva, Bill Wendling, Justin Stitt, linux-kernel
In an effort to separate intentional arithmetic wrap-around from
unexpected wrap-around, we need to refactor places that depend on this
kind of math. One of the most common code patterns of this is:
VAR + value < VAR
Notably, this is considered "undefined behavior" for signed and pointer
types, which the kernel works around by using the -fno-strict-overflow
option in the build[1] (which used to just be -fwrapv). Regardless, we
want to get the kernel source to the position where we can meaningfully
instrument arithmetic wrap-around conditions and catch them when they
are unexpected, regardless of whether they are signed[2], unsigned[3],
or pointer[4] types.
Refactor open-coded wrap-around addition test to use add_would_overflow().
This paves the way to enabling the wrap-around sanitizers in the future.
Link: https://git.kernel.org/linus/68df3755e383e6fecf2354a67b08f92f18536594 [1]
Link: https://github.com/KSPP/linux/issues/26 [2]
Link: https://github.com/KSPP/linux/issues/27 [3]
Link: https://github.com/KSPP/linux/issues/344 [4]
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: John Fastabend <john.fastabend@gmail.com>
Cc: Andrii Nakryiko <andrii@kernel.org>
Cc: Martin KaFai Lau <martin.lau@linux.dev>
Cc: Song Liu <song@kernel.org>
Cc: Yonghong Song <yonghong.song@linux.dev>
Cc: KP Singh <kpsingh@kernel.org>
Cc: Stanislav Fomichev <sdf@google.com>
Cc: Hao Luo <haoluo@google.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: bpf@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
kernel/bpf/verifier.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 65f598694d55..21e3f30c8757 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -12901,8 +12901,8 @@ static int adjust_ptr_min_max_vals(struct bpf_verifier_env *env,
dst_reg->smin_value = smin_ptr + smin_val;
dst_reg->smax_value = smax_ptr + smax_val;
}
- if (umin_ptr + umin_val < umin_ptr ||
- umax_ptr + umax_val < umax_ptr) {
+ if (add_would_overflow(umin_ptr, umin_val) ||
+ add_would_overflow(umax_ptr, umax_val)) {
dst_reg->umin_value = 0;
dst_reg->umax_value = U64_MAX;
} else {
@@ -13023,8 +13023,8 @@ static void scalar32_min_max_add(struct bpf_reg_state *dst_reg,
dst_reg->s32_min_value += smin_val;
dst_reg->s32_max_value += smax_val;
}
- if (dst_reg->u32_min_value + umin_val < umin_val ||
- dst_reg->u32_max_value + umax_val < umax_val) {
+ if (add_would_overflow(umin_val, dst_reg->u32_min_value) ||
+ add_would_overflow(umax_val, dst_reg->u32_max_value)) {
dst_reg->u32_min_value = 0;
dst_reg->u32_max_value = U32_MAX;
} else {
@@ -13049,8 +13049,8 @@ static void scalar_min_max_add(struct bpf_reg_state *dst_reg,
dst_reg->smin_value += smin_val;
dst_reg->smax_value += smax_val;
}
- if (dst_reg->umin_value + umin_val < umin_val ||
- dst_reg->umax_value + umax_val < umax_val) {
+ if (add_would_overflow(umin_val, dst_reg->umin_value) ||
+ add_would_overflow(umax_val, dst_reg->umax_value)) {
dst_reg->umin_value = 0;
dst_reg->umax_value = U64_MAX;
} else {
--
2.34.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 43/82] bpf: Refactor intentional wrap-around test
2024-01-23 0:27 ` [PATCH 43/82] bpf: Refactor intentional wrap-around test Kees Cook
@ 2024-01-23 4:00 ` Yonghong Song
2024-01-23 4:07 ` Kees Cook
0 siblings, 1 reply; 4+ messages in thread
From: Yonghong Song @ 2024-01-23 4:00 UTC (permalink / raw)
To: Kees Cook, linux-hardening
Cc: Alexei Starovoitov, Daniel Borkmann, John Fastabend,
Andrii Nakryiko, Martin KaFai Lau, Song Liu, KP Singh,
Stanislav Fomichev, Hao Luo, Jiri Olsa, bpf, Gustavo A. R. Silva,
Bill Wendling, Justin Stitt, linux-kernel
On 1/22/24 4:27 PM, Kees Cook wrote:
> In an effort to separate intentional arithmetic wrap-around from
> unexpected wrap-around, we need to refactor places that depend on this
> kind of math. One of the most common code patterns of this is:
>
> VAR + value < VAR
>
> Notably, this is considered "undefined behavior" for signed and pointer
> types, which the kernel works around by using the -fno-strict-overflow
> option in the build[1] (which used to just be -fwrapv). Regardless, we
> want to get the kernel source to the position where we can meaningfully
> instrument arithmetic wrap-around conditions and catch them when they
> are unexpected, regardless of whether they are signed[2], unsigned[3],
> or pointer[4] types.
>
> Refactor open-coded wrap-around addition test to use add_would_overflow().
> This paves the way to enabling the wrap-around sanitizers in the future.
>
> Link: https://git.kernel.org/linus/68df3755e383e6fecf2354a67b08f92f18536594 [1]
> Link: https://github.com/KSPP/linux/issues/26 [2]
> Link: https://github.com/KSPP/linux/issues/27 [3]
> Link: https://github.com/KSPP/linux/issues/344 [4]
> Cc: Alexei Starovoitov <ast@kernel.org>
> Cc: Daniel Borkmann <daniel@iogearbox.net>
> Cc: John Fastabend <john.fastabend@gmail.com>
> Cc: Andrii Nakryiko <andrii@kernel.org>
> Cc: Martin KaFai Lau <martin.lau@linux.dev>
> Cc: Song Liu <song@kernel.org>
> Cc: Yonghong Song <yonghong.song@linux.dev>
> Cc: KP Singh <kpsingh@kernel.org>
> Cc: Stanislav Fomichev <sdf@google.com>
> Cc: Hao Luo <haoluo@google.com>
> Cc: Jiri Olsa <jolsa@kernel.org>
> Cc: bpf@vger.kernel.org
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
> kernel/bpf/verifier.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 65f598694d55..21e3f30c8757 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -12901,8 +12901,8 @@ static int adjust_ptr_min_max_vals(struct bpf_verifier_env *env,
> dst_reg->smin_value = smin_ptr + smin_val;
> dst_reg->smax_value = smax_ptr + smax_val;
> }
> - if (umin_ptr + umin_val < umin_ptr ||
> - umax_ptr + umax_val < umax_ptr) {
> + if (add_would_overflow(umin_ptr, umin_val) ||
> + add_would_overflow(umax_ptr, umax_val)) {
Maybe you could give a reference to the definition of add_would_overflow()?
A link or a patch with add_would_overflow() defined cc'ed to bpf program.
The patch itselfs looks good to me.
> dst_reg->umin_value = 0;
> dst_reg->umax_value = U64_MAX;
> } else {
> @@ -13023,8 +13023,8 @@ static void scalar32_min_max_add(struct bpf_reg_state *dst_reg,
> dst_reg->s32_min_value += smin_val;
> dst_reg->s32_max_value += smax_val;
> }
> - if (dst_reg->u32_min_value + umin_val < umin_val ||
> - dst_reg->u32_max_value + umax_val < umax_val) {
> + if (add_would_overflow(umin_val, dst_reg->u32_min_value) ||
> + add_would_overflow(umax_val, dst_reg->u32_max_value)) {
> dst_reg->u32_min_value = 0;
> dst_reg->u32_max_value = U32_MAX;
> } else {
> @@ -13049,8 +13049,8 @@ static void scalar_min_max_add(struct bpf_reg_state *dst_reg,
> dst_reg->smin_value += smin_val;
> dst_reg->smax_value += smax_val;
> }
> - if (dst_reg->umin_value + umin_val < umin_val ||
> - dst_reg->umax_value + umax_val < umax_val) {
> + if (add_would_overflow(umin_val, dst_reg->umin_value) ||
> + add_would_overflow(umax_val, dst_reg->umax_value)) {
> dst_reg->umin_value = 0;
> dst_reg->umax_value = U64_MAX;
> } else {
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 43/82] bpf: Refactor intentional wrap-around test
2024-01-23 4:00 ` Yonghong Song
@ 2024-01-23 4:07 ` Kees Cook
2024-01-23 5:13 ` Yonghong Song
0 siblings, 1 reply; 4+ messages in thread
From: Kees Cook @ 2024-01-23 4:07 UTC (permalink / raw)
To: Yonghong Song, Kees Cook, linux-hardening
Cc: Alexei Starovoitov, Daniel Borkmann, John Fastabend,
Andrii Nakryiko, Martin KaFai Lau, Song Liu, KP Singh,
Stanislav Fomichev, Hao Luo, Jiri Olsa, bpf, Gustavo A. R. Silva,
Bill Wendling, Justin Stitt, linux-kernel
On January 22, 2024 8:00:26 PM PST, Yonghong Song <yonghong.song@linux.dev> wrote:
>
>On 1/22/24 4:27 PM, Kees Cook wrote:
>> In an effort to separate intentional arithmetic wrap-around from
>> unexpected wrap-around, we need to refactor places that depend on this
>> kind of math. One of the most common code patterns of this is:
>>
>> VAR + value < VAR
>>
>> Notably, this is considered "undefined behavior" for signed and pointer
>> types, which the kernel works around by using the -fno-strict-overflow
>> option in the build[1] (which used to just be -fwrapv). Regardless, we
>> want to get the kernel source to the position where we can meaningfully
>> instrument arithmetic wrap-around conditions and catch them when they
>> are unexpected, regardless of whether they are signed[2], unsigned[3],
>> or pointer[4] types.
>>
>> Refactor open-coded wrap-around addition test to use add_would_overflow().
>> This paves the way to enabling the wrap-around sanitizers in the future.
>>
>> Link: https://git.kernel.org/linus/68df3755e383e6fecf2354a67b08f92f18536594 [1]
>> Link: https://github.com/KSPP/linux/issues/26 [2]
>> Link: https://github.com/KSPP/linux/issues/27 [3]
>> Link: https://github.com/KSPP/linux/issues/344 [4]
>> Cc: Alexei Starovoitov <ast@kernel.org>
>> Cc: Daniel Borkmann <daniel@iogearbox.net>
>> Cc: John Fastabend <john.fastabend@gmail.com>
>> Cc: Andrii Nakryiko <andrii@kernel.org>
>> Cc: Martin KaFai Lau <martin.lau@linux.dev>
>> Cc: Song Liu <song@kernel.org>
>> Cc: Yonghong Song <yonghong.song@linux.dev>
>> Cc: KP Singh <kpsingh@kernel.org>
>> Cc: Stanislav Fomichev <sdf@google.com>
>> Cc: Hao Luo <haoluo@google.com>
>> Cc: Jiri Olsa <jolsa@kernel.org>
>> Cc: bpf@vger.kernel.org
>> Signed-off-by: Kees Cook <keescook@chromium.org>
>> ---
>> kernel/bpf/verifier.c | 12 ++++++------
>> 1 file changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index 65f598694d55..21e3f30c8757 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -12901,8 +12901,8 @@ static int adjust_ptr_min_max_vals(struct bpf_verifier_env *env,
>> dst_reg->smin_value = smin_ptr + smin_val;
>> dst_reg->smax_value = smax_ptr + smax_val;
>> }
>> - if (umin_ptr + umin_val < umin_ptr ||
>> - umax_ptr + umax_val < umax_ptr) {
>> + if (add_would_overflow(umin_ptr, umin_val) ||
>> + add_would_overflow(umax_ptr, umax_val)) {
>
>Maybe you could give a reference to the definition of add_would_overflow()?
>A link or a patch with add_would_overflow() defined cc'ed to bpf program.
Sure! It was earlier in the series:
https://lore.kernel.org/linux-hardening/20240123002814.1396804-2-keescook@chromium.org/
The cover letter also has more details:
https://lore.kernel.org/linux-hardening/20240122235208.work.748-kees@kernel.org/
>The patch itselfs looks good to me.
Thanks!
-Kees
--
Kees Cook
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 43/82] bpf: Refactor intentional wrap-around test
2024-01-23 4:07 ` Kees Cook
@ 2024-01-23 5:13 ` Yonghong Song
0 siblings, 0 replies; 4+ messages in thread
From: Yonghong Song @ 2024-01-23 5:13 UTC (permalink / raw)
To: Kees Cook, Kees Cook, linux-hardening
Cc: Alexei Starovoitov, Daniel Borkmann, John Fastabend,
Andrii Nakryiko, Martin KaFai Lau, Song Liu, KP Singh,
Stanislav Fomichev, Hao Luo, Jiri Olsa, bpf, Gustavo A. R. Silva,
Bill Wendling, Justin Stitt, linux-kernel
On 1/22/24 8:07 PM, Kees Cook wrote:
>
> On January 22, 2024 8:00:26 PM PST, Yonghong Song <yonghong.song@linux.dev> wrote:
>> On 1/22/24 4:27 PM, Kees Cook wrote:
>>> In an effort to separate intentional arithmetic wrap-around from
>>> unexpected wrap-around, we need to refactor places that depend on this
>>> kind of math. One of the most common code patterns of this is:
>>>
>>> VAR + value < VAR
>>>
>>> Notably, this is considered "undefined behavior" for signed and pointer
>>> types, which the kernel works around by using the -fno-strict-overflow
>>> option in the build[1] (which used to just be -fwrapv). Regardless, we
>>> want to get the kernel source to the position where we can meaningfully
>>> instrument arithmetic wrap-around conditions and catch them when they
>>> are unexpected, regardless of whether they are signed[2], unsigned[3],
>>> or pointer[4] types.
>>>
>>> Refactor open-coded wrap-around addition test to use add_would_overflow().
>>> This paves the way to enabling the wrap-around sanitizers in the future.
>>>
>>> Link: https://git.kernel.org/linus/68df3755e383e6fecf2354a67b08f92f18536594 [1]
>>> Link: https://github.com/KSPP/linux/issues/26 [2]
>>> Link: https://github.com/KSPP/linux/issues/27 [3]
>>> Link: https://github.com/KSPP/linux/issues/344 [4]
>>> Cc: Alexei Starovoitov <ast@kernel.org>
>>> Cc: Daniel Borkmann <daniel@iogearbox.net>
>>> Cc: John Fastabend <john.fastabend@gmail.com>
>>> Cc: Andrii Nakryiko <andrii@kernel.org>
>>> Cc: Martin KaFai Lau <martin.lau@linux.dev>
>>> Cc: Song Liu <song@kernel.org>
>>> Cc: Yonghong Song <yonghong.song@linux.dev>
>>> Cc: KP Singh <kpsingh@kernel.org>
>>> Cc: Stanislav Fomichev <sdf@google.com>
>>> Cc: Hao Luo <haoluo@google.com>
>>> Cc: Jiri Olsa <jolsa@kernel.org>
>>> Cc: bpf@vger.kernel.org
>>> Signed-off-by: Kees Cook <keescook@chromium.org>
>>> ---
>>> kernel/bpf/verifier.c | 12 ++++++------
>>> 1 file changed, 6 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>>> index 65f598694d55..21e3f30c8757 100644
>>> --- a/kernel/bpf/verifier.c
>>> +++ b/kernel/bpf/verifier.c
>>> @@ -12901,8 +12901,8 @@ static int adjust_ptr_min_max_vals(struct bpf_verifier_env *env,
>>> dst_reg->smin_value = smin_ptr + smin_val;
>>> dst_reg->smax_value = smax_ptr + smax_val;
>>> }
>>> - if (umin_ptr + umin_val < umin_ptr ||
>>> - umax_ptr + umax_val < umax_ptr) {
>>> + if (add_would_overflow(umin_ptr, umin_val) ||
>>> + add_would_overflow(umax_ptr, umax_val)) {
>> Maybe you could give a reference to the definition of add_would_overflow()?
>> A link or a patch with add_would_overflow() defined cc'ed to bpf program.
> Sure! It was earlier in the series:
> https://lore.kernel.org/linux-hardening/20240123002814.1396804-2-keescook@chromium.org/
>
> The cover letter also has more details:
> https://lore.kernel.org/linux-hardening/20240122235208.work.748-kees@kernel.org/
Thanks for the pointer.
Acked-by: Yonghong Song <yonghong.song@linux.dev>
>
>> The patch itselfs looks good to me.
> Thanks!
>
> -Kees
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-01-23 5:13 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20240122235208.work.748-kees@kernel.org>
2024-01-23 0:27 ` [PATCH 43/82] bpf: Refactor intentional wrap-around test Kees Cook
2024-01-23 4:00 ` Yonghong Song
2024-01-23 4:07 ` Kees Cook
2024-01-23 5:13 ` Yonghong Song
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox