On Thu, 8 Jan 2026 14:25:01 +0100 Loïc Molinari <[email protected]> wrote:
> On 07/01/2026 12:12, Boris Brezillon wrote: > > On Tue, 6 Jan 2026 17:49:35 +0100 > > Boris Brezillon <[email protected]> wrote: > > > >> drm_gem_object_lookup_at_offset() can return a valid object with > >> filp or filp->f_op->get_unmapped_area set to NULL. Make sure we still > >> release the ref we acquired on such objects. > >> > >> Cc: Loïc Molinari <[email protected]> > >> Fixes: 99bda20d6d4c ("drm/gem: Introduce drm_gem_get_unmapped_area() fop") > >> Signed-off-by: Boris Brezillon <[email protected]> > >> --- > >> drivers/gpu/drm/drm_gem.c | 10 ++++++---- > >> 1 file changed, 6 insertions(+), 4 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c > >> index 36c8af123877..f7cbf6e8d1e0 100644 > >> --- a/drivers/gpu/drm/drm_gem.c > >> +++ b/drivers/gpu/drm/drm_gem.c > >> @@ -1298,11 +1298,13 @@ unsigned long drm_gem_get_unmapped_area(struct > >> file *filp, unsigned long uaddr, > >> unsigned long ret; > >> > >> obj = drm_gem_object_lookup_at_offset(filp, pgoff, len >> PAGE_SHIFT); > >> - if (IS_ERR(obj) || !obj->filp || !obj->filp->f_op->get_unmapped_area) > >> - return mm_get_unmapped_area(filp, uaddr, len, 0, flags); > >> + if (IS_ERR(obj)) > >> + obj = NULL; > >> > >> - ret = obj->filp->f_op->get_unmapped_area(obj->filp, uaddr, len, 0, > >> - flags); > >> + if (!obj || !obj->filp || !obj->filp->f_op->get_unmapped_area) > >> + ret = mm_get_unmapped_area(filp, uaddr, len, 0, flags); > > > > Also, I'm wondering if we shouldn't pass pgoff instead of zero here to > > have the exact same behavior that existed before > > drm_gem_get_unmapped_area() was introduced. > > > > For mappings with a file (the DRM file in our case), if > filp->f_op->get_unmapped_area isn't set then generic_get_unmapped_area() > is used and it ignores the pgoff argument. That's true for architectures that rely on the default implementation (Arm64 for instance), but other architectures might have their own implementation. > So it wasn't 0 before but was > ignored anyway. Didn't check all of them but the Arm implementation does take this pgoff into account if I read the code correctly [1]. The fact this argument is passed around makes me think other architectures might take this into account too. I know this pgoff is just a fake offset into the /dev/dri pseudo-file, but if we want to err on the safe side, we'd rather do exactly what was done before drm_gem_get_unmapped_area() was introduced for the ->filp==NULL case. That's just my 2 cts, of course. Regards, Boris [1]https://elixir.bootlin.com/linux/v6.18.3/source/arch/arm/mm/mmap.c#L30
