| Message ID | 20230720101431.71640c8a@imladris.surriel.com |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | mm,ima,kexec: use memblock_free_late from ima_free_kexec_buffer | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | warning | total: 0 errors, 1 warnings, 26 lines checked |
| robh/patch-applied | fail | build log |
On Thu, Jul 20, 2023 at 10:14:31AM -0400, Rik van Riel wrote: > The code calling ima_free_kexec_buffer runs long after the memblock > allocator has already been torn down, potentially resulting in a use > after free in memblock_isolate_range. > > With KASAN or KFENCE, this use after free will result in a BUG > from the idle task, and a subsequent kernel panic. > > Switch ima_free_kexec_buffer over to memblock_free_late to avoid > that issue. > > Fixes: fee3ff99bc67 ("powerpc: Move arch independent ima kexec functions to drivers/of/kexec.c") Fixes: b69a2afd5afc ("x86/kexec: Carry forward IMA measurement log on kexec") Acked-by: Rob Herring <robh@kernel.org> (I'm assuming someone else is taking this) > Cc: stable@kernel.org > Signed-off-by: Rik van Riel <riel@surriel.com> > Suggested-by: Mike Rappoport <rppt@kernel.org> > --- > arch/x86/kernel/setup.c | 8 ++------ > drivers/of/kexec.c | 3 ++- > 2 files changed, 4 insertions(+), 7 deletions(-) > > diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c > index fd975a4a5200..aa0df37c1fe7 100644 > --- a/arch/x86/kernel/setup.c > +++ b/arch/x86/kernel/setup.c > @@ -359,15 +359,11 @@ static void __init add_early_ima_buffer(u64 phys_addr) > #if defined(CONFIG_HAVE_IMA_KEXEC) && !defined(CONFIG_OF_FLATTREE) > int __init ima_free_kexec_buffer(void) > { > - int rc; > - > if (!ima_kexec_buffer_size) > return -ENOENT; > > - rc = memblock_phys_free(ima_kexec_buffer_phys, > - ima_kexec_buffer_size); > - if (rc) > - return rc; > + memblock_free_late(ima_kexec_buffer_phys, > + ima_kexec_buffer_size); > > ima_kexec_buffer_phys = 0; > ima_kexec_buffer_size = 0; > diff --git a/drivers/of/kexec.c b/drivers/of/kexec.c > index f26d2ba8a371..68278340cecf 100644 > --- a/drivers/of/kexec.c > +++ b/drivers/of/kexec.c > @@ -184,7 +184,8 @@ int __init ima_free_kexec_buffer(void) > if (ret) > return ret; > > - return memblock_phys_free(addr, size); > + memblock_free_late(addr, size); > + return 0; > } > #endif > > -- > 2.41.0 >
On Fri, 2023-07-21 at 13:38 -0600, Rob Herring wrote: > On Thu, Jul 20, 2023 at 10:14:31AM -0400, Rik van Riel wrote: > > The code calling ima_free_kexec_buffer runs long after the memblock > > allocator has already been torn down, potentially resulting in a > > use > > after free in memblock_isolate_range. > > > > With KASAN or KFENCE, this use after free will result in a BUG > > from the idle task, and a subsequent kernel panic. > > > > Switch ima_free_kexec_buffer over to memblock_free_late to avoid > > that issue. > > > > Fixes: fee3ff99bc67 ("powerpc: Move arch independent ima kexec > > functions to drivers/of/kexec.c") > > Fixes: b69a2afd5afc ("x86/kexec: Carry forward IMA measurement log on > kexec") > Thank you for digging further back in the history of that code. > Acked-by: Rob Herring <robh@kernel.org> > > (I'm assuming someone else is taking this) I hope so, but I don't know who...
On Fri, Jul 21, 2023 at 08:40:41PM -0400, Rik van Riel wrote: > On Fri, 2023-07-21 at 13:38 -0600, Rob Herring wrote: > > On Thu, Jul 20, 2023 at 10:14:31AM -0400, Rik van Riel wrote: > > > The code calling ima_free_kexec_buffer runs long after the memblock > > > allocator has already been torn down, potentially resulting in a > > > use > > > after free in memblock_isolate_range. > > > > > > With KASAN or KFENCE, this use after free will result in a BUG > > > from the idle task, and a subsequent kernel panic. > > > > > > Switch ima_free_kexec_buffer over to memblock_free_late to avoid > > > that issue. > > > > > > Fixes: fee3ff99bc67 ("powerpc: Move arch independent ima kexec > > > functions to drivers/of/kexec.c") > > > > Fixes: b69a2afd5afc ("x86/kexec: Carry forward IMA measurement log on > > kexec") > > > Thank you for digging further back in the history of that code. > > > Acked-by: Rob Herring <robh@kernel.org> > > > > (I'm assuming someone else is taking this) > > I hope so, but I don't know who... You could split this into 2 patches, then there's no doubt. And each Fixes is clear. Rob
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c index fd975a4a5200..aa0df37c1fe7 100644 --- a/arch/x86/kernel/setup.c +++ b/arch/x86/kernel/setup.c @@ -359,15 +359,11 @@ static void __init add_early_ima_buffer(u64 phys_addr) #if defined(CONFIG_HAVE_IMA_KEXEC) && !defined(CONFIG_OF_FLATTREE) int __init ima_free_kexec_buffer(void) { - int rc; - if (!ima_kexec_buffer_size) return -ENOENT; - rc = memblock_phys_free(ima_kexec_buffer_phys, - ima_kexec_buffer_size); - if (rc) - return rc; + memblock_free_late(ima_kexec_buffer_phys, + ima_kexec_buffer_size); ima_kexec_buffer_phys = 0; ima_kexec_buffer_size = 0; diff --git a/drivers/of/kexec.c b/drivers/of/kexec.c index f26d2ba8a371..68278340cecf 100644 --- a/drivers/of/kexec.c +++ b/drivers/of/kexec.c @@ -184,7 +184,8 @@ int __init ima_free_kexec_buffer(void) if (ret) return ret; - return memblock_phys_free(addr, size); + memblock_free_late(addr, size); + return 0; } #endif
The code calling ima_free_kexec_buffer runs long after the memblock allocator has already been torn down, potentially resulting in a use after free in memblock_isolate_range. With KASAN or KFENCE, this use after free will result in a BUG from the idle task, and a subsequent kernel panic. Switch ima_free_kexec_buffer over to memblock_free_late to avoid that issue. Fixes: fee3ff99bc67 ("powerpc: Move arch independent ima kexec functions to drivers/of/kexec.c") Cc: stable@kernel.org Signed-off-by: Rik van Riel <riel@surriel.com> Suggested-by: Mike Rappoport <rppt@kernel.org> --- arch/x86/kernel/setup.c | 8 ++------ drivers/of/kexec.c | 3 ++- 2 files changed, 4 insertions(+), 7 deletions(-)