Re: [PATCH net] net/mlx4: avoid GCC 10 __bad_copy_from() false positive

[Tariq Toukan <tariqt@nvidia.com>]

On 29/05/2026 9:45, Yao Sang wrote:
> On Mon, May 25, 2026 at 01:47:59PM +0300, Tariq Toukan wrote:
>>
>>
>> On 20/05/2026 13:21, Yao Sang wrote:
>>> mlx4_init_user_cqes() allocates a single PAGE_SIZE buffer and fills it
>>> with the CQE initialization pattern. When entries_per_copy >= entries,
>>> the function copies array_size(entries, cqe_size) bytes from that buffer
>>> to userspace.
>>>
>>> That copy is actually bounded by PAGE_SIZE in the else branch because
>>> entries_per_copy >= entries implies entries * cqe_size <= PAGE_SIZE.
>>> However, GCC 10 does not derive that constraint and falsely triggers
>>> __bad_copy_from() in mlx4_init_user_cqes().
>>>
>>> Cap the single copy_to_user() length to PAGE_SIZE to make that bound
>>> explicit and avoid the GCC 10 false positive.
>>>
>>> Fixes: f69bf5dee7ef ("net/mlx4: Use array_size() helper in copy_to_user()")
>>> Signed-off-by: Yao Sang <sangyao@kylinos.cn>
>>> ---
>>>    drivers/net/ethernet/mellanox/mlx4/cq.c | 5 ++++-
>>>    1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/ethernet/mellanox/mlx4/cq.c b/drivers/net/ethernet/mellanox/mlx4/cq.c
>>> index e130e7259275..7b024a5e13c8 100644
>>> --- a/drivers/net/ethernet/mellanox/mlx4/cq.c
>>> +++ b/drivers/net/ethernet/mellanox/mlx4/cq.c
>>> @@ -314,8 +314,11 @@ static int mlx4_init_user_cqes(void *buf, int entries, int cqe_size)
>>>    			buf += PAGE_SIZE;
>>>    		}
>>>    	} else {
>>> +		size_t copy_bytes = min_t(size_t, array_size(entries, cqe_size),
>>> +					 PAGE_SIZE);
>>> +
>>>    		err = copy_to_user((void __user *)buf, init_ents,
>>> -				   array_size(entries, cqe_size)) ?
>>> +				   copy_bytes) ?
>>>    			-EFAULT : 0;
>>>    	}
>>
>> Thanks for your patch.
>>
>> This is a compiler issue.
>> Did you try fixing it there first?
> Hi Tariq,
> 
> Thanks for the review.
> 
> Yes, I agree this is triggered by a GCC 10 limitation / false positive.
> 
> I have not tried to make a compiler-side fix the gating item here,
> because GCC 10 is still within the documented compiler range for
> building the kernel, so I think the kernel should still build cleanly
> with it.
> 
> That said, I also agree that my v1 shape is not ideal. In particular,
> the silent min_t(..., PAGE_SIZE) clamp is too implicit.
> 
> I think Paolo's suggested direction is a better shape here, i.e. keep
> array_size(), but make the bound explicit with a runtime guard instead
> of silently clamping it, e.g.
> 
>          copy_bytes = array_size(entries, cqe_size);
>          if (WARN_ON_ONCE(copy_bytes > PAGE_SIZE))
>                  return -EINVAL;
> 
>          err = copy_to_user((void __user *)buf, init_ents, copy_bytes) ?
>                  -EFAULT : 0;
> 
> That would keep the overflow-safe multiplication, avoid the silent
> truncation in v1, and make the single-copy branch invariant explicit
> for GCC 10.
> 
> Regarding David's suggestion of using a memset_user() loop, I've also
> looked into it, but couldn't locate either of those APIs in the kernel
> after check.Please let me know if you have any additional information
> or suggestions.
> 
> If this approach looks good to you, I'll send out the full v2 patch shortly.
> 

That would work.
Thanks.