On Fri, Sep 08, 2023 at 09:59:40PM +0200, Philipp Stanner wrote:
> +static inline void *memdup_array_user(const void __user *src, size_t n, size_t size)
> +{
> + size_t nbytes;
> +
> + if (unlikely(check_mul_overflow(n, size, &nbytes)))
> + return ERR_PTR(-EOVERFLOW);
No need for an unlikely() because check_mul_overflow() already has one
inside. I feel like -ENOMEM is more traditional but I doubt anyone/userspace
cares.
> +
> + return memdup_user(src, nbytes);
> +}
regards,
dan carpenter
On Sat, Sep 16, 2023 at 05:32:42PM +0300, Dan Carpenter wrote:
> On Fri, Sep 08, 2023 at 09:59:40PM +0200, Philipp Stanner wrote:
...
> > +static inline void *memdup_array_user(const void __user *src, size_t n, size_t size)
> > +{
> > + size_t nbytes;
> > +
> > + if (unlikely(check_mul_overflow(n, size, &nbytes)))
> > + return ERR_PTR(-EOVERFLOW);
>
> No need for an unlikely() because check_mul_overflow() already has one
> inside.
Makes sense.
> I feel like -ENOMEM is more traditional but I doubt anyone/userspace
> cares.
ENOMEM is good for the real allocation calls, here is not the one (the one is
below). Hence ENOMEM is not good candidate above. And whenever functions returns
an error pointer the caller must not assume that it will be only ENOMEM for
allocators.
> > + return memdup_user(src, nbytes);
> > +}
--
With Best Regards,
Andy Shevchenko
On Sat, 2023-09-16 at 17:32 +0300, Dan Carpenter wrote:
> On Fri, Sep 08, 2023 at 09:59:40PM +0200, Philipp Stanner wrote:
> > +static inline void *memdup_array_user(const void __user *src,
> > size_t n, size_t size)
> > +{
> > + size_t nbytes;
> > +
> > + if (unlikely(check_mul_overflow(n, size, &nbytes)))
> > + return ERR_PTR(-EOVERFLOW);
>
> No need for an unlikely() because check_mul_overflow() already has
> one
> inside.
ACK. I overlooked that as it was hidden in the callstack.
> I feel like -ENOMEM is more traditional but I doubt
> anyone/userspace
> cares.
I agree with Andy here. We're not allocating, so -ENOMEM makes no sense
IMO.
P.
>
> > +
> > + return memdup_user(src, nbytes);
> > +}
>
> regards,
> dan carpenter
>