* [PATCH v2 0/7] netfilter: refactor deprecated strncpy
@ 2023-08-09 1:06 Justin Stitt
2023-08-09 1:06 ` [PATCH v2 1/7] netfilter: ipset: " Justin Stitt
` (6 more replies)
0 siblings, 7 replies; 17+ messages in thread
From: Justin Stitt @ 2023-08-09 1:06 UTC (permalink / raw)
To: Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: linux-hardening, Kees Cook, netfilter-devel, coreteam, netdev,
linux-kernel, Justin Stitt
`strncpy` is deprecated for use on NUL-terminated destination strings [1].
A suitable replacement is `strscpy` or `strscpy_pad` [2] due to the fact
that they guarantee NUL-termination on their destination buffer argument
which is _not_ the case for `strncpy`!
This series of patches aims to swap out `strncpy` for more a robust and
less ambiguous interface `strscpy_pad` . This patch series, if applied
in its entirety, removes most if not all instances of `strncpy` in the
`net/netfilter` directory.
[1]: www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings
[2]: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html
Link: https://github.com/KSPP/linux/issues/90
Related: commit 8556bceb9c40 ("netfilter: move from strlcpy with unused retval to strscpy")
---
Changes in v2:
- use `strscpy_pad` instead of `strscpy` since zero-padding is needed
(thanks Florian and Kees)
- Link to v1: https://lore.kernel.org/r/20230808-net-netfilter-v1-0-efbbe4ec60af@google.com
---
Justin Stitt (7):
netfilter: ipset: refactor deprecated strncpy
netfilter: nf_tables: refactor deprecated strncpy
netfilter: nf_tables: refactor deprecated strncpy
netfilter: nft_meta: refactor deprecated strncpy
netfilter: nft_osf: refactor deprecated strncpy
netfilter: x_tables: refactor deprecated strncpy
netfilter: xtables: refactor deprecated strncpy
net/netfilter/ipset/ip_set_core.c | 10 +++++-----
net/netfilter/nft_ct.c | 2 +-
net/netfilter/nft_fib.c | 2 +-
net/netfilter/nft_meta.c | 6 +++---
net/netfilter/nft_osf.c | 6 +++---
net/netfilter/x_tables.c | 5 ++---
net/netfilter/xt_repldata.h | 2 +-
7 files changed, 16 insertions(+), 17 deletions(-)
---
base-commit: 14f9643dc90adea074a0ffb7a17d337eafc6a5cc
change-id: 20230807-net-netfilter-4027219bb6e7
Best regards,
--
Justin Stitt <justinstitt@google.com>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 1/7] netfilter: ipset: refactor deprecated strncpy
2023-08-09 1:06 [PATCH v2 0/7] netfilter: refactor deprecated strncpy Justin Stitt
@ 2023-08-09 1:06 ` Justin Stitt
2023-08-09 20:19 ` Florian Westphal
2023-08-09 1:06 ` [PATCH v2 2/7] netfilter: nf_tables: " Justin Stitt
` (5 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Justin Stitt @ 2023-08-09 1:06 UTC (permalink / raw)
To: Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: linux-hardening, Kees Cook, netfilter-devel, coreteam, netdev,
linux-kernel, Justin Stitt
Use `strscpy_pad` instead of `strncpy`.
Link: https://github.com/KSPP/linux/issues/90
Cc: linux-hardening@vger.kernel.org
Signed-off-by: Justin Stitt <justinstitt@google.com>
---
net/netfilter/ipset/ip_set_core.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/net/netfilter/ipset/ip_set_core.c b/net/netfilter/ipset/ip_set_core.c
index 0b68e2e2824e..e564b5174261 100644
--- a/net/netfilter/ipset/ip_set_core.c
+++ b/net/netfilter/ipset/ip_set_core.c
@@ -872,7 +872,7 @@ ip_set_name_byindex(struct net *net, ip_set_id_t index, char *name)
BUG_ON(!set);
read_lock_bh(&ip_set_ref_lock);
- strncpy(name, set->name, IPSET_MAXNAMELEN);
+ strscpy_pad(name, set->name, IPSET_MAXNAMELEN);
read_unlock_bh(&ip_set_ref_lock);
}
EXPORT_SYMBOL_GPL(ip_set_name_byindex);
@@ -1326,7 +1326,7 @@ static int ip_set_rename(struct sk_buff *skb, const struct nfnl_info *info,
goto out;
}
}
- strncpy(set->name, name2, IPSET_MAXNAMELEN);
+ strscpy_pad(set->name, name2, IPSET_MAXNAMELEN);
out:
write_unlock_bh(&ip_set_ref_lock);
@@ -1380,9 +1380,9 @@ static int ip_set_swap(struct sk_buff *skb, const struct nfnl_info *info,
return -EBUSY;
}
- strncpy(from_name, from->name, IPSET_MAXNAMELEN);
- strncpy(from->name, to->name, IPSET_MAXNAMELEN);
- strncpy(to->name, from_name, IPSET_MAXNAMELEN);
+ strscpy_pad(from_name, from->name, IPSET_MAXNAMELEN);
+ strscpy_pad(from->name, to->name, IPSET_MAXNAMELEN);
+ strscpy_pad(to->name, from_name, IPSET_MAXNAMELEN);
swap(from->ref, to->ref);
ip_set(inst, from_id) = to;
--
2.41.0.640.ga95def55d0-goog
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v2 2/7] netfilter: nf_tables: refactor deprecated strncpy
2023-08-09 1:06 [PATCH v2 0/7] netfilter: refactor deprecated strncpy Justin Stitt
2023-08-09 1:06 ` [PATCH v2 1/7] netfilter: ipset: " Justin Stitt
@ 2023-08-09 1:06 ` Justin Stitt
2023-08-09 1:06 ` [PATCH v2 3/7] " Justin Stitt
` (4 subsequent siblings)
6 siblings, 0 replies; 17+ messages in thread
From: Justin Stitt @ 2023-08-09 1:06 UTC (permalink / raw)
To: Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: linux-hardening, Kees Cook, netfilter-devel, coreteam, netdev,
linux-kernel, Justin Stitt
Prefer `strscpy_pad` over `strncpy`.
Signed-off-by: Justin Stitt <justinstitt@google.com>
---
Note: Opt for `strscpy_pad` over `strscpy` since padding is wanted [1]
Link: https://lore.kernel.org/all/20230808234001.GJ9741@breakpoint.cc/ [1]
---
net/netfilter/nft_ct.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/netfilter/nft_ct.c b/net/netfilter/nft_ct.c
index 38958e067aa8..bbf11c4871b2 100644
--- a/net/netfilter/nft_ct.c
+++ b/net/netfilter/nft_ct.c
@@ -108,7 +108,7 @@ static void nft_ct_get_eval(const struct nft_expr *expr,
helper = rcu_dereference(help->helper);
if (helper == NULL)
goto err;
- strncpy((char *)dest, helper->name, NF_CT_HELPER_NAME_LEN);
+ strscpy_pad((char *)dest, helper->name, NF_CT_HELPER_NAME_LEN);
return;
#ifdef CONFIG_NF_CONNTRACK_LABELS
case NFT_CT_LABELS: {
--
2.41.0.640.ga95def55d0-goog
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v2 3/7] netfilter: nf_tables: refactor deprecated strncpy
2023-08-09 1:06 [PATCH v2 0/7] netfilter: refactor deprecated strncpy Justin Stitt
2023-08-09 1:06 ` [PATCH v2 1/7] netfilter: ipset: " Justin Stitt
2023-08-09 1:06 ` [PATCH v2 2/7] netfilter: nf_tables: " Justin Stitt
@ 2023-08-09 1:06 ` Justin Stitt
2023-08-09 1:06 ` [PATCH v2 4/7] netfilter: nft_meta: " Justin Stitt
` (3 subsequent siblings)
6 siblings, 0 replies; 17+ messages in thread
From: Justin Stitt @ 2023-08-09 1:06 UTC (permalink / raw)
To: Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: linux-hardening, Kees Cook, netfilter-devel, coreteam, netdev,
linux-kernel, Justin Stitt
Prefer `strscpy_pad` over `strncpy`.
Signed-off-by: Justin Stitt <justinstitt@google.com>
---
net/netfilter/nft_fib.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/netfilter/nft_fib.c b/net/netfilter/nft_fib.c
index 6e049fd48760..8c250a47a3b4 100644
--- a/net/netfilter/nft_fib.c
+++ b/net/netfilter/nft_fib.c
@@ -150,7 +150,7 @@ void nft_fib_store_result(void *reg, const struct nft_fib *priv,
if (priv->flags & NFTA_FIB_F_PRESENT)
*dreg = !!dev;
else
- strncpy(reg, dev ? dev->name : "", IFNAMSIZ);
+ strscpy_pad(reg, dev ? dev->name : "", IFNAMSIZ);
break;
default:
WARN_ON_ONCE(1);
--
2.41.0.640.ga95def55d0-goog
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v2 4/7] netfilter: nft_meta: refactor deprecated strncpy
2023-08-09 1:06 [PATCH v2 0/7] netfilter: refactor deprecated strncpy Justin Stitt
` (2 preceding siblings ...)
2023-08-09 1:06 ` [PATCH v2 3/7] " Justin Stitt
@ 2023-08-09 1:06 ` Justin Stitt
2023-08-09 1:06 ` [PATCH v2 5/7] netfilter: nft_osf: " Justin Stitt
` (2 subsequent siblings)
6 siblings, 0 replies; 17+ messages in thread
From: Justin Stitt @ 2023-08-09 1:06 UTC (permalink / raw)
To: Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: linux-hardening, Kees Cook, netfilter-devel, coreteam, netdev,
linux-kernel, Justin Stitt
Prefer `strscpy_pad` to `strncpy`.
Signed-off-by: Justin Stitt <justinstitt@google.com>
---
net/netfilter/nft_meta.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/netfilter/nft_meta.c b/net/netfilter/nft_meta.c
index 8fdc7318c03c..f7da7c43333b 100644
--- a/net/netfilter/nft_meta.c
+++ b/net/netfilter/nft_meta.c
@@ -185,12 +185,12 @@ static noinline bool nft_meta_get_eval_kind(enum nft_meta_keys key,
case NFT_META_IIFKIND:
if (!in || !in->rtnl_link_ops)
return false;
- strncpy((char *)dest, in->rtnl_link_ops->kind, IFNAMSIZ);
+ strscpy_pad((char *)dest, in->rtnl_link_ops->kind, IFNAMSIZ);
break;
case NFT_META_OIFKIND:
if (!out || !out->rtnl_link_ops)
return false;
- strncpy((char *)dest, out->rtnl_link_ops->kind, IFNAMSIZ);
+ strscpy_pad((char *)dest, out->rtnl_link_ops->kind, IFNAMSIZ);
break;
default:
return false;
@@ -206,7 +206,7 @@ static void nft_meta_store_ifindex(u32 *dest, const struct net_device *dev)
static void nft_meta_store_ifname(u32 *dest, const struct net_device *dev)
{
- strncpy((char *)dest, dev ? dev->name : "", IFNAMSIZ);
+ strscpy_pad((char *)dest, dev ? dev->name : "", IFNAMSIZ);
}
static bool nft_meta_store_iftype(u32 *dest, const struct net_device *dev)
--
2.41.0.640.ga95def55d0-goog
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v2 5/7] netfilter: nft_osf: refactor deprecated strncpy
2023-08-09 1:06 [PATCH v2 0/7] netfilter: refactor deprecated strncpy Justin Stitt
` (3 preceding siblings ...)
2023-08-09 1:06 ` [PATCH v2 4/7] netfilter: nft_meta: " Justin Stitt
@ 2023-08-09 1:06 ` Justin Stitt
2023-08-09 20:21 ` Florian Westphal
2023-08-09 1:06 ` [PATCH v2 6/7] netfilter: x_tables: " Justin Stitt
2023-08-09 1:06 ` [PATCH v2 7/7] netfilter: xtables: " Justin Stitt
6 siblings, 1 reply; 17+ messages in thread
From: Justin Stitt @ 2023-08-09 1:06 UTC (permalink / raw)
To: Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: linux-hardening, Kees Cook, netfilter-devel, coreteam, netdev,
linux-kernel, Justin Stitt
Use `strscpy_pad` over `strncpy` for NUL-terminated strings.
We can also drop the + 1 from `NFT_OSF_MAXGENRELEN + 1` since `strscpy`
will guarantee NUL-termination.
Signed-off-by: Justin Stitt <justinstitt@google.com>
---
Note: Should this usage @8556bceb9c409 also be changed to `strscpy_pad`
or is zero-padding not required here?
---
net/netfilter/nft_osf.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/netfilter/nft_osf.c b/net/netfilter/nft_osf.c
index 70820c66b591..7f61506e5b44 100644
--- a/net/netfilter/nft_osf.c
+++ b/net/netfilter/nft_osf.c
@@ -23,7 +23,7 @@ static void nft_osf_eval(const struct nft_expr *expr, struct nft_regs *regs,
struct nft_osf *priv = nft_expr_priv(expr);
u32 *dest = ®s->data[priv->dreg];
struct sk_buff *skb = pkt->skb;
- char os_match[NFT_OSF_MAXGENRELEN + 1];
+ char os_match[NFT_OSF_MAXGENRELEN];
const struct tcphdr *tcp;
struct nf_osf_data data;
struct tcphdr _tcph;
@@ -45,7 +45,7 @@ static void nft_osf_eval(const struct nft_expr *expr, struct nft_regs *regs,
}
if (!nf_osf_find(skb, nf_osf_fingers, priv->ttl, &data)) {
- strncpy((char *)dest, "unknown", NFT_OSF_MAXGENRELEN);
+ strscpy_pad((char *)dest, "unknown", NFT_OSF_MAXGENRELEN);
} else {
if (priv->flags & NFT_OSF_F_VERSION)
snprintf(os_match, NFT_OSF_MAXGENRELEN, "%s:%s",
@@ -53,7 +53,7 @@ static void nft_osf_eval(const struct nft_expr *expr, struct nft_regs *regs,
else
strscpy(os_match, data.genre, NFT_OSF_MAXGENRELEN);
- strncpy((char *)dest, os_match, NFT_OSF_MAXGENRELEN);
+ strscpy_pad((char *)dest, os_match, NFT_OSF_MAXGENRELEN);
}
}
--
2.41.0.640.ga95def55d0-goog
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v2 6/7] netfilter: x_tables: refactor deprecated strncpy
2023-08-09 1:06 [PATCH v2 0/7] netfilter: refactor deprecated strncpy Justin Stitt
` (4 preceding siblings ...)
2023-08-09 1:06 ` [PATCH v2 5/7] netfilter: nft_osf: " Justin Stitt
@ 2023-08-09 1:06 ` Justin Stitt
2023-08-09 20:20 ` Florian Westphal
2023-08-09 1:06 ` [PATCH v2 7/7] netfilter: xtables: " Justin Stitt
6 siblings, 1 reply; 17+ messages in thread
From: Justin Stitt @ 2023-08-09 1:06 UTC (permalink / raw)
To: Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: linux-hardening, Kees Cook, netfilter-devel, coreteam, netdev,
linux-kernel, Justin Stitt
Prefer `strscpy_pad` to `strncpy`.
Signed-off-by: Justin Stitt <justinstitt@google.com>
---
net/netfilter/x_tables.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
index 470282cf3fae..21624d68314f 100644
--- a/net/netfilter/x_tables.c
+++ b/net/netfilter/x_tables.c
@@ -768,7 +768,7 @@ void xt_compat_match_from_user(struct xt_entry_match *m, void **dstptr,
m->u.user.match_size = msize;
strscpy(name, match->name, sizeof(name));
module_put(match->me);
- strncpy(m->u.user.name, name, sizeof(m->u.user.name));
+ strscpy_pad(m->u.user.name, name, sizeof(m->u.user.name));
*size += off;
*dstptr += msize;
@@ -1148,7 +1148,7 @@ void xt_compat_target_from_user(struct xt_entry_target *t, void **dstptr,
t->u.user.target_size = tsize;
strscpy(name, target->name, sizeof(name));
module_put(target->me);
- strncpy(t->u.user.name, name, sizeof(t->u.user.name));
+ strscpy_pad(t->u.user.name, name, sizeof(t->u.user.name));
*size += off;
*dstptr += tsize;
@@ -2014,4 +2014,3 @@ static void __exit xt_fini(void)
module_init(xt_init);
module_exit(xt_fini);
-
--
2.41.0.640.ga95def55d0-goog
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v2 7/7] netfilter: xtables: refactor deprecated strncpy
2023-08-09 1:06 [PATCH v2 0/7] netfilter: refactor deprecated strncpy Justin Stitt
` (5 preceding siblings ...)
2023-08-09 1:06 ` [PATCH v2 6/7] netfilter: x_tables: " Justin Stitt
@ 2023-08-09 1:06 ` Justin Stitt
2023-08-09 20:20 ` Florian Westphal
6 siblings, 1 reply; 17+ messages in thread
From: Justin Stitt @ 2023-08-09 1:06 UTC (permalink / raw)
To: Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: linux-hardening, Kees Cook, netfilter-devel, coreteam, netdev,
linux-kernel, Justin Stitt
Prefer `strscpy_pad` as it's a more robust interface whilst maintaing
zero-padding behavior.
There may have existed a bug here due to both `tbl->repl.name` and
`info->name` having a size of 32 as defined below:
| #define XT_TABLE_MAXNAMELEN 32
This may lead to buffer overreads in some situations -- `strscpy` solves
this by guaranteeing NUL-termination of the dest buffer.
Signed-off-by: Justin Stitt <justinstitt@google.com>
---
Note: build tested only
---
net/netfilter/xt_repldata.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/netfilter/xt_repldata.h b/net/netfilter/xt_repldata.h
index 68ccbe50bb1e..5d1fb7018dba 100644
--- a/net/netfilter/xt_repldata.h
+++ b/net/netfilter/xt_repldata.h
@@ -29,7 +29,7 @@
if (tbl == NULL) \
return NULL; \
term = (struct type##_error *)&(((char *)tbl)[term_offset]); \
- strncpy(tbl->repl.name, info->name, sizeof(tbl->repl.name)); \
+ strscpy_pad(tbl->repl.name, info->name, sizeof(tbl->repl.name)); \
*term = (struct type##_error)typ2##_ERROR_INIT; \
tbl->repl.valid_hooks = hook_mask; \
tbl->repl.num_entries = nhooks + 1; \
--
2.41.0.640.ga95def55d0-goog
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v2 1/7] netfilter: ipset: refactor deprecated strncpy
2023-08-09 1:06 ` [PATCH v2 1/7] netfilter: ipset: " Justin Stitt
@ 2023-08-09 20:19 ` Florian Westphal
2023-08-09 21:40 ` Justin Stitt
0 siblings, 1 reply; 17+ messages in thread
From: Florian Westphal @ 2023-08-09 20:19 UTC (permalink / raw)
To: Justin Stitt
Cc: Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
linux-hardening, Kees Cook, netfilter-devel, coreteam, netdev,
linux-kernel
Justin Stitt <justinstitt@google.com> wrote:
> Use `strscpy_pad` instead of `strncpy`.
I don't think that any of these need zero-padding.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 6/7] netfilter: x_tables: refactor deprecated strncpy
2023-08-09 1:06 ` [PATCH v2 6/7] netfilter: x_tables: " Justin Stitt
@ 2023-08-09 20:20 ` Florian Westphal
0 siblings, 0 replies; 17+ messages in thread
From: Florian Westphal @ 2023-08-09 20:20 UTC (permalink / raw)
To: Justin Stitt
Cc: Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
linux-hardening, Kees Cook, netfilter-devel, coreteam, netdev,
linux-kernel
Justin Stitt <justinstitt@google.com> wrote:
> Prefer `strscpy_pad` to `strncpy`.
I don't think this instance needs _pad.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 7/7] netfilter: xtables: refactor deprecated strncpy
2023-08-09 1:06 ` [PATCH v2 7/7] netfilter: xtables: " Justin Stitt
@ 2023-08-09 20:20 ` Florian Westphal
0 siblings, 0 replies; 17+ messages in thread
From: Florian Westphal @ 2023-08-09 20:20 UTC (permalink / raw)
To: Justin Stitt
Cc: Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
linux-hardening, Kees Cook, netfilter-devel, coreteam, netdev,
linux-kernel
Justin Stitt <justinstitt@google.com> wrote:
> Prefer `strscpy_pad` as it's a more robust interface whilst maintaing
> zero-padding behavior.
>
> There may have existed a bug here due to both `tbl->repl.name` and
> `info->name` having a size of 32 as defined below:
> | #define XT_TABLE_MAXNAMELEN 32
>
> This may lead to buffer overreads in some situations -- `strscpy` solves
> this by guaranteeing NUL-termination of the dest buffer.
I don't think we need to use _pad here.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 5/7] netfilter: nft_osf: refactor deprecated strncpy
2023-08-09 1:06 ` [PATCH v2 5/7] netfilter: nft_osf: " Justin Stitt
@ 2023-08-09 20:21 ` Florian Westphal
0 siblings, 0 replies; 17+ messages in thread
From: Florian Westphal @ 2023-08-09 20:21 UTC (permalink / raw)
To: Justin Stitt
Cc: Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
linux-hardening, Kees Cook, netfilter-devel, coreteam, netdev,
linux-kernel
Justin Stitt <justinstitt@google.com> wrote:
---
> Note: Should this usage @8556bceb9c409 also be changed to `strscpy_pad`
> or is zero-padding not required here?
That commit converted strlcpy which doesn't do any zero-padding,
I don't see anything wrong with that change.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 1/7] netfilter: ipset: refactor deprecated strncpy
2023-08-09 20:19 ` Florian Westphal
@ 2023-08-09 21:40 ` Justin Stitt
2023-08-09 21:54 ` Jan Engelhardt
2023-08-09 21:58 ` Florian Westphal
0 siblings, 2 replies; 17+ messages in thread
From: Justin Stitt @ 2023-08-09 21:40 UTC (permalink / raw)
To: Florian Westphal
Cc: Pablo Neira Ayuso, Jozsef Kadlecsik, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, linux-hardening,
Kees Cook, netfilter-devel, coreteam, netdev, linux-kernel
On Wed, Aug 9, 2023 at 1:19 PM Florian Westphal <fw@strlen.de> wrote:
>
> Justin Stitt <justinstitt@google.com> wrote:
> > Use `strscpy_pad` instead of `strncpy`.
>
> I don't think that any of these need zero-padding.
It's a more consistent change with the rest of the series and I don't
believe it has much different behavior to `strncpy` (other than
NUL-termination) as that will continue to pad to `n` as well.
Do you think the `_pad` for 1/7, 6/7 and 7/7 should be changed back to
`strscpy` in a v3? I really am shooting in the dark as it is quite
hard to tell whether or not a buffer is expected to be NUL-padded or
not.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 1/7] netfilter: ipset: refactor deprecated strncpy
2023-08-09 21:40 ` Justin Stitt
@ 2023-08-09 21:54 ` Jan Engelhardt
2023-08-10 19:07 ` Kees Cook
2023-08-09 21:58 ` Florian Westphal
1 sibling, 1 reply; 17+ messages in thread
From: Jan Engelhardt @ 2023-08-09 21:54 UTC (permalink / raw)
To: Justin Stitt
Cc: Florian Westphal, Pablo Neira Ayuso, Jozsef Kadlecsik,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
linux-hardening, Kees Cook, netfilter-devel, coreteam, netdev,
linux-kernel
On Wednesday 2023-08-09 23:40, Justin Stitt wrote:
>On Wed, Aug 9, 2023 at 1:19 PM Florian Westphal <fw@strlen.de> wrote:
>>
>> Justin Stitt <justinstitt@google.com> wrote:
>> > Use `strscpy_pad` instead of `strncpy`.
>>
>> I don't think that any of these need zero-padding.
>It's a more consistent change with the rest of the series and I don't
>believe it has much different behavior to `strncpy` (other than
>NUL-termination) as that will continue to pad to `n` as well.
>
>Do you think the `_pad` for 1/7, 6/7 and 7/7 should be changed back to
>`strscpy` in a v3? I really am shooting in the dark as it is quite
>hard to tell whether or not a buffer is expected to be NUL-padded or
>not.
I don't recall either NF userspace or kernelspace code doing memcmp
with name-like fields, so padding should not be strictly needed.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 1/7] netfilter: ipset: refactor deprecated strncpy
2023-08-09 21:40 ` Justin Stitt
2023-08-09 21:54 ` Jan Engelhardt
@ 2023-08-09 21:58 ` Florian Westphal
2023-08-09 22:47 ` Justin Stitt
1 sibling, 1 reply; 17+ messages in thread
From: Florian Westphal @ 2023-08-09 21:58 UTC (permalink / raw)
To: Justin Stitt
Cc: Florian Westphal, Pablo Neira Ayuso, Jozsef Kadlecsik,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
linux-hardening, Kees Cook, netfilter-devel, coreteam, netdev,
linux-kernel
Justin Stitt <justinstitt@google.com> wrote:
> On Wed, Aug 9, 2023 at 1:19 PM Florian Westphal <fw@strlen.de> wrote:
> >
> > Justin Stitt <justinstitt@google.com> wrote:
> > > Use `strscpy_pad` instead of `strncpy`.
> >
> > I don't think that any of these need zero-padding.
> It's a more consistent change with the rest of the series and I don't
> believe it has much different behavior to `strncpy` (other than
> NUL-termination) as that will continue to pad to `n` as well.
>
> Do you think the `_pad` for 1/7, 6/7 and 7/7 should be changed back to
> `strscpy` in a v3? I really am shooting in the dark as it is quite
> hard to tell whether or not a buffer is expected to be NUL-padded or
> not.
No, you can keep it as-is. Which tree are you targetting with this?
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 1/7] netfilter: ipset: refactor deprecated strncpy
2023-08-09 21:58 ` Florian Westphal
@ 2023-08-09 22:47 ` Justin Stitt
0 siblings, 0 replies; 17+ messages in thread
From: Justin Stitt @ 2023-08-09 22:47 UTC (permalink / raw)
To: Florian Westphal
Cc: Pablo Neira Ayuso, Jozsef Kadlecsik, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, linux-hardening,
Kees Cook, netfilter-devel, coreteam, netdev, linux-kernel
On Wed, Aug 9, 2023 at 2:58 PM Florian Westphal <fw@strlen.de> wrote:
>
> Justin Stitt <justinstitt@google.com> wrote:
> > On Wed, Aug 9, 2023 at 1:19 PM Florian Westphal <fw@strlen.de> wrote:
> > >
> > > Justin Stitt <justinstitt@google.com> wrote:
> > > > Use `strscpy_pad` instead of `strncpy`.
> > >
> > > I don't think that any of these need zero-padding.
> > It's a more consistent change with the rest of the series and I don't
> > believe it has much different behavior to `strncpy` (other than
> > NUL-termination) as that will continue to pad to `n` as well.
> >
> > Do you think the `_pad` for 1/7, 6/7 and 7/7 should be changed back to
> > `strscpy` in a v3? I really am shooting in the dark as it is quite
> > hard to tell whether or not a buffer is expected to be NUL-padded or
> > not.
>
> No, you can keep it as-is. Which tree are you targetting with this?
Not sure, I let ./getmaintainer auto-add the mailing lists. Perhaps
netdev or netfilter next trees?
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 1/7] netfilter: ipset: refactor deprecated strncpy
2023-08-09 21:54 ` Jan Engelhardt
@ 2023-08-10 19:07 ` Kees Cook
0 siblings, 0 replies; 17+ messages in thread
From: Kees Cook @ 2023-08-10 19:07 UTC (permalink / raw)
To: Jan Engelhardt
Cc: Justin Stitt, Florian Westphal, Pablo Neira Ayuso,
Jozsef Kadlecsik, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, linux-hardening, netfilter-devel, coreteam, netdev,
linux-kernel
On Wed, Aug 09, 2023 at 11:54:48PM +0200, Jan Engelhardt wrote:
>
> On Wednesday 2023-08-09 23:40, Justin Stitt wrote:
> >On Wed, Aug 9, 2023 at 1:19 PM Florian Westphal <fw@strlen.de> wrote:
> >>
> >> Justin Stitt <justinstitt@google.com> wrote:
> >> > Use `strscpy_pad` instead of `strncpy`.
> >>
> >> I don't think that any of these need zero-padding.
> >It's a more consistent change with the rest of the series and I don't
> >believe it has much different behavior to `strncpy` (other than
> >NUL-termination) as that will continue to pad to `n` as well.
> >
> >Do you think the `_pad` for 1/7, 6/7 and 7/7 should be changed back to
> >`strscpy` in a v3? I really am shooting in the dark as it is quite
> >hard to tell whether or not a buffer is expected to be NUL-padded or
> >not.
>
> I don't recall either NF userspace or kernelspace code doing memcmp
> with name-like fields, so padding should not be strictly needed.
My only concern with padding is just to make sure any buffers copied to
userspace have been zeroed. I would need to take a close look at how
buffers are passed around here to know for sure...
--
Kees Cook
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2023-08-10 19:07 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-09 1:06 [PATCH v2 0/7] netfilter: refactor deprecated strncpy Justin Stitt
2023-08-09 1:06 ` [PATCH v2 1/7] netfilter: ipset: " Justin Stitt
2023-08-09 20:19 ` Florian Westphal
2023-08-09 21:40 ` Justin Stitt
2023-08-09 21:54 ` Jan Engelhardt
2023-08-10 19:07 ` Kees Cook
2023-08-09 21:58 ` Florian Westphal
2023-08-09 22:47 ` Justin Stitt
2023-08-09 1:06 ` [PATCH v2 2/7] netfilter: nf_tables: " Justin Stitt
2023-08-09 1:06 ` [PATCH v2 3/7] " Justin Stitt
2023-08-09 1:06 ` [PATCH v2 4/7] netfilter: nft_meta: " Justin Stitt
2023-08-09 1:06 ` [PATCH v2 5/7] netfilter: nft_osf: " Justin Stitt
2023-08-09 20:21 ` Florian Westphal
2023-08-09 1:06 ` [PATCH v2 6/7] netfilter: x_tables: " Justin Stitt
2023-08-09 20:20 ` Florian Westphal
2023-08-09 1:06 ` [PATCH v2 7/7] netfilter: xtables: " Justin Stitt
2023-08-09 20:20 ` Florian Westphal
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).