Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20221022110529.51857-1-liuxin350@huawei.com>
In-Reply-To: <20221022110529.51857-1-liuxin350@huawei.com>
From:   Andrii Nakryiko <andrii.nakryiko@gmail.com>
Date:   Mon, 24 Oct 2022 11:27:51 -0700
Message-ID: <CAEf4BzbhhcvAn4yCdHUZZSLCLotywRM5e_-12sYSjg7h8nbdmQ@mail.gmail.com>
Subject: Re: [PATCH] libbpf: glob_sym may be a NULL pointer and cause the
 program crash
To:     Xin Liu <liuxin350@huawei.com>
Cc:     ast@kernel.org, daniel@iogearbox.net, andrii@kernel.org,
        martin.lau@linux.dev, song@kernel.org, yhs@fb.com,
        john.fastabend@gmail.com, kpsingh@kernel.org, sdf@google.com,
        haoluo@google.com, jolsa@kernel.org, bpf@vger.kernel.org,
        linux-kernel@vger.kernel.org, yanan@huawei.com,
        wuchangye@huawei.com, xiesongyang@huawei.com, zhudi2@huawei.com,
        kongweibin2@huawei.com
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Sat, Oct 22, 2022 at 4:05 AM Xin Liu <liuxin350@huawei.com> wrote:
>
> I found that `glob_sym` does not check whether it is NULL when reading the
> code. `glob_sym` obtains the pointer of btf information in the linker from
> `find_glob_sym`, which may be return NULL pointer. However, the code then
> references `glob_sym->sec_id`. This may cause program to crash.
>

May cause a crash or did you actually see an example of such a crash?

As far as I can see from the code, such global_sym is guaranteed to
exist, see how btf_type_map is filled in linker_append_btf(), slightly
above the code you are trying to fix


> Fixes: a46349227cd8 ("libbpf: Add linker extern resolution support for functions and global variables")
> Signed-off-by: Xin Liu <liuxin350@huawei.com>
> Signed-off-by: Weibin Kong <kongweibin2@huawei.com>
> ---
>  tools/lib/bpf/linker.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/tools/lib/bpf/linker.c b/tools/lib/bpf/linker.c
> index 4ac02c28e152..d02d2754910f 100644
> --- a/tools/lib/bpf/linker.c
> +++ b/tools/lib/bpf/linker.c
> @@ -2355,6 +2355,11 @@ static int linker_append_btf(struct bpf_linker *linker, struct src_obj *obj)
>                         if (btf_is_non_static(t)) {
>                                 name = btf__str_by_offset(linker->btf, t->name_off);
>                                 glob_sym = find_glob_sym(linker, name);
> +                               if (!glob_sym) {
> +                                       pr_warn("global '%s': section mismatch %d\n", name,
> +                                               dst_sec->id);
> +                                       return -EINVAL;
> +                               }
>                                 if (glob_sym->sec_id != dst_sec->id) {
>                                         pr_warn("global '%s': section mismatch %d vs %d\n",
>                                                 name, glob_sym->sec_id, dst_sec->id);
> --
> 2.33.0
>