| Message ID | 20240909-strncpy-net-bridge-netfilter-nft_meta_bridge-c-v1-1-946180aa7909@google.com (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | netfilter: nf_tables: replace deprecated strncpy with strscpy_pad | expand |
On Mon, Sep 09, 2024 at 03:48:39PM -0700, Justin Stitt wrote: > strncpy() is deprecated for use on NUL-terminated destination strings [1] and > as such we should prefer more robust and less ambiguous string interfaces. > > In this particular instance, the usage of strncpy() is fine and works as > expected. However, towards the goal of [2], we should consider replacing > it with an alternative as many instances of strncpy() are bug-prone. Its > removal from the kernel promotes better long term health for the > codebase. > > The current usage of strncpy() likely just wants the NUL-padding > behavior offered by strncpy() and doesn't care about the > NUL-termination. Since the compiler doesn't know the size of @dest, we > can't use strtomem_pad(). Instead, use strscpy_pad() which behaves > functionally the same as strncpy() in this context -- as we expect > br_dev->name to be NUL-terminated itself. > > Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1] > Link: https://github.com/KSPP/linux/issues/90 [2] > Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html > Cc: Kees Cook <keescook@chromium.org> > Cc: linux-hardening@vger.kernel.org > Signed-off-by: Justin Stitt <justinstitt@google.com> > --- > Note: build-tested only. Reviewed-by: Simon Horman <horms@kernel.org>
On Mon, Sep 09, 2024 at 03:48:39PM -0700, Justin Stitt wrote: > strncpy() is deprecated for use on NUL-terminated destination strings [1] and > as such we should prefer more robust and less ambiguous string interfaces. > > In this particular instance, the usage of strncpy() is fine and works as > expected. However, towards the goal of [2], we should consider replacing > it with an alternative as many instances of strncpy() are bug-prone. Its > removal from the kernel promotes better long term health for the > codebase. > > The current usage of strncpy() likely just wants the NUL-padding > behavior offered by strncpy() and doesn't care about the > NUL-termination. Since the compiler doesn't know the size of @dest, we > can't use strtomem_pad(). Instead, use strscpy_pad() which behaves > functionally the same as strncpy() in this context -- as we expect > br_dev->name to be NUL-terminated itself. Applied to nf-next
diff --git a/net/bridge/netfilter/nft_meta_bridge.c b/net/bridge/netfilter/nft_meta_bridge.c index bd4d1b4d745f..2a17e88ab8ee 100644 --- a/net/bridge/netfilter/nft_meta_bridge.c +++ b/net/bridge/netfilter/nft_meta_bridge.c @@ -63,7 +63,7 @@ static void nft_meta_bridge_get_eval(const struct nft_expr *expr, return nft_meta_get_eval(expr, regs, pkt); } - strncpy((char *)dest, br_dev ? br_dev->name : "", IFNAMSIZ); + strscpy_pad((char *)dest, br_dev ? br_dev->name : "", IFNAMSIZ); return; err: regs->verdict.code = NFT_BREAK;
strncpy() is deprecated for use on NUL-terminated destination strings [1] and as such we should prefer more robust and less ambiguous string interfaces. In this particular instance, the usage of strncpy() is fine and works as expected. However, towards the goal of [2], we should consider replacing it with an alternative as many instances of strncpy() are bug-prone. Its removal from the kernel promotes better long term health for the codebase. The current usage of strncpy() likely just wants the NUL-padding behavior offered by strncpy() and doesn't care about the NUL-termination. Since the compiler doesn't know the size of @dest, we can't use strtomem_pad(). Instead, use strscpy_pad() which behaves functionally the same as strncpy() in this context -- as we expect br_dev->name to be NUL-terminated itself. Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1] Link: https://github.com/KSPP/linux/issues/90 [2] Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html Cc: Kees Cook <keescook@chromium.org> Cc: linux-hardening@vger.kernel.org Signed-off-by: Justin Stitt <justinstitt@google.com> --- Note: build-tested only. --- net/bridge/netfilter/nft_meta_bridge.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) --- base-commit: 521b1e7f4cf0b05a47995b103596978224b380a8 change-id: 20240909-strncpy-net-bridge-netfilter-nft_meta_bridge-c-09dd8aaad386 Best regards, -- Justin Stitt <justinstitt@google.com>