Thanks for your reply. We think that when intel_gvt_dma_map_guest_page() fails, ppgtt_invalidate_spt is called to handle this error. If the ppgtt_invalidate_spt is successful to kfree the spt object, then in the ppgtt_populate_spt function there is no need to kfree the spt again. And if the ppgtt_invalidate_spt failed, then in the ppgtt_populate_spt function there is need to kfree the spt for error handling. This is our fix, if it's not right, we are glad to discuss with you. Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> 于2022年9月5日周一 16:04写道: > > On Mon, Sep 05, 2022 at 03:46:09PM +0800, Zheng Hacker wrote: > > I rewrote the letter. Hope it works. > > > > There is a double-free security bug in split_2MB_gtt_entry. > > > > Here is a calling chain : > > ppgtt_populate_spt->ppgtt_populate_shadow_entry->split_2MB_gtt_entry. > > If intel_gvt_dma_map_guest_page failed, it will call > > ppgtt_invalidate_spt, which will finally call ppgtt_free_spt and > > kfree(spt). But the caller does not notice that, and it will call > > ppgtt_free_spt again in error path. > > > > Fix this by returning the result of ppgtt_invalidate_spt to split_2MB_gtt_entry. > > > > Signed-off-by: Zheng Wang > > > > --- > > drivers/gpu/drm/i915/gvt/gtt.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c > > index ce0eb03709c3..9f14fded8c0c 100644 > > --- a/drivers/gpu/drm/i915/gvt/gtt.c > > +++ b/drivers/gpu/drm/i915/gvt/gtt.c > > @@ -1215,7 +1215,7 @@ static int split_2MB_gtt_entry(struct intel_vgpu *vgpu, > > ret = intel_gvt_dma_map_guest_page(vgpu, start_gfn + sub_index, > > PAGE_SIZE, &dma_addr); > > if (ret) { > > - ppgtt_invalidate_spt(spt); > > + ret = ppgtt_invalidate_spt(spt); > > return ret; > > But now you just lost the original error, shouldn't this succeed even if > intel_gvt_dma_map_guest_page() failed? > > And how are you causing intel_gvt_dma_map_guest_page() to fail in a real > system? > > thanks, > > greg k-h