public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/1] x86/sev: Fix potential sign extension in amd_enc_status_change_finish()
@ 2026-04-19  9:37 YangWencheng
  0 siblings, 0 replies; 5+ messages in thread
From: YangWencheng @ 2026-04-19  9:37 UTC (permalink / raw)
  Cc: linux-kernel, YangWencheng, YangWencheng

When npages (int) is left-shifted by PAGE_SHIFT to calculate the size
in bytes, the operation is performed on a signed integer. If npages is
large enough, this can cause:

  1. Signed integer overflow (undefined behavior per C standard)
  2. Sign extension when the result is promoted to unsigned long for
     the enc_dec_hypercall() parameter

Cast npages to unsigned long before the shift to ensure:
  - The shift operation uses unsigned arithmetic
  - No sign extension occurs during parameter promotion
  - Correct size calculation for large page counts (>2^19 pages on 32-bit)

This is a defensive fix; in practice, npages values are typically small,
but the cast ensures correctness for edge cases and silences potential
compiler warnings with -Wshift-overflow or -Wsign-conversion.

Fixes: ac3f9c9f1b37 (x86/sev: Make enc_dec_hypercall() accept a size instead of npages)
Signed-off-by: YangWencheng <east.moutain.yang@gmail.cn>
---
 arch/x86/mm/mem_encrypt_amd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/mm/mem_encrypt_amd.c b/arch/x86/mm/mem_encrypt_amd.c
index 2f8c32173972..8134fb731a21 100644
--- a/arch/x86/mm/mem_encrypt_amd.c
+++ b/arch/x86/mm/mem_encrypt_amd.c
@@ -306,7 +306,7 @@ static int amd_enc_status_change_finish(unsigned long vaddr, int npages, bool en
 		snp_set_memory_private(vaddr, npages);
 
 	if (!cc_platform_has(CC_ATTR_HOST_MEM_ENCRYPT))
-		enc_dec_hypercall(vaddr, npages << PAGE_SHIFT, enc);
+		enc_dec_hypercall(vaddr, (unsigned long)npages << PAGE_SHIFT, enc);
 
 	return 0;
 }
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 1/1] x86/sev: Fix potential sign extension in amd_enc_status_change_finish()
@ 2026-04-19 10:03 YangWencheng
  0 siblings, 0 replies; 5+ messages in thread
From: YangWencheng @ 2026-04-19 10:03 UTC (permalink / raw)
  To: dave.hansen, srutherford
  Cc: YangWencheng, Andy Lutomirski, Peter Zijlstra, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, x86, H. Peter Anvin, Tom Lendacky,
	Pankaj Gupta, linux-kernel

When npages (int) is left-shifted by PAGE_SHIFT to calculate the size
in bytes, the operation is performed on a signed integer. If npages is
large enough, this can cause:

  1. Signed integer overflow (undefined behavior per C standard)
  2. Sign extension when the result is promoted to unsigned long for
     the enc_dec_hypercall() parameter

Cast npages to unsigned long before the shift to ensure:
  - The shift operation uses unsigned arithmetic
  - No sign extension occurs during parameter promotion
  - Correct size calculation for large page counts (>2^19 pages on 32-bit)

This is a defensive fix; in practice, npages values are typically small,
but the cast ensures correctness for edge cases and silences potential
compiler warnings with -Wshift-overflow or -Wsign-conversion.

Fixes: ac3f9c9f1b37 (x86/sev: Make enc_dec_hypercall() accept a size instead of npages)
Signed-off-by: YangWencheng <east.moutain.yang@gmail.com>
---
 arch/x86/mm/mem_encrypt_amd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/mm/mem_encrypt_amd.c b/arch/x86/mm/mem_encrypt_amd.c
index 2f8c32173972..8134fb731a21 100644
--- a/arch/x86/mm/mem_encrypt_amd.c
+++ b/arch/x86/mm/mem_encrypt_amd.c
@@ -306,7 +306,7 @@ static int amd_enc_status_change_finish(unsigned long vaddr, int npages, bool en
 		snp_set_memory_private(vaddr, npages);
 
 	if (!cc_platform_has(CC_ATTR_HOST_MEM_ENCRYPT))
-		enc_dec_hypercall(vaddr, npages << PAGE_SHIFT, enc);
+		enc_dec_hypercall(vaddr, (unsigned long)npages << PAGE_SHIFT, enc);
 
 	return 0;
 }
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 1/1] x86/sev: Fix potential sign extension in amd_enc_status_change_finish()
@ 2026-04-23  2:10 YangWencheng
  2026-04-23 17:07 ` Dave Hansen
  0 siblings, 1 reply; 5+ messages in thread
From: YangWencheng @ 2026-04-23  2:10 UTC (permalink / raw)
  To: dave.hansen, srutherford
  Cc: YangWencheng, Andy Lutomirski, Peter Zijlstra, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, x86, H. Peter Anvin, Tom Lendacky,
	Pankaj Gupta, linux-kernel

When npages (int) is left-shifted by PAGE_SHIFT to calculate the size
in bytes, the operation is performed on a signed integer. If npages is
large enough, this can cause:

  1. Signed integer overflow (undefined behavior per C standard)
  2. Sign extension when the result is promoted to unsigned long for
     the enc_dec_hypercall() parameter

Cast npages to unsigned long before the shift to ensure:
  - The shift operation uses unsigned arithmetic
  - No sign extension occurs during parameter promotion
  - Correct size calculation for large page counts (>2^19 pages on 32-bit)

This is a defensive fix; in practice, npages values are typically small,
but the cast ensures correctness for edge cases and silences potential
compiler warnings with -Wshift-overflow or -Wsign-conversion.

Fixes: ac3f9c9f1b37 (x86/sev: Make enc_dec_hypercall() accept a size instead of npages)
Signed-off-by: YangWencheng <east.moutain.yang@gmail.com>
---
 arch/x86/mm/mem_encrypt_amd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/mm/mem_encrypt_amd.c b/arch/x86/mm/mem_encrypt_amd.c
index 2f8c32173972..8134fb731a21 100644
--- a/arch/x86/mm/mem_encrypt_amd.c
+++ b/arch/x86/mm/mem_encrypt_amd.c
@@ -306,7 +306,7 @@ static int amd_enc_status_change_finish(unsigned long vaddr, int npages, bool en
 		snp_set_memory_private(vaddr, npages);
 
 	if (!cc_platform_has(CC_ATTR_HOST_MEM_ENCRYPT))
-		enc_dec_hypercall(vaddr, npages << PAGE_SHIFT, enc);
+		enc_dec_hypercall(vaddr, (unsigned long)npages << PAGE_SHIFT, enc);
 
 	return 0;
 }
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/1] x86/sev: Fix potential sign extension in amd_enc_status_change_finish()
  2026-04-23  2:10 [PATCH 1/1] x86/sev: Fix potential sign extension in amd_enc_status_change_finish() YangWencheng
@ 2026-04-23 17:07 ` Dave Hansen
  2026-04-27  2:39   ` Wencheng Yang
  0 siblings, 1 reply; 5+ messages in thread
From: Dave Hansen @ 2026-04-23 17:07 UTC (permalink / raw)
  To: YangWencheng, dave.hansen, srutherford
  Cc: Andy Lutomirski, Peter Zijlstra, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, x86, H. Peter Anvin, Tom Lendacky, Pankaj Gupta,
	linux-kernel

On 4/22/26 19:10, YangWencheng wrote:
> When npages (int) is left-shifted by PAGE_SHIFT to calculate the size
> in bytes, the operation is performed on a signed integer. If npages is
> large enough, this can cause:

I kinda think the whole API around encryption is busted. The majority of
the callers have a virtual address and a bytes-based length argument and
convert it to pages.

Shouldn't we fix the API instead of hacking around it in a billion
little places?

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/1] x86/sev: Fix potential sign extension in amd_enc_status_change_finish()
  2026-04-23 17:07 ` Dave Hansen
@ 2026-04-27  2:39   ` Wencheng Yang
  0 siblings, 0 replies; 5+ messages in thread
From: Wencheng Yang @ 2026-04-27  2:39 UTC (permalink / raw)
  To: Dave Hansen
  Cc: dave.hansen, srutherford, Andy Lutomirski, Peter Zijlstra,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86,
	H. Peter Anvin, Tom Lendacky, Pankaj Gupta, linux-kernel

Hi, Dave

Thank you very much for your reply.

I think it's not easy to fix the API for me, at the point of view of
the bug, the most
direct way and also the quickest way is converting the type of pages to ulong.


On Fri, Apr 24, 2026 at 1:07 AM Dave Hansen <dave.hansen@intel.com> wrote:
>
> On 4/22/26 19:10, YangWencheng wrote:
> > When npages (int) is left-shifted by PAGE_SHIFT to calculate the size
> > in bytes, the operation is performed on a signed integer. If npages is
> > large enough, this can cause:
>
> I kinda think the whole API around encryption is busted. The majority of
> the callers have a virtual address and a bytes-based length argument and
> convert it to pages.
>
> Shouldn't we fix the API instead of hacking around it in a billion
> little places?

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-04-27  2:39 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-23  2:10 [PATCH 1/1] x86/sev: Fix potential sign extension in amd_enc_status_change_finish() YangWencheng
2026-04-23 17:07 ` Dave Hansen
2026-04-27  2:39   ` Wencheng Yang
  -- strict thread matches above, loose matches on Subject: below --
2026-04-19 10:03 YangWencheng
2026-04-19  9:37 YangWencheng

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox