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

[Yao Sang <sangyao@kylinos.cn>]

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.

Thanks,
Yao