Hi Krzysztof
On Thu Jul 17, 2025 at 2:38 PM UTC, Krzysztof Karas wrote:
> Hi Sebastian,
>
>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
>> @@ -1529,6 +1529,7 @@ static int eb_relocate_vma(struct i915_execbuffer *eb,
>> struct eb_vma *ev)
>> struct drm_i915_gem_relocation_entry __user *urelocs =
>> u64_to_user_ptr(entry->relocs_ptr);
>> unsigned long remain = entry->relocation_count;
>> + int ret = 0;
>>
>> if (unlikely(remain > N_RELOC(INT_MAX)))
>> return -EINVAL;
>> @@ -1541,11 +1542,10 @@ static int eb_relocate_vma(struct i915_execbuffer
>> *eb, struct eb_vma *ev)
>> if (unlikely(!access_ok(urelocs, remain * sizeof(*urelocs))))
>> return -EFAULT;
>>
>> - do {
>> - struct drm_i915_gem_relocation_entry *r = stack;
>> - unsigned int count =
>> - min_t(unsigned long, remain, ARRAY_SIZE(stack));
>> + while (remain > 0) {
> Is it possible for "remain" variable to be initialized to 0?
> If that would be the case then after your change we'd skip this
> loop entirely, where previously we'd run at least one iteration.
> Would that be a problem?
--
This should not occur, as there are several direct and indirect safeguards
verifying that entry->relocation_count is non-zero.
e.g.
```eb_relocate_vma_slow
1622 › for (i = 0; i < entry->relocation_count; i++) {
```
or
```check_relocations
1642 › size = entry->relocation_count;
1643 › if (size == 0)
1644 › › return 0;
```
And it would be a peculiar choice to copy 0 bytes
Best regards,
Sebastian
