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.
