diff mbox series

[net-next,1/2] octeontx2-af: Pass string literal as format argument of alloc_workqueue()

Message ID 20240903-octeontx2-sparse-v1-1-f190309ecb0a@kernel.org (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series octeontx2: Address some Sparse warnings | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for 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: 7 this patch: 7
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 15 of 15 maintainers
netdev/build_clang success Errors and warnings before: 16 this patch: 16
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: 21 this patch: 21
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 11 lines checked
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

Simon Horman Sept. 3, 2024, 4:26 p.m. UTC
Recently I noticed that both gcc-14 and clang-18 report that passing
a non-string literal as the format argument of alloc_workqueue()
is potentially insecure.

E.g. clang-18 says:

.../rvu.c:2493:32: warning: format string is not a string literal (potentially insecure) [-Wformat-security]
 2493 |         mw->mbox_wq = alloc_workqueue(name,
      |                                       ^~~~
.../rvu.c:2493:32: note: treat the string as an argument to avoid this
 2493 |         mw->mbox_wq = alloc_workqueue(name,
      |                                       ^
      |                                       "%s",

It is always the case where the contents of name is safe to pass as the
format argument. That is, in my understanding, it never contains any
format escape sequences.

But, it seems better to be safe than sorry. And, as a bonus, compiler
output becomes less verbose by addressing this issue as suggested by
clang-18.

Compile tested only.

Signed-off-by: Simon Horman <horms@kernel.org>
---
 drivers/net/ethernet/marvell/octeontx2/af/rvu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Geetha sowjanya Sept. 4, 2024, 9:02 a.m. UTC | #1
>-----Original Message-----
>From: Simon Horman <horms@kernel.org>
>Sent: Tuesday, September 3, 2024 9:57 PM
>To: Sunil Kovvuri Goutham <sgoutham@marvell.com>; Linu Cherian
><lcherian@marvell.com>; Geethasowjanya Akula <gakula@marvell.com>;
>Jerin Jacob <jerinj@marvell.com>; Hariprasad Kelam <hkelam@marvell.com>;
>Subbaraya Sundeep Bhatta <sbhatta@marvell.com>
>Cc: David S. Miller <davem@davemloft.net>; Eric Dumazet
><edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo Abeni
><pabeni@redhat.com>; Nathan Chancellor <nathan@kernel.org>; Nick
>Desaulniers <ndesaulniers@google.com>; Bill Wendling
><morbo@google.com>; Justin Stitt <justinstitt@google.com>;
>netdev@vger.kernel.org; llvm@lists.linux.dev
>Subject: [EXTERNAL] [PATCH net-next 1/2] octeontx2-af: Pass string literal as
>format argument of alloc_workqueue()
>Recently I noticed that both gcc-14 and clang-18 report that passing a non-
>string literal as the format argument of alloc_workqueue() is potentially
>insecure.
>
>E.g. clang-18 says:
>
>.../rvu.c:2493:32: warning: format string is not a string literal (potentially
>insecure) [-Wformat-security]
> 2493 |         mw->mbox_wq = alloc_workqueue(name,
>      |                                       ^~~~
>.../rvu.c:2493:32: note: treat the string as an argument to avoid this
> 2493 |         mw->mbox_wq = alloc_workqueue(name,
>      |                                       ^
>      |                                       "%s",
>
>It is always the case where the contents of name is safe to pass as the format
>argument. That is, in my understanding, it never contains any format escape
>sequences.
>
>But, it seems better to be safe than sorry. And, as a bonus, compiler output
>becomes less verbose by addressing this issue as suggested by clang-18.
>
>Compile tested only.
>
>Signed-off-by: Simon Horman <horms@kernel.org>
>---
> drivers/net/ethernet/marvell/octeontx2/af/rvu.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu.c
>b/drivers/net/ethernet/marvell/octeontx2/af/rvu.c
>index ac7ee3f3598c..1a97fb9032fa 100644
>--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu.c
>+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu.c
>@@ -2479,9 +2479,9 @@ static int rvu_mbox_init(struct rvu *rvu, struct
>mbox_wq_info *mw,
> 		goto free_regions;
> 	}
>
>-	mw->mbox_wq = alloc_workqueue(name,
>+	mw->mbox_wq = alloc_workqueue("%s",
> 				      WQ_UNBOUND | WQ_HIGHPRI |
>WQ_MEM_RECLAIM,
>-				      num);
>+				      num, name);
> 	if (!mw->mbox_wq) {
> 		err = -ENOMEM;
> 		goto unmap_regions;
>
>--
>2.45.2

Tested-by: Geetha sowjanya <gakula@marvell.com>
diff mbox series

Patch

diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu.c
index ac7ee3f3598c..1a97fb9032fa 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu.c
@@ -2479,9 +2479,9 @@  static int rvu_mbox_init(struct rvu *rvu, struct mbox_wq_info *mw,
 		goto free_regions;
 	}
 
-	mw->mbox_wq = alloc_workqueue(name,
+	mw->mbox_wq = alloc_workqueue("%s",
 				      WQ_UNBOUND | WQ_HIGHPRI | WQ_MEM_RECLAIM,
-				      num);
+				      num, name);
 	if (!mw->mbox_wq) {
 		err = -ENOMEM;
 		goto unmap_regions;