Takashi Iwai
2023-Jan-03 14:07 UTC
[Nouveau] [PATCH] drm/nouveau/mmu: fix Use after Free bug in nvkm_vmm_node_split
On Fri, 30 Dec 2022 08:27:58 +0100, Zheng Wang wrote:> > Here is a function call chain. > nvkm_vmm_pfn_map->nvkm_vmm_pfn_split_merge->nvkm_vmm_node_split > If nvkm_vma_tail return NULL in nvkm_vmm_node_split, it will > finally invoke nvkm_vmm_node_merge->nvkm_vmm_node_delete, which > will free the vma. However, nvkm_vmm_pfn_map didn't notice that. > It goes into next label and UAF happens. > > Fix it by returning the return-value of nvkm_vmm_node_merge > instead of NULL. > > Signed-off-by: Zheng Wang <zyytlz.wz at 163.com>FWIW, CVE-2023-0030 has been assigned to this bug. It's a question whether it really deserves as a security issue, but a bug is a bug... Ben, could you review this please? thanks, Takashi> --- > drivers/gpu/drm/nouveau/nvkm/subdev/mmu/vmm.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/mmu/vmm.c b/drivers/gpu/drm/nouveau/nvkm/subdev/mmu/vmm.c > index ae793f400ba1..84d6fc87b2e8 100644 > --- a/drivers/gpu/drm/nouveau/nvkm/subdev/mmu/vmm.c > +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/mmu/vmm.c > @@ -937,8 +937,8 @@ nvkm_vmm_node_split(struct nvkm_vmm *vmm, > if (vma->size != size) { > struct nvkm_vma *tmp; > if (!(tmp = nvkm_vma_tail(vma, vma->size - size))) { > - nvkm_vmm_node_merge(vmm, prev, vma, NULL, vma->size); > - return NULL; > + tmp = nvkm_vmm_node_merge(vmm, prev, vma, NULL, vma->size); > + return tmp; > } > tmp->part = true; > nvkm_vmm_node_insert(vmm, tmp); > -- > 2.25.1 >
Takashi Iwai
2023-Jan-27 12:10 UTC
[Nouveau] [PATCH] drm/nouveau/mmu: fix Use after Free bug in nvkm_vmm_node_split
On Tue, 03 Jan 2023 15:07:55 +0100, Takashi Iwai wrote:> > On Fri, 30 Dec 2022 08:27:58 +0100, > Zheng Wang wrote: > > > > Here is a function call chain. > > nvkm_vmm_pfn_map->nvkm_vmm_pfn_split_merge->nvkm_vmm_node_split > > If nvkm_vma_tail return NULL in nvkm_vmm_node_split, it will > > finally invoke nvkm_vmm_node_merge->nvkm_vmm_node_delete, which > > will free the vma. However, nvkm_vmm_pfn_map didn't notice that. > > It goes into next label and UAF happens. > > > > Fix it by returning the return-value of nvkm_vmm_node_merge > > instead of NULL. > > > > Signed-off-by: Zheng Wang <zyytlz.wz at 163.com> > > FWIW, CVE-2023-0030 has been assigned to this bug. > It's a question whether it really deserves as a security issue, but a > bug is a bug... > > Ben, could you review this please?A gentle ping as reminder. The bug is still present. thanks, Takashi> > > thanks, > > Takashi > > > --- > > drivers/gpu/drm/nouveau/nvkm/subdev/mmu/vmm.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/mmu/vmm.c b/drivers/gpu/drm/nouveau/nvkm/subdev/mmu/vmm.c > > index ae793f400ba1..84d6fc87b2e8 100644 > > --- a/drivers/gpu/drm/nouveau/nvkm/subdev/mmu/vmm.c > > +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/mmu/vmm.c > > @@ -937,8 +937,8 @@ nvkm_vmm_node_split(struct nvkm_vmm *vmm, > > if (vma->size != size) { > > struct nvkm_vma *tmp; > > if (!(tmp = nvkm_vma_tail(vma, vma->size - size))) { > > - nvkm_vmm_node_merge(vmm, prev, vma, NULL, vma->size); > > - return NULL; > > + tmp = nvkm_vmm_node_merge(vmm, prev, vma, NULL, vma->size); > > + return tmp; > > } > > tmp->part = true; > > nvkm_vmm_node_insert(vmm, tmp); > > -- > > 2.25.1 > >