Message-ID: <CAADnVQJ+OVPnBz8z3vNu8gKXX42jCUqfuvhWAyCQDu8N_yqqwQ@mail.gmail.com>
Date:   Thu, 10 Feb 2022 14:34:57 -0800
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     Yonghong Song <yhs@...com>, "Connor O'Brien" <connoro@...gle.com>
Cc:     Michal Suchánek <msuchanek@...e.de>,
        Shung-Hsi Yu <shung-hsi.yu@...e.com>,
        bpf <bpf@...r.kernel.org>,
        Network Development <netdev@...r.kernel.org>,
        Andrii Nakryiko <andrii@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Alexei Starovoitov <ast@...nel.org>
Subject: Re: BTF compatibility issue across builds

On Thu, Feb 10, 2022 at 10:17 AM Yonghong Song <yhs@...com> wrote:
>
>
>
> On 2/10/22 2:01 AM, Michal Suchánek wrote:
> > Hello,
> >
> > On Mon, Jan 31, 2022 at 09:36:44AM -0800, Yonghong Song wrote:
> >>
> >>
> >> On 1/27/22 7:10 AM, Shung-Hsi Yu wrote:
> >>> Hi,
> >>>
> >>> We recently run into module load failure related to split BTF on openSUSE
> >>> Tumbleweed[1], which I believe is something that may also happen on other
> >>> rolling distros.
> >>>
> >>> The error looks like the follow (though failure is not limited to ipheth)
> >>>
> >>>       BPF:[103111] STRUCT BPF:size=152 vlen=2 BPF: BPF:Invalid name BPF:
> >>>
> >>>       failed to validate module [ipheth] BTF: -22
> >>>
> >>> The error comes down to trying to load BTF of *kernel modules from a
> >>> different build* than the runtime kernel (but the source is the same), where
> >>> the base BTF of the two build is different.
> >>>
> >>> While it may be too far stretched to call this a bug, solving this might
> >>> make BTF adoption easier. I'd natively think that we could further split
> >>> base BTF into two part to avoid this issue, where .BTF only contain exported
> >>> types, and the other (still residing in vmlinux) holds the unexported types.
> >>
> >> What is the exported types? The types used by export symbols?
> >> This for sure will increase btf handling complexity.
> >
> > And it will not actually help.
> >
> > We have modversion ABI which checks the checksum of the symbols that the
> > module imports and fails the load if the checksum for these symbols does
> > not match. It's not concerned with symbols not exported, it's not
> > concerned with symbols not used by the module. This is something that is
> > sustainable across kernel rebuilds with minor fixes/features and what
> > distributions watch for.
> >
> > Now with BTF the situation is vastly different. There are at least three
> > bugs:
> >
> >   - The BTF check is global for all symbols, not for the symbols the
> >     module uses. This is not sustainable. Given the BTF is supposed to
> >     allow linking BPF programs that were built in completely different
> >     environment with the kernel it is completely within the scope of BTF
> >     to solve this problem, it's just neglected.
> >   - It is possible to load modules with no BTF but not modules with
> >     non-matching BTF. Surely the non-matching BTF could be discarded.
> >   - BTF is part of vermagic. This is completely pointless since modules
> >     without BTF can be loaded on BTF kernel. Surely it would not be too
> >     difficult to do the reverse as well. Given BTF must pass extra check
> >     to be used having it in vermagic is just useless moise.
> >
> >>> Does that sound like something reasonable to work on?
> >>>
> >>>
> >>> ## Root case (in case anyone is interested in a verbose version)
> >>>
> >>> On openSUSE Tumbleweed there can be several builds of the same source. Since
> >>> the source is the same, the binaries are simply replaced when a package with
> >>> a larger build number is installed during upgrade.
> >>>
> >>> In our case, a rebuild is triggered[2], and resulted in changes in base BTF.
> >>> More precisely, the BTF_KIND_FUNC{,_PROTO} of i2c_smbus_check_pec(u8 cpec,
> >>> struct i2c_msg *msg) and inet_lhash2_bucket_sk(struct inet_hashinfo *h,
> >>> struct sock *sk) was added to the base BTF of 5.15.12-1.3. Those functions
> >>> are previously missing in base BTF of 5.15.12-1.1.
> >>
> >> As stated in [2] below, I think we should understand why rebuild is
> >> triggered. If the rebuild for vmlinux is triggered, why the modules cannot
> >> be rebuild at the same time?
> >
> > They do get rebuilt. However, if you are running the kernel and install
> > the update you get the new modules with the old kernel. If the install
> > script fails to copy the kernel to your EFI partition based on the fact
> > a kernel with the same filename is alreasy there you get the same.
> >
> > If you have 'stable' distribution adding new symbols is normal and it
> > does not break module loading without BTF but it breaks BTF.
>
> Okay, I see. One possible solution is that if kernel module btf
> does not match vmlinux btf, the kernel module btf will be ignored
> with a dmesg warning but kernel module load will proceed as normal.
> I think this might be also useful for bpf lskel kernel modules as
> well which tries to be portable (with CO-RE) for different kernels.

That sounds like #2 that Michal is proposing:
"It is possible to load modules with no BTF but not modules with
 non-matching BTF. Surely the non-matching BTF could be discarded."

That's probably the simplest way forward.

The patch
https://patchwork.kernel.org/project/netdevbpf/patch/20220209052141.140063-1-connoro@google.com/
shouldn't be necessary too.

Powered by blists - more mailing lists