diff mbox series

[1/1] net/mlx4: Fix build error use array_size() helper in copy_to_user()

Message ID 20230107072725.673064-1-liyupeng@zbhlos.com
State New
Headers
Series [1/1] net/mlx4: Fix build error use array_size() helper in copy_to_user() |

Commit Message

Yupeng Li Jan. 7, 2023, 7:27 a.m. UTC 
  When CONFIG_64BIT was disabled, check_copy_size() was declared with
attribute error: copy source size is too small, array_size() for 32BIT
was wrong size, some compiled msg with error like:

  CALL    scripts/checksyscalls.sh
  CC [M]  drivers/net/ethernet/mellanox/mlx4/cq.o
In file included from ./arch/x86/include/asm/preempt.h:7,
                 from ./include/linux/preempt.h:78,
                 from ./include/linux/percpu.h:6,
                 from ./include/linux/context_tracking_state.h:5,
                 from ./include/linux/hardirq.h:5,
                 from drivers/net/ethernet/mellanox/mlx4/cq.c:37:
In function ‘check_copy_size’,
    inlined from ‘copy_to_user’ at ./include/linux/uaccess.h:168:6,
    inlined from ‘mlx4_init_user_cqes’ at drivers/net/ethernet/mellanox/mlx4/cq.c:317:9,
    inlined from ‘mlx4_cq_alloc’ at drivers/net/ethernet/mellanox/mlx4/cq.c:394:10:
./include/linux/thread_info.h:228:4: error: call to ‘__bad_copy_from’ declared with attribute error: copy source size is too small
  228 |    __bad_copy_from();
      |    ^~~~~~~~~~~~~~~~~
make[6]: *** [scripts/Makefile.build:250：drivers/net/ethernet/mellanox/mlx4/cq.o] 错误 1
make[5]: *** [scripts/Makefile.build:500：drivers/net/ethernet/mellanox/mlx4] 错误 2
make[5]: *** 正在等待未完成的任务....
make[4]: *** [scripts/Makefile.build:500：drivers/net/ethernet/mellanox] 错误 2
make[3]: *** [scripts/Makefile.build:500：drivers/net/ethernet] 错误 2
make[3]: *** 正在等待未完成的任务....
make[2]: *** [scripts/Makefile.build:500：drivers/net] 错误 2
make[2]: *** 正在等待未完成的任务....
make[1]: *** [scripts/Makefile.build:500：drivers] 错误 2
make: *** [Makefile:1992：.] 错误 2

Signed-off-by: Yupeng Li <liyupeng@zbhlos.com>
Reviewed-by: Caicai <caizp2008@163.com>
---
 drivers/net/ethernet/mellanox/mlx4/cq.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Jason Gunthorpe Jan. 9, 2023, 1:51 p.m. UTC | #1 
On Sat, Jan 07, 2023 at 03:27:25PM +0800, Yupeng Li wrote:
> When CONFIG_64BIT was disabled, check_copy_size() was declared with
> attribute error: copy source size is too small, array_size() for 32BIT
> was wrong size, some compiled msg with error like:
> 
>   CALL    scripts/checksyscalls.sh
>   CC [M]  drivers/net/ethernet/mellanox/mlx4/cq.o
> In file included from ./arch/x86/include/asm/preempt.h:7,
>                  from ./include/linux/preempt.h:78,
>                  from ./include/linux/percpu.h:6,
>                  from ./include/linux/context_tracking_state.h:5,
>                  from ./include/linux/hardirq.h:5,
>                  from drivers/net/ethernet/mellanox/mlx4/cq.c:37:
> In function ‘check_copy_size’,
>     inlined from ‘copy_to_user’ at ./include/linux/uaccess.h:168:6,
>     inlined from ‘mlx4_init_user_cqes’ at drivers/net/ethernet/mellanox/mlx4/cq.c:317:9,
>     inlined from ‘mlx4_cq_alloc’ at drivers/net/ethernet/mellanox/mlx4/cq.c:394:10:
> ./include/linux/thread_info.h:228:4: error: call to ‘__bad_copy_from’ declared with attribute error: copy source size is too small
>   228 |    __bad_copy_from();
>       |    ^~~~~~~~~~~~~~~~~
> make[6]: *** [scripts/Makefile.build:250：drivers/net/ethernet/mellanox/mlx4/cq.o] 错误 1
> make[5]: *** [scripts/Makefile.build:500：drivers/net/ethernet/mellanox/mlx4] 错误 2
> make[5]: *** 正在等待未完成的任务....
> make[4]: *** [scripts/Makefile.build:500：drivers/net/ethernet/mellanox] 错误 2
> make[3]: *** [scripts/Makefile.build:500：drivers/net/ethernet] 错误 2
> make[3]: *** 正在等待未完成的任务....
> make[2]: *** [scripts/Makefile.build:500：drivers/net] 错误 2
> make[2]: *** 正在等待未完成的任务....
> make[1]: *** [scripts/Makefile.build:500：drivers] 错误 2
> make: *** [Makefile:1992：.] 错误 2
> 
> Signed-off-by: Yupeng Li <liyupeng@zbhlos.com>
> Reviewed-by: Caicai <caizp2008@163.com>
> ---
>  drivers/net/ethernet/mellanox/mlx4/cq.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx4/cq.c b/drivers/net/ethernet/mellanox/mlx4/cq.c
> index 4d4f9cf9facb..7dadd7227480 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/cq.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/cq.c
> @@ -315,7 +315,11 @@ static int mlx4_init_user_cqes(void *buf, int entries, int cqe_size)
>  		}
>  	} else {
>  		err = copy_to_user((void __user *)buf, init_ents,
> +#ifdef CONFIG_64BIT
>  				   array_size(entries, cqe_size)) ?
> +#else
> +				   entries * cqe_size) ?
> +#endif
>  			-EFAULT : 0;

This can't possibly make sense, Kees?

Jason
Kees Cook Jan. 13, 2023, 6:41 p.m. UTC | #2 
On Mon, Jan 09, 2023 at 09:51:18AM -0400, Jason Gunthorpe wrote:
> On Sat, Jan 07, 2023 at 03:27:25PM +0800, Yupeng Li wrote:
> > When CONFIG_64BIT was disabled, check_copy_size() was declared with
> > attribute error: copy source size is too small, array_size() for 32BIT
> > was wrong size, some compiled msg with error like:
> > 
> >   CALL    scripts/checksyscalls.sh
> >   CC [M]  drivers/net/ethernet/mellanox/mlx4/cq.o
> > In file included from ./arch/x86/include/asm/preempt.h:7,
> >                  from ./include/linux/preempt.h:78,
> >                  from ./include/linux/percpu.h:6,
> >                  from ./include/linux/context_tracking_state.h:5,
> >                  from ./include/linux/hardirq.h:5,
> >                  from drivers/net/ethernet/mellanox/mlx4/cq.c:37:
> > In function ‘check_copy_size’,
> >     inlined from ‘copy_to_user’ at ./include/linux/uaccess.h:168:6,
> >     inlined from ‘mlx4_init_user_cqes’ at drivers/net/ethernet/mellanox/mlx4/cq.c:317:9,
> >     inlined from ‘mlx4_cq_alloc’ at drivers/net/ethernet/mellanox/mlx4/cq.c:394:10:
> > ./include/linux/thread_info.h:228:4: error: call to ‘__bad_copy_from’ declared with attribute error: copy source size is too small
> >   228 |    __bad_copy_from();
> >       |    ^~~~~~~~~~~~~~~~~
> > make[6]: *** [scripts/Makefile.build:250：drivers/net/ethernet/mellanox/mlx4/cq.o] 错误 1
> > make[5]: *** [scripts/Makefile.build:500：drivers/net/ethernet/mellanox/mlx4] 错误 2
> > make[5]: *** 正在等待未完成的任务....
> > make[4]: *** [scripts/Makefile.build:500：drivers/net/ethernet/mellanox] 错误 2
> > make[3]: *** [scripts/Makefile.build:500：drivers/net/ethernet] 错误 2
> > make[3]: *** 正在等待未完成的任务....
> > make[2]: *** [scripts/Makefile.build:500：drivers/net] 错误 2
> > make[2]: *** 正在等待未完成的任务....
> > make[1]: *** [scripts/Makefile.build:500：drivers] 错误 2
> > make: *** [Makefile:1992：.] 错误 2
> > 
> > Signed-off-by: Yupeng Li <liyupeng@zbhlos.com>
> > Reviewed-by: Caicai <caizp2008@163.com>
> > ---
> >  drivers/net/ethernet/mellanox/mlx4/cq.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/drivers/net/ethernet/mellanox/mlx4/cq.c b/drivers/net/ethernet/mellanox/mlx4/cq.c
> > index 4d4f9cf9facb..7dadd7227480 100644
> > --- a/drivers/net/ethernet/mellanox/mlx4/cq.c
> > +++ b/drivers/net/ethernet/mellanox/mlx4/cq.c
> > @@ -315,7 +315,11 @@ static int mlx4_init_user_cqes(void *buf, int entries, int cqe_size)
> >  		}
> >  	} else {
> >  		err = copy_to_user((void __user *)buf, init_ents,
> > +#ifdef CONFIG_64BIT
> >  				   array_size(entries, cqe_size)) ?
> > +#else
> > +				   entries * cqe_size) ?
> > +#endif
> >  			-EFAULT : 0;
> 
> This can't possibly make sense, Kees?

Uuuuh, that's really weird. What compiler version and arch? I'll see if
I can reproduce this.
Kees Cook Jan. 13, 2023, 11:01 p.m. UTC | #3 
On Fri, Jan 13, 2023 at 10:41:01AM -0800, Kees Cook wrote:
> On Mon, Jan 09, 2023 at 09:51:18AM -0400, Jason Gunthorpe wrote:
> > On Sat, Jan 07, 2023 at 03:27:25PM +0800, Yupeng Li wrote:
> > > When CONFIG_64BIT was disabled, check_copy_size() was declared with
> > > attribute error: copy source size is too small, array_size() for 32BIT
> > > was wrong size, some compiled msg with error like:
> > > 
> > >   CALL    scripts/checksyscalls.sh
> > >   CC [M]  drivers/net/ethernet/mellanox/mlx4/cq.o
> > > In file included from ./arch/x86/include/asm/preempt.h:7,
> > >                  from ./include/linux/preempt.h:78,
> > >                  from ./include/linux/percpu.h:6,
> > >                  from ./include/linux/context_tracking_state.h:5,
> > >                  from ./include/linux/hardirq.h:5,
> > >                  from drivers/net/ethernet/mellanox/mlx4/cq.c:37:
> > > In function ‘check_copy_size’,
> > >     inlined from ‘copy_to_user’ at ./include/linux/uaccess.h:168:6,
> > >     inlined from ‘mlx4_init_user_cqes’ at drivers/net/ethernet/mellanox/mlx4/cq.c:317:9,
> > >     inlined from ‘mlx4_cq_alloc’ at drivers/net/ethernet/mellanox/mlx4/cq.c:394:10:
> > > ./include/linux/thread_info.h:228:4: error: call to ‘__bad_copy_from’ declared with attribute error: copy source size is too small
> > >   228 |    __bad_copy_from();
> > >       |    ^~~~~~~~~~~~~~~~~
> > > make[6]: *** [scripts/Makefile.build:250：drivers/net/ethernet/mellanox/mlx4/cq.o] 错误 1
> > > make[5]: *** [scripts/Makefile.build:500：drivers/net/ethernet/mellanox/mlx4] 错误 2
> > > make[5]: *** 正在等待未完成的任务....
> > > make[4]: *** [scripts/Makefile.build:500：drivers/net/ethernet/mellanox] 错误 2
> > > make[3]: *** [scripts/Makefile.build:500：drivers/net/ethernet] 错误 2
> > > make[3]: *** 正在等待未完成的任务....
> > > make[2]: *** [scripts/Makefile.build:500：drivers/net] 错误 2
> > > make[2]: *** 正在等待未完成的任务....
> > > make[1]: *** [scripts/Makefile.build:500：drivers] 错误 2
> > > make: *** [Makefile:1992：.] 错误 2
> > > 
> > > Signed-off-by: Yupeng Li <liyupeng@zbhlos.com>
> > > Reviewed-by: Caicai <caizp2008@163.com>
> > > ---
> > >  drivers/net/ethernet/mellanox/mlx4/cq.c | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > > 
> > > diff --git a/drivers/net/ethernet/mellanox/mlx4/cq.c b/drivers/net/ethernet/mellanox/mlx4/cq.c
> > > index 4d4f9cf9facb..7dadd7227480 100644
> > > --- a/drivers/net/ethernet/mellanox/mlx4/cq.c
> > > +++ b/drivers/net/ethernet/mellanox/mlx4/cq.c
> > > @@ -315,7 +315,11 @@ static int mlx4_init_user_cqes(void *buf, int entries, int cqe_size)
> > >  		}
> > >  	} else {
> > >  		err = copy_to_user((void __user *)buf, init_ents,
> > > +#ifdef CONFIG_64BIT
> > >  				   array_size(entries, cqe_size)) ?
> > > +#else
> > > +				   entries * cqe_size) ?
> > > +#endif
> > >  			-EFAULT : 0;
> > 
> > This can't possibly make sense, Kees?
> 
> Uuuuh, that's really weird. What compiler version and arch? I'll see if
> I can reproduce this.

I can't reproduce this. I'm assuming this is being seen on a 32-bit
loongarch build? I have no idea how to get that compiler. Neither Debian
nor Fedora seem to package it. (It looks like it was added in GCC 12?)
Perhaps it's just "mips"? But I also can't figure out how to choose a
32-bit mips build. Wheee.

Anyway, I would assume this is a compiler bug around inlining or the
check_mul_overflow implementation?

static inline size_t __must_check size_mul(size_t factor1, size_t factor2)
{
        size_t bytes;

        if (check_mul_overflow(factor1, factor2, &bytes))
                return SIZE_MAX;

        return bytes;
}

#define array_size(a, b)        size_mul(a, b)


-Kees
Kees Cook Jan. 18, 2023, 8:54 p.m. UTC | #4 
On Sun, Jan 15, 2023 at 07:53:34PM +0800, caizp2008 wrote:
> my kernel config is config-mlx4-error.

Where can I find this config?
diff mbox series

Patch

diff --git a/drivers/net/ethernet/mellanox/mlx4/cq.c b/drivers/net/ethernet/mellanox/mlx4/cq.c
index 4d4f9cf9facb..7dadd7227480 100644
--- a/drivers/net/ethernet/mellanox/mlx4/cq.c
+++ b/drivers/net/ethernet/mellanox/mlx4/cq.c
@@ -315,7 +315,11 @@  static int mlx4_init_user_cqes(void *buf, int entries, int cqe_size)
 		}
 	} else {
 		err = copy_to_user((void __user *)buf, init_ents,
+#ifdef CONFIG_64BIT
 				   array_size(entries, cqe_size)) ?
+#else
+				   entries * cqe_size) ?
+#endif
 			-EFAULT : 0;
 	}