Netfilter fixes for net
Subsystems (4)
Pablo Neira Ayuso
maintainercover Netfilter fixes for net
Hi David,
The following patchset contains Netfilter fixes for your net tree, they are:
1) Fixed hashtable representation doesn't support timeout flag, skip it otherwise rules to add elements from the packet fail bogusly fail with EOPNOTSUPP.
2) Fix bogus error with 32-bits ebtables userspace and 64-bits kernel, patch from Florian Westphal.
3) Sanitize proc names in several x_tables extensions, also from Florian.
4) Add sanitization to ebt_among wormhash logic, from Florian.
5) Missing release of hook array in flowtable.
You can pull these changes from:
git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf.git
Thanks!
----------------------------------------------------------------
The following changes since commit ce380619fab99036f5e745c7a865b21c59f005f6:
Merge tag 'please-pull-ia64_misc' of git://git.kernel.org/pub/scm/linux/kernel/git/aegl/linux (2018-03-05 20:31:14 -0800)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf.git HEAD
for you to fetch changes up to c04a3f730021c304c7cc4bc30ee57ee70ad98d57:
netfilter: nf_tables: release flowtable hooks (2018-03-11 21:24:56 +0100)
----------------------------------------------------------------
Florian Westphal (3):
netfilter: ebtables: fix erroneous reject of last rule
netfilter: x_tables: add and use xt_check_proc_name
netfilter: bridge: ebt_among: add more missing match size checks
Pablo Neira Ayuso (2):
netfilter: nft_set_hash: skip fixed hash if timeout is specified
netfilter: nf_tables: release flowtable hooks
include/linux/netfilter/x_tables.h | 2 ++ net/bridge/netfilter/ebt_among.c | 34 ++++++++++++++++++++++++++++++++++ net/bridge/netfilter/ebtables.c | 6 +++++- net/netfilter/nf_tables_api.c | 1 + net/netfilter/nft_set_hash.c | 2 +- net/netfilter/x_tables.c | 30 ++++++++++++++++++++++++++++++ net/netfilter/xt_hashlimit.c | 16 ++++++++++------ net/netfilter/xt_recent.c | 6 +++--- 8 files changed, 86 insertions(+), 11 deletions(-)
Pablo Neira Ayuso
maintainerPATCH 1/5 netfilter: nft_set_hash: skip fixed hash if timeout is specified
Fixed hash supports to timeouts, so skip it. Otherwise, userspace hits EOPNOTSUPP.
--- net/netfilter/nft_set_hash.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
net/netfilter/nft_set_hash.c +1 -1
@@ -674,7 +674,7 @@ static const struct nft_set_ops * nft_hash_select_ops(const struct nft_ctx *ctx, const struct nft_set_desc *desc, u32 flags) { - if (desc->size) { + if (desc->size && !(flags & NFT_SET_TIMEOUT)) { switch (desc->klen) {case 4 :return &nft_hash_fast_ops;
Signature · 1 line
2.11.0
Pablo Neira Ayuso
maintainerPATCH 2/5 netfilter: ebtables: fix erroneous reject of last rule
In reply to Pablo Neira AyusoFrom: Florian Westphal <[email protected]>
The last rule in the blob has next_entry offset that is same as total size. This made "ebtables32 -A OUTPUT -d de:ad:be:ef:01:02" fail on 64 bit kernel.
--- net/bridge/netfilter/ebtables.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
net/bridge/netfilter/ebtables.c +5 -1
@@ -2119,8 +2119,12 @@ static int size_entry_mwt(struct ebt_entry *entry, const unsigned char *base,* offsets are relative to beginning of struct ebt_entry (i.e., 0). */ for (i =0 ; i <4 ; ++i) { -if (offsets[i] >= *total) +if (offsets[i] > *total)return -EINVAL ; + +if (i <3 && offsets[i] == *total) +return -EINVAL ; +if (i ==0 )continue ;if (offsets[i-1 ] > offsets[i])
Signature · 1 line
2.11.0
Pablo Neira Ayuso
maintainerPATCH 3/5 netfilter: x_tables: add and use xt_check_proc_name
In reply to Pablo Neira AyusoFrom: Florian Westphal <[email protected]>
recent and hashlimit both create /proc files, but only check that name is 0 terminated.
This can trigger WARN() from procfs when name is "" or "/". Add helper for this and then use it for both.
--- include/linux/netfilter/x_tables.h | 2 ++ net/netfilter/x_tables.c | 30 ++++++++++++++++++++++++++++++ net/netfilter/xt_hashlimit.c | 16 ++++++++++------ net/netfilter/xt_recent.c | 6 +++--- 4 files changed, 45 insertions(+), 9 deletions(-)
include/linux/netfilter/x_tables.h +2 -0
@@ -285,6 +285,8 @@ unsigned int *xt_alloc_entry_offsets(unsigned int size);bool xt_find_jump_offset (const unsigned int *offsets,unsigned int target,unsigned int size); +int xt_check_proc_name (const char *name,unsigned int size); +int xt_check_match (struct xt_mtchk_param *,unsigned int size,u_int8_t proto,bool inv_proto); int xt_check_target(struct xt_tgchk_param *, unsigned int size, u_int8_t proto,
net/netfilter/x_tables.c +30 -0
@@ -423,6 +423,36 @@ textify_hooks(char *buf, size_t size, unsigned int mask, uint8_t nfproto)return buf; } +/** +* xt_check_proc_name - check that name is suitable for /proc file creation +* +* @name: file name candidate +* @size: length of buffer +* +* some x_tables modules wish to create a file in /proc. +* This function makes sure that the name is suitable for this +* purpose, it checks that name is NUL terminated and isn't a 'special' +* name, like "..". +* +* returns negative number on error or 0 if name is useable. +*/ +int xt_check_proc_name (const char *name,unsigned int size) +{ +if (name[0 ] =='\0' ) +return -EINVAL ; + +if (strnlen (name, size) == size) +return -ENAMETOOLONG ; + +if (strcmp (name,"." ) ==0 || +strcmp (name,".." ) ==0 || +strchr (name,'/' )) +return -EINVAL ; + +return 0 ; +} +EXPORT_SYMBOL (xt_check_proc_name); +int xt_check_match (struct xt_mtchk_param *par,unsigned int size,u_int8_t proto,bool inv_proto) {
net/netfilter/xt_hashlimit.c +10 -6
@@ -917,8 +917,9 @@ static int hashlimit_mt_check_v1(const struct xt_mtchk_param *par)struct hashlimit_cfg3 cfg = {};int ret; -if (info->name[sizeof (info->name) -1 ] !='\0' ) -return -EINVAL ; + ret =xt_check_proc_name (info->name,sizeof (info->name)); +if (ret) +return ret; ret =cfg_copy (&cfg, (void *)&info->cfg,1 );
@@ -935,8 +936,9 @@ static int hashlimit_mt_check_v2(const struct xt_mtchk_param *par)struct hashlimit_cfg3 cfg = {};int ret; -if (info->name[sizeof (info->name) -1 ] !='\0' ) -return -EINVAL ; + ret =xt_check_proc_name (info->name,sizeof (info->name)); +if (ret) +return ret; ret =cfg_copy (&cfg, (void *)&info->cfg,2 );
@@ -950,9 +952,11 @@ static int hashlimit_mt_check_v2(const struct xt_mtchk_param *par)static int hashlimit_mt_check (const struct xt_mtchk_param *par) {struct xt_hashlimit_mtinfo3 *info = par->matchinfo; +int ret; -if (info->name[sizeof (info->name) -1 ] !='\0' ) -return -EINVAL ; + ret =xt_check_proc_name (info->name,sizeof (info->name)); +if (ret) +return ret;return hashlimit_mt_check_common (par, &info->hinfo, &info->cfg, info->name,3 );
net/netfilter/xt_recent.c +3 -3
@@ -361,9 +361,9 @@ static int recent_mt_check(const struct xt_mtchk_param *par, info->hit_count, XT_RECENT_MAX_NSTAMPS - 1);return -EINVAL ; } -if (info->name[0 ] =='\0' || -strnlen (info->name,XT_RECENT_NAME_LEN ) ==XT_RECENT_NAME_LEN ) -return -EINVAL ; + ret =xt_check_proc_name (info->name,sizeof (info->name)); +if (ret) +return ret;if (ip_pkt_list_tot && info->hit_count < ip_pkt_list_tot) nstamp_mask =roundup_pow_of_two (ip_pkt_list_tot) -1 ;
Signature · 1 line
2.11.0
Pablo Neira Ayuso
maintainerPATCH 4/5 netfilter: bridge: ebt_among: add more missing match size checks
In reply to Pablo Neira AyusoFrom: Florian Westphal <[email protected]>
ebt_among is special, it has a dynamic match size and is exempt from the central size checks.
commit c4585a2823edf ("bridge: ebt_among: add missing match size checks") added validation for pool size, but missed fact that the macros ebt_among_wh_src/dst can already return out-of-bound result because they do not check value of wh_src/dst_ofs (an offset) vs. the size of the match that userspace gave to us.
v2:
check that offset has correct alignment.
Paolo Abeni points out that we should also check that src/dst wormhash arrays do not overlap, and src + length lines up with start of dst (or vice versa).
v3: compact wormhash_sizes_valid() part
NB: Fixes tag is intentionally wrong, this bug exists from day one when match was added for 2.6 kernel. Tag is there so stable maintainers will notice this one too.
Tested with same rules from the earlier patch.
--- net/bridge/netfilter/ebt_among.c | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+)
net/bridge/netfilter/ebt_among.c +34 -0
@@ -177,6 +177,28 @@ static bool poolsize_invalid(const struct ebt_mac_wormhash *w)return w && w->poolsize >= (INT_MAX /sizeof (struct ebt_mac_wormhash_tuple )); } +static bool wormhash_offset_invalid (int off,unsigned int len) +{ +if (off ==0 )/* not present */ +return false; + +if (off < (int )sizeof (struct ebt_among_info ) || + off % __alignof__(struct ebt_mac_wormhash )) +return true; + + off +=sizeof (struct ebt_mac_wormhash ); + +return off > len; +} + +static bool wormhash_sizes_valid (const struct ebt_mac_wormhash *wh,int a,int b) +{ +if (a ==0 ) + a =sizeof (struct ebt_among_info ); + +return ebt_mac_wormhash_size (wh) + a == b; +} +static int ebt_among_mt_check (const struct xt_mtchk_param *par) {const struct ebt_among_info *info = par->matchinfo;
@@ -189,6 +211,10 @@ static int ebt_among_mt_check(const struct xt_mtchk_param *par)if (expected_length > em->match_size)return -EINVAL ; +if (wormhash_offset_invalid (info->wh_dst_ofs, em->match_size) || +wormhash_offset_invalid (info->wh_src_ofs, em->match_size)) +return -EINVAL ; + wh_dst =ebt_among_wh_dst (info);if (poolsize_invalid (wh_dst))return -EINVAL ;
@@ -201,6 +227,14 @@ static int ebt_among_mt_check(const struct xt_mtchk_param *par)if (poolsize_invalid (wh_src))return -EINVAL ; +if (info->wh_src_ofs < info->wh_dst_ofs) { +if (!wormhash_sizes_valid (wh_src, info->wh_src_ofs, info->wh_dst_ofs)) +return -EINVAL ; + }else { +if (!wormhash_sizes_valid (wh_dst, info->wh_dst_ofs, info->wh_src_ofs)) +return -EINVAL ; + } + expected_length +=ebt_mac_wormhash_size (wh_src);if (em->match_size !=EBT_ALIGN (expected_length)) {
Signature · 1 line
2.11.0
Pablo Neira Ayuso
maintainerPATCH 5/5 netfilter: nf_tables: release flowtable hooks
In reply to Pablo Neira AyusoOtherwise we leak this array.
--- net/netfilter/nf_tables_api.c | 1 + 1 file changed, 1 insertion(+)
net/netfilter/nf_tables_api.c +1 -0
@@ -5423,6 +5423,7 @@ static void nf_tables_flowtable_notify(struct nft_ctx *ctx,static void nf_tables_flowtable_destroy (struct nft_flowtable *flowtable) {cancel_delayed_work_sync (&flowtable->data.gc_work); +kfree (flowtable->ops);kfree (flowtable->name); flowtable->data.type->free (&flowtable->data);rhashtable_destroy (&flowtable->data.rhashtable);
Signature · 1 line
2.11.0
David Miller
maintainerFrom: Pablo Neira Ayuso <[email protected]>
Date: Mon, 12 Mar 2018 17:15:59 +0100
Quoted 6 lines
The following patchset contains Netfilter fixes for your net tree, they are:
1) Fixed hashtable representation doesn't support timeout flag, skip it otherwise rules to add elements from the packet fail bogusly fail with EOPNOTSUPP.
Quoted 8 lines
2) Fix bogus error with 32-bits ebtables userspace and 64-bits kernel, patch from Florian Westphal.
3) Sanitize proc names in several
x_tablesextensions, also from Florian.4) Add sanitization to
ebt_amongwormhash logic, from Florian.5) Missing release of hook array in flowtable.
Pulled, thank you.