linux-kernel - Re: [PATCH] gpu: ttm: fix GPF in ttm_bo

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <20210708130910.63a15c73@gmail.com>
Date:   Thu, 8 Jul 2021 13:09:10 +0300
From:   Pavel Skripkin <paskripkin@...il.com>
To:     Christian König <christian.koenig@....com>,
        ray.huang@....com, airlied@...ux.ie
Cc:     dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] gpu: ttm: fix GPF in ttm_bo_release

On Thu, 8 Jul 2021 11:37:01 +0300
Pavel Skripkin <paskripkin@...il.com> wrote:

> On Thu, 8 Jul 2021 08:49:48 +0200
> Christian König <christian.koenig@....com> wrote:
> 
> > Am 07.07.21 um 20:51 schrieb Pavel Skripkin:
> > > My local syzbot instance hit GPF in ttm_bo_release().
> > > Unfortunately, syzbot didn't produce a reproducer for this, but I
> > > found out possible scenario:
> > >
> > > drm_gem_vram_create()            <-- drm_gem_vram_object kzalloced
> > > 				     (bo embedded in this object)
> > >    ttm_bo_init()
> > >      ttm_bo_init_reserved()
> > >        ttm_resource_alloc()
> > >          man->func->alloc()       <-- allocation failure
> > >        ttm_bo_put()
> > > 	ttm_bo_release()
> > > 	  ttm_mem_io_free()      <-- bo->resource == NULL passed
> > > 				     as second argument
> > > 	     *GPF*
> > >
> > > So, I've added check in ttm_bo_release() to avoid passing
> > > NULL as second argument to ttm_mem_io_free().
> 
> Hi, Christian!
> 
> Thank you for quick feedback :)
> 
> > 
> > There is another ocassion of this a bit down before we call 
> > ttm_bo_move_to_lru_tail() apart from that good catch.
> > 
> 
> Did you mean, that ttm_bo_move_to_lru_tail() should have NULL check
> too? I checked it's realization, and, I think, NULL check is necessary
> there, since mem pointer is dereferenced w/o any checking
> 
> > But I'm wondering if we should make the functions NULL save instead
> > of the external check.
> > 
> 
> I tried to find more possible scenarios of GPF in ttm_bo_release(),
> but I didn't find one. But, yes, moving NULL check inside
> ttm_mem_io_free() is more general approach and it will defend this
> function from GPFs in the future.
> 
> 
> 
> With regards,
> Pavel Skripkin
> 

I misclicked and sent this email to Christian privately :(

Added all thread participants back, sorry.



With regards,
Pavel Skripkin