linux-kernel - Re: [PATCH] powerpc/xmon: symbol lookup length fixed

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] [day] [month] [year] [list]

Message-ID: <87a5eskj4v.fsf@mail.lhotse>
Date: Fri, 25 Oct 2024 13:32:00 +1100
From: Michael Ellerman <mpe@...erman.id.au>
To: Mukesh Kumar Chaurasiya <mchauras@...ux.ibm.com>
Cc: npiggin@...il.com, christophe.leroy@...roup.eu, naveen@...nel.org,
 maddy@...ux.ibm.com, peterx@...hat.com, groug@...d.org,
 sshegde@...ux.ibm.com, linuxppc-dev@...ts.ozlabs.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH] powerpc/xmon: symbol lookup length fixed

Mukesh Kumar Chaurasiya <mchauras@...ux.ibm.com> writes:
> On Thu, Oct 24, 2024 at 10:42:12AM +0530, Mukesh Kumar Chaurasiya wrote:
>> On Thu, Oct 24, 2024 at 12:00:53PM +1100, Michael Ellerman wrote:
>> > Mukesh Kumar Chaurasiya <mchauras@...ux.ibm.com> writes:
>> > > Currently xmon cannot lookup symbol beyond 64 characters in some cases.
>> > 
>> > Can you mention which commands? It looks like it's "ls" and "lp".
>> Sure.
>> > 
>> > > Fix this by using KSYM_NAME_LEN instead of fixed 64 characters.
>> > >
>> > > Signed-off-by: Mukesh Kumar Chaurasiya <mchauras@...ux.ibm.com>
>> > > ---
>> > >  arch/powerpc/xmon/xmon.c | 6 +++---
>> > >  1 file changed, 3 insertions(+), 3 deletions(-)
>> > >
>> > > diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
>> > > index e6cddbb2305f..22b8b5cc4df0 100644
>> > > --- a/arch/powerpc/xmon/xmon.c
>> > > +++ b/arch/powerpc/xmon/xmon.c
>> > > @@ -3662,7 +3662,7 @@ symbol_lookup(void)
>> > >  	int type = inchar();
>> > >  	unsigned long addr, cpu;
>> > >  	void __percpu *ptr = NULL;
>> > > -	static char tmp[64];
>> > > +	static char tmp[KSYM_NAME_LEN];
>> >   
>> > I think you could use the existing tmpstr buffer.
>> > 
>> > It is global so it's a little hard to track down all the users, but I
>> > think it's only used briefly in get_function_bounds(),
>> > xmon_print_symbol() and scanhex(). ie. none of the uses persist across
>> > function calls.
>> > 
>> > We don't want to have two 512 byte static arrays lying around if we can
>> > get by with one.
>> > 
>> > cheers
>> Sure.

> For now i don't think so we can get by with only one. There is a lookup being
> done when the scanhex is using the tmpstr for another lookup.

Yeah I see it, when handling `lp <symbol> <cpu>`.

That's a bit annoying because for that command the CPU number is only
meant to be a number, it never makes sense to do a $symbol lookup there.

But I agree with the way the code is structured it's best not to reuse
tmpstr there.

cheers