| Message ID | 20240806-bonding-fmt-v1-1-e75027e45775@kernel.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 93b828cc8e2a87355ee5e852d27c21fdee27591b |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] bonding: Pass string literal as format argument of alloc_ordered_workqueue() | expand |
Simon Horman <horms@kernel.org> wrote: >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@kernel.org> Acked-by: Jay Vosburgh <jv@jvosburgh.net> >--- > 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; > >
Hello: This patch was applied to netdev/net-next.git (main) by Jakub Kicinski <kuba@kernel.org>: On Tue, 06 Aug 2024 10:56:52 +0100 you wrote: > 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) > | ^ > > [...] Here is the summary with links: - [net-next] bonding: Pass string literal as format argument of alloc_ordered_workqueue() https://git.kernel.org/netdev/net-next/c/93b828cc8e2a You are awesome, thank you!
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;
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@kernel.org> --- drivers/net/bonding/bond_main.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)