[PATCH 0/2] drm/msm/gem: A couple VM_BIND fixes

[PATCH 0/2] drm/msm/gem: A couple VM_BIND fixes

[Rob Clark]

Fix possible overflow in range check, and disallow passing BOs table to
GEM_SUBMIT for VM_BIND contexts.

Rob Clark (2):
  drm/msm/gem: Fix VM_BIND range check
  drm/msm/gem: Disallow nr_bos for VM_BIND contexts

 drivers/gpu/drm/msm/msm_gem_submit.c | 5 ++++-
 drivers/gpu/drm/msm/msm_gem_vma.c    | 2 +-
 2 files changed, 5 insertions(+), 2 deletions(-)

-- 
2.56.0

[PATCH 1/2] drm/msm/gem: Fix VM_BIND range check

[Rob Clark]

Use size_add() to guard against overflow.

Reported-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Fixes: 3a3bef68a6c1 ("drm/msm: Fix missing VM_BIND offset/range validation")
Signed-off-by: Rob Clark <robin.clark@oss.qualcomm.com>
---
 drivers/gpu/drm/msm/msm_gem_vma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/msm_gem_vma.c b/drivers/gpu/drm/msm/msm_gem_vma.c
index f687a629629d..16b3870d5e90 100644
--- a/drivers/gpu/drm/msm/msm_gem_vma.c
+++ b/drivers/gpu/drm/msm/msm_gem_vma.c
@@ -1118,7 +1118,7 @@ vm_bind_job_lookup_ops(struct msm_vm_bind_job *job, struct drm_msm_vm_bind *args
 		op->obj = obj;
 		cnt++;
 
-		if ((op->range + op->obj_offset) > obj->size) {
+		if (size_add(op->range, op->obj_offset) > obj->size) {
 			ret = UERR(EINVAL, dev, "invalid range: %016llx + %016llx > %016zx\n",
 				   op->range, op->obj_offset, obj->size);
 			goto out_unlock;
-- 
2.56.0

Re: [PATCH 1/2] drm/msm/gem: Fix VM_BIND range check

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] drm/msm: 64-to-32-bit truncation in size_add() in vm_bind_job_lookup_ops()
--

commit 0bb1fc19abe2c004d338802c56cd54e0621aa9a6
Author: Rob Clark <robin.clark@oss.qualcomm.com>

drm/msm/gem: Fix VM_BIND range check

This commit replaces the addition of op->range and op->obj_offset with
size_add() in vm_bind_job_lookup_ops() to guard against integer overflow when
validating the range against obj->size.

> diff --git a/drivers/gpu/drm/msm/msm_gem_vma.c b/drivers/gpu/drm/msm/msm_gem_vma.c
> index f687a629629d3..16b3870d5e90e 100644
> --- a/drivers/gpu/drm/msm/msm_gem_vma.c
> +++ b/drivers/gpu/drm/msm/msm_gem_vma.c
> @@ -1118,7 +1118,7 @@ vm_bind_job_lookup_ops(struct msm_vm_bind_job *job, struct drm_msm_vm_bind *args
>  		op->obj = obj;
>  		cnt++;
>  
> -		if ((op->range + op->obj_offset) > obj->size) {
> +		if (size_add(op->range, op->obj_offset) > obj->size) {

[Severity: Medium]
On 32-bit builds, does passing op->range and op->obj_offset to size_add()
truncate the upper 32 bits of both 64-bit operands before checking for
overflow?

In struct msm_vm_bind_op, obj_offset and range are 64-bit values:

drivers/gpu/drm/msm/msm_gem_vma.c:
struct msm_vm_bind_op {
	...
	uint64_t obj_offset;
	uint64_t iova;
	uint64_t range;
	...
};

However, size_add() in include/linux/overflow.h takes size_t arguments:

static __always_inline size_t __must_check size_add(size_t addend1, size_t addend2)

Earlier in lookup_op(), op->obj_offset is only checked with
invalid_alignment(op->obj_offset), which does not reject non-zero upper 32
bits. On 32-bit architectures where size_t is 32 bits wide, an offset such as
0x100000000ULL is truncated to 0 when passed to size_add(), allowing an
out-of-bounds 64-bit op->obj_offset or op->range to bypass the obj->size
check in vm_bind_job_lookup_ops() and be stored into vma->gem.offset.

Would check_add_overflow() with a uint64_t sum (or range_overflows_t(u64,
op->obj_offset, op->range, obj->size)) avoid narrowing the operands on 32-bit
targets?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261101174911.25184-1-robin.clark@oss.qualcomm.com?part=1

Re: [PATCH 1/2] drm/msm/gem: Fix VM_BIND range check

[Konrad Dybcio]

On 11/1/26 6:49 PM, Rob Clark wrote:
> Use size_add() to guard against overflow.
> 
> Reported-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> Fixes: 3a3bef68a6c1 ("drm/msm: Fix missing VM_BIND offset/range validation")
> Signed-off-by: Rob Clark <robin.clark@oss.qualcomm.com>
> ---
>  drivers/gpu/drm/msm/msm_gem_vma.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/msm/msm_gem_vma.c b/drivers/gpu/drm/msm/msm_gem_vma.c
> index f687a629629d..16b3870d5e90 100644
> --- a/drivers/gpu/drm/msm/msm_gem_vma.c
> +++ b/drivers/gpu/drm/msm/msm_gem_vma.c
> @@ -1118,7 +1118,7 @@ vm_bind_job_lookup_ops(struct msm_vm_bind_job *job, struct drm_msm_vm_bind *args
>  		op->obj = obj;
>  		cnt++;
>  
> -		if ((op->range + op->obj_offset) > obj->size) {
> +		if (size_add(op->range, op->obj_offset) > obj->size) {

GPT and Sashiko are unhappy about what this does on 32b

Konrad

Re: [PATCH 1/2] drm/msm/gem: Fix VM_BIND range check

[Dmitry Baryshkov]

On Mon, Nov 02, 2026 at 10:40:06AM +0100, Konrad Dybcio wrote:
> On 11/1/26 6:49 PM, Rob Clark wrote:
> > Use size_add() to guard against overflow.
> > 
> > Reported-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> > Fixes: 3a3bef68a6c1 ("drm/msm: Fix missing VM_BIND offset/range validation")
> > Signed-off-by: Rob Clark <robin.clark@oss.qualcomm.com>
> > ---
> >  drivers/gpu/drm/msm/msm_gem_vma.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/msm/msm_gem_vma.c b/drivers/gpu/drm/msm/msm_gem_vma.c
> > index f687a629629d..16b3870d5e90 100644
> > --- a/drivers/gpu/drm/msm/msm_gem_vma.c
> > +++ b/drivers/gpu/drm/msm/msm_gem_vma.c
> > @@ -1118,7 +1118,7 @@ vm_bind_job_lookup_ops(struct msm_vm_bind_job *job, struct drm_msm_vm_bind *args
> >  		op->obj = obj;
> >  		cnt++;
> >  
> > -		if ((op->range + op->obj_offset) > obj->size) {
> > +		if (size_add(op->range, op->obj_offset) > obj->size) {
> 
> GPT and Sashiko are unhappy about what this does on 32b

I'd somewhat doubt that we would ever need VM_BIND on arm32 platforms.
a2xx isn't Vulkan-compatible, a3xx is questionable.

-- 
With best wishes
Dmitry

Re: [PATCH 1/2] drm/msm/gem: Fix VM_BIND range check

[Rob Clark]

On Mon, Nov 2, 2026 at 1:40 AM Konrad Dybcio
<konrad.dybcio@oss.qualcomm.com> wrote:
>
> On 11/1/26 6:49 PM, Rob Clark wrote:
> > Use size_add() to guard against overflow.
> >
> > Reported-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> > Fixes: 3a3bef68a6c1 ("drm/msm: Fix missing VM_BIND offset/range validation")
> > Signed-off-by: Rob Clark <robin.clark@oss.qualcomm.com>
> > ---
> >  drivers/gpu/drm/msm/msm_gem_vma.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/msm/msm_gem_vma.c b/drivers/gpu/drm/msm/msm_gem_vma.c
> > index f687a629629d..16b3870d5e90 100644
> > --- a/drivers/gpu/drm/msm/msm_gem_vma.c
> > +++ b/drivers/gpu/drm/msm/msm_gem_vma.c
> > @@ -1118,7 +1118,7 @@ vm_bind_job_lookup_ops(struct msm_vm_bind_job *job, struct drm_msm_vm_bind *args
> >               op->obj = obj;
> >               cnt++;
> >
> > -             if ((op->range + op->obj_offset) > obj->size) {
> > +             if (size_add(op->range, op->obj_offset) > obj->size) {
>
> GPT and Sashiko are unhappy about what this does on 32b

Considering that we can't have >4gb BOs on 32b, I think the concerns
are probably unwarranted.  But yes, I do look at Sashiko's reports

BR,
-R

Re: [PATCH 1/2] drm/msm/gem: Fix VM_BIND range check

[Rob Clark]

On Mon, Nov 2, 2026 at 5:59 AM Rob Clark <rob.clark@oss.qualcomm.com> wrote:
>
> On Mon, Nov 2, 2026 at 1:40 AM Konrad Dybcio
> <konrad.dybcio@oss.qualcomm.com> wrote:
> >
> > On 11/1/26 6:49 PM, Rob Clark wrote:
> > > Use size_add() to guard against overflow.
> > >
> > > Reported-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> > > Fixes: 3a3bef68a6c1 ("drm/msm: Fix missing VM_BIND offset/range validation")
> > > Signed-off-by: Rob Clark <robin.clark@oss.qualcomm.com>
> > > ---
> > >  drivers/gpu/drm/msm/msm_gem_vma.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/msm/msm_gem_vma.c b/drivers/gpu/drm/msm/msm_gem_vma.c
> > > index f687a629629d..16b3870d5e90 100644
> > > --- a/drivers/gpu/drm/msm/msm_gem_vma.c
> > > +++ b/drivers/gpu/drm/msm/msm_gem_vma.c
> > > @@ -1118,7 +1118,7 @@ vm_bind_job_lookup_ops(struct msm_vm_bind_job *job, struct drm_msm_vm_bind *args
> > >               op->obj = obj;
> > >               cnt++;
> > >
> > > -             if ((op->range + op->obj_offset) > obj->size) {
> > > +             if (size_add(op->range, op->obj_offset) > obj->size) {
> >
> > GPT and Sashiko are unhappy about what this does on 32b
>
> Considering that we can't have >4gb BOs on 32b, I think the concerns
> are probably unwarranted.  But yes, I do look at Sashiko's reports

Probably we should add checks that obj_offset/range are not more than SIZE_MAX?

BR,
-R

Re: [PATCH 1/2] drm/msm/gem: Fix VM_BIND range check

[Konrad Dybcio]

On 11/2/26 4:18 PM, Rob Clark wrote:
> On Mon, Nov 2, 2026 at 5:59 AM Rob Clark <rob.clark@oss.qualcomm.com> wrote:
>>
>> On Mon, Nov 2, 2026 at 1:40 AM Konrad Dybcio
>> <konrad.dybcio@oss.qualcomm.com> wrote:
>>>
>>> On 11/1/26 6:49 PM, Rob Clark wrote:
>>>> Use size_add() to guard against overflow.
>>>>
>>>> Reported-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>>>> Fixes: 3a3bef68a6c1 ("drm/msm: Fix missing VM_BIND offset/range validation")
>>>> Signed-off-by: Rob Clark <robin.clark@oss.qualcomm.com>
>>>> ---
>>>>  drivers/gpu/drm/msm/msm_gem_vma.c | 2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/msm/msm_gem_vma.c b/drivers/gpu/drm/msm/msm_gem_vma.c
>>>> index f687a629629d..16b3870d5e90 100644
>>>> --- a/drivers/gpu/drm/msm/msm_gem_vma.c
>>>> +++ b/drivers/gpu/drm/msm/msm_gem_vma.c
>>>> @@ -1118,7 +1118,7 @@ vm_bind_job_lookup_ops(struct msm_vm_bind_job *job, struct drm_msm_vm_bind *args
>>>>               op->obj = obj;
>>>>               cnt++;
>>>>
>>>> -             if ((op->range + op->obj_offset) > obj->size) {
>>>> +             if (size_add(op->range, op->obj_offset) > obj->size) {
>>>
>>> GPT and Sashiko are unhappy about what this does on 32b
>>
>> Considering that we can't have >4gb BOs on 32b, I think the concerns
>> are probably unwarranted.  But yes, I do look at Sashiko's reports
> 
> Probably we should add checks that obj_offset/range are not more than SIZE_MAX?

Wouldn't hurt

Konrad

Re: [PATCH 1/2] drm/msm/gem: Fix VM_BIND range check

[Dmitry Baryshkov]

On Sun, Nov 01, 2026 at 09:49:09AM -0800, Rob Clark wrote:
> Use size_add() to guard against overflow.
> 
> Reported-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> Fixes: 3a3bef68a6c1 ("drm/msm: Fix missing VM_BIND offset/range validation")
> Signed-off-by: Rob Clark <robin.clark@oss.qualcomm.com>
> ---
>  drivers/gpu/drm/msm/msm_gem_vma.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>


-- 
With best wishes
Dmitry

[PATCH 2/2] drm/msm/gem: Disallow nr_bos for VM_BIND contexts

[Rob Clark]

If using VM_BIND, it is invalid to attach BOs to a GEM_SUBMIT ioctl.
Only the VM_BIND ioctl should be used for binding objects.

Fixes: 2e6a8a1fe2b2 ("drm/msm: Add VM_BIND ioctl")
Signed-off-by: Rob Clark <robin.clark@oss.qualcomm.com>
---
 drivers/gpu/drm/msm/msm_gem_submit.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c b/drivers/gpu/drm/msm/msm_gem_submit.c
index ff6cf39587c9..b558d07ed2b6 100644
--- a/drivers/gpu/drm/msm/msm_gem_submit.c
+++ b/drivers/gpu/drm/msm/msm_gem_submit.c
@@ -272,7 +272,7 @@ static int submit_lock_objects_vmbind(struct msm_gem_submit *submit)
 	struct drm_exec *exec = &submit->exec;
 	int ret = 0;
 
-	drm_exec_init(&submit->exec, flags, submit->nr_bos);
+	drm_exec_init(&submit->exec, flags, 0);
 	submit->has_exec = true;
 
 	drm_exec_until_all_locked (&submit->exec) {
@@ -589,6 +589,9 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data,
 			return -EINVAL;
 	}
 
+	if (msm_context_is_vmbind(ctx) && args->nr_bos)
+		return UERR(EINVAL, dev, "invalid nr_bos, must be zero for VM_BIND contexts");
+
 	queue = msm_submitqueue_get(ctx, args->queueid);
 	if (!queue)
 		return -ENOENT;
-- 
2.56.0

Re: [PATCH 2/2] drm/msm/gem: Disallow nr_bos for VM_BIND contexts

[Dmitry Baryshkov]

On Sun, Nov 01, 2026 at 09:49:10AM -0800, Rob Clark wrote:
> If using VM_BIND, it is invalid to attach BOs to a GEM_SUBMIT ioctl.
> Only the VM_BIND ioctl should be used for binding objects.
> 
> Fixes: 2e6a8a1fe2b2 ("drm/msm: Add VM_BIND ioctl")
> Signed-off-by: Rob Clark <robin.clark@oss.qualcomm.com>
> ---
>  drivers/gpu/drm/msm/msm_gem_submit.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>


-- 
With best wishes
Dmitry