[v6,03/17] arm64: hibernate: check pgd table allocation
diff mbox series

Message ID 20191004185234.31471-4-pasha.tatashin@soleen.com
State New
Headers show
Series
  • arm64: MMU enabled kexec relocation
Related show

Commit Message

Pavel Tatashin Oct. 4, 2019, 6:52 p.m. UTC
There is a bug in create_safe_exec_page(), when page table is allocated
it is not checked that table is allocated successfully:

But it is dereferenced in: pgd_none(READ_ONCE(*pgdp)).  Check that
allocation was successful.

Fixes: 82869ac57b5d ("arm64: kernel: Add support for hibernate/suspend-to-disk")

Signed-off-by: Pavel Tatashin <pasha.tatashin@soleen.com>
---
 arch/arm64/kernel/hibernate.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

James Morse Oct. 11, 2019, 6:17 p.m. UTC | #1
Hi Pavel,

On 04/10/2019 19:52, Pavel Tatashin wrote:
> There is a bug in create_safe_exec_page(), when page table is allocated
> it is not checked that table is allocated successfully:
> 
> But it is dereferenced in: pgd_none(READ_ONCE(*pgdp)).  Check that
> allocation was successful.


> Fixes: 82869ac57b5d ("arm64: kernel: Add support for hibernate/suspend-to-disk")
> 
> Signed-off-by: Pavel Tatashin <pasha.tatashin@soleen.com>

Nit: Please remove the stray newline so all the tags appear together.


> diff --git a/arch/arm64/kernel/hibernate.c b/arch/arm64/kernel/hibernate.c
> index d52f69462c8f..ef46ce66d7e8 100644
> --- a/arch/arm64/kernel/hibernate.c
> +++ b/arch/arm64/kernel/hibernate.c
> @@ -217,6 +217,11 @@ static int create_safe_exec_page(void *src_start, size_t length,
>  	__flush_icache_range(dst, dst + length);
>  
>  	trans_pgd = allocator(mask);
> +	if (!trans_pgd) {
> +		rc = -ENOMEM;
> +		goto out;
> +	}
> +
>  	pgdp = pgd_offset_raw(trans_pgd, dst_addr);
>  	if (pgd_none(READ_ONCE(*pgdp))) {
>  		pudp = allocator(mask);
> 

Thanks for splitting [0] into two ... but this fix depends on the previous patch - which
isn't an issue that anyone can hit, and doesn't match Greg's 'stable-kernel-rules'.

Please separate out this patch - and post it on its own as a stand-alone fix that can be
sent to the stable trees.


Mixing fixes with other patches leads to problems like this. It isn't possible to pick
this fix independently of the cleanup in the previous patch.


Thanks,

James

[0] https://lore.kernel.org/linux-arm-kernel/ddd81093-89fc-5146-0b33-ad3bd9a1c10c@arm.com/
Pavel Tatashin Oct. 14, 2019, 2:51 p.m. UTC | #2
> Thanks for splitting [0] into two ... but this fix depends on the previous patch - which
> isn't an issue that anyone can hit, and doesn't match Greg's 'stable-kernel-rules'.
> 
> Please separate out this patch - and post it on its own as a stand-alone fix that can be
> sent to the stable trees.
> 
> 
> Mixing fixes with other patches leads to problems like this. It isn't possible to pick
> this fix independently of the cleanup in the previous patch.

Thank you, I sent it out as a separate fix.

Patch
diff mbox series

diff --git a/arch/arm64/kernel/hibernate.c b/arch/arm64/kernel/hibernate.c
index d52f69462c8f..ef46ce66d7e8 100644
--- a/arch/arm64/kernel/hibernate.c
+++ b/arch/arm64/kernel/hibernate.c
@@ -217,6 +217,11 @@  static int create_safe_exec_page(void *src_start, size_t length,
 	__flush_icache_range(dst, dst + length);
 
 	trans_pgd = allocator(mask);
+	if (!trans_pgd) {
+		rc = -ENOMEM;
+		goto out;
+	}
+
 	pgdp = pgd_offset_raw(trans_pgd, dst_addr);
 	if (pgd_none(READ_ONCE(*pgdp))) {
 		pudp = allocator(mask);