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
Subject: RE: [PATCH v4 2/3] bpf: make bpf_dump_raw_ok() based on
 CONFIG_KALLSYMS
Reply-To: maninder1.s@samsung.com
Sender: Maninder Singh <maninder1.s@samsung.com>
From:   Maninder Singh <maninder1.s@samsung.com>
To:     Andrii Nakryiko <andrii.nakryiko@gmail.com>
CC:     "ast@kernel.org" <ast@kernel.org>,
        "daniel@iogearbox.net" <daniel@iogearbox.net>,
        "john.fastabend@gmail.com" <john.fastabend@gmail.com>,
        "andrii@kernel.org" <andrii@kernel.org>,
        "martin.lau@linux.dev" <martin.lau@linux.dev>,
        "song@kernel.org" <song@kernel.org>, "yhs@fb.com" <yhs@fb.com>,
        "kpsingh@kernel.org" <kpsingh@kernel.org>,
        "sdf@google.com" <sdf@google.com>,
        "haoluo@google.com" <haoluo@google.com>,
        "jolsa@kernel.org" <jolsa@kernel.org>,
        "thunder.leizhen@huawei.com" <thunder.leizhen@huawei.com>,
        "mcgrof@kernel.org" <mcgrof@kernel.org>,
        "boqun.feng@gmail.com" <boqun.feng@gmail.com>,
        "vincenzopalazzodev@gmail.com" <vincenzopalazzodev@gmail.com>,
        "ojeda@kernel.org" <ojeda@kernel.org>,
        "jgross@suse.com" <jgross@suse.com>,
        "brauner@kernel.org" <brauner@kernel.org>,
        "michael.christie@oracle.com" <michael.christie@oracle.com>,
        "samitolvanen@google.com" <samitolvanen@google.com>,
        "glider@google.com" <glider@google.com>,
        "peterz@infradead.org" <peterz@infradead.org>,
        "keescook@chromium.org" <keescook@chromium.org>,
        "stephen.s.brennan@oracle.com" <stephen.s.brennan@oracle.com>,
        "alan.maguire@oracle.com" <alan.maguire@oracle.com>,
        "pmladek@suse.com" <pmladek@suse.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "bpf@vger.kernel.org" <bpf@vger.kernel.org>,
        Onkarnath <onkarnath.1@samsung.com>
In-Reply-To: <CAEf4BzYavyL431eA_HZ-X8+wTeO4Cyt7tGDUbPB0yqPru=ZUSw@mail.gmail.com>
Message-ID: <20230607034028epcms5p8ed013806c42bd79b76368ac015a7b6ba@epcms5p8>
Date:   Wed, 07 Jun 2023 09:10:28 +0530
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset="utf-8"
CMS-TYPE: 105P
References: <CAEf4BzYavyL431eA_HZ-X8+wTeO4Cyt7tGDUbPB0yqPru=ZUSw@mail.gmail.com>
        <20230606042802.508954-1-maninder1.s@samsung.com>
        <20230606042802.508954-2-maninder1.s@samsung.com>
        <CGME20230606042819epcas5p4f0601efb42d59007cba023c73fa0624a@epcms5p8>
Precedence: bulk

Hi Andrii Nakryiko,

>>
>> bpf_dump_raw_ok() depends on kallsyms_show_value() and we already
>> have a false definition for the =21CONFIG_KALLSYMS case. But we'll
>> soon expand on kallsyms_show_value() and so to make the code
>> easier to follow just provide a direct =21CONFIG_KALLSYMS definition
>> for bpf_dump_raw_ok() as well.
>
> I'm sorry, I'm failing to follow the exact reasoning about
> simplification. It seems simpler to have
>=20
> static inline bool kallsyms_show_value(const struct cred *cred)
> =7B
> =C2=A0=20=C2=A0=20return=20false;=0D=0A>=20=7D=0D=0A>=20=0D=0A>=20and=20c=
ontrol=20it=20from=20kallsyms-related=20internal=20header,=20rather=20than=
=0D=0A>=20adding=20CONFIG_KALLSYMS=20ifdef-ery=20to=20include/linux/filter.=
h=20and=0D=0A>=20redefining=20that=20=60return=20false=60=20decision.=20Wha=
t=20if=20in=20the=20future=20we=0D=0A>=20decide=20that=20if=20=21CONFIG_KAL=
LSYMS=20it's=20ok=20to=20show=20raw=20addresses,=20now=0D=0A>=20we'll=20hav=
e=20to=20remember=20to=20update=20it=20in=20two=20places.=0D=0A>=20=0D=0A>=
=20Unless=20I'm=20missing=20some=20other=20complications?=0D=0A>=20=0D=0A=
=0D=0APatch=203/3=20does=20the=20same,=20it=20extends=20functionality=20of=
=20kallsyms_show_value()=0D=0Ain=20case=20of=20=20=21CONFIG_KALLSYMS.=0D=0A=
=0D=0AAll=20other=20users=20likes=20modules=20code,=20kprobe=20codes=20are=
=20using=20this=20API=0D=0Afor=20sanity/permission,=20and=20then=20prints=
=20the=20address=20like=20below:=0D=0A=0D=0Astatic=20int=20kprobe_blacklist=
_seq_show(struct=20seq_file=20*m,=20void=20*v)=0D=0A=7B=0D=0A...=0D=0A=20=
=20=20=20=20=20=20=20if=20(=21kallsyms_show_value(m->file->f_cred))=0D=0A=
=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20seq_printf(m,=20=220x%px-0x=
%px=5Ct%ps=5Cn=22,=20NULL,=20NULL,=0D=0A=20=20=20=20=20=20=20=20=20=20=20=
=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20(void=20*)ent->start_addr);=
=0D=0A=20=20=20=20=20=20=20=20else=0D=0A=20=20=20=20=20=20=20=20=20=20=20=
=20=20=20=20=20seq_printf(m,=20=220x%px-0x%px=5Ct%ps=5Cn=22,=20(void=20*)en=
t->start_addr,=0D=0A=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=
=20=20=20=20=20=20=20=20=20(void=20*)ent->end_addr,=20(void=20*)ent->start_=
addr);=0D=0A..=0D=0A=7D=0D=0A=0D=0Aso=20there=20will=20be=20no=20issues=20i=
f=20we=20move=20kallsyms_show_value()=20out=20of=20KALLSYMS=20dependency.=
=0D=0Aand=20these=20codes=20will=20work=20in=20case=20of=20=21KALLSYMS=20al=
so.=0D=0A=0D=0Abut=20BPF=20code=20logic=20was=20complex=20and=20seems=20thi=
s=20API=20was=20used=20as=20checking=20for=20whether=20KALLSYMS=20is=0D=0Ae=
nabled=20or=20not=20as=20per=20comment=20in=20bpf_dump_raw_ok():=0D=0A=0D=
=0A/*=0D=0A=20*=20Reconstruction=20of=20call-sites=20is=20dependent=20on=20=
kallsyms,=0D=0A=20*=20thus=20make=20dump=20the=20same=20restriction.=0D=0A=
=20*/=0D=0A=0D=0Aalso=20as=20per=20below=20code:=20=0D=0A(we=20were=20not=
=20sure=20whether=20BPF=20will=20work=20or=20not=20with=20patch=203/3=20bec=
ause=20of=20no=20expertise=20on=20BPF=20code,=0D=0Aso=20we=20keep=20the=20b=
ehaviour=20same)=0D=0A=0D=0A=20=20=20=20=20=20=20if=20(ulen)=20=7B=0D=0A=20=
=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20if=20(bpf_dump_raw_ok(file->f_=
cred))=20=7B=0D=0A=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=
=20=20=20=20=20unsigned=20long=20ksym_addr;=0D=0A=20=20=20=20=20=20=20=20=
=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20u64=20__user=20*user_ksyms;=
=0D=0A=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=
=20u32=20i;=0D=0A=0D=0A=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=
=20=20=20=20=20=20=20/*=20copy=20the=20address=20of=20the=20kernel=20symbol=
=0D=0A=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=
=20=20*=20corresponding=20to=20each=20function=0D=0A=20=20=20=20=20=20=20=
=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20*/=0D=0A=20=20=20=20=
=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20ulen=20=3D=20mi=
n_t(u32,=20info.nr_jited_ksyms,=20ulen);=0D=0A=20=20=20=20=20=20=20=20=20=
=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20user_ksyms=20=3D=20u64_to_user=
_ptr(info.jited_ksyms);=0D=0A=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=
=20=20=20=20=20=20=20=20=20if=20(prog->aux->func_cnt)=20=7B=0D=0A=20=20=20=
=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=
=20=20=20=20for=20(i=20=3D=200;=20i=20<=20ulen;=20i++)=20=7B=0D=0A=20=20=20=
...=0D=0A=20=20=20=7D=0D=0A=20=20=20=0D=0Aearlier=20conversation=20for=20th=
is=20change:=0D=0Ahttps://lkml.org/lkml/2022/4/13/326=0D=0A=0D=0Ahere=20Pet=
r=20CC'ed=20BPF=20maintainers=20to=20know=20their=20opinion=20whether=20BPF=
=20code=20can=20work=20with=20patch=203/3,=0D=0Aif=20not=20then=20we=20need=
=20this=20patch.=0D=0A=0D=0AThanks,=0D=0AManinder=20Singh