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;
Date:   Thu, 8 Jun 2023 15:05:42 -0700
From:   Krister Johansen <kjlx@templeofstupid.com>
To:     Yonghong Song <yhs@meta.com>
Cc:     bpf@vger.kernel.org, Alexei Starovoitov <ast@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>,
        Andrii Nakryiko <andrii@kernel.org>,
        Martin KaFai Lau <martin.lau@linux.dev>,
        Song Liu <song@kernel.org>, Yonghong Song <yhs@fb.com>,
        John Fastabend <john.fastabend@gmail.com>,
        KP Singh <kpsingh@kernel.org>,
        Stanislav Fomichev <sdf@google.com>,
        Hao Luo <haoluo@google.com>, Jiri Olsa <jolsa@kernel.org>,
        Mykola Lysenko <mykolal@fb.com>, Shuah Khan <shuah@kernel.org>,
        linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org,
        stable@vger.kernel.org
Subject: Re: [PATCH bpf v2 2/2] bpf: ensure main program has an extable
Message-ID: <20230608220542.GA2057@templeofstupid.com>
References: <cover.1686166633.git.kjlx@templeofstupid.com>
 <de425e99876dc6c344e1a4254894a3c81e71a2ec.1686166633.git.kjlx@templeofstupid.com>
 <33c52b4c-2a16-7578-f782-51267deff750@meta.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <33c52b4c-2a16-7578-f782-51267deff750@meta.com>
Precedence: bulk

On Thu, Jun 08, 2023 at 10:38:12AM -0700, Yonghong Song wrote:
> 
> 
> On 6/7/23 2:04 PM, Krister Johansen wrote:
> > When bpf subprograms are in use, the main program is not jit'd after the
> > subprograms because jit_subprogs sets a value for prog->bpf_func upon
> > success.  Subsequent calls to the JIT are bypassed when this value is
> > non-NULL.  This leads to a situation where the main program and its
> > func[0] counterpart are both in the bpf kallsyms tree, but only func[0]
> > has an extable.  Extables are only created during JIT.  Now there are
> > two nearly identical program ksym entries in the tree, but only one has
> > an extable.  Depending upon how the entries are placed, there's a chance
> > that a fault will call search_extable on the aux with the NULL entry.
> > 
> > Since jit_subprogs already copies state from func[0] to the main
> > program, include the extable pointer in this state duplication.  The
> > alternative is to skip adding the main program to the bpf_kallsyms
> > table, but that would mean adding a check for subprograms into the
> > middle of bpf_prog_load.
> 
> I think having two early identical program ksym entries is bad.
> When people 'cat /proc/kallsyms | grep <their program name>',
> they will find two programs with identical kernel address but different
> hash value. This is just very confusing. I think removing the
> duplicate in kallsyms is better from user's perspective.

Thanks for all the feedback.

In terms of resolving this confusion my inclination is to use the main
program. That way users see in kallsyms the same tag that is reported by
bpftool.  On the other hand, the tag in kallsyms won't match the sha1 of
that actual chunk of code.  Is anything relying on the hash in the tag
and the digest of the code agreeing?

-K