[PATCH] kdb: use correct pointer when 'btc' calls 'btt'

[PATCH] kdb: use correct pointer when 'btc' calls 'btt'

[Christophe Leroy]

On a powerpc 8xx, 'btc' fails as follows:

Entering kdb (current=0x(ptrval), pid 282) due to Keyboard Entry
kdb> btc
btc: cpu status: Currently on cpu 0
Available cpus: 0
kdb_getarea: Bad address 0x0

when booting the kernel with 'debug_boot_weak_hash', it fails as well

Entering kdb (current=0xba99ad80, pid 284) due to Keyboard Entry
kdb> btc
btc: cpu status: Currently on cpu 0
Available cpus: 0
kdb_getarea: Bad address 0xba99ad80

On other platforms, Oopses have been observed too, see
https://github.com/linuxppc/linux/issues/139

This is due to btc calling 'btt' with %p pointer as an argument.

This patch replaces %p by %px to get the real pointer value as
expected by 'btt'

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
Cc: <stable@vger.kernel.org> # 4.15+
---
 kernel/debug/kdb/kdb_bt.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/debug/kdb/kdb_bt.c b/kernel/debug/kdb/kdb_bt.c
index 6ad4a9fcbd6f..7921ae4fca8d 100644
--- a/kernel/debug/kdb/kdb_bt.c
+++ b/kernel/debug/kdb/kdb_bt.c
@@ -179,14 +179,14 @@ kdb_bt(int argc, const char **argv)
 				kdb_printf("no process for cpu %ld\n", cpu);
 				return 0;
 			}
-			sprintf(buf, "btt 0x%p\n", KDB_TSK(cpu));
+			sprintf(buf, "btt 0x%px\n", KDB_TSK(cpu));
 			kdb_parse(buf);
 			return 0;
 		}
 		kdb_printf("btc: cpu status: ");
 		kdb_parse("cpu\n");
 		for_each_online_cpu(cpu) {
-			sprintf(buf, "btt 0x%p\n", KDB_TSK(cpu));
+			sprintf(buf, "btt 0x%px\n", KDB_TSK(cpu));
 			kdb_parse(buf);
 			touch_nmi_watchdog();
 		}
-- 
2.13.3

Re: [PATCH] kdb: use correct pointer when 'btc' calls 'btt'

[Daniel Thompson]

On Fri, Sep 14, 2018 at 12:35:44PM +0000, Christophe Leroy wrote:
> On a powerpc 8xx, 'btc' fails as follows:
> 
> Entering kdb (current=0x(ptrval), pid 282) due to Keyboard Entry
> kdb> btc
> btc: cpu status: Currently on cpu 0
> Available cpus: 0
> kdb_getarea: Bad address 0x0
> 
> when booting the kernel with 'debug_boot_weak_hash', it fails as well
> 
> Entering kdb (current=0xba99ad80, pid 284) due to Keyboard Entry
> kdb> btc
> btc: cpu status: Currently on cpu 0
> Available cpus: 0
> kdb_getarea: Bad address 0xba99ad80
> 
> On other platforms, Oopses have been observed too, see
> https://github.com/linuxppc/linux/issues/139
> 
> This is due to btc calling 'btt' with %p pointer as an argument.
> 
> This patch replaces %p by %px to get the real pointer value as
> expected by 'btt'
> 
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> Cc: <stable@vger.kernel.org> # 4.15+

Would a Fixes: be better here?
Fixes: ad67b74d2469d9b82 ("printk: hash addresses printed with %p")

No blame attached to Tobin, but the fixes makes it super clear what
changed and why this breaks kdb (which was not explicitly called out
the patch description).


Daniel.

> ---
>  kernel/debug/kdb/kdb_bt.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/debug/kdb/kdb_bt.c b/kernel/debug/kdb/kdb_bt.c
> index 6ad4a9fcbd6f..7921ae4fca8d 100644
> --- a/kernel/debug/kdb/kdb_bt.c
> +++ b/kernel/debug/kdb/kdb_bt.c
> @@ -179,14 +179,14 @@ kdb_bt(int argc, const char **argv)
>  				kdb_printf("no process for cpu %ld\n", cpu);
>  				return 0;
>  			}
> -			sprintf(buf, "btt 0x%p\n", KDB_TSK(cpu));
> +			sprintf(buf, "btt 0x%px\n", KDB_TSK(cpu));
>  			kdb_parse(buf);
>  			return 0;
>  		}
>  		kdb_printf("btc: cpu status: ");
>  		kdb_parse("cpu\n");
>  		for_each_online_cpu(cpu) {
> -			sprintf(buf, "btt 0x%p\n", KDB_TSK(cpu));
> +			sprintf(buf, "btt 0x%px\n", KDB_TSK(cpu));
>  			kdb_parse(buf);
>  			touch_nmi_watchdog();
>  		}
> -- 
> 2.13.3
> 

Re: [PATCH] kdb: use correct pointer when 'btc' calls 'btt'

[Tobin C. Harding]

On Sun, Sep 16, 2018 at 12:06:10PM -0700, Daniel Thompson wrote:
> On Fri, Sep 14, 2018 at 12:35:44PM +0000, Christophe Leroy wrote:
> > On a powerpc 8xx, 'btc' fails as follows:
> > 
> > Entering kdb (current=0x(ptrval), pid 282) due to Keyboard Entry
> > kdb> btc
> > btc: cpu status: Currently on cpu 0
> > Available cpus: 0
> > kdb_getarea: Bad address 0x0
> > 
> > when booting the kernel with 'debug_boot_weak_hash', it fails as well
> > 
> > Entering kdb (current=0xba99ad80, pid 284) due to Keyboard Entry
> > kdb> btc
> > btc: cpu status: Currently on cpu 0
> > Available cpus: 0
> > kdb_getarea: Bad address 0xba99ad80
> > 
> > On other platforms, Oopses have been observed too, see
> > https://github.com/linuxppc/linux/issues/139
> > 
> > This is due to btc calling 'btt' with %p pointer as an argument.
> > 
> > This patch replaces %p by %px to get the real pointer value as
> > expected by 'btt'
> > 
> > Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> > Cc: <stable@vger.kernel.org> # 4.15+
> 
> Would a Fixes: be better here?
> Fixes: ad67b74d2469d9b82 ("printk: hash addresses printed with %p")
> 
> No blame attached to Tobin, but the fixes makes it super clear what

:)

> changed and why this breaks kdb (which was not explicitly called out
> the patch description).
> 
> 
> Daniel.

Re: Re: [PATCH] kdb: use correct pointer when 'btc' calls 'btt'

[Daniel Thompson]

On 16/09/2018 20:06, Daniel Thompson wrote:
> On Fri, Sep 14, 2018 at 12:35:44PM +0000, Christophe Leroy wrote:
>> On a powerpc 8xx, 'btc' fails as follows:
>>
>> Entering kdb (current=0x(ptrval), pid 282) due to Keyboard Entry
>> kdb> btc
>> btc: cpu status: Currently on cpu 0
>> Available cpus: 0
>> kdb_getarea: Bad address 0x0
>>
>> when booting the kernel with 'debug_boot_weak_hash', it fails as well
>>
>> Entering kdb (current=0xba99ad80, pid 284) due to Keyboard Entry
>> kdb> btc
>> btc: cpu status: Currently on cpu 0
>> Available cpus: 0
>> kdb_getarea: Bad address 0xba99ad80
>>
>> On other platforms, Oopses have been observed too, see
>> https://github.com/linuxppc/linux/issues/139
>>
>> This is due to btc calling 'btt' with %p pointer as an argument.
>>
>> This patch replaces %p by %px to get the real pointer value as
>> expected by 'btt'
>>
>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>> Cc: <stable@vger.kernel.org> # 4.15+
> 
> Would a Fixes: be better here?
> Fixes: ad67b74d2469d9b82 ("printk: hash addresses printed with %p")

Christophe, When you add the Fixes: could you also add my

Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>


Thanks.


> 
> No blame attached to Tobin, but the fixes makes it super clear what
> changed and why this breaks kdb (which was not explicitly called out
> the patch description).
> 
> 
> Daniel.
> 
>> ---
>>   kernel/debug/kdb/kdb_bt.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/kernel/debug/kdb/kdb_bt.c b/kernel/debug/kdb/kdb_bt.c
>> index 6ad4a9fcbd6f..7921ae4fca8d 100644
>> --- a/kernel/debug/kdb/kdb_bt.c
>> +++ b/kernel/debug/kdb/kdb_bt.c
>> @@ -179,14 +179,14 @@ kdb_bt(int argc, const char **argv)
>>   				kdb_printf("no process for cpu %ld\n", cpu);
>>   				return 0;
>>   			}
>> -			sprintf(buf, "btt 0x%p\n", KDB_TSK(cpu));
>> +			sprintf(buf, "btt 0x%px\n", KDB_TSK(cpu));
>>   			kdb_parse(buf);
>>   			return 0;
>>   		}
>>   		kdb_printf("btc: cpu status: ");
>>   		kdb_parse("cpu\n");
>>   		for_each_online_cpu(cpu) {
>> -			sprintf(buf, "btt 0x%p\n", KDB_TSK(cpu));
>> +			sprintf(buf, "btt 0x%px\n", KDB_TSK(cpu));
>>   			kdb_parse(buf);
>>   			touch_nmi_watchdog();
>>   		}
>> -- 
>> 2.13.3
>>

Re: [PATCH] kdb: use correct pointer when 'btc' calls 'btt'

[Christophe LEROY]

Le 26/09/2018 à 13:11, Daniel Thompson a écrit :
> On 16/09/2018 20:06, Daniel Thompson wrote:
>> On Fri, Sep 14, 2018 at 12:35:44PM +0000, Christophe Leroy wrote:
>>> On a powerpc 8xx, 'btc' fails as follows:
>>>
>>> Entering kdb (current=0x(ptrval), pid 282) due to Keyboard Entry
>>> kdb> btc
>>> btc: cpu status: Currently on cpu 0
>>> Available cpus: 0
>>> kdb_getarea: Bad address 0x0
>>>
>>> when booting the kernel with 'debug_boot_weak_hash', it fails as well
>>>
>>> Entering kdb (current=0xba99ad80, pid 284) due to Keyboard Entry
>>> kdb> btc
>>> btc: cpu status: Currently on cpu 0
>>> Available cpus: 0
>>> kdb_getarea: Bad address 0xba99ad80
>>>
>>> On other platforms, Oopses have been observed too, see
>>> https://github.com/linuxppc/linux/issues/139
>>>
>>> This is due to btc calling 'btt' with %p pointer as an argument.
>>>
>>> This patch replaces %p by %px to get the real pointer value as
>>> expected by 'btt'
>>>
>>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>>> Cc: <stable@vger.kernel.org> # 4.15+
>>
>> Would a Fixes: be better here?
>> Fixes: ad67b74d2469d9b82 ("printk: hash addresses printed with %p")
> 
> Christophe, When you add the Fixes: could you also add my
> 
> Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>


Ok, thanks for the review, but do I have to do anything really ?

The Fixes: and now your Reviewed-by: appear automatically in patchwork 
(https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=65715), 
so I believe they'll be automatically included when Jason or someone 
else takes the patch, no ?

Christophe

> 
> 
> Thanks.
> 
> 
>>
>> No blame attached to Tobin, but the fixes makes it super clear what
>> changed and why this breaks kdb (which was not explicitly called out
>> the patch description).
>>
>>
>> Daniel.
>>
>>> ---
>>>   kernel/debug/kdb/kdb_bt.c | 4 ++--
>>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/kernel/debug/kdb/kdb_bt.c b/kernel/debug/kdb/kdb_bt.c
>>> index 6ad4a9fcbd6f..7921ae4fca8d 100644
>>> --- a/kernel/debug/kdb/kdb_bt.c
>>> +++ b/kernel/debug/kdb/kdb_bt.c
>>> @@ -179,14 +179,14 @@ kdb_bt(int argc, const char **argv)
>>>                   kdb_printf("no process for cpu %ld\n", cpu);
>>>                   return 0;
>>>               }
>>> -            sprintf(buf, "btt 0x%p\n", KDB_TSK(cpu));
>>> +            sprintf(buf, "btt 0x%px\n", KDB_TSK(cpu));
>>>               kdb_parse(buf);
>>>               return 0;
>>>           }
>>>           kdb_printf("btc: cpu status: ");
>>>           kdb_parse("cpu\n");
>>>           for_each_online_cpu(cpu) {
>>> -            sprintf(buf, "btt 0x%p\n", KDB_TSK(cpu));
>>> +            sprintf(buf, "btt 0x%px\n", KDB_TSK(cpu));
>>>               kdb_parse(buf);
>>>               touch_nmi_watchdog();
>>>           }
>>> -- 
>>> 2.13.3
>>>

Re: [PATCH] kdb: use correct pointer when 'btc' calls 'btt'

[Michael Ellerman]

Christophe LEROY <christophe.leroy@c-s.fr> writes:
> Le 26/09/2018 à 13:11, Daniel Thompson a écrit :
>> On 16/09/2018 20:06, Daniel Thompson wrote:
>>> On Fri, Sep 14, 2018 at 12:35:44PM +0000, Christophe Leroy wrote:
>>>> On a powerpc 8xx, 'btc' fails as follows:
>>>> Entering kdb (current=0x(ptrval), pid 282) due to Keyboard Entry
...
>>>>
>>>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>>>> Cc: <stable@vger.kernel.org> # 4.15+
>>>
>>> Would a Fixes: be better here?
>>> Fixes: ad67b74d2469d9b82 ("printk: hash addresses printed with %p")
>> 
>> Christophe, When you add the Fixes: could you also add my
>> 
>> Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>
>
> Ok, thanks for the review, but do I have to do anything really ?
>
> The Fixes: and now your Reviewed-by: appear automatically in patchwork 
> (https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=65715), 
> so I believe they'll be automatically included when Jason or someone 
> else takes the patch, no ?

patchwork won't add the Fixes tag from the reply, it needs to be in the
original mail.

See:
  https://github.com/getpatchwork/patchwork/issues/151


cheers

Re: [PATCH] kdb: use correct pointer when 'btc' calls 'btt'

[Christophe LEROY]

Le 27/09/2018 à 13:09, Michael Ellerman a écrit :
> Christophe LEROY <christophe.leroy@c-s.fr> writes:
>> Le 26/09/2018 à 13:11, Daniel Thompson a écrit :
>>> On 16/09/2018 20:06, Daniel Thompson wrote:
>>>> On Fri, Sep 14, 2018 at 12:35:44PM +0000, Christophe Leroy wrote:
>>>>> On a powerpc 8xx, 'btc' fails as follows:
>>>>> Entering kdb (current=0x(ptrval), pid 282) due to Keyboard Entry
> ...
>>>>>
>>>>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>>>>> Cc: <stable@vger.kernel.org> # 4.15+
>>>>
>>>> Would a Fixes: be better here?
>>>> Fixes: ad67b74d2469d9b82 ("printk: hash addresses printed with %p")
>>>
>>> Christophe, When you add the Fixes: could you also add my
>>>
>>> Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>
>>
>> Ok, thanks for the review, but do I have to do anything really ?
>>
>> The Fixes: and now your Reviewed-by: appear automatically in patchwork
>> (https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=65715),
>> so I believe they'll be automatically included when Jason or someone
>> else takes the patch, no ?
> 
> patchwork won't add the Fixes tag from the reply, it needs to be in the
> original mail.
> 
> See:
>    https://github.com/getpatchwork/patchwork/issues/151
> 

Ok, so it accounts it and adds a '1' in the F column in the patches 
list, but won't take it into account.

Then I'll send a v2 with revised commit text.

Christophe

Re: [PATCH] kdb: use correct pointer when 'btc' calls 'btt'

[Michael Ellerman]

Christophe LEROY <christophe.leroy@c-s.fr> writes:
> Le 27/09/2018 à 13:09, Michael Ellerman a écrit :
>> Christophe LEROY <christophe.leroy@c-s.fr> writes:
>>> Le 26/09/2018 à 13:11, Daniel Thompson a écrit :
>>>> On 16/09/2018 20:06, Daniel Thompson wrote:
>>>>> On Fri, Sep 14, 2018 at 12:35:44PM +0000, Christophe Leroy wrote:
>>>>>> On a powerpc 8xx, 'btc' fails as follows:
>>>>>> Entering kdb (current=0x(ptrval), pid 282) due to Keyboard Entry
>> ...
>>>>>>
>>>>>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>>>>>> Cc: <stable@vger.kernel.org> # 4.15+
>>>>>
>>>>> Would a Fixes: be better here?
>>>>> Fixes: ad67b74d2469d9b82 ("printk: hash addresses printed with %p")
>>>>
>>>> Christophe, When you add the Fixes: could you also add my
>>>>
>>>> Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>
>>>
>>> Ok, thanks for the review, but do I have to do anything really ?
>>>
>>> The Fixes: and now your Reviewed-by: appear automatically in patchwork
>>> (https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=65715),
>>> so I believe they'll be automatically included when Jason or someone
>>> else takes the patch, no ?
>> 
>> patchwork won't add the Fixes tag from the reply, it needs to be in the
>> original mail.
>> 
>> See:
>>    https://github.com/getpatchwork/patchwork/issues/151
>> 
>
> Ok, so it accounts it and adds a '1' in the F column in the patches 
> list, but won't take it into account.

Yes. The logic that populates the columns is separate from the logic
that scrapes the tags, which is a bug :)

> Then I'll send a v2 with revised commit text.

Thanks.

cheers

Re: [PATCH] kdb: use correct pointer when 'btc' calls 'btt'

[Jason Wessel]

On 09/28/2018 07:57 AM, Michael Ellerman wrote:
> Christophe LEROY <christophe.leroy@c-s.fr> writes:
>> Le 27/09/2018 à 13:09, Michael Ellerman a écrit :
>>> Christophe LEROY <christophe.leroy@c-s.fr> writes:
>>>> Le 26/09/2018 à 13:11, Daniel Thompson a écrit :
>>>>
>>>> The Fixes: and now your Reviewed-by: appear automatically in patchwork
>>>> (https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=65715),
>>>> so I believe they'll be automatically included when Jason or someone
>>>> else takes the patch, no ?
>>>
>>> patchwork won't add the Fixes tag from the reply, it needs to be in the
>>> original mail.
>>>
>>> See:
>>>     https://github.com/getpatchwork/patchwork/issues/151
>>>
>>
>> Ok, so it accounts it and adds a '1' in the F column in the patches
>> list, but won't take it into account.
> 
> Yes. The logic that populates the columns is separate from the logic
> that scrapes the tags, which is a bug :)
> 
>> Then I'll send a v2 with revised commit text.
> 


No need.  https://git.kernel.org/pub/scm/linux/kernel/git/jwessel/kgdb.git/commit/?h=kgdb-next

Since it is a regression fix, we'll try and get it merged as soon as we can.

Cheers,
Jason.

Re: [PATCH] kdb: use correct pointer when 'btc' calls 'btt'

[Olof Johansson]

Hi Jason,

On Mon, Oct 1, 2018 at 12:52 PM Jason Wessel <jason.wessel@windriver.com> wrote:
>
> On 09/28/2018 07:57 AM, Michael Ellerman wrote:
> > Christophe LEROY <christophe.leroy@c-s.fr> writes:
> >> Le 27/09/2018 à 13:09, Michael Ellerman a écrit :
> >>> Christophe LEROY <christophe.leroy@c-s.fr> writes:
> >>>> Le 26/09/2018 à 13:11, Daniel Thompson a écrit :
> >>>>
> >>>> The Fixes: and now your Reviewed-by: appear automatically in patchwork
> >>>> (https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=65715),
> >>>> so I believe they'll be automatically included when Jason or someone
> >>>> else takes the patch, no ?
> >>>
> >>> patchwork won't add the Fixes tag from the reply, it needs to be in the
> >>> original mail.
> >>>
> >>> See:
> >>>     https://github.com/getpatchwork/patchwork/issues/151
> >>>
> >>
> >> Ok, so it accounts it and adds a '1' in the F column in the patches
> >> list, but won't take it into account.
> >
> > Yes. The logic that populates the columns is separate from the logic
> > that scrapes the tags, which is a bug :)
> >
> >> Then I'll send a v2 with revised commit text.
> >
>
>
> No need.  https://git.kernel.org/pub/scm/linux/kernel/git/jwessel/kgdb.git/commit/?h=kgdb-next
>
> Since it is a regression fix, we'll try and get it merged as soon as we can.

Looks like this didn't make it in yet, even with a merge window inbetween? :(

I know first-hand that time to do upstream work can sometimes be hard
to find. I also know that Daniel has shown interest in helping out
here, and is listed as a maintainer. May I suggest that he starts a
tree to collect patches and submit pull requests for a while, until
you find more time for it?

Having a tag-team maintainer setup like we have had for arm-soc has
been very useful especially when one of us get too busy for a while,
etc.


-Olof