Re: [Intel-gfx] [PATCH v3] drm/i915/gvt: fix double free bug in split_2MB_gtt_entry

[Wang, Zhi A]

On 12/19/2022 9:57 AM, Zheng Wang wrote:
> Hi Zhi,
> 
> Thanks again for your reply and clear explaination about the function.
> I still have some doubt about the fix. Here is a invoke chain :
> ppgtt_populate_spt
>    ->ppgtt_populate_shadow_entry
>      ->split_2MB_gtt_entry
> As far as I'm concerned, when something error happens in DMA mapping,
> which will make intel_gvt_dma_map_guest_page return none-zero code,
> It will invoke ppgtt_invalidate_spt and call ppgtt_free_spt,which will
> finally free spt by kfree. But the caller doesn't notice that and frees
> spt by calling ppgtt_free_spt again. This is a typical UAF/Double Free
> vulnerability. So I think the key point is about how to handle spt properly.
> The handle newly allocated spt (aka sub_spt) is not the root cause of this
> issue. Could you please give me more advice about how to fix this security
> bug? Besides, I'm not sure if there are more similar problems in othe location.
> 
> Best regards,
> Zheng Wang
> 

I think it is a case-by-case thing. For example:

The current scenario in this function looks like below:

caller pass spt a
function
	alloc spt b
	something error
	free spt a
	return error

The problem is: the function wrongly frees the spt a instead free what 
it allocates.

A proper fix should be:

caller pass spt a
function
	alloc spt b
	something error
	*free spt b*
	return error

Thanks,
Zhi.