Re: [PATCH v5 1/3] x86/tdx: Fix off-by-one in port I/O handling

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] SEV-ES vc_handle_ioio incorrectly zero-extends 8-bit and 16-bit IN instructions.
--

commit df84d995a704008239da9f3f6b57944d2f071dc7
Author: Kiryl Shutsemau (Meta) <kas@kernel.org>
x86/tdx: Fix off-by-one in port I/O handling

This commit fixes an off-by-one error in the GENMASK calculation for port
I/O handling in the TDX module. It corrects the mask to prevent it from
being one bit too wide for all I/O sizes.

Link: https://lore.kernel.org/all/CAKw_Dz96rfSQc6Rn+9QBcUFHhmkK+9zu+P=bxowfZwxrATCBRg@mail.gmail.com/

> diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c
> index 29b6f1ed59ec5..b8bbd715fb624 100644
> --- a/arch/x86/coco/tdx/tdx.c
> +++ b/arch/x86/coco/tdx/tdx.c
> @@ -694,7 +694,7 @@ static bool handle_in(struct pt_regs *regs, int size, int port)
>  		.r13 = PORT_READ,
>  		.r14 = port,
>  	};
> -	u64 mask = GENMASK(BITS_PER_BYTE * size, 0);
> +	u64 mask = GENMASK(BITS_PER_BYTE * size - 1, 0);
>  	bool success;
>  
>  	/*

[ ... ]

While reviewing the port I/O handling changes here, I noticed a pre-existing
issue in the related SEV-ES code:

arch/x86/coco/sev/vc-shared.c:vc_handle_ioio() {
[ ... ]
> 		if (exit_info_1 & IOIO_TYPE_IN) {
> 			if (!ghcb_rax_is_valid(ghcb))
> 				return ES_VMM_ERROR;
> 			regs->ax = lower_bits(ghcb->save.rax, bits);
                        ^^^^^^^^

[Severity: High]
This is a pre-existing issue, but does this incorrectly zero-extend 8-bit and
16-bit in instructions?

When a guest executes an 8-bit or 16-bit in instruction, it triggers a #VC
exception (SVM_EXIT_IOIO) and calls vc_handle_ioio(). The function extracts
the result using lower_bits(ghcb->save.rax, bits) and assigns it directly to
the 64-bit regs->ax field, overwriting the entire register.

This violates x86-64 architectural semantics, which dictate that 8-bit and
16-bit register writes must leave the upper bits untouched.

Can this cause data corruption or crashes in guest code if the compiler or
assembly relies on the upper bits of rax being preserved across the in
instruction?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260701110547.764083-1-kirill@shutemov.name?part=1