Re: FAILED: patch "[PATCH] bpf: Fix toctou on read-only map's constant scalar tracking" failed to apply to 5.4-stable tree

From: Rafael David Tinoco <rafaeldtinoco@gmail.com>
To: Daniel Borkmann <daniel@iogearbox.net>
Cc: Lee Jones <lee.jones@linaro.org>,
	Andrii Nakryiko <andrii@kernel.org>,
	Alexei Starovoitov <ast@kernel.org>,
	stable@vger.kernel.org, Greg KH <gregkh@linuxfoundation.org>
Subject: Re: FAILED: patch "[PATCH] bpf: Fix toctou on read-only map's constant scalar tracking" failed to apply to 5.4-stable tree
Date: Tue, 1 Mar 2022 19:04:40 -0300	[thread overview]
Message-ID: <577A5957-B1ED-41D8-A17C-227E15C23925@gmail.com> (raw)
In-Reply-To: <f01b6557-ed8f-1385-c5f6-95f73b940b7f@iogearbox.net>

>> The bad-commit mentioned in "the Fixes tag":
>> Fixes: a23740ec43ba ("bpf: Track contents of read-only maps as scalars")
>> Which as you say, could well have been fixing another issue.
>> In fact, yes it was:
>> https://lore.kernel.org/stable/20210821203108.215937-2-rafaeldtinoco@gmail.com/
>> Daniel, what do you suggest please?
> 
> Hm, okay, so a23740ec43ba ("bpf: Track contents of read-only maps as scalars") was
> backported to 5.4.144 given Rafael needed it to fix a failing regression test [0].
> 
> Normally, I would have said that we should just revert a23740ec43ba given it was
> not a 'fix' in the first place, but then we are getting into a situation where it
> would break Rafael's now functioning test case again on 5.4.144+ released kernels.
> 

IIRC, Without this patch, eBPF programs with extern variables, either from ksyms
or kconfig relocations, done by libbpf, used as branch conditions, won't work in
<= 5.4.144.

Something like:

extern u32 CONFIG_ARCH_HAS_SYSCALL_WRAPPER __kconfig;
...
if (CONFIG_ARCH_HAS_SYSCALL_WRAPPER) {
   valid BTF type declared/used
} else {
   <dead code>: invalid BTF type declared/used
}
...

The dead code is always evaluated and object load does not pass the verifier.

The workaround to mitigate this is to always rely in type/field existence checks
for the branch conditions, instead of relying in kconfig/ksyms relocations.

We've been doing this to support same CO-RE BPF obj in kernels < 5.4 so I guess
we could continue doing this for 5.4 as well (allowing you to drop this "fix").

Sorry for the burden (about having to introduce another fix, needed because of
that patch). I hope nobody else is relying on it and, if they are, there is a
mitigation described above.

So, feel free to drop it if it's easier for 5.4 maintenance, I'll mitigate
code on our side.

And, Thanks a lot for checking!

> Rafael, given you need this, do you have some cycles to help out Lee on this backport
> for 5.4 stable?
> 
> Thanks guys,
> Daniel
> 
> [0] https://lore.kernel.org/stable/20210821203108.215937-1-rafaeldtinoco@gmail.com/