Re: [Bug 206175] Fedora >= 5.4 kernels instantly freeze on boot without producing any display output

Re: [Bug 206175] Fedora >= 5.4 kernels instantly freeze on boot without producing any display output

[Christoph Hellwig]

FYI, Linus' latest rant shows up in bugzilla, but never made it to me,
just as the other replies from Artem and Hans.  This just shows how
broken bugzilla is as a reporting tool.  Please be a little more calm,
I've always taken reported regressions series and as a first priority,
but it really does not help if information is hidden away.  Adding
Artem to the Cc list and drop bugzilla to make this work a bit better.

Artem, can you test the patch below?  This fixes the broken dma_mask
handling in platform_device_register_full that could override a
perfectly valid mask with 0.  If this doesn't work, can you throw
in a dump_stack() into the working kernel build to see where
platform_device_register_full and setup_pdev_dma_masks get called
for your system?

diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index 7fa654f1288b..03035661eb6b 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -662,19 +662,6 @@ struct platform_device *platform_device_register_full(
 	pdev->dev.of_node_reused = pdevinfo->of_node_reused;
 
 	if (pdevinfo->dma_mask) {
-		/*
-		 * This memory isn't freed when the device is put,
-		 * I don't have a nice idea for that though.  Conceptually
-		 * dma_mask in struct device should not be a pointer.
-		 * See http://thread.gmane.org/gmane.linux.kernel.pci/9081
-		 */
-		pdev->dev.dma_mask =
-			kmalloc(sizeof(*pdev->dev.dma_mask), GFP_KERNEL);
-		if (!pdev->dev.dma_mask)
-			goto err;
-
-		kmemleak_ignore(pdev->dev.dma_mask);
-
 		*pdev->dev.dma_mask = pdevinfo->dma_mask;
 		pdev->dev.coherent_dma_mask = pdevinfo->dma_mask;
 	}

Re: [Bug 206175] Fedora >= 5.4 kernels instantly freeze on boot without producing any display output

[Christoph Hellwig]

As you seem to have a mfd based usb card reader per bugzilla, can you
try the patch form Robin below?  This ensures mfd doesn't mess with the
dma mask and thus entangling it with the parent.  And please try to
reply to the actual mail.  I found some updates in bugzilla when
I checked it after I haven't seen any reply for a while, but that isn't
a very efficient way to communicate.

--- a/drivers/mfd/mfd-core.c
+++ b/drivers/mfd/mfd-core.c
@@ -138,7 +138,7 @@ static int mfd_add_device(struct device *parent, int id,

  	pdev->dev.parent = parent;
  	pdev->dev.type = &mfd_dev_type;
-	pdev->dev.dma_mask = parent->dma_mask;
+	pdev->dma_mask = parent->dma_mask ? *parent->dma_mask : 0;
  	pdev->dev.dma_parms = parent->dma_parms;
  	pdev->dev.coherent_dma_mask = parent->coherent_dma_mask;

Re: [Bug 206175] Fedora >= 5.4 kernels instantly freeze on boot without producing any display output

[Artem S. Tashkinov]

On 3/11/20 3:24 PM, Christoph Hellwig wrote:
> pdev->dma_mask = parent->dma_mask ? *parent->dma_mask : 0;

This patch makes no difference.

The kernel panics with the same call trace which starts with:

RIP: 0010:kmem_cache_alloc_trace
...

? acpi_ds_create_walk_state
acpi_ds_create_walk_state
acpi_ds_call_control_method
acpi_ds_parse_aml
acpi_ps_execute_method
acpi_ns_evaluate
acpi_ut_evaluate_object
...

Re: [Bug 206175] Fedora >= 5.4 kernels instantly freeze on boot without producing any display output

[Christoph Hellwig]

On Wed, Mar 11, 2020 at 03:34:38PM +0000, Artem S. Tashkinov wrote:
> On 3/11/20 3:24 PM, Christoph Hellwig wrote:
>> pdev->dma_mask = parent->dma_mask ? *parent->dma_mask : 0;
>
> This patch makes no difference.
>
> The kernel panics with the same call trace which starts with:

This looks really strange and not dma mask related, but there must be
some odd interactions somewhere.

Can you call gdb on the vmlinux file for the 5.5.8-200.fc31 kernel
in the jpg and then do

l *(kmem_cache_alloc_trace+0x7e)

l *(acpi_processor_add+0x3a)

and send the output?

Re: [Bug 206175] Fedora >= 5.4 kernels instantly freeze on boot without producing any display output

[Christoph Hellwig]

And actually one more idea after looking at what slab interactions
could exist.  platform_device_register_full frees the dma_mask
unconditionally, even if it didn't allocated it, which might lead
to weird memory corruption if we hit the failure path.  So let's try
something like this, replacing the earlier patch in that file.

diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index b230beb6ccb4..04080a8d94e2 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -632,19 +632,6 @@ struct platform_device *platform_device_register_full(
 	pdev->dev.of_node_reused = pdevinfo->of_node_reused;
 
 	if (pdevinfo->dma_mask) {
-		/*
-		 * This memory isn't freed when the device is put,
-		 * I don't have a nice idea for that though.  Conceptually
-		 * dma_mask in struct device should not be a pointer.
-		 * See http://thread.gmane.org/gmane.linux.kernel.pci/9081
-		 */
-		pdev->dev.dma_mask =
-			kmalloc(sizeof(*pdev->dev.dma_mask), GFP_KERNEL);
-		if (!pdev->dev.dma_mask)
-			goto err;
-
-		kmemleak_ignore(pdev->dev.dma_mask);
-
 		*pdev->dev.dma_mask = pdevinfo->dma_mask;
 		pdev->dev.coherent_dma_mask = pdevinfo->dma_mask;
 	}
@@ -670,7 +657,6 @@ struct platform_device *platform_device_register_full(
 	if (ret) {
 err:
 		ACPI_COMPANION_SET(&pdev->dev, NULL);
-		kfree(pdev->dev.dma_mask);
 		platform_device_put(pdev);
 		return ERR_PTR(ret);
 	}

Re: [Bug 206175] Fedora >= 5.4 kernels instantly freeze on boot without producing any display output

[Artem S. Tashkinov]

On 3/11/20 3:43 PM, Christoph Hellwig wrote:
> On Wed, Mar 11, 2020 at 03:34:38PM +0000, Artem S. Tashkinov wrote:
>> On 3/11/20 3:24 PM, Christoph Hellwig wrote:
>>> pdev->dma_mask = parent->dma_mask ? *parent->dma_mask : 0;
>>
>> This patch makes no difference.
>>
>> The kernel panics with the same call trace which starts with:
>
> This looks really strange and not dma mask related, but there must be
> some odd interactions somewhere.
>
> Can you call gdb on the vmlinux file for the 5.5.8-200.fc31 kernel
> in the jpg and then do
>
> l *(kmem_cache_alloc_trace+0x7e)
>
> l *(acpi_processor_add+0x3a)
>
> and send the output?
>

I'm not sure I can call or do anything because the system is dead and
I'm looking at the kernel panic message. The console is dead. The root
file system is not yet mounted. Initrd can't be loaded either. I have no
COM port/console. I have no debugging abilities whatsoever. I can only
compile kernels and try running them.

Re: [Bug 206175] Fedora >= 5.4 kernels instantly freeze on boot without producing any display output

[Linus Torvalds]

On Wed, Mar 11, 2020 at 8:53 AM Artem S. Tashkinov <aros@gmx.com> wrote:
>
> I'm not sure I can call or do anything because the system is dead and
> I'm looking at the kernel panic message. The console is dead. The root
> file system is not yet mounted. Initrd can't be loaded either. I have no
> COM port/console. I have no debugging abilities whatsoever. I can only
> compile kernels and try running them.

Christoph just wanted you to use the image you booted with - you can
do it while using a working and entirely unrealted kernel.

But I think Christoph's second email was right on the money: the
platform device code used to (accidentally) always use that special
kmalloc()'ed memory, and the "always use kfree() to release" then
happened to work.

But with the change, platform devices use that allocations inside the
platform device itself, and the kfree() now does bad things and
corrupts the kmalloc lists.

So that finally makes sense of why that commit would cause odd
problems for you. I'm actually surprised it didn't cause problems for
others, but it's an error path, and presumably it normally never
triggers.

                Linus

              Linus

Re: [Bug 206175] Fedora >= 5.4 kernels instantly freeze on boot without producing any display output

[Artem S. Tashkinov]

On 3/11/20 3:47 PM, Christoph Hellwig wrote:
> And actually one more idea after looking at what slab interactions
> could exist.  platform_device_register_full frees the dma_mask
> unconditionally, even if it didn't allocated it, which might lead
> to weird memory corruption if we hit the failure path.  So let's try
> something like this, replacing the earlier patch in that file.
>
> diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> index b230beb6ccb4..04080a8d94e2 100644
> --- a/drivers/base/platform.c
> +++ b/drivers/base/platform.c
> @@ -632,19 +632,6 @@ struct platform_device *platform_device_register_full(
>   	pdev->dev.of_node_reused = pdevinfo->of_node_reused;
>
>   	if (pdevinfo->dma_mask) {
> -		/*
> -		 * This memory isn't freed when the device is put,
> -		 * I don't have a nice idea for that though.  Conceptually
> -		 * dma_mask in struct device should not be a pointer.
> -		 * See http://thread.gmane.org/gmane.linux.kernel.pci/9081
> -		 */
> -		pdev->dev.dma_mask =
> -			kmalloc(sizeof(*pdev->dev.dma_mask), GFP_KERNEL);
> -		if (!pdev->dev.dma_mask)
> -			goto err;
> -
> -		kmemleak_ignore(pdev->dev.dma_mask);
> -
>   		*pdev->dev.dma_mask = pdevinfo->dma_mask;
>   		pdev->dev.coherent_dma_mask = pdevinfo->dma_mask;
>   	}
> @@ -670,7 +657,6 @@ struct platform_device *platform_device_register_full(
>   	if (ret) {
>   err:
>   		ACPI_COMPANION_SET(&pdev->dev, NULL);
> -		kfree(pdev->dev.dma_mask);
>   		platform_device_put(pdev);
>   		return ERR_PTR(ret);
>   	}
>

With this patch the system works (I haven't created an initrd, so it
doesn't completely boot and panics on not being able to mount root fs
but that's expected).

Re: [Bug 206175] Fedora >= 5.4 kernels instantly freeze on boot without producing any display output

[Robin Murphy]

On 11/03/2020 4:02 pm, Artem S. Tashkinov wrote:
> On 3/11/20 3:47 PM, Christoph Hellwig wrote:
>> And actually one more idea after looking at what slab interactions
>> could exist.  platform_device_register_full frees the dma_mask
>> unconditionally, even if it didn't allocated it, which might lead
>> to weird memory corruption if we hit the failure path.  So let's try
>> something like this, replacing the earlier patch in that file.
>>
>> diff --git a/drivers/base/platform.c b/drivers/base/platform.c
>> index b230beb6ccb4..04080a8d94e2 100644
>> --- a/drivers/base/platform.c
>> +++ b/drivers/base/platform.c
>> @@ -632,19 +632,6 @@ struct platform_device 
>> *platform_device_register_full(
>>       pdev->dev.of_node_reused = pdevinfo->of_node_reused;
>>
>>       if (pdevinfo->dma_mask) {
>> -        /*
>> -         * This memory isn't freed when the device is put,
>> -         * I don't have a nice idea for that though.  Conceptually
>> -         * dma_mask in struct device should not be a pointer.
>> -         * See http://thread.gmane.org/gmane.linux.kernel.pci/9081
>> -         */
>> -        pdev->dev.dma_mask =
>> -            kmalloc(sizeof(*pdev->dev.dma_mask), GFP_KERNEL);
>> -        if (!pdev->dev.dma_mask)
>> -            goto err;
>> -
>> -        kmemleak_ignore(pdev->dev.dma_mask);
>> -
>>           *pdev->dev.dma_mask = pdevinfo->dma_mask;
>>           pdev->dev.coherent_dma_mask = pdevinfo->dma_mask;
>>       }
>> @@ -670,7 +657,6 @@ struct platform_device 
>> *platform_device_register_full(
>>       if (ret) {
>>   err:
>>           ACPI_COMPANION_SET(&pdev->dev, NULL);
>> -        kfree(pdev->dev.dma_mask);
>>           platform_device_put(pdev);
>>           return ERR_PTR(ret);
>>       }
>>
> 
> With this patch the system works (I haven't created an initrd, so it
> doesn't completely boot and panics on not being able to mount root fs
> but that's expected).

Yup, a few lines earlier in the log you can see the wdat_wdt driver 
failing in platform_device_add(), which since it called into 
platform_device_register_full() with pdevinfo.dma_mask = 0, will have 
unwound into that kfree() of pdev.dma_mask corrupting the heap.

Robin.

Re: [Bug 206175] Fedora >= 5.4 kernels instantly freeze on boot without producing any display output

[Linus Torvalds]

On Wed, Mar 11, 2020 at 9:02 AM Artem S. Tashkinov <aros@gmx.com> wrote:
>
> With this patch the system works (I haven't created an initrd, so it
> doesn't completely boot and panics on not being able to mount root fs
> but that's expected).

Perfect.

I ended up applying my earlier cleanup patch with just the added
removal of the kfree(), which was the actual trigger of the bug.

It's commit e423fb6929d4 ("driver code: clarify and fix platform
device DMA mask allocation") in my tree. I've not pushed it out yet (I
have a few pending pull requests), but it should be out shortly.

           Linus

Re: [Bug 206175] Fedora >= 5.4 kernels instantly freeze on boot without producing any display output

[Linus Torvalds]

On Wed, Mar 11, 2020 at 9:21 AM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> It's commit e423fb6929d4 ("driver code: clarify and fix platform
> device DMA mask allocation") in my tree. I've not pushed it out yet (I
> have a few pending pull requests), but it should be out shortly.

Actually, looking at other emails in my mailbox I see that Christoph
send a patch with a sign-off, and there's a reviewed-by too, so since
I haven't pushed mine out yet, I'll edit that up and give credit to
Christoph properly, and add the reviewed-by.

So it will have a different commit ID, updated message, and be a mix
of my patch and Christoph's.

             Linus

Re: [Bug 206175] Fedora >= 5.4 kernels instantly freeze on boot without producing any display output

[Linus Torvalds]

On Wed, Mar 11, 2020 at 9:24 AM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> So it will have a different commit ID, updated message, and be a mix
> of my patch and Christoph's.

I ended up pushing it out before starting on the pull requests, so
it's out there now.

Artem, it would be good to have confirmation that my (modified) tip of
tree now works for you. I don't actually doubt it does, but a final
confirmation would be appreciated.

            Linus

Re: [Bug 206175] Fedora >= 5.4 kernels instantly freeze on boot without producing any display output

[Artem S. Tashkinov]

On 3/11/20 4:48 PM, Linus Torvalds wrote:
> On Wed, Mar 11, 2020 at 9:24 AM Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
>>
>> So it will have a different commit ID, updated message, and be a mix
>> of my patch and Christoph's.
>
> I ended up pushing it out before starting on the pull requests, so
> it's out there now.
>
> Artem, it would be good to have confirmation that my (modified) tip of
> tree now works for you. I don't actually doubt it does, but a final
> confirmation would be appreciated.
>

Should I test kernel 5.6-rc5 with this patch applied or wait for you to
commit it?

Re: [Bug 206175] Fedora >= 5.4 kernels instantly freeze on boot without producing any display output

[Artem S. Tashkinov]

On 3/11/20 4:21 PM, Linus Torvalds wrote:
> On Wed, Mar 11, 2020 at 9:02 AM Artem S. Tashkinov <aros@gmx.com> wrote:
>>
>> With this patch the system works (I haven't created an initrd, so it
>> doesn't completely boot and panics on not being able to mount root fs
>> but that's expected).
>
> Perfect.
>
> I ended up applying my earlier cleanup patch with just the added
> removal of the kfree(), which was the actual trigger of the bug.
>
> It's commit e423fb6929d4 ("driver code: clarify and fix platform
> device DMA mask allocation") in my tree. I've not pushed it out yet (I
> have a few pending pull requests), but it should be out shortly.


I've been able to compile and run
e3a36eb6dfaeea8175c05d5915dcf0b939be6dab successfully. I won't claim
this patch doesn't break something for other people :-)

Best regards,
Artem

Re: [Bug 206175] Fedora >= 5.4 kernels instantly freeze on boot without producing any display output

[Linus Torvalds]

On Wed, Mar 11, 2020 at 10:21 AM Artem S. Tashkinov <aros@gmx.com> wrote:
>
> I've been able to compile and run
> e3a36eb6dfaeea8175c05d5915dcf0b939be6dab successfully. I won't claim
> this patch doesn't break something for other people :-)

Thanks, that's all I was looking for.

If it breaks something for somebody else, I have a solid plan for that
too: I'll just sit in a corner and cry.

               Linus