diff mbox series

net/mlx4: Fix build errors with gcc 10.3.1

Message ID 20241026100221.2242565-1-lihuafei1@huawei.com (mailing list archive)
State Rejected
Delegated to: Netdev Maintainers
Headers show
Series net/mlx4: Fix build errors with gcc 10.3.1 | expand

Checks

Context Check Description
netdev/series_format warning Single patches do not need cover letters; Target tree name not specified in the subject
netdev/tree_selection success Guessed tree name to be net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 5 this patch: 5
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 7 of 7 maintainers
netdev/build_clang success Errors and warnings before: 3 this patch: 3
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 4 this patch: 4
netdev/checkpatch warning WARNING: line length of 86 exceeds 80 columns
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Li Huafei Oct. 26, 2024, 10:02 a.m. UTC
When compiling the kernel in my environment (with gcc version gcc
10.3.1), I encountered the following compilation check error:

 In function ‘check_copy_size’,
     inlined from ‘copy_to_user’ at ./include/linux/uaccess.h:210:7,
     inlined from ‘mlx4_init_user_cqes’ at drivers/net/ethernet/mellanox/mlx4/cq.c:317:9:
 ./include/linux/thread_info.h:244:4: error: call to ‘__bad_copy_from’ declared with attribute error: copy source size is too small
   244 |    __bad_copy_from();

mlx4_init_user_cqes() checks the size of the buf before copying data,
ensuring that there will be no out-of-bounds copying, so this should be
a false positive. I searched the git commit history and found that the
commit 75da0eba0a47 ("rapidio: avoid bogus __alloc_size warning") fixed
a similar issue, where the compiler encountered an error when expanding
the arguments of check_copy_size().  Saving the result of array_size()
to a temporary variable and using this variable as the argument of
copy_to_user() can avoid this gcc warning.

Additionally, I tested older (9.4.0) and newer (10.3.5) versions and did
not encounter the same problem, so this should be a bug in a specific
intermediate version.

Signed-off-by: Li Huafei <lihuafei1@huawei.com>
---
 drivers/net/ethernet/mellanox/mlx4/cq.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

Comments

Tariq Toukan Oct. 28, 2024, 9:18 a.m. UTC | #1
On 26/10/2024 13:02, Li Huafei wrote:
> When compiling the kernel in my environment (with gcc version gcc
> 10.3.1), I encountered the following compilation check error:
> 
>   In function ‘check_copy_size’,
>       inlined from ‘copy_to_user’ at ./include/linux/uaccess.h:210:7,
>       inlined from ‘mlx4_init_user_cqes’ at drivers/net/ethernet/mellanox/mlx4/cq.c:317:9:
>   ./include/linux/thread_info.h:244:4: error: call to ‘__bad_copy_from’ declared with attribute error: copy source size is too small
>     244 |    __bad_copy_from();
> 
> mlx4_init_user_cqes() checks the size of the buf before copying data,
> ensuring that there will be no out-of-bounds copying, so this should be
> a false positive. I searched the git commit history and found that the
> commit 75da0eba0a47 ("rapidio: avoid bogus __alloc_size warning") fixed
> a similar issue, where the compiler encountered an error when expanding
> the arguments of check_copy_size().  Saving the result of array_size()
> to a temporary variable and using this variable as the argument of
> copy_to_user() can avoid this gcc warning.
> 
> Additionally, I tested older (9.4.0) and newer (10.3.5) versions and did
> not encounter the same problem, so this should be a bug in a specific
> intermediate version.
> 
> Signed-off-by: Li Huafei <lihuafei1@huawei.com>
> ---
>   drivers/net/ethernet/mellanox/mlx4/cq.c | 5 ++---
>   1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx4/cq.c b/drivers/net/ethernet/mellanox/mlx4/cq.c
> index e130e7259275..5169c7a4097b 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/cq.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/cq.c
> @@ -293,6 +293,7 @@ static int mlx4_init_user_cqes(void *buf, int entries, int cqe_size)
>   	void *init_ents;
>   	int err = 0;
>   	int i;
> +	size_t size = array_size(entries, cqe_size);
>   
>   	init_ents = kmalloc(PAGE_SIZE, GFP_KERNEL);
>   	if (!init_ents)
> @@ -314,9 +315,7 @@ static int mlx4_init_user_cqes(void *buf, int entries, int cqe_size)
>   			buf += PAGE_SIZE;
>   		}
>   	} else {
> -		err = copy_to_user((void __user *)buf, init_ents,
> -				   array_size(entries, cqe_size)) ?
> -			-EFAULT : 0;
> +		err = copy_to_user((void __user *)buf, init_ents, size) ? -EFAULT : 0;
>   	}
>   
>   out:


As you mention, the bug is in the compiler, in a very specific 
intermediate version.
Why would you modify the driver then?
diff mbox series

Patch

diff --git a/drivers/net/ethernet/mellanox/mlx4/cq.c b/drivers/net/ethernet/mellanox/mlx4/cq.c
index e130e7259275..5169c7a4097b 100644
--- a/drivers/net/ethernet/mellanox/mlx4/cq.c
+++ b/drivers/net/ethernet/mellanox/mlx4/cq.c
@@ -293,6 +293,7 @@  static int mlx4_init_user_cqes(void *buf, int entries, int cqe_size)
 	void *init_ents;
 	int err = 0;
 	int i;
+	size_t size = array_size(entries, cqe_size);
 
 	init_ents = kmalloc(PAGE_SIZE, GFP_KERNEL);
 	if (!init_ents)
@@ -314,9 +315,7 @@  static int mlx4_init_user_cqes(void *buf, int entries, int cqe_size)
 			buf += PAGE_SIZE;
 		}
 	} else {
-		err = copy_to_user((void __user *)buf, init_ents,
-				   array_size(entries, cqe_size)) ?
-			-EFAULT : 0;
+		err = copy_to_user((void __user *)buf, init_ents, size) ? -EFAULT : 0;
 	}
 
 out: