* [PATCH nf-next 0/6] netfilter: remove support for variably-sized extensions
@ 2017-04-15 23:29 Florian Westphal
2017-04-15 23:29 ` [PATCH nf-next 1/6] netfilter: conntrack: move helper struct to nf_conntrack_helper.h Florian Westphal
` (6 more replies)
0 siblings, 7 replies; 8+ messages in thread
From: Florian Westphal @ 2017-04-15 23:29 UTC (permalink / raw)
To: netfilter-devel
3 years ago we had to bump the offsets to the extensions
(223b02d923ecd7c84cf9780bb3686f455d279279,
"netfilter: nf_conntrack: reserve two bytes for nf_ct_ext->len")
because total size of all extensions had increased to a point where u8
did overflow.
We already dieted the extensions back to more reasonable sizes, however,
I never wanted to switch back because overflow produces hard to diagnose
crash bugs, and we could not add compile-time assert because extensions
can be dynamically sized.
This series makes the last veriable-sized extension (helper)
fixed in size by adding a 32byte scratch area for helpers to use
and then adds the compile-time asserts to catch overflow during build
time.
include/net/netfilter/nf_conntrack.h | 19 ----------------
include/net/netfilter/nf_conntrack_extend.h | 12 ++--------
include/net/netfilter/nf_conntrack_helper.h | 31 ++++++++++++++++++++++----
net/netfilter/nf_conntrack_amanda.c | 2 +
net/netfilter/nf_conntrack_core.c | 33 ++++++++++++++++++++++++++++
net/netfilter/nf_conntrack_extend.c | 16 +++++--------
net/netfilter/nf_conntrack_ftp.c | 8 +++---
net/netfilter/nf_conntrack_h323_main.c | 6 +----
net/netfilter/nf_conntrack_helper.c | 6 +----
net/netfilter/nf_conntrack_irc.c | 2 -
net/netfilter/nf_conntrack_netbios_ns.c | 2 +
net/netfilter/nf_conntrack_pptp.c | 3 +-
net/netfilter/nf_conntrack_sane.c | 8 +++---
net/netfilter/nf_conntrack_sip.c | 14 +++++------
net/netfilter/nf_conntrack_tftp.c | 6 +++--
net/netfilter/nfnetlink_cthelper.c | 10 ++++++--
16 files changed, 106 insertions(+), 72 deletions(-)
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH nf-next 1/6] netfilter: conntrack: move helper struct to nf_conntrack_helper.h 2017-04-15 23:29 [PATCH nf-next 0/6] netfilter: remove support for variably-sized extensions Florian Westphal @ 2017-04-15 23:29 ` Florian Westphal 2017-04-15 23:29 ` [PATCH nf-next 2/6] netfilter: helper: add build-time asserts for helper data size Florian Westphal ` (5 subsequent siblings) 6 siblings, 0 replies; 8+ messages in thread From: Florian Westphal @ 2017-04-15 23:29 UTC (permalink / raw) To: netfilter-devel; +Cc: Florian Westphal its definition is not needed in nf_conntrack.h. Signed-off-by: Florian Westphal <fw@strlen.de> --- include/net/netfilter/nf_conntrack.h | 19 ------------------- include/net/netfilter/nf_conntrack_helper.h | 17 +++++++++++++++++ 2 files changed, 17 insertions(+), 19 deletions(-) diff --git a/include/net/netfilter/nf_conntrack.h b/include/net/netfilter/nf_conntrack.h index 4978a82b75fa..8ece3612d0cd 100644 --- a/include/net/netfilter/nf_conntrack.h +++ b/include/net/netfilter/nf_conntrack.h @@ -50,25 +50,6 @@ union nf_conntrack_expect_proto { #define NF_CT_ASSERT(x) #endif -struct nf_conntrack_helper; - -/* Must be kept in sync with the classes defined by helpers */ -#define NF_CT_MAX_EXPECT_CLASSES 4 - -/* nf_conn feature for connections that have a helper */ -struct nf_conn_help { - /* Helper. if any */ - struct nf_conntrack_helper __rcu *helper; - - struct hlist_head expectations; - - /* Current number of expected connections */ - u8 expecting[NF_CT_MAX_EXPECT_CLASSES]; - - /* private helper information. */ - char data[]; -}; - #include <net/netfilter/ipv4/nf_conntrack_ipv4.h> #include <net/netfilter/ipv6/nf_conntrack_ipv6.h> diff --git a/include/net/netfilter/nf_conntrack_helper.h b/include/net/netfilter/nf_conntrack_helper.h index 1eaac1f4cd6a..15d746558665 100644 --- a/include/net/netfilter/nf_conntrack_helper.h +++ b/include/net/netfilter/nf_conntrack_helper.h @@ -52,6 +52,23 @@ struct nf_conntrack_helper { unsigned int queue_num; /* For user-space helpers. */ }; +/* Must be kept in sync with the classes defined by helpers */ +#define NF_CT_MAX_EXPECT_CLASSES 4 + +/* nf_conn feature for connections that have a helper */ +struct nf_conn_help { + /* Helper. if any */ + struct nf_conntrack_helper __rcu *helper; + + struct hlist_head expectations; + + /* Current number of expected connections */ + u8 expecting[NF_CT_MAX_EXPECT_CLASSES]; + + /* private helper information. */ + char data[]; +}; + struct nf_conntrack_helper *__nf_conntrack_helper_find(const char *name, u16 l3num, u8 protonum); -- 2.10.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH nf-next 2/6] netfilter: helper: add build-time asserts for helper data size 2017-04-15 23:29 [PATCH nf-next 0/6] netfilter: remove support for variably-sized extensions Florian Westphal 2017-04-15 23:29 ` [PATCH nf-next 1/6] netfilter: conntrack: move helper struct to nf_conntrack_helper.h Florian Westphal @ 2017-04-15 23:29 ` Florian Westphal 2017-04-15 23:29 ` [PATCH nf-next 3/6] netfilter: nfnetlink_cthelper: reject too large userspace allocation requests Florian Westphal ` (4 subsequent siblings) 6 siblings, 0 replies; 8+ messages in thread From: Florian Westphal @ 2017-04-15 23:29 UTC (permalink / raw) To: netfilter-devel; +Cc: Florian Westphal add a 32 byte scratch area in the helper struct instead of relying on variable sized helpers plus compile-time asserts to let us know if 32 bytes aren't enough anymore. Not having variable sized helpers will later allow to add BUILD_BUG_ON for the total size of conntrack extensions -- the helper extension is the only one that doesn't have a fixed size. The (useless!) NF_CT_HELPER_BUILD_BUG_ON(0); are added so that in case someone adds a new helper and copy-pastes from one that doesn't store private data at least some indication that this macro should be used somehow is there... Signed-off-by: Florian Westphal <fw@strlen.de> --- include/net/netfilter/nf_conntrack_helper.h | 5 ++++- net/netfilter/nf_conntrack_amanda.c | 2 ++ net/netfilter/nf_conntrack_ftp.c | 2 ++ net/netfilter/nf_conntrack_h323_main.c | 2 ++ net/netfilter/nf_conntrack_netbios_ns.c | 2 ++ net/netfilter/nf_conntrack_pptp.c | 2 ++ net/netfilter/nf_conntrack_sane.c | 2 ++ net/netfilter/nf_conntrack_sip.c | 2 ++ net/netfilter/nf_conntrack_tftp.c | 2 ++ 9 files changed, 20 insertions(+), 1 deletion(-) diff --git a/include/net/netfilter/nf_conntrack_helper.h b/include/net/netfilter/nf_conntrack_helper.h index 15d746558665..29539ed1008f 100644 --- a/include/net/netfilter/nf_conntrack_helper.h +++ b/include/net/netfilter/nf_conntrack_helper.h @@ -66,9 +66,12 @@ struct nf_conn_help { u8 expecting[NF_CT_MAX_EXPECT_CLASSES]; /* private helper information. */ - char data[]; + char data[32] __aligned(8); }; +#define NF_CT_HELPER_BUILD_BUG_ON(structsize) \ + BUILD_BUG_ON((structsize) > FIELD_SIZEOF(struct nf_conn_help, data)) + struct nf_conntrack_helper *__nf_conntrack_helper_find(const char *name, u16 l3num, u8 protonum); diff --git a/net/netfilter/nf_conntrack_amanda.c b/net/netfilter/nf_conntrack_amanda.c index 57a26cc90c9f..03d2ccffa9fa 100644 --- a/net/netfilter/nf_conntrack_amanda.c +++ b/net/netfilter/nf_conntrack_amanda.c @@ -207,6 +207,8 @@ static int __init nf_conntrack_amanda_init(void) { int ret, i; + NF_CT_HELPER_BUILD_BUG_ON(0); + for (i = 0; i < ARRAY_SIZE(search); i++) { search[i].ts = textsearch_prepare(ts_algo, search[i].string, search[i].len, diff --git a/net/netfilter/nf_conntrack_ftp.c b/net/netfilter/nf_conntrack_ftp.c index 4aecef4a89fb..58e1256cd05d 100644 --- a/net/netfilter/nf_conntrack_ftp.c +++ b/net/netfilter/nf_conntrack_ftp.c @@ -577,6 +577,8 @@ static int __init nf_conntrack_ftp_init(void) { int i, ret = 0; + NF_CT_HELPER_BUILD_BUG_ON(sizeof(struct nf_ct_ftp_master)); + ftp_buffer = kmalloc(65536, GFP_KERNEL); if (!ftp_buffer) return -ENOMEM; diff --git a/net/netfilter/nf_conntrack_h323_main.c b/net/netfilter/nf_conntrack_h323_main.c index f65d93639d12..e98204349efe 100644 --- a/net/netfilter/nf_conntrack_h323_main.c +++ b/net/netfilter/nf_conntrack_h323_main.c @@ -1836,6 +1836,8 @@ static int __init nf_conntrack_h323_init(void) { int ret; + NF_CT_HELPER_BUILD_BUG_ON(sizeof(struct nf_ct_h323_master)); + h323_buffer = kmalloc(65536, GFP_KERNEL); if (!h323_buffer) return -ENOMEM; diff --git a/net/netfilter/nf_conntrack_netbios_ns.c b/net/netfilter/nf_conntrack_netbios_ns.c index 4c8f30a3d6d2..496ce173f0c1 100644 --- a/net/netfilter/nf_conntrack_netbios_ns.c +++ b/net/netfilter/nf_conntrack_netbios_ns.c @@ -58,6 +58,8 @@ static struct nf_conntrack_helper helper __read_mostly = { static int __init nf_conntrack_netbios_ns_init(void) { + NF_CT_HELPER_BUILD_BUG_ON(0); + exp_policy.timeout = timeout; return nf_conntrack_helper_register(&helper); } diff --git a/net/netfilter/nf_conntrack_pptp.c b/net/netfilter/nf_conntrack_pptp.c index f60a4755d71e..34fac4c52c4c 100644 --- a/net/netfilter/nf_conntrack_pptp.c +++ b/net/netfilter/nf_conntrack_pptp.c @@ -607,6 +607,8 @@ static struct nf_conntrack_helper pptp __read_mostly = { static int __init nf_conntrack_pptp_init(void) { + NF_CT_HELPER_BUILD_BUG_ON(sizeof(struct nf_ct_pptp_master)); + return nf_conntrack_helper_register(&pptp); } diff --git a/net/netfilter/nf_conntrack_sane.c b/net/netfilter/nf_conntrack_sane.c index 9dcb9ee9b97d..1121db08d048 100644 --- a/net/netfilter/nf_conntrack_sane.c +++ b/net/netfilter/nf_conntrack_sane.c @@ -184,6 +184,8 @@ static int __init nf_conntrack_sane_init(void) { int i, ret = 0; + NF_CT_HELPER_BUILD_BUG_ON(sizeof(struct nf_ct_sane_master)); + sane_buffer = kmalloc(65536, GFP_KERNEL); if (!sane_buffer) return -ENOMEM; diff --git a/net/netfilter/nf_conntrack_sip.c b/net/netfilter/nf_conntrack_sip.c index 91a9c97b7e9a..79bbcc4d52ee 100644 --- a/net/netfilter/nf_conntrack_sip.c +++ b/net/netfilter/nf_conntrack_sip.c @@ -1622,6 +1622,8 @@ static int __init nf_conntrack_sip_init(void) { int i, ret; + NF_CT_HELPER_BUILD_BUG_ON(sizeof(struct nf_ct_sip_master)); + if (ports_c == 0) ports[ports_c++] = SIP_PORT; diff --git a/net/netfilter/nf_conntrack_tftp.c b/net/netfilter/nf_conntrack_tftp.c index b1227dc6f75e..27e2f6f904dc 100644 --- a/net/netfilter/nf_conntrack_tftp.c +++ b/net/netfilter/nf_conntrack_tftp.c @@ -113,6 +113,8 @@ static int __init nf_conntrack_tftp_init(void) { int i, ret; + NF_CT_HELPER_BUILD_BUG_ON(0); + if (ports_c == 0) ports[ports_c++] = TFTP_PORT; -- 2.10.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH nf-next 3/6] netfilter: nfnetlink_cthelper: reject too large userspace allocation requests 2017-04-15 23:29 [PATCH nf-next 0/6] netfilter: remove support for variably-sized extensions Florian Westphal 2017-04-15 23:29 ` [PATCH nf-next 1/6] netfilter: conntrack: move helper struct to nf_conntrack_helper.h Florian Westphal 2017-04-15 23:29 ` [PATCH nf-next 2/6] netfilter: helper: add build-time asserts for helper data size Florian Westphal @ 2017-04-15 23:29 ` Florian Westphal 2017-04-15 23:29 ` [PATCH nf-next 4/6] netfilter: helpers: remove data_len usage for inkernel helpers Florian Westphal ` (3 subsequent siblings) 6 siblings, 0 replies; 8+ messages in thread From: Florian Westphal @ 2017-04-15 23:29 UTC (permalink / raw) To: netfilter-devel; +Cc: Florian Westphal Userspace should not abuse the kernel to store large amounts of data, reject requests larger than the private area can accommodate. Signed-off-by: Florian Westphal <fw@strlen.de> --- net/netfilter/nfnetlink_cthelper.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/net/netfilter/nfnetlink_cthelper.c b/net/netfilter/nfnetlink_cthelper.c index 9a50bf93dd16..eef7120e1f74 100644 --- a/net/netfilter/nfnetlink_cthelper.c +++ b/net/netfilter/nfnetlink_cthelper.c @@ -104,7 +104,7 @@ nfnl_cthelper_from_nlattr(struct nlattr *attr, struct nf_conn *ct) if (help->helper->data_len == 0) return -EINVAL; - memcpy(help->data, nla_data(attr), help->helper->data_len); + nla_memcpy(help->data, nla_data(attr), sizeof(help->data)); return 0; } @@ -216,6 +216,7 @@ nfnl_cthelper_create(const struct nlattr * const tb[], { struct nf_conntrack_helper *helper; struct nfnl_cthelper *nfcth; + unsigned int size; int ret; if (!tb[NFCTH_TUPLE] || !tb[NFCTH_POLICY] || !tb[NFCTH_PRIV_DATA_LEN]) @@ -231,7 +232,12 @@ nfnl_cthelper_create(const struct nlattr * const tb[], goto err1; strncpy(helper->name, nla_data(tb[NFCTH_NAME]), NF_CT_HELPER_NAME_LEN); - helper->data_len = ntohl(nla_get_be32(tb[NFCTH_PRIV_DATA_LEN])); + size = ntohl(nla_get_be32(tb[NFCTH_PRIV_DATA_LEN])); + if (size > FIELD_SIZEOF(struct nf_conn_help, data)) { + ret = -ENOMEM; + goto err2; + } + helper->flags |= NF_CT_HELPER_F_USERSPACE; memcpy(&helper->tuple, tuple, sizeof(struct nf_conntrack_tuple)); -- 2.10.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH nf-next 4/6] netfilter: helpers: remove data_len usage for inkernel helpers 2017-04-15 23:29 [PATCH nf-next 0/6] netfilter: remove support for variably-sized extensions Florian Westphal ` (2 preceding siblings ...) 2017-04-15 23:29 ` [PATCH nf-next 3/6] netfilter: nfnetlink_cthelper: reject too large userspace allocation requests Florian Westphal @ 2017-04-15 23:29 ` Florian Westphal 2017-04-15 23:29 ` [PATCH nf-next 5/6] netfilter: remove last traces of variable-sized extensions Florian Westphal ` (2 subsequent siblings) 6 siblings, 0 replies; 8+ messages in thread From: Florian Westphal @ 2017-04-15 23:29 UTC (permalink / raw) To: netfilter-devel; +Cc: Florian Westphal No need to track this for inkernel helpers anymore as NF_CT_HELPER_BUILD_BUG_ON checks do this now. All inkernel helpers know what kind of structure they stored in helper->data. Signed-off-by: Florian Westphal <fw@strlen.de> --- include/net/netfilter/nf_conntrack_helper.h | 11 ++++++----- net/netfilter/nf_conntrack_ftp.c | 6 ++---- net/netfilter/nf_conntrack_h323_main.c | 4 ---- net/netfilter/nf_conntrack_helper.c | 6 ++---- net/netfilter/nf_conntrack_irc.c | 2 +- net/netfilter/nf_conntrack_pptp.c | 1 - net/netfilter/nf_conntrack_sane.c | 6 ++---- net/netfilter/nf_conntrack_sip.c | 12 ++++-------- net/netfilter/nf_conntrack_tftp.c | 4 ++-- 9 files changed, 19 insertions(+), 33 deletions(-) diff --git a/include/net/netfilter/nf_conntrack_helper.h b/include/net/netfilter/nf_conntrack_helper.h index 29539ed1008f..e04fa7691e5d 100644 --- a/include/net/netfilter/nf_conntrack_helper.h +++ b/include/net/netfilter/nf_conntrack_helper.h @@ -29,9 +29,6 @@ struct nf_conntrack_helper { struct module *me; /* pointer to self */ const struct nf_conntrack_expect_policy *expect_policy; - /* length of internal data, ie. sizeof(struct nf_ct_*_master) */ - size_t data_len; - /* Tuple of things we will help (compared against server response) */ struct nf_conntrack_tuple tuple; @@ -49,7 +46,11 @@ struct nf_conntrack_helper { unsigned int expect_class_max; unsigned int flags; - unsigned int queue_num; /* For user-space helpers. */ + + /* For user-space helpers: */ + unsigned int queue_num; + /* length of userspace private data stored in nf_conn_help->data */ + u16 data_len; }; /* Must be kept in sync with the classes defined by helpers */ @@ -82,7 +83,7 @@ void nf_ct_helper_init(struct nf_conntrack_helper *helper, u16 l3num, u16 protonum, const char *name, u16 default_port, u16 spec_port, u32 id, const struct nf_conntrack_expect_policy *exp_pol, - u32 expect_class_max, u32 data_len, + u32 expect_class_max, int (*help)(struct sk_buff *skb, unsigned int protoff, struct nf_conn *ct, enum ip_conntrack_info ctinfo), diff --git a/net/netfilter/nf_conntrack_ftp.c b/net/netfilter/nf_conntrack_ftp.c index 58e1256cd05d..f0e9a7511e1a 100644 --- a/net/netfilter/nf_conntrack_ftp.c +++ b/net/netfilter/nf_conntrack_ftp.c @@ -591,12 +591,10 @@ static int __init nf_conntrack_ftp_init(void) for (i = 0; i < ports_c; i++) { nf_ct_helper_init(&ftp[2 * i], AF_INET, IPPROTO_TCP, "ftp", FTP_PORT, ports[i], ports[i], &ftp_exp_policy, - 0, sizeof(struct nf_ct_ftp_master), help, - nf_ct_ftp_from_nlattr, THIS_MODULE); + 0, help, nf_ct_ftp_from_nlattr, THIS_MODULE); nf_ct_helper_init(&ftp[2 * i + 1], AF_INET6, IPPROTO_TCP, "ftp", FTP_PORT, ports[i], ports[i], &ftp_exp_policy, - 0, sizeof(struct nf_ct_ftp_master), help, - nf_ct_ftp_from_nlattr, THIS_MODULE); + 0, help, nf_ct_ftp_from_nlattr, THIS_MODULE); } ret = nf_conntrack_helpers_register(ftp, ports_c * 2); diff --git a/net/netfilter/nf_conntrack_h323_main.c b/net/netfilter/nf_conntrack_h323_main.c index e98204349efe..3bcdc718484e 100644 --- a/net/netfilter/nf_conntrack_h323_main.c +++ b/net/netfilter/nf_conntrack_h323_main.c @@ -637,7 +637,6 @@ static const struct nf_conntrack_expect_policy h245_exp_policy = { static struct nf_conntrack_helper nf_conntrack_helper_h245 __read_mostly = { .name = "H.245", .me = THIS_MODULE, - .data_len = sizeof(struct nf_ct_h323_master), .tuple.src.l3num = AF_UNSPEC, .tuple.dst.protonum = IPPROTO_UDP, .help = h245_help, @@ -1215,7 +1214,6 @@ static struct nf_conntrack_helper nf_conntrack_helper_q931[] __read_mostly = { { .name = "Q.931", .me = THIS_MODULE, - .data_len = sizeof(struct nf_ct_h323_master), .tuple.src.l3num = AF_INET, .tuple.src.u.tcp.port = cpu_to_be16(Q931_PORT), .tuple.dst.protonum = IPPROTO_TCP, @@ -1800,7 +1798,6 @@ static struct nf_conntrack_helper nf_conntrack_helper_ras[] __read_mostly = { { .name = "RAS", .me = THIS_MODULE, - .data_len = sizeof(struct nf_ct_h323_master), .tuple.src.l3num = AF_INET, .tuple.src.u.udp.port = cpu_to_be16(RAS_PORT), .tuple.dst.protonum = IPPROTO_UDP, @@ -1810,7 +1807,6 @@ static struct nf_conntrack_helper nf_conntrack_helper_ras[] __read_mostly = { { .name = "RAS", .me = THIS_MODULE, - .data_len = sizeof(struct nf_ct_h323_master), .tuple.src.l3num = AF_INET6, .tuple.src.u.udp.port = cpu_to_be16(RAS_PORT), .tuple.dst.protonum = IPPROTO_UDP, diff --git a/net/netfilter/nf_conntrack_helper.c b/net/netfilter/nf_conntrack_helper.c index 33ebb78649f8..8239b4406f56 100644 --- a/net/netfilter/nf_conntrack_helper.c +++ b/net/netfilter/nf_conntrack_helper.c @@ -178,8 +178,7 @@ nf_ct_helper_ext_add(struct nf_conn *ct, { struct nf_conn_help *help; - help = nf_ct_ext_add_length(ct, NF_CT_EXT_HELPER, - helper->data_len, gfp); + help = nf_ct_ext_add(ct, NF_CT_EXT_HELPER, gfp); if (help) INIT_HLIST_HEAD(&help->expectations); else @@ -484,7 +483,7 @@ void nf_ct_helper_init(struct nf_conntrack_helper *helper, u16 l3num, u16 protonum, const char *name, u16 default_port, u16 spec_port, u32 id, const struct nf_conntrack_expect_policy *exp_pol, - u32 expect_class_max, u32 data_len, + u32 expect_class_max, int (*help)(struct sk_buff *skb, unsigned int protoff, struct nf_conn *ct, enum ip_conntrack_info ctinfo), @@ -497,7 +496,6 @@ void nf_ct_helper_init(struct nf_conntrack_helper *helper, helper->tuple.src.u.all = htons(spec_port); helper->expect_policy = exp_pol; helper->expect_class_max = expect_class_max; - helper->data_len = data_len; helper->help = help; helper->from_nlattr = from_nlattr; helper->me = module; diff --git a/net/netfilter/nf_conntrack_irc.c b/net/netfilter/nf_conntrack_irc.c index 1a5af4d4af2d..5523acce9d69 100644 --- a/net/netfilter/nf_conntrack_irc.c +++ b/net/netfilter/nf_conntrack_irc.c @@ -263,7 +263,7 @@ static int __init nf_conntrack_irc_init(void) for (i = 0; i < ports_c; i++) { nf_ct_helper_init(&irc[i], AF_INET, IPPROTO_TCP, "irc", IRC_PORT, ports[i], i, &irc_exp_policy, - 0, 0, help, NULL, THIS_MODULE); + 0, help, NULL, THIS_MODULE); } ret = nf_conntrack_helpers_register(&irc[0], ports_c); diff --git a/net/netfilter/nf_conntrack_pptp.c b/net/netfilter/nf_conntrack_pptp.c index 34fac4c52c4c..126031909fc7 100644 --- a/net/netfilter/nf_conntrack_pptp.c +++ b/net/netfilter/nf_conntrack_pptp.c @@ -596,7 +596,6 @@ static const struct nf_conntrack_expect_policy pptp_exp_policy = { static struct nf_conntrack_helper pptp __read_mostly = { .name = "pptp", .me = THIS_MODULE, - .data_len = sizeof(struct nf_ct_pptp_master), .tuple.src.l3num = AF_INET, .tuple.src.u.tcp.port = cpu_to_be16(PPTP_CONTROL_PORT), .tuple.dst.protonum = IPPROTO_TCP, diff --git a/net/netfilter/nf_conntrack_sane.c b/net/netfilter/nf_conntrack_sane.c index 1121db08d048..ae457f39d5ce 100644 --- a/net/netfilter/nf_conntrack_sane.c +++ b/net/netfilter/nf_conntrack_sane.c @@ -198,13 +198,11 @@ static int __init nf_conntrack_sane_init(void) for (i = 0; i < ports_c; i++) { nf_ct_helper_init(&sane[2 * i], AF_INET, IPPROTO_TCP, "sane", SANE_PORT, ports[i], ports[i], - &sane_exp_policy, 0, - sizeof(struct nf_ct_sane_master), help, NULL, + &sane_exp_policy, 0, help, NULL, THIS_MODULE); nf_ct_helper_init(&sane[2 * i + 1], AF_INET6, IPPROTO_TCP, "sane", SANE_PORT, ports[i], ports[i], - &sane_exp_policy, 0, - sizeof(struct nf_ct_sane_master), help, NULL, + &sane_exp_policy, 0, help, NULL, THIS_MODULE); } diff --git a/net/netfilter/nf_conntrack_sip.c b/net/netfilter/nf_conntrack_sip.c index 79bbcc4d52ee..d38af4274335 100644 --- a/net/netfilter/nf_conntrack_sip.c +++ b/net/netfilter/nf_conntrack_sip.c @@ -1630,23 +1630,19 @@ static int __init nf_conntrack_sip_init(void) for (i = 0; i < ports_c; i++) { nf_ct_helper_init(&sip[4 * i], AF_INET, IPPROTO_UDP, "sip", SIP_PORT, ports[i], i, sip_exp_policy, - SIP_EXPECT_MAX, - sizeof(struct nf_ct_sip_master), sip_help_udp, + SIP_EXPECT_MAX, sip_help_udp, NULL, THIS_MODULE); nf_ct_helper_init(&sip[4 * i + 1], AF_INET, IPPROTO_TCP, "sip", SIP_PORT, ports[i], i, sip_exp_policy, - SIP_EXPECT_MAX, - sizeof(struct nf_ct_sip_master), sip_help_tcp, + SIP_EXPECT_MAX, sip_help_tcp, NULL, THIS_MODULE); nf_ct_helper_init(&sip[4 * i + 2], AF_INET6, IPPROTO_UDP, "sip", SIP_PORT, ports[i], i, sip_exp_policy, - SIP_EXPECT_MAX, - sizeof(struct nf_ct_sip_master), sip_help_udp, + SIP_EXPECT_MAX, sip_help_udp, NULL, THIS_MODULE); nf_ct_helper_init(&sip[4 * i + 3], AF_INET6, IPPROTO_TCP, "sip", SIP_PORT, ports[i], i, sip_exp_policy, - SIP_EXPECT_MAX, - sizeof(struct nf_ct_sip_master), sip_help_tcp, + SIP_EXPECT_MAX, sip_help_tcp, NULL, THIS_MODULE); } diff --git a/net/netfilter/nf_conntrack_tftp.c b/net/netfilter/nf_conntrack_tftp.c index 27e2f6f904dc..0ec6779fd5d9 100644 --- a/net/netfilter/nf_conntrack_tftp.c +++ b/net/netfilter/nf_conntrack_tftp.c @@ -121,10 +121,10 @@ static int __init nf_conntrack_tftp_init(void) for (i = 0; i < ports_c; i++) { nf_ct_helper_init(&tftp[2 * i], AF_INET, IPPROTO_UDP, "tftp", TFTP_PORT, ports[i], i, &tftp_exp_policy, - 0, 0, tftp_help, NULL, THIS_MODULE); + 0, tftp_help, NULL, THIS_MODULE); nf_ct_helper_init(&tftp[2 * i + 1], AF_INET6, IPPROTO_UDP, "tftp", TFTP_PORT, ports[i], i, &tftp_exp_policy, - 0, 0, tftp_help, NULL, THIS_MODULE); + 0, tftp_help, NULL, THIS_MODULE); } ret = nf_conntrack_helpers_register(tftp, ports_c * 2); -- 2.10.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH nf-next 5/6] netfilter: remove last traces of variable-sized extensions 2017-04-15 23:29 [PATCH nf-next 0/6] netfilter: remove support for variably-sized extensions Florian Westphal ` (3 preceding siblings ...) 2017-04-15 23:29 ` [PATCH nf-next 4/6] netfilter: helpers: remove data_len usage for inkernel helpers Florian Westphal @ 2017-04-15 23:29 ` Florian Westphal 2017-04-15 23:29 ` [PATCH nf-next 6/6] netfilter: conntrack: use u8 for extension sizes again Florian Westphal 2017-04-19 15:56 ` [PATCH nf-next 0/6] netfilter: remove support for variably-sized extensions Pablo Neira Ayuso 6 siblings, 0 replies; 8+ messages in thread From: Florian Westphal @ 2017-04-15 23:29 UTC (permalink / raw) To: netfilter-devel; +Cc: Florian Westphal get rid of the (now unused) nf_ct_ext_add_length define and also rename the function to plain nf_ct_ext_add(). Signed-off-by: Florian Westphal <fw@strlen.de> --- include/net/netfilter/nf_conntrack_extend.h | 8 +------- net/netfilter/nf_conntrack_extend.c | 16 +++++++--------- 2 files changed, 8 insertions(+), 16 deletions(-) diff --git a/include/net/netfilter/nf_conntrack_extend.h b/include/net/netfilter/nf_conntrack_extend.h index 1c3035dda31f..4ec645c8b647 100644 --- a/include/net/netfilter/nf_conntrack_extend.h +++ b/include/net/netfilter/nf_conntrack_extend.h @@ -86,13 +86,7 @@ static inline void nf_ct_ext_free(struct nf_conn *ct) } /* Add this type, returns pointer to data or NULL. */ -void *__nf_ct_ext_add_length(struct nf_conn *ct, enum nf_ct_ext_id id, - size_t var_alloc_len, gfp_t gfp); - -#define nf_ct_ext_add(ct, id, gfp) \ - ((id##_TYPE *)__nf_ct_ext_add_length((ct), (id), 0, (gfp))) -#define nf_ct_ext_add_length(ct, id, len, gfp) \ - ((id##_TYPE *)__nf_ct_ext_add_length((ct), (id), (len), (gfp))) +void *nf_ct_ext_add(struct nf_conn *ct, enum nf_ct_ext_id id, gfp_t gfp); #define NF_CT_EXT_F_PREALLOC 0x0001 diff --git a/net/netfilter/nf_conntrack_extend.c b/net/netfilter/nf_conntrack_extend.c index 008299b7f78f..b5879a9c748d 100644 --- a/net/netfilter/nf_conntrack_extend.c +++ b/net/netfilter/nf_conntrack_extend.c @@ -44,8 +44,7 @@ void __nf_ct_ext_destroy(struct nf_conn *ct) EXPORT_SYMBOL(__nf_ct_ext_destroy); static void * -nf_ct_ext_create(struct nf_ct_ext **ext, enum nf_ct_ext_id id, - size_t var_alloc_len, gfp_t gfp) +nf_ct_ext_create(struct nf_ct_ext **ext, enum nf_ct_ext_id id, gfp_t gfp) { unsigned int off, len; struct nf_ct_ext_type *t; @@ -59,8 +58,8 @@ nf_ct_ext_create(struct nf_ct_ext **ext, enum nf_ct_ext_id id, } off = ALIGN(sizeof(struct nf_ct_ext), t->align); - len = off + t->len + var_alloc_len; - alloc_size = t->alloc_size + var_alloc_len; + len = off + t->len; + alloc_size = t->alloc_size; rcu_read_unlock(); *ext = kzalloc(alloc_size, gfp); @@ -73,8 +72,7 @@ nf_ct_ext_create(struct nf_ct_ext **ext, enum nf_ct_ext_id id, return (void *)(*ext) + off; } -void *__nf_ct_ext_add_length(struct nf_conn *ct, enum nf_ct_ext_id id, - size_t var_alloc_len, gfp_t gfp) +void *nf_ct_ext_add(struct nf_conn *ct, enum nf_ct_ext_id id, gfp_t gfp) { struct nf_ct_ext *old, *new; int newlen, newoff; @@ -85,7 +83,7 @@ void *__nf_ct_ext_add_length(struct nf_conn *ct, enum nf_ct_ext_id id, old = ct->ext; if (!old) - return nf_ct_ext_create(&ct->ext, id, var_alloc_len, gfp); + return nf_ct_ext_create(&ct->ext, id, gfp); if (__nf_ct_ext_exist(old, id)) return NULL; @@ -98,7 +96,7 @@ void *__nf_ct_ext_add_length(struct nf_conn *ct, enum nf_ct_ext_id id, } newoff = ALIGN(old->len, t->align); - newlen = newoff + t->len + var_alloc_len; + newlen = newoff + t->len; rcu_read_unlock(); new = __krealloc(old, newlen, gfp); @@ -115,7 +113,7 @@ void *__nf_ct_ext_add_length(struct nf_conn *ct, enum nf_ct_ext_id id, memset((void *)new + newoff, 0, newlen - newoff); return (void *)new + newoff; } -EXPORT_SYMBOL(__nf_ct_ext_add_length); +EXPORT_SYMBOL(nf_ct_ext_add); static void update_alloc_size(struct nf_ct_ext_type *type) { -- 2.10.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH nf-next 6/6] netfilter: conntrack: use u8 for extension sizes again 2017-04-15 23:29 [PATCH nf-next 0/6] netfilter: remove support for variably-sized extensions Florian Westphal ` (4 preceding siblings ...) 2017-04-15 23:29 ` [PATCH nf-next 5/6] netfilter: remove last traces of variable-sized extensions Florian Westphal @ 2017-04-15 23:29 ` Florian Westphal 2017-04-19 15:56 ` [PATCH nf-next 0/6] netfilter: remove support for variably-sized extensions Pablo Neira Ayuso 6 siblings, 0 replies; 8+ messages in thread From: Florian Westphal @ 2017-04-15 23:29 UTC (permalink / raw) To: netfilter-devel; +Cc: Florian Westphal commit 223b02d923ecd7c84cf9780bb3686f455d279279 ("netfilter: nf_conntrack: reserve two bytes for nf_ct_ext->len") had to increase size of the extension offsets because total size of the extensions had increased to a point where u8 did overflow. 3 years later we've managed to diet extensions a bit and we no longer need u16. Furthermore we can now add a compile-time assertion for this problem. Signed-off-by: Florian Westphal <fw@strlen.de> --- include/net/netfilter/nf_conntrack_extend.h | 4 ++-- net/netfilter/nf_conntrack_core.c | 33 +++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/include/net/netfilter/nf_conntrack_extend.h b/include/net/netfilter/nf_conntrack_extend.h index 4ec645c8b647..5fc908dc9f32 100644 --- a/include/net/netfilter/nf_conntrack_extend.h +++ b/include/net/netfilter/nf_conntrack_extend.h @@ -43,8 +43,8 @@ enum nf_ct_ext_id { /* Extensions: optional stuff which isn't permanently in struct. */ struct nf_ct_ext { struct rcu_head rcu; - u16 offset[NF_CT_EXT_NUM]; - u16 len; + u8 offset[NF_CT_EXT_NUM]; + u8 len; char data[0]; }; diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c index 03150f60714d..62368b05cef5 100644 --- a/net/netfilter/nf_conntrack_core.c +++ b/net/netfilter/nf_conntrack_core.c @@ -1804,12 +1804,45 @@ EXPORT_SYMBOL_GPL(nf_conntrack_set_hashsize); module_param_call(hashsize, nf_conntrack_set_hashsize, param_get_uint, &nf_conntrack_htable_size, 0600); +static unsigned int total_extension_size(void) +{ + /* remember to add new extensions below */ + BUILD_BUG_ON(NF_CT_EXT_NUM > 9); + + return sizeof(struct nf_ct_ext) + + sizeof(struct nf_conn_help) +#if IS_ENABLED(CONFIG_NF_NAT) + + sizeof(struct nf_conn_nat) +#endif + + sizeof(struct nf_conn_seqadj) + + sizeof(struct nf_conn_acct) +#ifdef CONFIG_NF_CONNTRACK_EVENTS + + sizeof(struct nf_conntrack_ecache) +#endif +#ifdef CONFIG_NF_CONNTRACK_TIMESTAMP + + sizeof(struct nf_conn_tstamp) +#endif +#ifdef CONFIG_NF_CONNTRACK_TIMEOUT + + sizeof(struct nf_conn_timeout) +#endif +#ifdef CONFIG_NF_CONNTRACK_LABELS + + sizeof(struct nf_conn_labels) +#endif +#if IS_ENABLED(CONFIG_NETFILTER_SYNPROXY) + + sizeof(struct nf_conn_synproxy) +#endif + ; +}; + int nf_conntrack_init_start(void) { int max_factor = 8; int ret = -ENOMEM; int i; + /* struct nf_ct_ext uses u8 to store offsets/size */ + BUILD_BUG_ON(total_extension_size() > 255u); + seqcount_init(&nf_conntrack_generation); for (i = 0; i < CONNTRACK_LOCKS; i++) -- 2.10.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH nf-next 0/6] netfilter: remove support for variably-sized extensions 2017-04-15 23:29 [PATCH nf-next 0/6] netfilter: remove support for variably-sized extensions Florian Westphal ` (5 preceding siblings ...) 2017-04-15 23:29 ` [PATCH nf-next 6/6] netfilter: conntrack: use u8 for extension sizes again Florian Westphal @ 2017-04-19 15:56 ` Pablo Neira Ayuso 6 siblings, 0 replies; 8+ messages in thread From: Pablo Neira Ayuso @ 2017-04-19 15:56 UTC (permalink / raw) To: Florian Westphal; +Cc: netfilter-devel On Sun, Apr 16, 2017 at 01:29:13AM +0200, Florian Westphal wrote: > 3 years ago we had to bump the offsets to the extensions > (223b02d923ecd7c84cf9780bb3686f455d279279, > "netfilter: nf_conntrack: reserve two bytes for nf_ct_ext->len") > because total size of all extensions had increased to a point where u8 > did overflow. > > We already dieted the extensions back to more reasonable sizes, however, > I never wanted to switch back because overflow produces hard to diagnose > crash bugs, and we could not add compile-time assert because extensions > can be dynamically sized. > > This series makes the last veriable-sized extension (helper) > fixed in size by adding a 32byte scratch area for helpers to use > and then adds the compile-time asserts to catch overflow during build > time. Series applied. ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-04-19 15:56 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-04-15 23:29 [PATCH nf-next 0/6] netfilter: remove support for variably-sized extensions Florian Westphal 2017-04-15 23:29 ` [PATCH nf-next 1/6] netfilter: conntrack: move helper struct to nf_conntrack_helper.h Florian Westphal 2017-04-15 23:29 ` [PATCH nf-next 2/6] netfilter: helper: add build-time asserts for helper data size Florian Westphal 2017-04-15 23:29 ` [PATCH nf-next 3/6] netfilter: nfnetlink_cthelper: reject too large userspace allocation requests Florian Westphal 2017-04-15 23:29 ` [PATCH nf-next 4/6] netfilter: helpers: remove data_len usage for inkernel helpers Florian Westphal 2017-04-15 23:29 ` [PATCH nf-next 5/6] netfilter: remove last traces of variable-sized extensions Florian Westphal 2017-04-15 23:29 ` [PATCH nf-next 6/6] netfilter: conntrack: use u8 for extension sizes again Florian Westphal 2017-04-19 15:56 ` [PATCH nf-next 0/6] netfilter: remove support for variably-sized extensions Pablo Neira Ayuso
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).