netdev - [PATCH net-next] bonding: Pass string literal as format argument of alloc_ordered

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20240806-bonding-fmt-v1-1-e75027e45775@kernel.org>
Date: Tue, 06 Aug 2024 10:56:52 +0100
From: Simon Horman <horms@...nel.org>
To: "David S. Miller" <davem@...emloft.net>, 
 Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, 
 Paolo Abeni <pabeni@...hat.com>
Cc: Jay Vosburgh <jv@...sburgh.net>, Andy Gospodarek <andy@...yhouse.net>, 
 Nathan Chancellor <nathan@...nel.org>, 
 Nick Desaulniers <ndesaulniers@...gle.com>, 
 Bill Wendling <morbo@...gle.com>, Justin Stitt <justinstitt@...gle.com>, 
 netdev@...r.kernel.org, llvm@...ts.linux.dev
Subject: [PATCH net-next] bonding: Pass string literal as format argument
 of alloc_ordered_workqueue()

Recently I noticed that both gcc-14 and clang-18 report that passing
a non-string literal as the format argument of alloc_ordered_workqueue
is potentially insecure.

F.e. clang-18 says:

.../bond_main.c:6384:37: warning: format string is not a string literal (potentially insecure) [-Wformat-security]
 6384 |         bond->wq = alloc_ordered_workqueue(bond_dev->name, WQ_MEM_RECLAIM);
      |                                            ^~~~~~~~~~~~~~
.../workqueue.h:524:18: note: expanded from macro 'alloc_ordered_workqueue'
  524 |         alloc_workqueue(fmt, WQ_UNBOUND | __WQ_ORDERED | (flags), 1, ##args)
      |                         ^~~
.../bond_main.c:6384:37: note: treat the string as an argument to avoid this
 6384 |         bond->wq = alloc_ordered_workqueue(bond_dev->name, WQ_MEM_RECLAIM);
      |                                            ^
      |                                            "%s",
..../workqueue.h:524:18: note: expanded from macro 'alloc_ordered_workqueue'
  524 |         alloc_workqueue(fmt, WQ_UNBOUND | __WQ_ORDERED | (flags), 1, ##args)
      |                         ^

Perhaps it is always the case where the contents of bond_dev->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.

Signed-off-by: Simon Horman <horms@...nel.org>
---
 drivers/net/bonding/bond_main.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 1cd92c12e782..f9633a6f8571 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -6338,7 +6338,8 @@ static int bond_init(struct net_device *bond_dev)

 	netdev_dbg(bond_dev, "Begin bond_init\n");

-	bond->wq = alloc_ordered_workqueue(bond_dev->name, WQ_MEM_RECLAIM);
+	bond->wq = alloc_ordered_workqueue("%s", WQ_MEM_RECLAIM,
+					   bond_dev->name);
 	if (!bond->wq)
 		return -ENOMEM;