* [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