diff mbox series

[v1] netfilter: conntrack: validate cta_ip via parsing

Message ID 20230711032257.3561166-1-linma@zju.edu.cn (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series [v1] netfilter: conntrack: validate cta_ip via parsing | expand

Checks

Context Check Description
netdev/series_format warning Single patches do not need cover letters; Target tree name not specified in the subject
netdev/tree_selection success Guessed tree name to be net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 1342 this patch: 1342
netdev/cc_maintainers fail 1 blamed authors not CCed: johannes.berg@intel.com; 1 maintainers not CCed: johannes.berg@intel.com
netdev/build_clang success Errors and warnings before: 1364 this patch: 1364
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 1365 this patch: 1365
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 17 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Lin Ma July 11, 2023, 3:22 a.m. UTC
In current ctnetlink_parse_tuple_ip() function, nested parsing and
validation is splitting as two parts. This is unnecessary as the
nla_parse_nested_deprecated function supports validation in the fly.
These two finially reach same place __nla_validate_parse with same
validate flag.

nla_parse_nested_deprecated
  __nla_parse(.., NL_VALIDATE_LIBERAL, ..)
    __nla_validate_parse

nla_validate_nested_deprecated
  __nla_validate_nested(.., NL_VALIDATE_LIBERAL, ..)
    __nla_validate
      __nla_validate_parse

This commit removes the call to nla_validate_nested_deprecated and pass
cta_ip_nla_policy when do parsing.

Fixes: 8cb081746c03 ("netlink: make validation more configurable for future strictness")
Signed-off-by: Lin Ma <linma@zju.edu.cn>
---
 net/netfilter/nf_conntrack_netlink.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

Comments

Simon Horman July 12, 2023, 1:16 p.m. UTC | #1
On Tue, Jul 11, 2023 at 11:22:57AM +0800, Lin Ma wrote:
> In current ctnetlink_parse_tuple_ip() function, nested parsing and
> validation is splitting as two parts. This is unnecessary as the
> nla_parse_nested_deprecated function supports validation in the fly.
> These two finially reach same place __nla_validate_parse with same
> validate flag.
> 
> nla_parse_nested_deprecated
>   __nla_parse(.., NL_VALIDATE_LIBERAL, ..)
>     __nla_validate_parse
> 
> nla_validate_nested_deprecated
>   __nla_validate_nested(.., NL_VALIDATE_LIBERAL, ..)
>     __nla_validate
>       __nla_validate_parse
> 
> This commit removes the call to nla_validate_nested_deprecated and pass
> cta_ip_nla_policy when do parsing.
> 
> Fixes: 8cb081746c03 ("netlink: make validation more configurable for future strictness")

I don't think this warrants a fixes tag, as it's not fixing any
user-visible behaviour. Rather, it is a clean-up.

> Signed-off-by: Lin Ma <linma@zju.edu.cn>
> ---
>  net/netfilter/nf_conntrack_netlink.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
> index 69c8c8c7e9b8..334db22199c1 100644
> --- a/net/netfilter/nf_conntrack_netlink.c
> +++ b/net/netfilter/nf_conntrack_netlink.c
> @@ -1321,15 +1321,11 @@ static int ctnetlink_parse_tuple_ip(struct nlattr *attr,
>  	struct nlattr *tb[CTA_IP_MAX+1];
>  	int ret = 0;
>  
> -	ret = nla_parse_nested_deprecated(tb, CTA_IP_MAX, attr, NULL, NULL);
> +	ret = nla_parse_nested_deprecated(tb, CTA_IP_MAX, attr,
> +					  cta_ip_nla_policy, NULL);
>  	if (ret < 0)
>  		return ret;
>  
> -	ret = nla_validate_nested_deprecated(attr, CTA_IP_MAX,
> -					     cta_ip_nla_policy, NULL);
> -	if (ret)
> -		return ret;
> -
>  	switch (tuple->src.l3num) {
>  	case NFPROTO_IPV4:
>  		ret = ipv4_nlattr_to_tuple(tb, tuple, flags);
> -- 
> 2.17.1
> 
>
Lin Ma July 12, 2023, 1:26 p.m. UTC | #2
Hello Simon,

> 
> I don't think this warrants a fixes tag, as it's not fixing any
> user-visible behaviour. Rather, it is a clean-up.
> 

My bad, I will resend one with adjusted message.

Regards
Lin
Simon Horman July 13, 2023, 8:52 a.m. UTC | #3
On Wed, Jul 12, 2023 at 09:26:09PM +0800, Lin Ma wrote:
> Hello Simon,
> 
> > 
> > I don't think this warrants a fixes tag, as it's not fixing any
> > user-visible behaviour. Rather, it is a clean-up.
> > 
> 
> My bad, I will resend one with adjusted message.

Thanks, much appreciated.
diff mbox series

Patch

diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
index 69c8c8c7e9b8..334db22199c1 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -1321,15 +1321,11 @@  static int ctnetlink_parse_tuple_ip(struct nlattr *attr,
 	struct nlattr *tb[CTA_IP_MAX+1];
 	int ret = 0;
 
-	ret = nla_parse_nested_deprecated(tb, CTA_IP_MAX, attr, NULL, NULL);
+	ret = nla_parse_nested_deprecated(tb, CTA_IP_MAX, attr,
+					  cta_ip_nla_policy, NULL);
 	if (ret < 0)
 		return ret;
 
-	ret = nla_validate_nested_deprecated(attr, CTA_IP_MAX,
-					     cta_ip_nla_policy, NULL);
-	if (ret)
-		return ret;
-
 	switch (tuple->src.l3num) {
 	case NFPROTO_IPV4:
 		ret = ipv4_nlattr_to_tuple(tb, tuple, flags);