struct dev_pagemap corruption
diff mbox series

Message ID 7885dce0-edbe-db04-b5ec-bd271c9a0612@arm.com
State New
Headers show
Series
  • struct dev_pagemap corruption
Related show

Commit Message

Anshuman Khandual April 5, 2019, 4:40 a.m. UTC
Hello,

On arm64 platform "struct dev_pagemap" is getting corrupted during ZONE_DEVICE
unmapping path through device_destroy(). Its device memory range end address
(pgmap->res.end) which is getting corrupted in this particular case. AFAICS
pgmap which gets initialized by the driver and mapped with devm_memremap_pages()
should retain it's values during the unmapping path as well. Is this assumption
right ?

[   62.779412] Call trace:
[   62.779808]  dump_backtrace+0x0/0x118
[   62.780460]  show_stack+0x14/0x20
[   62.781204]  dump_stack+0xa8/0xcc
[   62.781941]  devm_memremap_pages_release+0x24/0x1d8
[   62.783021]  devm_action_release+0x10/0x18
[   62.783911]  release_nodes+0x1b0/0x220
[   62.784732]  devres_release_all+0x34/0x50
[   62.785623]  device_release+0x24/0x90
[   62.786454]  kobject_put+0x74/0xe8
[   62.787214]  device_destroy+0x48/0x58
[   62.788041]  zone_device_public_altmap_init+0x404/0x42c [zone_device_public_altmap]
[   62.789675]  do_one_initcall+0x74/0x190
[   62.790528]  do_init_module+0x50/0x1c0
[   62.791346]  load_module+0x1be4/0x2140
[   62.792192]  __se_sys_finit_module+0xb8/0xc8
[   62.793128]  __arm64_sys_finit_module+0x18/0x20
[   62.794128]  el0_svc_handler+0x88/0x100
[   62.794989]  el0_svc+0x8/0xc

The problem can be traced down here.


dev_pagemap being added:

#define ZONE_DEVICE_PHYS_START 0x680000000UL
#define ZONE_DEVICE_PHYS_END   0x6BFFFFFFFUL
#define ALTMAP_FREE 4096
#define ALTMAP_RESV 1024

	pgmap->type = MEMORY_DEVICE_PUBLIC;
	pgmap->res.start = ZONE_DEVICE_PHYS_START;
	pgmap->res.end = ZONE_DEVICE_PHYS_END;
	pgmap->ref = ref;
	pgmap->kill = zone_device_percpu_kill;
	pgmap->dev = dev;

	memset(&pgmap->altmap, 0, sizeof(struct vmem_altmap));
	pgmap->altmap.free = ALTMAP_FREE;
	pgmap->altmap.alloc = 0;
	pgmap->altmap.align = 0;
	pgmap->altmap_valid = 1;

	tmp = (unsigned long *)&pgmap->altmap.base_pfn;
	tmp1 = (unsigned long *)&pgmap->altmap.reserve;

	*tmp = pgmap->res.start >> PAGE_SHIFT;
	*tmp1 = ALTMAP_RESV;

With the patch:

[   53.027865] XXX: zone_device_public_altmap_init pgmap ffff8005de634218 resource ffff8005de634250 res->start 680000000 res->end 6bfffffff size 40000000
[   53.029840] XXX: devm_memremap_pages_release pgmap ffff8005de634218 resource ffff8005de634250 res->start 680000000 res->end 6bfffffff size 40000000

Without the patch:

[   34.326066] XXX: zone_device_public_altmap_init pgmap ffff8005de530a80 resource ffff8005de530ab8 res->start 680000000 res->end 6bfffffff size 40000000
[   34.328063] XXX: devm_memremap_pages_release pgmap ffff8005de530a80 resource ffff8005de530ab8 res->start 680000000 res->end 0 size fffffff980000001

Though this prevents the above corruption I wonder what was causing it in the
first place and how we can address the problem.

- Anshuman

Comments

Robin Murphy April 5, 2019, 1:37 p.m. UTC | #1
On 05/04/2019 05:40, Anshuman Khandual wrote:
> Hello,
> 
> On arm64 platform "struct dev_pagemap" is getting corrupted during ZONE_DEVICE
> unmapping path through device_destroy(). Its device memory range end address
> (pgmap->res.end) which is getting corrupted in this particular case. AFAICS
> pgmap which gets initialized by the driver and mapped with devm_memremap_pages()
> should retain it's values during the unmapping path as well. Is this assumption
> right ?
> 
> [   62.779412] Call trace:
> [   62.779808]  dump_backtrace+0x0/0x118
> [   62.780460]  show_stack+0x14/0x20
> [   62.781204]  dump_stack+0xa8/0xcc
> [   62.781941]  devm_memremap_pages_release+0x24/0x1d8
> [   62.783021]  devm_action_release+0x10/0x18
> [   62.783911]  release_nodes+0x1b0/0x220
> [   62.784732]  devres_release_all+0x34/0x50
> [   62.785623]  device_release+0x24/0x90
> [   62.786454]  kobject_put+0x74/0xe8
> [   62.787214]  device_destroy+0x48/0x58
> [   62.788041]  zone_device_public_altmap_init+0x404/0x42c [zone_device_public_altmap]
> [   62.789675]  do_one_initcall+0x74/0x190
> [   62.790528]  do_init_module+0x50/0x1c0
> [   62.791346]  load_module+0x1be4/0x2140
> [   62.792192]  __se_sys_finit_module+0xb8/0xc8
> [   62.793128]  __arm64_sys_finit_module+0x18/0x20
> [   62.794128]  el0_svc_handler+0x88/0x100
> [   62.794989]  el0_svc+0x8/0xc
> 
> The problem can be traced down here.
> 
> diff --git a/drivers/base/devres.c b/drivers/base/devres.c
> index e038e2b3b7ea..2a410c88c596 100644
> --- a/drivers/base/devres.c
> +++ b/drivers/base/devres.c
> @@ -33,7 +33,7 @@ struct devres {
>           * Thus we use ARCH_KMALLOC_MINALIGN here and get exactly the same
>           * buffer alignment as if it was allocated by plain kmalloc().
>           */
> -       u8 __aligned(ARCH_KMALLOC_MINALIGN) data[];
> +       u8 __aligned(__alignof__(unsigned long long)) data[];
>   };
> 
> On arm64 ARCH_KMALLOC_MINALIGN -> ARCH_DMA_MINALIGN -> 128
> 
> dev_pagemap being added:
> 
> #define ZONE_DEVICE_PHYS_START 0x680000000UL
> #define ZONE_DEVICE_PHYS_END   0x6BFFFFFFFUL
> #define ALTMAP_FREE 4096
> #define ALTMAP_RESV 1024
> 
> 	pgmap->type = MEMORY_DEVICE_PUBLIC;

Given that what seems to ultimately get corrupted is the memory pointed 
to by pgmap here, how is *that* being allocated?

Robin.

> 	pgmap->res.start = ZONE_DEVICE_PHYS_START;
> 	pgmap->res.end = ZONE_DEVICE_PHYS_END;
> 	pgmap->ref = ref;
> 	pgmap->kill = zone_device_percpu_kill;
> 	pgmap->dev = dev;
> 
> 	memset(&pgmap->altmap, 0, sizeof(struct vmem_altmap));
> 	pgmap->altmap.free = ALTMAP_FREE;
> 	pgmap->altmap.alloc = 0;
> 	pgmap->altmap.align = 0;
> 	pgmap->altmap_valid = 1;
> 
> 	tmp = (unsigned long *)&pgmap->altmap.base_pfn;
> 	tmp1 = (unsigned long *)&pgmap->altmap.reserve;
> 
> 	*tmp = pgmap->res.start >> PAGE_SHIFT;
> 	*tmp1 = ALTMAP_RESV;
> 
> With the patch:
> 
> [   53.027865] XXX: zone_device_public_altmap_init pgmap ffff8005de634218 resource ffff8005de634250 res->start 680000000 res->end 6bfffffff size 40000000
> [   53.029840] XXX: devm_memremap_pages_release pgmap ffff8005de634218 resource ffff8005de634250 res->start 680000000 res->end 6bfffffff size 40000000
> 
> Without the patch:
> 
> [   34.326066] XXX: zone_device_public_altmap_init pgmap ffff8005de530a80 resource ffff8005de530ab8 res->start 680000000 res->end 6bfffffff size 40000000
> [   34.328063] XXX: devm_memremap_pages_release pgmap ffff8005de530a80 resource ffff8005de530ab8 res->start 680000000 res->end 0 size fffffff980000001
> 
> Though this prevents the above corruption I wonder what was causing it in the
> first place and how we can address the problem.
> 
> - Anshuman
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
Catalin Marinas April 5, 2019, 1:42 p.m. UTC | #2
Hi Anshuman,

On Fri, Apr 05, 2019 at 10:10:22AM +0530, Anshuman Khandual wrote:
> On arm64 platform "struct dev_pagemap" is getting corrupted during ZONE_DEVICE
> unmapping path through device_destroy(). Its device memory range end address
> (pgmap->res.end) which is getting corrupted in this particular case. AFAICS
> pgmap which gets initialized by the driver and mapped with devm_memremap_pages()
> should retain it's values during the unmapping path as well. Is this assumption
> right ?
[...]
> The problem can be traced down here.
> 
> diff --git a/drivers/base/devres.c b/drivers/base/devres.c
> index e038e2b3b7ea..2a410c88c596 100644
> --- a/drivers/base/devres.c
> +++ b/drivers/base/devres.c
> @@ -33,7 +33,7 @@ struct devres {
>          * Thus we use ARCH_KMALLOC_MINALIGN here and get exactly the same
>          * buffer alignment as if it was allocated by plain kmalloc().
>          */
> -       u8 __aligned(ARCH_KMALLOC_MINALIGN) data[];
> +       u8 __aligned(__alignof__(unsigned long long)) data[];
>  };
[...]
> With the patch:
> 
> [   53.027865] XXX: zone_device_public_altmap_init pgmap ffff8005de634218 resource ffff8005de634250 res->start 680000000 res->end 6bfffffff size 40000000
> [   53.029840] XXX: devm_memremap_pages_release pgmap ffff8005de634218 resource ffff8005de634250 res->start 680000000 res->end 6bfffffff size 40000000
> 
> Without the patch:
> 
> [   34.326066] XXX: zone_device_public_altmap_init pgmap ffff8005de530a80 resource ffff8005de530ab8 res->start 680000000 res->end 6bfffffff size 40000000
> [   34.328063] XXX: devm_memremap_pages_release pgmap ffff8005de530a80 resource ffff8005de530ab8 res->start 680000000 res->end 0 size fffffff980000001

OK, so without this patch pgmap->res.end becomes 0 while it should stay
at 0x6bfffffff. Is it easy to reproduce with mainline?

What's zone_device_public_altmap_init? I couldn't grep it in mainline.
How's the pgmap allocated?

I'd suggest you enable kasan and see if it spots anything.
Jerome Glisse April 5, 2019, 1:53 p.m. UTC | #3
On Fri, Apr 05, 2019 at 10:10:22AM +0530, Anshuman Khandual wrote:
> Hello,
> 
> On arm64 platform "struct dev_pagemap" is getting corrupted during ZONE_DEVICE
> unmapping path through device_destroy(). Its device memory range end address
> (pgmap->res.end) which is getting corrupted in this particular case. AFAICS
> pgmap which gets initialized by the driver and mapped with devm_memremap_pages()
> should retain it's values during the unmapping path as well. Is this assumption
> right ?
> 
> [   62.779412] Call trace:
> [   62.779808]  dump_backtrace+0x0/0x118
> [   62.780460]  show_stack+0x14/0x20
> [   62.781204]  dump_stack+0xa8/0xcc
> [   62.781941]  devm_memremap_pages_release+0x24/0x1d8
> [   62.783021]  devm_action_release+0x10/0x18
> [   62.783911]  release_nodes+0x1b0/0x220
> [   62.784732]  devres_release_all+0x34/0x50
> [   62.785623]  device_release+0x24/0x90
> [   62.786454]  kobject_put+0x74/0xe8
> [   62.787214]  device_destroy+0x48/0x58
> [   62.788041]  zone_device_public_altmap_init+0x404/0x42c [zone_device_public_altmap]
> [   62.789675]  do_one_initcall+0x74/0x190
> [   62.790528]  do_init_module+0x50/0x1c0
> [   62.791346]  load_module+0x1be4/0x2140
> [   62.792192]  __se_sys_finit_module+0xb8/0xc8
> [   62.793128]  __arm64_sys_finit_module+0x18/0x20
> [   62.794128]  el0_svc_handler+0x88/0x100
> [   62.794989]  el0_svc+0x8/0xc
> 
> The problem can be traced down here.
> 
> diff --git a/drivers/base/devres.c b/drivers/base/devres.c
> index e038e2b3b7ea..2a410c88c596 100644
> --- a/drivers/base/devres.c
> +++ b/drivers/base/devres.c
> @@ -33,7 +33,7 @@ struct devres {
>          * Thus we use ARCH_KMALLOC_MINALIGN here and get exactly the same
>          * buffer alignment as if it was allocated by plain kmalloc().
>          */
> -       u8 __aligned(ARCH_KMALLOC_MINALIGN) data[];
> +       u8 __aligned(__alignof__(unsigned long long)) data[];
>  };

I doubt that pgmap->res.end get corrupted during device_destroy() but given
that the above changes fix the issue it kind of boggle the mind. If i where
to debug this i would probably run a kernel with qemu -s to get a gdbserver
and then attach gdb and set breakpoint on devm_memremap_pages() then when
that trigger i would set memory watch on the pgmap->res.end (there use to be
way to use memory as pmem through kernel boot option).

A printk alternative solution is, assuming you only have one pgmap, add a
global static struct pgmap *debug_pgmap = NULL; in memremap.c set that in
devm_memremap_pages() and add an helper function:

void debug_pgmap(const char *file, unsigned line)
{
    printk(... file, line);
    printk(... debug_pmap->res.end);
}

In a header:
#define DEBUG_PGMAP debug_pgmap(__FILE__, __LINE__);

Then sprinkle DEBUG_PGMAP within device_destroy(), device_unregister(),
device_release() and see when it get corrupted.

gdb would be faster but sometime i got issue with memory watchpoint and virt.

Cheers,
Jérôme
Anshuman Khandual April 5, 2019, 2:54 p.m. UTC | #4
On 04/05/2019 07:07 PM, Robin Murphy wrote:
> On 05/04/2019 05:40, Anshuman Khandual wrote:
>> Hello,
>>
>> On arm64 platform "struct dev_pagemap" is getting corrupted during ZONE_DEVICE
>> unmapping path through device_destroy(). Its device memory range end address
>> (pgmap->res.end) which is getting corrupted in this particular case. AFAICS
>> pgmap which gets initialized by the driver and mapped with devm_memremap_pages()
>> should retain it's values during the unmapping path as well. Is this assumption
>> right ?
>>
>> [   62.779412] Call trace:
>> [   62.779808]  dump_backtrace+0x0/0x118
>> [   62.780460]  show_stack+0x14/0x20
>> [   62.781204]  dump_stack+0xa8/0xcc
>> [   62.781941]  devm_memremap_pages_release+0x24/0x1d8
>> [   62.783021]  devm_action_release+0x10/0x18
>> [   62.783911]  release_nodes+0x1b0/0x220
>> [   62.784732]  devres_release_all+0x34/0x50
>> [   62.785623]  device_release+0x24/0x90
>> [   62.786454]  kobject_put+0x74/0xe8
>> [   62.787214]  device_destroy+0x48/0x58
>> [   62.788041]  zone_device_public_altmap_init+0x404/0x42c [zone_device_public_altmap]
>> [   62.789675]  do_one_initcall+0x74/0x190
>> [   62.790528]  do_init_module+0x50/0x1c0
>> [   62.791346]  load_module+0x1be4/0x2140
>> [   62.792192]  __se_sys_finit_module+0xb8/0xc8
>> [   62.793128]  __arm64_sys_finit_module+0x18/0x20
>> [   62.794128]  el0_svc_handler+0x88/0x100
>> [   62.794989]  el0_svc+0x8/0xc
>>
>> The problem can be traced down here.
>>
>> diff --git a/drivers/base/devres.c b/drivers/base/devres.c
>> index e038e2b3b7ea..2a410c88c596 100644
>> --- a/drivers/base/devres.c
>> +++ b/drivers/base/devres.c
>> @@ -33,7 +33,7 @@ struct devres {
>>           * Thus we use ARCH_KMALLOC_MINALIGN here and get exactly the same
>>           * buffer alignment as if it was allocated by plain kmalloc().
>>           */
>> -       u8 __aligned(ARCH_KMALLOC_MINALIGN) data[];
>> +       u8 __aligned(__alignof__(unsigned long long)) data[];
>>   };
>>
>> On arm64 ARCH_KMALLOC_MINALIGN -> ARCH_DMA_MINALIGN -> 128
>>
>> dev_pagemap being added:
>>
>> #define ZONE_DEVICE_PHYS_START 0x680000000UL
>> #define ZONE_DEVICE_PHYS_END   0x6BFFFFFFFUL
>> #define ALTMAP_FREE 4096
>> #define ALTMAP_RESV 1024
>>
>>     pgmap->type = MEMORY_DEVICE_PUBLIC;
> 
> Given that what seems to ultimately get corrupted is the memory pointed to by pgmap here, how is *that* being allocated?

struct dev_pagemap *pgmap;

pgmap = devm_kzalloc(dev, sizeof(struct dev_pagemap), GFP_KERNEL);

Is it problematic to use dev_kzalloc here instead of generic kmalloc/kzalloc
functions ? kzalloc() seems to solve the problem. Will do some more testing
tomorrow.
Anshuman Khandual April 5, 2019, 2:58 p.m. UTC | #5
On 04/05/2019 07:12 PM, Catalin Marinas wrote:
> Hi Anshuman,
> 
> On Fri, Apr 05, 2019 at 10:10:22AM +0530, Anshuman Khandual wrote:
>> On arm64 platform "struct dev_pagemap" is getting corrupted during ZONE_DEVICE
>> unmapping path through device_destroy(). Its device memory range end address
>> (pgmap->res.end) which is getting corrupted in this particular case. AFAICS
>> pgmap which gets initialized by the driver and mapped with devm_memremap_pages()
>> should retain it's values during the unmapping path as well. Is this assumption
>> right ?
> [...]
>> The problem can be traced down here.
>>
>> diff --git a/drivers/base/devres.c b/drivers/base/devres.c
>> index e038e2b3b7ea..2a410c88c596 100644
>> --- a/drivers/base/devres.c
>> +++ b/drivers/base/devres.c
>> @@ -33,7 +33,7 @@ struct devres {
>>          * Thus we use ARCH_KMALLOC_MINALIGN here and get exactly the same
>>          * buffer alignment as if it was allocated by plain kmalloc().
>>          */
>> -       u8 __aligned(ARCH_KMALLOC_MINALIGN) data[];
>> +       u8 __aligned(__alignof__(unsigned long long)) data[];
>>  };
> [...]
>> With the patch:
>>
>> [   53.027865] XXX: zone_device_public_altmap_init pgmap ffff8005de634218 resource ffff8005de634250 res->start 680000000 res->end 6bfffffff size 40000000
>> [   53.029840] XXX: devm_memremap_pages_release pgmap ffff8005de634218 resource ffff8005de634250 res->start 680000000 res->end 6bfffffff size 40000000
>>
>> Without the patch:
>>
>> [   34.326066] XXX: zone_device_public_altmap_init pgmap ffff8005de530a80 resource ffff8005de530ab8 res->start 680000000 res->end 6bfffffff size 40000000
>> [   34.328063] XXX: devm_memremap_pages_release pgmap ffff8005de530a80 resource ffff8005de530ab8 res->start 680000000 res->end 0 size fffffff980000001
> 
> OK, so without this patch pgmap->res.end becomes 0 while it should stay
> at 0x6bfffffff. Is it easy to reproduce with mainline?

Yeah but with some ZONE_DEVICE series patches.

> 
> What's zone_device_public_altmap_init? I couldn't grep it in mainline.

Test module inti function (should have mentioned it clearly).

> How's the pgmap allocated?

devm_kzalloc() but the problem seems to be gone with kzalloc().

> 
> I'd suggest you enable kasan and see if it spots anything.
> 

Sure.
Robin Murphy April 5, 2019, 3:57 p.m. UTC | #6
On 05/04/2019 15:54, Anshuman Khandual wrote:
> 
> 
> On 04/05/2019 07:07 PM, Robin Murphy wrote:
>> On 05/04/2019 05:40, Anshuman Khandual wrote:
>>> Hello,
>>>
>>> On arm64 platform "struct dev_pagemap" is getting corrupted during ZONE_DEVICE
>>> unmapping path through device_destroy(). Its device memory range end address
>>> (pgmap->res.end) which is getting corrupted in this particular case. AFAICS
>>> pgmap which gets initialized by the driver and mapped with devm_memremap_pages()
>>> should retain it's values during the unmapping path as well. Is this assumption
>>> right ?
>>>
>>> [   62.779412] Call trace:
>>> [   62.779808]  dump_backtrace+0x0/0x118
>>> [   62.780460]  show_stack+0x14/0x20
>>> [   62.781204]  dump_stack+0xa8/0xcc
>>> [   62.781941]  devm_memremap_pages_release+0x24/0x1d8
>>> [   62.783021]  devm_action_release+0x10/0x18
>>> [   62.783911]  release_nodes+0x1b0/0x220
>>> [   62.784732]  devres_release_all+0x34/0x50
>>> [   62.785623]  device_release+0x24/0x90
>>> [   62.786454]  kobject_put+0x74/0xe8
>>> [   62.787214]  device_destroy+0x48/0x58
>>> [   62.788041]  zone_device_public_altmap_init+0x404/0x42c [zone_device_public_altmap]
>>> [   62.789675]  do_one_initcall+0x74/0x190
>>> [   62.790528]  do_init_module+0x50/0x1c0
>>> [   62.791346]  load_module+0x1be4/0x2140
>>> [   62.792192]  __se_sys_finit_module+0xb8/0xc8
>>> [   62.793128]  __arm64_sys_finit_module+0x18/0x20
>>> [   62.794128]  el0_svc_handler+0x88/0x100
>>> [   62.794989]  el0_svc+0x8/0xc
>>>
>>> The problem can be traced down here.
>>>
>>> diff --git a/drivers/base/devres.c b/drivers/base/devres.c
>>> index e038e2b3b7ea..2a410c88c596 100644
>>> --- a/drivers/base/devres.c
>>> +++ b/drivers/base/devres.c
>>> @@ -33,7 +33,7 @@ struct devres {
>>>            * Thus we use ARCH_KMALLOC_MINALIGN here and get exactly the same
>>>            * buffer alignment as if it was allocated by plain kmalloc().
>>>            */
>>> -       u8 __aligned(ARCH_KMALLOC_MINALIGN) data[];
>>> +       u8 __aligned(__alignof__(unsigned long long)) data[];
>>>    };
>>>
>>> On arm64 ARCH_KMALLOC_MINALIGN -> ARCH_DMA_MINALIGN -> 128
>>>
>>> dev_pagemap being added:
>>>
>>> #define ZONE_DEVICE_PHYS_START 0x680000000UL
>>> #define ZONE_DEVICE_PHYS_END   0x6BFFFFFFFUL
>>> #define ALTMAP_FREE 4096
>>> #define ALTMAP_RESV 1024
>>>
>>>      pgmap->type = MEMORY_DEVICE_PUBLIC;
>>
>> Given that what seems to ultimately get corrupted is the memory pointed to by pgmap here, how is *that* being allocated?
> 
> struct dev_pagemap *pgmap;
> 
> pgmap = devm_kzalloc(dev, sizeof(struct dev_pagemap), GFP_KERNEL);
> 
> Is it problematic to use dev_kzalloc here instead of generic kmalloc/kzalloc
> functions ? kzalloc() seems to solve the problem. Will do some more testing
> tomorrow.

The important point is that your pgmap is a devres allocation itself, 
thus changing the alignment of devres::data is moving the thing which 
was getting corrupted to a different relative location:

(gdb) p sizeof(struct devres) + sizeof(struct dev_pagemap)
$1 = 296
(gdb) p sizeof(struct devres) + (size_t)&(((struct dev_pagemap*)0)->res.end)
$2 = 192

vs.

(gdb) p sizeof(struct devres_node) + sizeof(struct dev_pagemap)
$3 = 192
(gdb) p sizeof(struct devres_node) + (size_t)&(((struct 
dev_pagemap*)0)->res.end)
$4 = 88

The fact that the corruption of the 128-byte-aligned devres::data occurs 
exactly where the 8-byte-aligned devres::data would have ended is 
particularly suspicious...

Robin.
Dan Williams April 7, 2019, 10:25 p.m. UTC | #7
On Fri, Apr 5, 2019 at 7:54 AM Anshuman Khandual
<anshuman.khandual@arm.com> wrote:
[..]
> > Given that what seems to ultimately get corrupted is the memory pointed to by pgmap here, how is *that* being allocated?
>
> struct dev_pagemap *pgmap;
>
> pgmap = devm_kzalloc(dev, sizeof(struct dev_pagemap), GFP_KERNEL);
>
> Is it problematic to use dev_kzalloc here instead of generic kmalloc/kzalloc
> functions ?

On this specific question, no. devm_kzalloc() is how the pmem and
device-dax drivers allocate the pgmap passed to devm_memremap_pages().
The unwind order of the devres resources ensures that the
devm_memremap_pages() devres actions occur before the release of the
pgmap allocation.

Patch
diff mbox series

diff --git a/drivers/base/devres.c b/drivers/base/devres.c
index e038e2b3b7ea..2a410c88c596 100644
--- a/drivers/base/devres.c
+++ b/drivers/base/devres.c
@@ -33,7 +33,7 @@  struct devres {
         * Thus we use ARCH_KMALLOC_MINALIGN here and get exactly the same
         * buffer alignment as if it was allocated by plain kmalloc().
         */
-       u8 __aligned(ARCH_KMALLOC_MINALIGN) data[];
+       u8 __aligned(__alignof__(unsigned long long)) data[];
 };

On arm64 ARCH_KMALLOC_MINALIGN -> ARCH_DMA_MINALIGN -> 128