Re: [PATCH v2 2/2] virt: tdx-guest: Allocate Quote buffer dynamically

On Fri, Jul 17, 2026 at 09:55:19PM +0000, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] Premature exposure of the misc device allows a Use-After-Free (UAF) on the module initialization error path if `alloc_quote_buf()` fails.
> --
> 
> > @@ -417,7 +442,7 @@ static int __init tdx_guest_init(void)
> >  	if (ret)
> >  		goto deinit_mr;
> >  
> > -	quote_data = alloc_quote_buf();
> > +	quote_data = alloc_quote_buf(&quote_data_len);
> 
> [Severity: High]
> This is a pre-existing issue, but this patch makes it significantly more
> likely to trigger by introducing a large contiguous memory allocation (up to
> 4MB) that can easily fail on fragmented systems.
> 
> Can this lead to a Use-After-Free on the initialization error path?
> 
> If misc_register(&tdx_misc_dev) (called just before this hunk) succeeds, 
> the /dev/tdx_guest device is immediately exposed to userspace. A userspace 
> process can open it and call ioctl(TDX_CMD_GET_REPORT0), entering
> tdx_do_report() which uses tdx_report_buf.
> 
> If alloc_quote_buf() then fails, the error path calls misc_deregister() and 
> tdx_mr_deinit(). 
>     
> tdx_mr_deinit() executes:
> 
>     kfree(tdx_report_buf);
>     
> This freeing happens without acquiring mr_lock, meaning it can race with the 
> ongoing ioctl. Could the concurrent ioctl in tdx_do_report() then access the 
> freed tdx_report_buf?
> 
> Should misc_register() be moved to the end of tdx_guest_init() after all
> resource allocations succeed?

Good catch... I think this race could happen. /dev/tdx_guest is
considered a legacy interface now but this needs to be fixed. I'll add
it to the cleanup list.

> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2