* [PATCH AUTOSEL 6.3 04/59] bpf: Annotate data races in bpf_local_storage
[not found] <20230504194142.3805425-1-sashal@kernel.org>
@ 2023-05-04 19:40 ` Sasha Levin
2023-05-04 19:40 ` [PATCH AUTOSEL 6.3 08/59] bpf, mips: Implement DADDI workarounds for JIT Sasha Levin
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Sasha Levin @ 2023-05-04 19:40 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Kumar Kartikeya Dwivedi, Martin KaFai Lau, KP Singh,
Alexei Starovoitov, Sasha Levin, martin.lau, daniel, andrii, bpf
From: Kumar Kartikeya Dwivedi <memxor@gmail.com>
[ Upstream commit 0a09a2f933c73dc76ab0b72da6855f44342a8903 ]
There are a few cases where hlist_node is checked to be unhashed without
holding the lock protecting its modification. In this case, one must use
hlist_unhashed_lockless to avoid load tearing and KCSAN reports. Fix
this by using lockless variant in places not protected by the lock.
Since this is not prompted by any actual KCSAN reports but only from
code review, I have not included a fixes tag.
Cc: Martin KaFai Lau <martin.lau@kernel.org>
Cc: KP Singh <kpsingh@kernel.org>
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
Link: https://lore.kernel.org/r/20230221200646.2500777-4-memxor@gmail.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
kernel/bpf/bpf_local_storage.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/kernel/bpf/bpf_local_storage.c b/kernel/bpf/bpf_local_storage.c
index 35f4138a54dc1..58da17ae51241 100644
--- a/kernel/bpf/bpf_local_storage.c
+++ b/kernel/bpf/bpf_local_storage.c
@@ -51,11 +51,21 @@ owner_storage(struct bpf_local_storage_map *smap, void *owner)
return map->ops->map_owner_storage_ptr(owner);
}
+static bool selem_linked_to_storage_lockless(const struct bpf_local_storage_elem *selem)
+{
+ return !hlist_unhashed_lockless(&selem->snode);
+}
+
static bool selem_linked_to_storage(const struct bpf_local_storage_elem *selem)
{
return !hlist_unhashed(&selem->snode);
}
+static bool selem_linked_to_map_lockless(const struct bpf_local_storage_elem *selem)
+{
+ return !hlist_unhashed_lockless(&selem->map_node);
+}
+
static bool selem_linked_to_map(const struct bpf_local_storage_elem *selem)
{
return !hlist_unhashed(&selem->map_node);
@@ -174,7 +184,7 @@ static void __bpf_selem_unlink_storage(struct bpf_local_storage_elem *selem,
bool free_local_storage = false;
unsigned long flags;
- if (unlikely(!selem_linked_to_storage(selem)))
+ if (unlikely(!selem_linked_to_storage_lockless(selem)))
/* selem has already been unlinked from sk */
return;
@@ -208,7 +218,7 @@ void bpf_selem_unlink_map(struct bpf_local_storage_elem *selem)
struct bpf_local_storage_map_bucket *b;
unsigned long flags;
- if (unlikely(!selem_linked_to_map(selem)))
+ if (unlikely(!selem_linked_to_map_lockless(selem)))
/* selem has already be unlinked from smap */
return;
@@ -420,7 +430,7 @@ bpf_local_storage_update(void *owner, struct bpf_local_storage_map *smap,
err = check_flags(old_sdata, map_flags);
if (err)
return ERR_PTR(err);
- if (old_sdata && selem_linked_to_storage(SELEM(old_sdata))) {
+ if (old_sdata && selem_linked_to_storage_lockless(SELEM(old_sdata))) {
copy_map_value_locked(&smap->map, old_sdata->data,
value, false);
return old_sdata;
--
2.39.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH AUTOSEL 6.3 08/59] bpf, mips: Implement DADDI workarounds for JIT
[not found] <20230504194142.3805425-1-sashal@kernel.org>
2023-05-04 19:40 ` [PATCH AUTOSEL 6.3 04/59] bpf: Annotate data races in bpf_local_storage Sasha Levin
@ 2023-05-04 19:40 ` Sasha Levin
2023-05-05 12:04 ` Jiaxun Yang
2023-05-04 19:41 ` [PATCH AUTOSEL 6.3 31/59] samples/bpf: Fix fout leak in hbm's run_bpf_prog Sasha Levin
2023-05-04 19:41 ` [PATCH AUTOSEL 6.3 32/59] bpf: Add preempt_count_{sub,add} into btf id deny list Sasha Levin
3 siblings, 1 reply; 6+ messages in thread
From: Sasha Levin @ 2023-05-04 19:40 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Jiaxun Yang, Daniel Borkmann, Philippe Mathieu-Daudé,
Johan Almbladh, Sasha Levin, tsbogend, ast, andrii, paulburton,
linux-mips, bpf
From: Jiaxun Yang <jiaxun.yang@flygoat.com>
[ Upstream commit bbefef2f07080cd502a93cb1c529e1c8a6c4ac8e ]
For DADDI errata we just workaround by disable immediate operation
for BPF_ADD / BPF_SUB to avoid generation of DADDIU.
All other use cases in JIT won't cause overflow thus they are all safe.
Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Acked-by: Johan Almbladh <johan.almbladh@anyfinetworks.com>
Link: https://lore.kernel.org/bpf/20230228113305.83751-2-jiaxun.yang@flygoat.com
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/mips/Kconfig | 1 -
arch/mips/net/bpf_jit_comp.c | 4 ++++
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index e2f3ca73f40d6..edc7d8790f1e8 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -64,7 +64,6 @@ config MIPS
select HAVE_DMA_CONTIGUOUS
select HAVE_DYNAMIC_FTRACE
select HAVE_EBPF_JIT if !CPU_MICROMIPS && \
- !CPU_DADDI_WORKAROUNDS && \
!CPU_R4000_WORKAROUNDS && \
!CPU_R4400_WORKAROUNDS
select HAVE_EXIT_THREAD
diff --git a/arch/mips/net/bpf_jit_comp.c b/arch/mips/net/bpf_jit_comp.c
index b17130d510d49..a40d926b65139 100644
--- a/arch/mips/net/bpf_jit_comp.c
+++ b/arch/mips/net/bpf_jit_comp.c
@@ -218,9 +218,13 @@ bool valid_alu_i(u8 op, s32 imm)
/* All legal eBPF values are valid */
return true;
case BPF_ADD:
+ if (IS_ENABLED(CONFIG_CPU_DADDI_WORKAROUNDS))
+ return false;
/* imm must be 16 bits */
return imm >= -0x8000 && imm <= 0x7fff;
case BPF_SUB:
+ if (IS_ENABLED(CONFIG_CPU_DADDI_WORKAROUNDS))
+ return false;
/* -imm must be 16 bits */
return imm >= -0x7fff && imm <= 0x8000;
case BPF_AND:
--
2.39.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH AUTOSEL 6.3 08/59] bpf, mips: Implement DADDI workarounds for JIT
2023-05-04 19:40 ` [PATCH AUTOSEL 6.3 08/59] bpf, mips: Implement DADDI workarounds for JIT Sasha Levin
@ 2023-05-05 12:04 ` Jiaxun Yang
2023-05-18 17:03 ` Sasha Levin
0 siblings, 1 reply; 6+ messages in thread
From: Jiaxun Yang @ 2023-05-05 12:04 UTC (permalink / raw)
To: Sasha Levin
Cc: linux-kernel, stable@vger.kernel.org, Daniel Borkmann,
Philippe Mathieu-Daudé, Johan Almbladh, Thomas Bogendoerfer,
Alexei Starovoitov, Andrii Nakryiko, paulburton@kernel.org,
linux-mips@vger.kernel.org, bpf
> 2023年5月4日 20:40，Sasha Levin <sashal@kernel.org> 写道：
>
> From: Jiaxun Yang <jiaxun.yang@flygoat.com>
>
> [ Upstream commit bbefef2f07080cd502a93cb1c529e1c8a6c4ac8e ]
>
> For DADDI errata we just workaround by disable immediate operation
> for BPF_ADD / BPF_SUB to avoid generation of DADDIU.
>
> All other use cases in JIT won't cause overflow thus they are all safe.
>
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Acked-by: Johan Almbladh <johan.almbladh@anyfinetworks.com>
> Link: https://lore.kernel.org/bpf/20230228113305.83751-2-jiaxun.yang@flygoat.com
> Signed-off-by: Sasha Levin <sashal@kernel.org>
Hi Sasha,
I think this patch should count as a functional improvement instead of regression fix.
Please drop it from stable queue.
Thanks
Jiaxun
> ---
> arch/mips/Kconfig | 1 -
> arch/mips/net/bpf_jit_comp.c | 4 ++++
> 2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
> index e2f3ca73f40d6..edc7d8790f1e8 100644
> --- a/arch/mips/Kconfig
> +++ b/arch/mips/Kconfig
> @@ -64,7 +64,6 @@ config MIPS
> select HAVE_DMA_CONTIGUOUS
> select HAVE_DYNAMIC_FTRACE
> select HAVE_EBPF_JIT if !CPU_MICROMIPS && \
> - !CPU_DADDI_WORKAROUNDS && \
> !CPU_R4000_WORKAROUNDS && \
> !CPU_R4400_WORKAROUNDS
> select HAVE_EXIT_THREAD
> diff --git a/arch/mips/net/bpf_jit_comp.c b/arch/mips/net/bpf_jit_comp.c
> index b17130d510d49..a40d926b65139 100644
> --- a/arch/mips/net/bpf_jit_comp.c
> +++ b/arch/mips/net/bpf_jit_comp.c
> @@ -218,9 +218,13 @@ bool valid_alu_i(u8 op, s32 imm)
> /* All legal eBPF values are valid */
> return true;
> case BPF_ADD:
> + if (IS_ENABLED(CONFIG_CPU_DADDI_WORKAROUNDS))
> + return false;
> /* imm must be 16 bits */
> return imm >= -0x8000 && imm <= 0x7fff;
> case BPF_SUB:
> + if (IS_ENABLED(CONFIG_CPU_DADDI_WORKAROUNDS))
> + return false;
> /* -imm must be 16 bits */
> return imm >= -0x7fff && imm <= 0x8000;
> case BPF_AND:
> --
> 2.39.2
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH AUTOSEL 6.3 08/59] bpf, mips: Implement DADDI workarounds for JIT
2023-05-05 12:04 ` Jiaxun Yang
@ 2023-05-18 17:03 ` Sasha Levin
0 siblings, 0 replies; 6+ messages in thread
From: Sasha Levin @ 2023-05-18 17:03 UTC (permalink / raw)
To: Jiaxun Yang
Cc: linux-kernel, stable@vger.kernel.org, Daniel Borkmann,
Philippe Mathieu-Daudé, Johan Almbladh, Thomas Bogendoerfer,
Alexei Starovoitov, Andrii Nakryiko, paulburton@kernel.org,
linux-mips@vger.kernel.org, bpf
On Fri, May 05, 2023 at 01:04:14PM +0100, Jiaxun Yang wrote:
>
>
>> 2023年5月4日 20:40，Sasha Levin <sashal@kernel.org> 写道：
>>
>> From: Jiaxun Yang <jiaxun.yang@flygoat.com>
>>
>> [ Upstream commit bbefef2f07080cd502a93cb1c529e1c8a6c4ac8e ]
>>
>> For DADDI errata we just workaround by disable immediate operation
>> for BPF_ADD / BPF_SUB to avoid generation of DADDIU.
>>
>> All other use cases in JIT won't cause overflow thus they are all safe.
>>
>> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
>> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> Acked-by: Johan Almbladh <johan.almbladh@anyfinetworks.com>
>> Link: https://lore.kernel.org/bpf/20230228113305.83751-2-jiaxun.yang@flygoat.com
>> Signed-off-by: Sasha Levin <sashal@kernel.org>
>
>Hi Sasha,
>
>I think this patch should count as a functional improvement instead of regression fix.
>
>Please drop it from stable queue.
Dropped, thanks!
--
Thanks,
Sasha
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH AUTOSEL 6.3 31/59] samples/bpf: Fix fout leak in hbm's run_bpf_prog
[not found] <20230504194142.3805425-1-sashal@kernel.org>
2023-05-04 19:40 ` [PATCH AUTOSEL 6.3 04/59] bpf: Annotate data races in bpf_local_storage Sasha Levin
2023-05-04 19:40 ` [PATCH AUTOSEL 6.3 08/59] bpf, mips: Implement DADDI workarounds for JIT Sasha Levin
@ 2023-05-04 19:41 ` Sasha Levin
2023-05-04 19:41 ` [PATCH AUTOSEL 6.3 32/59] bpf: Add preempt_count_{sub,add} into btf id deny list Sasha Levin
3 siblings, 0 replies; 6+ messages in thread
From: Sasha Levin @ 2023-05-04 19:41 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Hao Zeng, Daniel Borkmann, Sasha Levin, ast, andrii, bpf
From: Hao Zeng <zenghao@kylinos.cn>
[ Upstream commit 23acb14af1914010dd0aae1bbb7fab28bf518b8e ]
Fix fout being fopen'ed but then not subsequently fclose'd. In the affected
branch, fout is otherwise going out of scope.
Signed-off-by: Hao Zeng <zenghao@kylinos.cn>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Link: https://lore.kernel.org/bpf/20230411084349.1999628-1-zenghao@kylinos.cn
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
samples/bpf/hbm.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/samples/bpf/hbm.c b/samples/bpf/hbm.c
index 516fbac28b716..7f89700a17b69 100644
--- a/samples/bpf/hbm.c
+++ b/samples/bpf/hbm.c
@@ -315,6 +315,7 @@ static int run_bpf_prog(char *prog, int cg_id)
fout = fopen(fname, "w");
fprintf(fout, "id:%d\n", cg_id);
fprintf(fout, "ERROR: Could not lookup queue_stats\n");
+ fclose(fout);
} else if (stats_flag && qstats.lastPacketTime >
qstats.firstPacketTime) {
long long delta_us = (qstats.lastPacketTime -
--
2.39.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH AUTOSEL 6.3 32/59] bpf: Add preempt_count_{sub,add} into btf id deny list
[not found] <20230504194142.3805425-1-sashal@kernel.org>
` (2 preceding siblings ...)
2023-05-04 19:41 ` [PATCH AUTOSEL 6.3 31/59] samples/bpf: Fix fout leak in hbm's run_bpf_prog Sasha Levin
@ 2023-05-04 19:41 ` Sasha Levin
3 siblings, 0 replies; 6+ messages in thread
From: Sasha Levin @ 2023-05-04 19:41 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Yafang, Steven Rostedt, Masami Hiramatsu, Jiri Olsa, Hao Luo,
Alexei Starovoitov, Sasha Levin, daniel, andrii, bpf
From: Yafang <laoar.shao@gmail.com>
[ Upstream commit c11bd046485d7bf1ca200db0e7d0bdc4bafdd395 ]
The recursion check in __bpf_prog_enter* and __bpf_prog_exit*
leave preempt_count_{sub,add} unprotected. When attaching trampoline to
them we get panic as follows,
[ 867.843050] BUG: TASK stack guard page was hit at 0000000009d325cf (stack is 0000000046a46a15..00000000537e7b28)
[ 867.843064] stack guard page: 0000 [#1] PREEMPT SMP NOPTI
[ 867.843067] CPU: 8 PID: 11009 Comm: trace Kdump: loaded Not tainted 6.2.0+ #4
[ 867.843100] Call Trace:
[ 867.843101] <TASK>
[ 867.843104] asm_exc_int3+0x3a/0x40
[ 867.843108] RIP: 0010:preempt_count_sub+0x1/0xa0
[ 867.843135] __bpf_prog_enter_recur+0x17/0x90
[ 867.843148] bpf_trampoline_6442468108_0+0x2e/0x1000
[ 867.843154] ? preempt_count_sub+0x1/0xa0
[ 867.843157] preempt_count_sub+0x5/0xa0
[ 867.843159] ? migrate_enable+0xac/0xf0
[ 867.843164] __bpf_prog_exit_recur+0x2d/0x40
[ 867.843168] bpf_trampoline_6442468108_0+0x55/0x1000
...
[ 867.843788] preempt_count_sub+0x5/0xa0
[ 867.843793] ? migrate_enable+0xac/0xf0
[ 867.843829] __bpf_prog_exit_recur+0x2d/0x40
[ 867.843837] BUG: IRQ stack guard page was hit at 0000000099bd8228 (stack is 00000000b23e2bc4..000000006d95af35)
[ 867.843841] BUG: IRQ stack guard page was hit at 000000005ae07924 (stack is 00000000ffd69623..0000000014eb594c)
[ 867.843843] BUG: IRQ stack guard page was hit at 00000000028320f0 (stack is 00000000034b6438..0000000078d1bcec)
[ 867.843842] bpf_trampoline_6442468108_0+0x55/0x1000
...
That is because in __bpf_prog_exit_recur, the preempt_count_{sub,add} are
called after prog->active is decreased.
Fixing this by adding these two functions into btf ids deny list.
Suggested-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Yafang <laoar.shao@gmail.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Jiri Olsa <olsajiri@gmail.com>
Acked-by: Hao Luo <haoluo@google.com>
Link: https://lore.kernel.org/r/20230413025248.79764-1-laoar.shao@gmail.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
kernel/bpf/verifier.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 767e8930b0bd5..d8a01180ddf76 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -17551,6 +17551,10 @@ BTF_ID(func, migrate_enable)
#if !defined CONFIG_PREEMPT_RCU && !defined CONFIG_TINY_RCU
BTF_ID(func, rcu_read_unlock_strict)
#endif
+#if defined(CONFIG_DEBUG_PREEMPT) || defined(CONFIG_TRACE_PREEMPT_TOGGLE)
+BTF_ID(func, preempt_count_add)
+BTF_ID(func, preempt_count_sub)
+#endif
BTF_SET_END(btf_id_deny)
static bool can_be_sleepable(struct bpf_prog *prog)
--
2.39.2
^ permalink raw reply related [flat|nested] 6+ messages in thread