| Message ID | 20241018-nf-mod-fmt-v1-1-b5a275d6861c@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [nf-next] netfilter: bpf: Pass string literal as format argument of request_module() | expand |
Simon Horman <horms@kernel.org> wrote: > Both gcc-14 and clang-18 report that passing a non-string literal as the > format argument of request_module() is potentially insecure. Reviewed-by: Florian Westphal <fw@strlen.de>
Simon Horman <horms@kernel.org> writes: > Both gcc-14 and clang-18 report that passing a non-string literal as the > format argument of request_module() is potentially insecure. > > E.g. clang-18 says: > > .../nf_bpf_link.c:46:24: warning: format string is not a string literal (potentially insecure) [-Wformat-security] > 46 | err = request_module(mod); > | ^~~ > .../kmod.h:25:55: note: expanded from macro 'request_module' > 25 | #define request_module(mod...) __request_module(true, mod) > | ^~~ > .../nf_bpf_link.c:46:24: note: treat the string as an argument to avoid this > 46 | err = request_module(mod); > | ^ > | "%s", > .../kmod.h:25:55: note: expanded from macro 'request_module' > 25 | #define request_module(mod...) __request_module(true, mod) > | ^ > > It is always the case where the contents of mod 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. > > No functional change intended. > Compile tested only. > > Signed-off-by: Simon Horman <horms@kernel.org> Reviewed-by: Toke Høiland-Jørgensen <toke@redhat.com>
diff --git a/net/netfilter/nf_bpf_link.c b/net/netfilter/nf_bpf_link.c index 5257d5e7eb09..6b9c9d71906d 100644 --- a/net/netfilter/nf_bpf_link.c +++ b/net/netfilter/nf_bpf_link.c @@ -42,7 +42,7 @@ get_proto_defrag_hook(struct bpf_nf_link *link, hook = rcu_dereference(*ptr_global_hook); if (!hook) { rcu_read_unlock(); - err = request_module(mod); + err = request_module("%s", mod); if (err) return ERR_PTR(err < 0 ? err : -EINVAL);
Both gcc-14 and clang-18 report that passing a non-string literal as the format argument of request_module() is potentially insecure. E.g. clang-18 says: .../nf_bpf_link.c:46:24: warning: format string is not a string literal (potentially insecure) [-Wformat-security] 46 | err = request_module(mod); | ^~~ .../kmod.h:25:55: note: expanded from macro 'request_module' 25 | #define request_module(mod...) __request_module(true, mod) | ^~~ .../nf_bpf_link.c:46:24: note: treat the string as an argument to avoid this 46 | err = request_module(mod); | ^ | "%s", .../kmod.h:25:55: note: expanded from macro 'request_module' 25 | #define request_module(mod...) __request_module(true, mod) | ^ It is always the case where the contents of mod 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. No functional change intended. Compile tested only. Signed-off-by: Simon Horman <horms@kernel.org> --- net/netfilter/nf_bpf_link.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)