Hi Boris,
On 08/01/2026 15:45, Boris Brezillon wrote:
On Thu, 8 Jan 2026 15:04:33 +0100
Loïc Molinari <[email protected]> wrote:
On 08/01/2026 14:43, Boris Brezillon wrote:
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.
You're right, I focused on arm64 here and there's obviously other archs
so we'd better pass pgoff for the fallback case.
Do you want me to send a patch doing that, or should I take care of it?
I'll take care of it. I'd better wait for that GEM leak fix to be pulled
first to avoid a conflict, right?
Regards,
Loïc
