| Message ID | 20160914125959.GA3020@sonyv |
|---|---|
| State | Accepted |
| Delegated to: | Pablo Neira |
| Headers | show |
On Wed, Sep 14, 2016 at 03:00:02PM +0200, Laura Garcia Liebana wrote: > Check storage of u32 netlink attributes in smaller resources. This > validation is usually required when the u32 netlink attributes are being > stored in a private structure size of u8 in the kernel. Applied with changes, no need to resend. If I break anything, just follow up on top. I have rewritten this description and the documentation on the code a bit. More changes: > 4da449a ("netfilter: nft_exthdr: Add size check on u8 nft_exthdr > attributes") Always use 12 bytes commit-ids. 4da449a is too short, given the number of changes we're getting in the kernel tree, this may become ambiguous at some point so it won't be unique. You can achieve this via: git log --oneline --abbrev=12 More comments below. > Suggested-by: Pablo Neira Ayuso <pablo@netfilter.org> > Signed-off-by: Laura Garcia Liebana <nevola@gmail.com> > --- > include/net/netfilter/nf_tables.h | 2 ++ > net/netfilter/nf_tables_api.c | 26 ++++++++++++++++++++++++++ > net/netfilter/nft_bitwise.c | 10 +++++++++- > net/netfilter/nft_byteorder.c | 17 +++++++++++++++-- > net/netfilter/nft_cmp.c | 6 +++++- > net/netfilter/nft_exthdr.c | 19 ++++++++++++------- > net/netfilter/nft_immediate.c | 4 ++++ > 7 files changed, 73 insertions(+), 11 deletions(-) > > diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h > index f2f1339..608130f 100644 > --- a/include/net/netfilter/nf_tables.h > +++ b/include/net/netfilter/nf_tables.h > @@ -127,6 +127,8 @@ static inline enum nft_registers nft_type_to_reg(enum nft_data_types type) > return type == NFT_DATA_VERDICT ? NFT_REG_VERDICT : NFT_REG_1 * NFT_REG_SIZE / NFT_REG32_SIZE; > } > > +unsigned int nft_parse_u32_check(const struct nlattr *attr, int maxlen, > + u32 *dest); I have renamed maxlen to max, so this fits in one line. > unsigned int nft_parse_register(const struct nlattr *attr); > int nft_dump_register(struct sk_buff *skb, unsigned int attr, unsigned int reg); > > diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c > index 7e1c876..d7b000f 100644 > --- a/net/netfilter/nf_tables_api.c > +++ b/net/netfilter/nf_tables_api.c > @@ -4343,6 +4343,32 @@ static int nf_tables_check_loops(const struct nft_ctx *ctx, > } > > /** > + * nft_parse_u32_check - parse a u32 value to store the value into a > + * smaller resource. > + * > + * @attr: netlink attribute > + * @maxlen: maximum value to be stored in dest > + * @dest: pointer to the resource > + * > + * Parse and store a given u32 value into a resource. Returns an error > + * ERANGE if the value will overload the maxlen, otherwise a 0 will be > + * returned and the value is stored into dest. I've rewritten this a bit. > + */ > +unsigned int nft_parse_u32_check(const struct nlattr *attr, int maxlen, I have rename maxlen to max. Probably maxval would have been better, anyway. > + u32 *dest) > +{ > + int reg; Variable names are important, they provide context when reading the code. Look, from the 'reg' name it seems we're fetching a register. However, we are obtaining a value, so I renamed this to val. Try to select the right name next time. > + > + reg = ntohl(nla_get_be32(attr)); > + if (reg > maxlen) > + return -ERANGE; > + > + *dest = reg; > + return 0; > +} > +EXPORT_SYMBOL_GPL(nft_parse_u32_check); > + > +/** > * nft_parse_register - parse a register value from a netlink attribute > * > * @attr: netlink attribute > diff --git a/net/netfilter/nft_bitwise.c b/net/netfilter/nft_bitwise.c > index d71cc18..e7f23a1 100644 > --- a/net/netfilter/nft_bitwise.c > +++ b/net/netfilter/nft_bitwise.c > @@ -14,6 +14,7 @@ > #include <linux/netlink.h> > #include <linux/netfilter.h> > #include <linux/netfilter/nf_tables.h> > +#include <linux/static_key.h> > #include <net/netfilter/nf_tables_core.h> > #include <net/netfilter/nf_tables.h> > > @@ -52,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; > + u32 len; > int err; > > if (tb[NFTA_BITWISE_SREG] == NULL || > @@ -61,7 +63,13 @@ 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])); > + err = nft_parse_u32_check(tb[NFTA_BITWISE_LEN], U8_MAX, > + &len); ^^^^ This line break was unnecessary. We can have lines 80-chars long, and for some reason you broken the line before the limit. I have repaired this. > + if (err < 0) > + return err; > + > + 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..8cac219 100644 > --- a/net/netfilter/nft_byteorder.c > +++ b/net/netfilter/nft_byteorder.c > @@ -99,6 +99,7 @@ static int nft_byteorder_init(const struct nft_ctx *ctx, > const struct nlattr * const tb[]) > { > struct nft_byteorder *priv = nft_expr_priv(expr); > + u32 size, len; > int err; > > if (tb[NFTA_BYTEORDER_SREG] == NULL || > @@ -117,7 +118,13 @@ static int nft_byteorder_init(const struct nft_ctx *ctx, > return -EINVAL; > } > > - priv->size = ntohl(nla_get_be32(tb[NFTA_BYTEORDER_SIZE])); > + err = nft_parse_u32_check(tb[NFTA_BYTEORDER_SIZE], U8_MAX, > + &size); Same here, and everywhere else. > + if (err < 0) > + return err; > + > + priv->size = size; > + > switch (priv->size) { > case 2: > case 4: > @@ -128,7 +135,13 @@ 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])); > + err = nft_parse_u32_check(tb[NFTA_BYTEORDER_LEN], U8_MAX, > + &len); > + if (err < 0) > + return err; > + > + 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..3d31432 100644 > --- a/net/netfilter/nft_cmp.c > +++ b/net/netfilter/nft_cmp.c > @@ -84,8 +84,12 @@ 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])); Look, this line has moved for no reason. This just adds noise to the patch, so modify only the code that you strictly need, to get it smaller. -- 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
On Thu, Sep 22, 2016 at 04:58:36PM +0200, Pablo Neira Ayuso wrote: > On Wed, Sep 14, 2016 at 03:00:02PM +0200, Laura Garcia Liebana wrote: > > Check storage of u32 netlink attributes in smaller resources. This > > validation is usually required when the u32 netlink attributes are being > > stored in a private structure size of u8 in the kernel. > > Applied with changes, no need to resend. If I break anything, just > follow up on top. And for the record: netfilter: nf_tables: validate maximum value of u32 netlink attributes I have renamed the titled to this, slightly shorter. -- 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
On Thu, 2016-09-22 at 16:58 +0200, Pablo Neira Ayuso wrote: > attributes") > > Always use 12 bytes commit-ids. 4da449a is too short, given the number > of changes we're getting in the kernel tree, this may become ambiguous > at some point so it won't be unique. > > You can achieve this via: git log --oneline --abbrev=12 and Documentation/SubmittingPatches has these tips : You should also be sure to use at least the first twelve characters of the SHA-1 ID. The kernel repository holds a *lot* of objects, making collisions with shorter IDs a real possibility. Bear in mind that, even if there is no collision with your six-character ID now, that condition may change five years from now. If your patch fixes a bug in a specific commit, e.g. you found an issue using git-bisect, please use the 'Fixes:' tag with the first 12 characters of the SHA-1 ID, and the one line summary. For example: Fixes: e21d2170f366 ("video: remove unnecessary platform_set_drvdata()") The following git-config settings can be used to add a pretty format for outputting the above style in the git log or git show commands [core] abbrev = 12 [pretty] fixes = Fixes: %h (\"%s\") -- 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
On Thu, Sep 22, 2016 at 04:58:36PM +0200, Pablo Neira Ayuso wrote: > On Wed, Sep 14, 2016 at 03:00:02PM +0200, Laura Garcia Liebana wrote: > > Check storage of u32 netlink attributes in smaller resources. This > > validation is usually required when the u32 netlink attributes are being > > stored in a private structure size of u8 in the kernel. > > Applied with changes, no need to resend. If I break anything, just > follow up on top. > > I have rewritten this description and the documentation on the code a bit. > > More changes: > > > 4da449a ("netfilter: nft_exthdr: Add size check on u8 nft_exthdr > > attributes") > > Always use 12 bytes commit-ids. 4da449a is too short, given the number > of changes we're getting in the kernel tree, this may become ambiguous > at some point so it won't be unique. > > You can achieve this via: git log --oneline --abbrev=12 > > More comments below. > > > Suggested-by: Pablo Neira Ayuso <pablo@netfilter.org> > > Signed-off-by: Laura Garcia Liebana <nevola@gmail.com> > > --- > > include/net/netfilter/nf_tables.h | 2 ++ > > net/netfilter/nf_tables_api.c | 26 ++++++++++++++++++++++++++ > > net/netfilter/nft_bitwise.c | 10 +++++++++- > > net/netfilter/nft_byteorder.c | 17 +++++++++++++++-- > > net/netfilter/nft_cmp.c | 6 +++++- > > net/netfilter/nft_exthdr.c | 19 ++++++++++++------- > > net/netfilter/nft_immediate.c | 4 ++++ > > 7 files changed, 73 insertions(+), 11 deletions(-) > > > > diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h > > index f2f1339..608130f 100644 > > --- a/include/net/netfilter/nf_tables.h > > +++ b/include/net/netfilter/nf_tables.h > > @@ -127,6 +127,8 @@ static inline enum nft_registers nft_type_to_reg(enum nft_data_types type) > > return type == NFT_DATA_VERDICT ? NFT_REG_VERDICT : NFT_REG_1 * NFT_REG_SIZE / NFT_REG32_SIZE; > > } > > > > +unsigned int nft_parse_u32_check(const struct nlattr *attr, int maxlen, > > + u32 *dest); > > I have renamed maxlen to max, so this fits in one line. > > > unsigned int nft_parse_register(const struct nlattr *attr); > > int nft_dump_register(struct sk_buff *skb, unsigned int attr, unsigned int reg); > > > > diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c > > index 7e1c876..d7b000f 100644 > > --- a/net/netfilter/nf_tables_api.c > > +++ b/net/netfilter/nf_tables_api.c > > @@ -4343,6 +4343,32 @@ static int nf_tables_check_loops(const struct nft_ctx *ctx, > > } > > > > /** > > + * nft_parse_u32_check - parse a u32 value to store the value into a > > + * smaller resource. > > + * > > + * @attr: netlink attribute > > + * @maxlen: maximum value to be stored in dest > > + * @dest: pointer to the resource > > + * > > + * Parse and store a given u32 value into a resource. Returns an error > > + * ERANGE if the value will overload the maxlen, otherwise a 0 will be > > + * returned and the value is stored into dest. > > I've rewritten this a bit. > > > + */ > > +unsigned int nft_parse_u32_check(const struct nlattr *attr, int maxlen, > > I have rename maxlen to max. Probably maxval would have been better, > anyway. > > > + u32 *dest) > > +{ > > + int reg; > > Variable names are important, they provide context when reading the > code. Look, from the 'reg' name it seems we're fetching a register. > However, we are obtaining a value, so I renamed this to val. Try to > select the right name next time. > > > + > > + reg = ntohl(nla_get_be32(attr)); > > + if (reg > maxlen) > > + return -ERANGE; > > + > > + *dest = reg; > > + return 0; > > +} > > +EXPORT_SYMBOL_GPL(nft_parse_u32_check); > > + > > +/** > > * nft_parse_register - parse a register value from a netlink attribute > > * > > * @attr: netlink attribute > > diff --git a/net/netfilter/nft_bitwise.c b/net/netfilter/nft_bitwise.c > > index d71cc18..e7f23a1 100644 > > --- a/net/netfilter/nft_bitwise.c > > +++ b/net/netfilter/nft_bitwise.c > > @@ -14,6 +14,7 @@ > > #include <linux/netlink.h> > > #include <linux/netfilter.h> > > #include <linux/netfilter/nf_tables.h> > > +#include <linux/static_key.h> > > #include <net/netfilter/nf_tables_core.h> > > #include <net/netfilter/nf_tables.h> > > > > @@ -52,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; > > + u32 len; > > int err; > > > > if (tb[NFTA_BITWISE_SREG] == NULL || > > @@ -61,7 +63,13 @@ 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])); > > + err = nft_parse_u32_check(tb[NFTA_BITWISE_LEN], U8_MAX, > > + &len); > ^^^^ > > This line break was unnecessary. We can have lines 80-chars long, and > for some reason you broken the line before the limit. I have repaired > this. > > > + if (err < 0) > > + return err; > > + > > + 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..8cac219 100644 > > --- a/net/netfilter/nft_byteorder.c > > +++ b/net/netfilter/nft_byteorder.c > > @@ -99,6 +99,7 @@ static int nft_byteorder_init(const struct nft_ctx *ctx, > > const struct nlattr * const tb[]) > > { > > struct nft_byteorder *priv = nft_expr_priv(expr); > > + u32 size, len; > > int err; > > > > if (tb[NFTA_BYTEORDER_SREG] == NULL || > > @@ -117,7 +118,13 @@ static int nft_byteorder_init(const struct nft_ctx *ctx, > > return -EINVAL; > > } > > > > - priv->size = ntohl(nla_get_be32(tb[NFTA_BYTEORDER_SIZE])); > > + err = nft_parse_u32_check(tb[NFTA_BYTEORDER_SIZE], U8_MAX, > > + &size); > > Same here, and everywhere else. > > > + if (err < 0) > > + return err; > > + > > + priv->size = size; > > + > > switch (priv->size) { > > case 2: > > case 4: > > @@ -128,7 +135,13 @@ 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])); > > + err = nft_parse_u32_check(tb[NFTA_BYTEORDER_LEN], U8_MAX, > > + &len); > > + if (err < 0) > > + return err; > > + > > + 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..3d31432 100644 > > --- a/net/netfilter/nft_cmp.c > > +++ b/net/netfilter/nft_cmp.c > > @@ -84,8 +84,12 @@ 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])); > > Look, this line has moved for no reason. This just adds noise to the > patch, so modify only the code that you strictly need, to get it > smaller. The reason of this movement was to return the error as soon as possible. -- 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
On Thu, Sep 22, 2016 at 09:16:07AM -0700, Eric Dumazet wrote: > On Thu, 2016-09-22 at 16:58 +0200, Pablo Neira Ayuso wrote: > > attributes") > > > > Always use 12 bytes commit-ids. 4da449a is too short, given the number > > of changes we're getting in the kernel tree, this may become ambiguous > > at some point so it won't be unique. > > > > You can achieve this via: git log --oneline --abbrev=12 > > and Documentation/SubmittingPatches has these tips : > > > You should also be sure to use at least the first twelve characters of the > SHA-1 ID. The kernel repository holds a *lot* of objects, making > collisions with shorter IDs a real possibility. Bear in mind that, even if > there is no collision with your six-character ID now, that condition may > change five years from now. > > If your patch fixes a bug in a specific commit, e.g. you found an issue using > git-bisect, please use the 'Fixes:' tag with the first 12 characters of the > SHA-1 ID, and the one line summary. For example: > > Fixes: e21d2170f366 ("video: remove unnecessary platform_set_drvdata()") > > The following git-config settings can be used to add a pretty format for > outputting the above style in the git log or git show commands > > [core] > abbrev = 12 > [pretty] > fixes = Fixes: %h (\"%s\") > > Duly noted, thanks. In this case, it was not referred to a fix but an extension of the older patch to other files with the same problem. -- 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
diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h index f2f1339..608130f 100644 --- a/include/net/netfilter/nf_tables.h +++ b/include/net/netfilter/nf_tables.h @@ -127,6 +127,8 @@ static inline enum nft_registers nft_type_to_reg(enum nft_data_types type) return type == NFT_DATA_VERDICT ? NFT_REG_VERDICT : NFT_REG_1 * NFT_REG_SIZE / NFT_REG32_SIZE; } +unsigned int nft_parse_u32_check(const struct nlattr *attr, int maxlen, + u32 *dest); unsigned int nft_parse_register(const struct nlattr *attr); int nft_dump_register(struct sk_buff *skb, unsigned int attr, unsigned int reg); diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c index 7e1c876..d7b000f 100644 --- a/net/netfilter/nf_tables_api.c +++ b/net/netfilter/nf_tables_api.c @@ -4343,6 +4343,32 @@ static int nf_tables_check_loops(const struct nft_ctx *ctx, } /** + * nft_parse_u32_check - parse a u32 value to store the value into a + * smaller resource. + * + * @attr: netlink attribute + * @maxlen: maximum value to be stored in dest + * @dest: pointer to the resource + * + * Parse and store a given u32 value into a resource. Returns an error + * ERANGE if the value will overload the maxlen, otherwise a 0 will be + * returned and the value is stored into dest. + */ +unsigned int nft_parse_u32_check(const struct nlattr *attr, int maxlen, + u32 *dest) +{ + int reg; + + reg = ntohl(nla_get_be32(attr)); + if (reg > maxlen) + return -ERANGE; + + *dest = reg; + return 0; +} +EXPORT_SYMBOL_GPL(nft_parse_u32_check); + +/** * nft_parse_register - parse a register value from a netlink attribute * * @attr: netlink attribute diff --git a/net/netfilter/nft_bitwise.c b/net/netfilter/nft_bitwise.c index d71cc18..e7f23a1 100644 --- a/net/netfilter/nft_bitwise.c +++ b/net/netfilter/nft_bitwise.c @@ -14,6 +14,7 @@ #include <linux/netlink.h> #include <linux/netfilter.h> #include <linux/netfilter/nf_tables.h> +#include <linux/static_key.h> #include <net/netfilter/nf_tables_core.h> #include <net/netfilter/nf_tables.h> @@ -52,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; + u32 len; int err; if (tb[NFTA_BITWISE_SREG] == NULL || @@ -61,7 +63,13 @@ 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])); + err = nft_parse_u32_check(tb[NFTA_BITWISE_LEN], U8_MAX, + &len); + if (err < 0) + return err; + + 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..8cac219 100644 --- a/net/netfilter/nft_byteorder.c +++ b/net/netfilter/nft_byteorder.c @@ -99,6 +99,7 @@ static int nft_byteorder_init(const struct nft_ctx *ctx, const struct nlattr * const tb[]) { struct nft_byteorder *priv = nft_expr_priv(expr); + u32 size, len; int err; if (tb[NFTA_BYTEORDER_SREG] == NULL || @@ -117,7 +118,13 @@ static int nft_byteorder_init(const struct nft_ctx *ctx, return -EINVAL; } - priv->size = ntohl(nla_get_be32(tb[NFTA_BYTEORDER_SIZE])); + err = nft_parse_u32_check(tb[NFTA_BYTEORDER_SIZE], U8_MAX, + &size); + if (err < 0) + return err; + + priv->size = size; + switch (priv->size) { case 2: case 4: @@ -128,7 +135,13 @@ 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])); + err = nft_parse_u32_check(tb[NFTA_BYTEORDER_LEN], U8_MAX, + &len); + if (err < 0) + return err; + + 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..3d31432 100644 --- a/net/netfilter/nft_cmp.c +++ b/net/netfilter/nft_cmp.c @@ -84,8 +84,12 @@ 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])); + return 0; } diff --git a/net/netfilter/nft_exthdr.c b/net/netfilter/nft_exthdr.c index 82c264e..613732d8 100644 --- a/net/netfilter/nft_exthdr.c +++ b/net/netfilter/nft_exthdr.c @@ -59,7 +59,7 @@ static int nft_exthdr_init(const struct nft_ctx *ctx, const struct nlattr * const tb[]) { struct nft_exthdr *priv = nft_expr_priv(expr); - u32 offset, len; + u32 offset, len, err; if (tb[NFTA_EXTHDR_DREG] == NULL || tb[NFTA_EXTHDR_TYPE] == NULL || @@ -67,15 +67,20 @@ static int nft_exthdr_init(const struct nft_ctx *ctx, tb[NFTA_EXTHDR_LEN] == NULL) return -EINVAL; - offset = ntohl(nla_get_be32(tb[NFTA_EXTHDR_OFFSET])); - len = ntohl(nla_get_be32(tb[NFTA_EXTHDR_LEN])); + err = nft_parse_u32_check(tb[NFTA_EXTHDR_OFFSET], U8_MAX, + &offset); + if (err < 0) + return err; - if (offset > U8_MAX || len > U8_MAX) - return -ERANGE; + priv->offset = offset; + err = nft_parse_u32_check(tb[NFTA_EXTHDR_LEN], U8_MAX, + &len); + if (err < 0) + return err; + + priv->len = len; priv->type = nla_get_u8(tb[NFTA_EXTHDR_TYPE]); - priv->offset = offset; - priv->len = len; priv->dreg = nft_parse_register(tb[NFTA_EXTHDR_DREG]); return nft_validate_register_store(ctx, priv->dreg, NULL, diff --git a/net/netfilter/nft_immediate.c b/net/netfilter/nft_immediate.c index db3b746..d17018f 100644 --- a/net/netfilter/nft_immediate.c +++ b/net/netfilter/nft_immediate.c @@ -53,6 +53,10 @@ 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]);
Check storage of u32 netlink attributes in smaller resources. This validation is usually required when the u32 netlink attributes are being stored in a private structure size of u8 in the kernel. 4da449a ("netfilter: nft_exthdr: Add size check on u8 nft_exthdr attributes") Suggested-by: Pablo Neira Ayuso <pablo@netfilter.org> Signed-off-by: Laura Garcia Liebana <nevola@gmail.com> --- include/net/netfilter/nf_tables.h | 2 ++ net/netfilter/nf_tables_api.c | 26 ++++++++++++++++++++++++++ net/netfilter/nft_bitwise.c | 10 +++++++++- net/netfilter/nft_byteorder.c | 17 +++++++++++++++-- net/netfilter/nft_cmp.c | 6 +++++- net/netfilter/nft_exthdr.c | 19 ++++++++++++------- net/netfilter/nft_immediate.c | 4 ++++ 7 files changed, 73 insertions(+), 11 deletions(-)