linux-kernel - Re: [PATCH RFC 01/19] kallsyms: Simplify update_iter

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20180514175509.GV13491@kernel.org>
Date:   Mon, 14 May 2018 14:55:10 -0300
From:   Arnaldo Carvalho de Melo <acme@...nel.org>
To:     "Hunter, Adrian" <adrian.hunter@...el.com>
Cc:     Jiri Olsa <jolsa@...hat.com>, Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Andy Lutomirski <luto@...nel.org>,
        "H. Peter Anvin" <hpa@...or.com>, Andi Kleen <ak@...ux.intel.com>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Joerg Roedel <joro@...tes.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "x86@...nel.org" <x86@...nel.org>
Subject: Re: [PATCH RFC 01/19] kallsyms: Simplify update_iter_mod()

Em Thu, May 10, 2018 at 05:02:18PM +0000, Hunter, Adrian escreveu:
> > -----Original Message-----
> > From: Jiri Olsa [mailto:jolsa@...hat.com]
> > Sent: Thursday, May 10, 2018 4:02 PM
> > To: Hunter, Adrian <adrian.hunter@...el.com>
> > Cc: Thomas Gleixner <tglx@...utronix.de>; Arnaldo Carvalho de Melo
> > <acme@...nel.org>; Ingo Molnar <mingo@...hat.com>; Peter Zijlstra
> > <peterz@...radead.org>; Andy Lutomirski <luto@...nel.org>; H. Peter
> > Anvin <hpa@...or.com>; Andi Kleen <ak@...ux.intel.com>; Alexander
> > Shishkin <alexander.shishkin@...ux.intel.com>; Dave Hansen
> > <dave.hansen@...ux.intel.com>; Joerg Roedel <joro@...tes.org>; linux-
> > kernel@...r.kernel.org; x86@...nel.org
> > Subject: Re: [PATCH RFC 01/19] kallsyms: Simplify update_iter_mod()
> > 
> > On Wed, May 09, 2018 at 02:43:30PM +0300, Adrian Hunter wrote:
> > > Simplify logic in update_iter_mod().
> > >
> > > Signed-off-by: Adrian Hunter <adrian.hunter@...el.com>
> > > ---
> > >  kernel/kallsyms.c | 20 ++++++--------------
> > >  1 file changed, 6 insertions(+), 14 deletions(-)
> > >
> > > diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c index
> > > a23e21ada81b..eda4b0222dab 100644
> > > --- a/kernel/kallsyms.c
> > > +++ b/kernel/kallsyms.c
> > > @@ -510,23 +510,15 @@ static int update_iter_mod(struct kallsym_iter
> > > *iter, loff_t pos)  {
> > >  	iter->pos = pos;
> > >
> > > -	if (iter->pos_ftrace_mod_end > 0 &&
> > > -	    iter->pos_ftrace_mod_end < iter->pos)
> > > -		return get_ksymbol_bpf(iter);
> > > -
> > > -	if (iter->pos_mod_end > 0 &&
> > > -	    iter->pos_mod_end < iter->pos) {
> > > -		if (!get_ksymbol_ftrace_mod(iter))
> > > -			return get_ksymbol_bpf(iter);
> > > +	if ((!iter->pos_mod_end || iter->pos_mod_end > pos) &&
> > > +	    get_ksymbol_mod(iter))
> > >  		return 1;
> > 
> > hum, should that be iter-> pos_mod_end >= pos ?
> 
> But module_get_kallsym() returned -1 when pos_mod_end was set to pos.

After looking at the reset_iter(), etc, i.e. the lifetime of reading
/proc/kallsyms, I _think_ this is ok, but if it works already, why
simplify it? Because it paves the way for later changes in this function
in this patchset? If so, please state that, and then spelling out why
this is better than before helps with reviewing, please expand on this
cset comment log.

- Arnaldo
 
> > 
> > 
> > > -	}
> > >
> > > -	if (!get_ksymbol_mod(iter)) {
> > > -		if (!get_ksymbol_ftrace_mod(iter))
> > > -			return get_ksymbol_bpf(iter);
> > > -	}
> > > +	if ((!iter->pos_ftrace_mod_end || iter->pos_ftrace_mod_end >
> > pos) &&
> > > +	    get_ksymbol_ftrace_mod(iter))
> > 
> > same here? iter->pos_ftrace_mod_end >= pos
> > 
> > jirka
> > 
> > > +		return 1;
> > >
> > > -	return 1;
> > > +	return get_ksymbol_bpf(iter);
> > >  }
> > >
> > >  /* Returns false if pos at or past end of file. */
> > > --
> > > 1.9.1
> > >