[v3] netfilter: nf_tables: Ensure init attributes are within the bounds
diff mbox

Message ID 20160818114521.GA32566@sonyv
State Changes Requested
Delegated to: Pablo Neira
Headers show

Commit Message

Laura Garcia Aug. 18, 2016, 11:45 a.m. UTC
Check for overflow of u8 fields from u32 netlink attributes and maximum
values.

Refer to 4da449ae1df

Signed-off-by: Laura Garcia Liebana <nevola@gmail.com>
---
(was: netfilter: nf_tables: Check for overflow of u8 fields from u32
netlink attributes)

Changes in V3:
	- Use ERANGE instead of EINVAL when validating the value is
within the bounds.
	- Add op verification in nft_cmp_init().
	- Remove additional bounds verification for family attribute in
nft_nat_init().

 net/netfilter/nft_bitwise.c   |  7 ++++++-
 net/netfilter/nft_byteorder.c | 13 +++++++++++--
 net/netfilter/nft_cmp.c       |  7 ++++++-
 net/netfilter/nft_immediate.c |  3 +++
 4 files changed, 26 insertions(+), 4 deletions(-)

Comments

kernel test robot Aug. 22, 2016, 9:10 a.m. UTC | #1
Hi Laura,

[auto build test ERROR on nf-next/master]
[also build test ERROR on v4.8-rc3 next-20160822]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
[Suggest to use git(>=2.9.0) format-patch --base=<commit> (or --base=auto for convenience) to record what (public, well-known) commit your patch series was built on]
[Check https://git-scm.com/docs/git-format-patch for more information]

url:    https://github.com/0day-ci/linux/commits/Laura-Garcia-Liebana/netfilter-nf_tables-Ensure-init-attributes-are-within-the-bounds/20160818-194709
base:   https://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf-next.git master
config: x86_64-rhel (attached as .config)
compiler: gcc-6 (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   net/netfilter/nft_cmp.c: In function 'nft_cmp_init':
>> net/netfilter/nft_cmp.c:91:18: error: 'NFT_CMP_MAX' undeclared (first use in this function)
     if (priv->op >= NFT_CMP_MAX)
                     ^~~~~~~~~~~
   net/netfilter/nft_cmp.c:91:18: note: each undeclared identifier is reported only once for each function it appears in

vim +/NFT_CMP_MAX +91 net/netfilter/nft_cmp.c

    85			return err;
    86	
    87		if (desc.len > U8_MAX)
    88			return -ERANGE;
    89		priv->len = desc.len;
    90		priv->op  = ntohl(nla_get_be32(tb[NFTA_CMP_OP]));
  > 91		if (priv->op >= NFT_CMP_MAX)
    92			return -ERANGE;
    93	
    94		return 0;

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
Laura Garcia Aug. 22, 2016, 9:17 a.m. UTC | #2
On Mon, Aug 22, 2016 at 05:10:02PM +0800, kbuild test robot wrote:
> Hi Laura,
> 
> [auto build test ERROR on nf-next/master]
> [also build test ERROR on v4.8-rc3 next-20160822]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
> [Suggest to use git(>=2.9.0) format-patch --base=<commit> (or --base=auto for convenience) to record what (public, well-known) commit your patch series was built on]
> [Check https://git-scm.com/docs/git-format-patch for more information]
> 
> url:    https://github.com/0day-ci/linux/commits/Laura-Garcia-Liebana/netfilter-nf_tables-Ensure-init-attributes-are-within-the-bounds/20160818-194709
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf-next.git master
> config: x86_64-rhel (attached as .config)
> compiler: gcc-6 (Debian 6.1.1-9) 6.1.1 20160705
> reproduce:
>         # save the attached .config to linux build tree
>         make ARCH=x86_64 
> 
> All errors (new ones prefixed by >>):
> 
>    net/netfilter/nft_cmp.c: In function 'nft_cmp_init':
> >> net/netfilter/nft_cmp.c:91:18: error: 'NFT_CMP_MAX' undeclared (first use in this function)
>      if (priv->op >= NFT_CMP_MAX)
>                      ^~~~~~~~~~~
>    net/netfilter/nft_cmp.c:91:18: note: each undeclared identifier is reported only once for each function it appears in
> 
> vim +/NFT_CMP_MAX +91 net/netfilter/nft_cmp.c
> 
>     85			return err;
>     86	
>     87		if (desc.len > U8_MAX)
>     88			return -ERANGE;
>     89		priv->len = desc.len;
>     90		priv->op  = ntohl(nla_get_be32(tb[NFTA_CMP_OP]));
>   > 91		if (priv->op >= NFT_CMP_MAX)
>     92			return -ERANGE;
>     93	
>     94		return 0;
>

A later patch version (v4) was sent to fix that.

Thanks.

> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation


--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
kernel test robot Aug. 22, 2016, 3:27 p.m. UTC | #3
Hi Laura,

[auto build test WARNING on nf-next/master]
[also build test WARNING on v4.8-rc3 next-20160822]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
[Suggest to use git(>=2.9.0) format-patch --base=<commit> (or --base=auto for convenience) to record what (public, well-known) commit your patch series was built on]
[Check https://git-scm.com/docs/git-format-patch for more information]

url:    https://github.com/0day-ci/linux/commits/Laura-Garcia-Liebana/netfilter-nf_tables-Ensure-init-attributes-are-within-the-bounds/20160818-194709
base:   https://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf-next.git master
config: x86_64-randconfig-s4-08190653 (attached as .config)
compiler: gcc-6 (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   In file included from include/linux/linkage.h:4:0,
                    from include/linux/kernel.h:6,
                    from net/netfilter/nft_cmp.c:11:
   net/netfilter/nft_cmp.c: In function 'nft_cmp_init':
   net/netfilter/nft_cmp.c:91:18: error: 'NFT_CMP_MAX' undeclared (first use in this function)
     if (priv->op >= NFT_CMP_MAX)
                     ^
   include/linux/compiler.h:149:30: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                 ^~~~
>> net/netfilter/nft_cmp.c:91:2: note: in expansion of macro 'if'
     if (priv->op >= NFT_CMP_MAX)
     ^~
   net/netfilter/nft_cmp.c:91:18: note: each undeclared identifier is reported only once for each function it appears in
     if (priv->op >= NFT_CMP_MAX)
                     ^
   include/linux/compiler.h:149:30: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                 ^~~~
>> net/netfilter/nft_cmp.c:91:2: note: in expansion of macro 'if'
     if (priv->op >= NFT_CMP_MAX)
     ^~

vim +/if +91 net/netfilter/nft_cmp.c

     5	 * it under the terms of the GNU General Public License version 2 as
     6	 * published by the Free Software Foundation.
     7	 *
     8	 * Development of this code funded by Astaro AG (http://www.astaro.com/)
     9	 */
    10	
  > 11	#include <linux/kernel.h>
    12	#include <linux/init.h>
    13	#include <linux/module.h>
    14	#include <linux/netlink.h>
    15	#include <linux/netfilter.h>
    16	#include <linux/netfilter/nf_tables.h>
    17	#include <net/netfilter/nf_tables_core.h>
    18	#include <net/netfilter/nf_tables.h>
    19	
    20	struct nft_cmp_expr {
    21		struct nft_data		data;
    22		enum nft_registers	sreg:8;
    23		u8			len;
    24		enum nft_cmp_ops	op:8;
    25	};
    26	
    27	static void nft_cmp_eval(const struct nft_expr *expr,
    28				 struct nft_regs *regs,
    29				 const struct nft_pktinfo *pkt)
    30	{
    31		const struct nft_cmp_expr *priv = nft_expr_priv(expr);
    32		int d;
    33	
    34		d = memcmp(&regs->data[priv->sreg], &priv->data, priv->len);
    35		switch (priv->op) {
    36		case NFT_CMP_EQ:
    37			if (d != 0)
    38				goto mismatch;
    39			break;
    40		case NFT_CMP_NEQ:
    41			if (d == 0)
    42				goto mismatch;
    43			break;
    44		case NFT_CMP_LT:
    45			if (d == 0)
    46				goto mismatch;
    47		case NFT_CMP_LTE:
    48			if (d > 0)
    49				goto mismatch;
    50			break;
    51		case NFT_CMP_GT:
    52			if (d == 0)
    53				goto mismatch;
    54		case NFT_CMP_GTE:
    55			if (d < 0)
    56				goto mismatch;
    57			break;
    58		}
    59		return;
    60	
    61	mismatch:
    62		regs->verdict.code = NFT_BREAK;
    63	}
    64	
    65	static const struct nla_policy nft_cmp_policy[NFTA_CMP_MAX + 1] = {
    66		[NFTA_CMP_SREG]		= { .type = NLA_U32 },
    67		[NFTA_CMP_OP]		= { .type = NLA_U32 },
    68		[NFTA_CMP_DATA]		= { .type = NLA_NESTED },
    69	};
    70	
    71	static int nft_cmp_init(const struct nft_ctx *ctx, const struct nft_expr *expr,
    72				const struct nlattr * const tb[])
    73	{
    74		struct nft_cmp_expr *priv = nft_expr_priv(expr);
    75		struct nft_data_desc desc;
    76		int err;
    77	
    78		err = nft_data_init(NULL, &priv->data, sizeof(priv->data), &desc,
    79				    tb[NFTA_CMP_DATA]);
    80		BUG_ON(err < 0);
    81	
    82		priv->sreg = nft_parse_register(tb[NFTA_CMP_SREG]);
    83		err = nft_validate_register_load(priv->sreg, desc.len);
    84		if (err < 0)
    85			return err;
    86	
    87		if (desc.len > U8_MAX)
    88			return -ERANGE;
    89		priv->len = desc.len;
    90		priv->op  = ntohl(nla_get_be32(tb[NFTA_CMP_OP]));
  > 91		if (priv->op >= NFT_CMP_MAX)
    92			return -ERANGE;
    93	
    94		return 0;

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

Patch
diff mbox

diff --git a/net/netfilter/nft_bitwise.c b/net/netfilter/nft_bitwise.c
index d71cc18..6e09b1e 100644
--- a/net/netfilter/nft_bitwise.c
+++ b/net/netfilter/nft_bitwise.c
@@ -53,6 +53,7 @@  static int nft_bitwise_init(const struct nft_ctx *ctx,
 	struct nft_bitwise *priv = nft_expr_priv(expr);
 	struct nft_data_desc d1, d2;
 	int err;
+	u32 len;
 
 	if (tb[NFTA_BITWISE_SREG] == NULL ||
 	    tb[NFTA_BITWISE_DREG] == NULL ||
@@ -61,7 +62,11 @@  static int nft_bitwise_init(const struct nft_ctx *ctx,
 	    tb[NFTA_BITWISE_XOR] == NULL)
 		return -EINVAL;
 
-	priv->len  = ntohl(nla_get_be32(tb[NFTA_BITWISE_LEN]));
+	len  = ntohl(nla_get_be32(tb[NFTA_BITWISE_LEN]));
+	if (len > U8_MAX)
+		return -ERANGE;
+	priv->len = len;
+
 	priv->sreg = nft_parse_register(tb[NFTA_BITWISE_SREG]);
 	err = nft_validate_register_load(priv->sreg, priv->len);
 	if (err < 0)
diff --git a/net/netfilter/nft_byteorder.c b/net/netfilter/nft_byteorder.c
index b78c28b..763cf15 100644
--- a/net/netfilter/nft_byteorder.c
+++ b/net/netfilter/nft_byteorder.c
@@ -100,6 +100,7 @@  static int nft_byteorder_init(const struct nft_ctx *ctx,
 {
 	struct nft_byteorder *priv = nft_expr_priv(expr);
 	int err;
+	u32 len, size;
 
 	if (tb[NFTA_BYTEORDER_SREG] == NULL ||
 	    tb[NFTA_BYTEORDER_DREG] == NULL ||
@@ -117,7 +118,10 @@  static int nft_byteorder_init(const struct nft_ctx *ctx,
 		return -EINVAL;
 	}
 
-	priv->size = ntohl(nla_get_be32(tb[NFTA_BYTEORDER_SIZE]));
+	size = ntohl(nla_get_be32(tb[NFTA_BYTEORDER_SIZE]));
+	if (size > U8_MAX)
+		return -ERANGE;
+	priv->size = size;
 	switch (priv->size) {
 	case 2:
 	case 4:
@@ -128,7 +132,12 @@  static int nft_byteorder_init(const struct nft_ctx *ctx,
 	}
 
 	priv->sreg = nft_parse_register(tb[NFTA_BYTEORDER_SREG]);
-	priv->len  = ntohl(nla_get_be32(tb[NFTA_BYTEORDER_LEN]));
+
+	len  = ntohl(nla_get_be32(tb[NFTA_BYTEORDER_LEN]));
+	if (len > U8_MAX)
+		return -ERANGE;
+	priv->len = len;
+
 	err = nft_validate_register_load(priv->sreg, priv->len);
 	if (err < 0)
 		return err;
diff --git a/net/netfilter/nft_cmp.c b/net/netfilter/nft_cmp.c
index e25b35d..7412c82 100644
--- a/net/netfilter/nft_cmp.c
+++ b/net/netfilter/nft_cmp.c
@@ -84,8 +84,13 @@  static int nft_cmp_init(const struct nft_ctx *ctx, const struct nft_expr *expr,
 	if (err < 0)
 		return err;
 
-	priv->op  = ntohl(nla_get_be32(tb[NFTA_CMP_OP]));
+	if (desc.len > U8_MAX)
+		return -ERANGE;
 	priv->len = desc.len;
+	priv->op  = ntohl(nla_get_be32(tb[NFTA_CMP_OP]));
+	if (priv->op >= NFT_CMP_MAX)
+		return -ERANGE;
+
 	return 0;
 }
 
diff --git a/net/netfilter/nft_immediate.c b/net/netfilter/nft_immediate.c
index db3b746..b5f899c 100644
--- a/net/netfilter/nft_immediate.c
+++ b/net/netfilter/nft_immediate.c
@@ -53,6 +53,9 @@  static int nft_immediate_init(const struct nft_ctx *ctx,
 			    tb[NFTA_IMMEDIATE_DATA]);
 	if (err < 0)
 		return err;
+
+	if (desc.len > U8_MAX)
+		return -ERANGE;
 	priv->dlen = desc.len;
 
 	priv->dreg = nft_parse_register(tb[NFTA_IMMEDIATE_DREG]);