netdev - Re: [PATCH ipsec-next 09/11] xfrm: remove afinfo pointer from xfrm

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 28 Mar 2019 15:13:33 +0100
From:   Sabrina Dubroca <sd@...asysnail.net>
To:     Florian Westphal <fw@...len.de>
Cc:     netdev@...r.kernel.org, steffen.klassert@...unet.com
Subject: Re: [PATCH ipsec-next 09/11] xfrm: remove afinfo pointer from
 xfrm_mode

(just a couple of checkpatch-like comments)

2019-03-27, 18:31:38 +0100, Florian Westphal wrote:
> diff --git a/net/ipv4/xfrm4_output.c b/net/ipv4/xfrm4_output.c
> index 6802d1aee424..cff048ad8562 100644
> --- a/net/ipv4/xfrm4_output.c
> +++ b/net/ipv4/xfrm4_output.c
> @@ -72,6 +72,8 @@ int xfrm4_output_finish(struct sock *sk, struct sk_buff *skb)
>  static int __xfrm4_output(struct net *net, struct sock *sk, struct sk_buff *skb)
>  {
>  	struct xfrm_state *x = skb_dst(skb)->xfrm;
> +	const struct xfrm_state_afinfo *afinfo;
> +	int ret = -EAFNOSUPPORT;
>  
>  #ifdef CONFIG_NETFILTER
>  	if (!x) {
> @@ -80,7 +82,14 @@ static int __xfrm4_output(struct net *net, struct sock *sk, struct sk_buff *skb)
>  	}
>  #endif
>  
> -	return x->outer_mode->afinfo->output_finish(sk, skb);
> +	rcu_read_lock();
> +	afinfo = xfrm_state_afinfo_get_rcu(x->outer_mode->family);
> +	if (afinfo)
> +		ret = afinfo->output_finish(sk, skb);
> +	else
> +		kfree_skb(skb);
> +	rcu_read_unlock();

Maybe add a blank line before the return, like you did in __xfrm6_output_state_finish().

> +	return ret;
>  }
>  
>  int xfrm4_output(struct net *net, struct sock *sk, struct sk_buff *skb)
> diff --git a/net/ipv6/xfrm6_output.c b/net/ipv6/xfrm6_output.c
> index 2b663d2ffdcd..82168de60e6b 100644
> --- a/net/ipv6/xfrm6_output.c
> +++ b/net/ipv6/xfrm6_output.c
> @@ -122,11 +122,27 @@ int xfrm6_output_finish(struct sock *sk, struct sk_buff *skb)
>  	return xfrm_output(sk, skb);
>  }
>  
> +static int __xfrm6_output_state_finish(struct xfrm_state *x, struct sock *sk, struct sk_buff *skb)

Over 80 chars.

> +{
> +	const struct xfrm_state_afinfo *afinfo;
> +	int ret = -EAFNOSUPPORT;
> +
> +	rcu_read_lock();
> +	afinfo = xfrm_state_afinfo_get_rcu(x->outer_mode->family);
> +	if (afinfo)
> +		ret = afinfo->output_finish(sk, skb);
> +	else
> +		kfree_skb(skb);
> +	rcu_read_unlock();
> +
> +	return ret;
> +}
> +
>  static int __xfrm6_output_finish(struct net *net, struct sock *sk, struct sk_buff *skb)
>  {
>  	struct xfrm_state *x = skb_dst(skb)->xfrm;
>  
> -	return x->outer_mode->afinfo->output_finish(sk, skb);
> +	return __xfrm6_output_state_finish(x, sk, skb);
>  }
>  
>  static int __xfrm6_output(struct net *net, struct sock *sk, struct sk_buff *skb)
> @@ -168,7 +184,8 @@ static int __xfrm6_output(struct net *net, struct sock *sk, struct sk_buff *skb)
>  				    __xfrm6_output_finish);
>  
>  skip_frag:
> -	return x->outer_mode->afinfo->output_finish(sk, skb);
> +

You could skip the extra blank line.

> +	return __xfrm6_output_state_finish(x, sk, skb);
>  }
>  
>  int xfrm6_output(struct net *net, struct sock *sk, struct sk_buff *skb)

-- 
Sabrina