[PATCH] powerpc/xmon: Fix tmpstr length check in scanhex

[PATCH] powerpc/xmon: Fix tmpstr length check in scanhex

[Madhavan Srinivasan]

If a function name is greater than 63 char long, xmon command
may not find them. For example, here is a test that
executed an illegal instruction in a kernel function and one of
call stack function has name >63 char long,

cpu 0x0: Vector: 700 (Program Check) at [c00000000a6577e0]
    pc: c0000000001aacb8: check__allowed__function__name__for__symbol__r4+0x8/0x10
    lr: c00000000019c1e0: check__allowed__function__name__for__symbol__r1+0x20/0x40
    sp: c00000000a657a80
   msr: 800000000288b033
  current = 0xc00000000a439900
  paca    = 0xc000000003e90000	 irqmask: 0x03	 irq_happened: 0x01
.....
[link register   ] c00000000019c1e0 check__allowed__function__name__for__symbol__r1+0x20/0x40
[c00000000a657a80] c00000000a439900 (unreliable)
[c00000000a657aa0] c0000000001021d8 check__allowed__function__name__for__symbol__r2_resolution_symbol+0x38/0x4c
[c00000000a657ac0] c00000000019b424 power_pmu_event_init+0xa4/0xa50

and when executing a dump instruction (di) command for long function name,
xmon fails to find the function symbol

0:mon> di $check__allowed__function__name__for__symbol__r2_resolution_symbol
unknown symbol 'check__allowed__function__name__for__symbol__r2_resolution_symb'
0000000000000000  ********

This is because, in the scanhex(), tmpstr loop index is checked only for a upper bound
of 63. Proposed fix is to replace the upper bound value with "KSYM_NAME_LEN"

With fix:

0:mon> di $check__allowed__function__name__for__symbol__r2_resolution_symbol
c0000000001021a0  3c4c0249	addis   r2,r12,585
c0000000001021a4  3842ae60	addi    r2,r2,-20896
c0000000001021a8  7c0802a6	mflr    r0
c0000000001021ac  60000000	nop
.....

Reported-by: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
Signed-off-by: Madhavan Srinivasan <maddy@linux.ibm.com>
Link - https://lore.kernel.org/linuxppc-dev/87ilc8ym6v.fsf@mail.lhotse/
---
 arch/powerpc/xmon/xmon.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index bd4813bad317..2f6a61d85e22 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -3543,7 +3543,7 @@ scanhex(unsigned long *vp)
 		}
 	} else if (c == '$') {
 		int i;
-		for (i=0; i<63; i++) {
+		for (i=0; i<KSYM_NAME_LEN; i++) {
 			c = inchar();
 			if (isspace(c) || c == '\0') {
 				termch = c;
-- 
2.45.2

Re: [PATCH] powerpc/xmon: Fix tmpstr length check in scanhex

[Miguel Ojeda]

On Wed, Aug 14, 2024 at 12:10 PM Madhavan Srinivasan
<maddy@linux.ibm.com> wrote:
>
> Reported-by: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
> Signed-off-by: Madhavan Srinivasan <maddy@linux.ibm.com>
> Link - https://lore.kernel.org/linuxppc-dev/87ilc8ym6v.fsf@mail.lhotse/

Thanks for fixing this!

The "Link -" should be a tag, i.e. "Link:". And, in this case, since
it was a report, I think it should be a "Closes:" instead, and thus it
should be put below the "Reported-by:" and it should point to the
message of the original report, i.e.

    https://lore.kernel.org/linuxppc-dev/CANiq72=QeTgtZL4k9=4CJP6C_Hv=rh3fsn3B9S3KFoPXkyWk3w@mail.gmail.com/

I am not sure if a "Fixes:" tag would apply here, though.

Cheers,
Miguel

Re: [PATCH] powerpc/xmon: Fix tmpstr length check in scanhex

[Madhavan Srinivasan]

On 8/14/24 3:53 PM, Miguel Ojeda wrote:
> On Wed, Aug 14, 2024 at 12:10 PM Madhavan Srinivasan
> <maddy@linux.ibm.com> wrote:
>> Reported-by: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
>> Signed-off-by: Madhavan Srinivasan <maddy@linux.ibm.com>
>> Link - https://lore.kernel.org/linuxppc-dev/87ilc8ym6v.fsf@mail.lhotse/
> Thanks for fixing this!
>
> The "Link -" should be a tag, i.e. "Link:". And, in this case, since
> it was a report, I think it should be a "Closes:" instead, and thus it
> should be put below the "Reported-by:" and it should point to the
> message of the original report, i.e.
>
>      https://lore.kernel.org/linuxppc-dev/CANiq72=QeTgtZL4k9=4CJP6C_Hv=rh3fsn3B9S3KFoPXkyWk3w@mail.gmail.com/


Ok thats was fast :)
Thanks for the review,  Will fix it in v2 and also will check and add 
"Fixes" tag part.

Maddy

>
> I am not sure if a "Fixes:" tag would apply here, though.
>
> Cheers,
> Miguel