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.
