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:	Fri, 13 Sep 2013 22:37:21 +0200
From:	Hannes Frederic Sowa <hannes@...essinduktion.org>
To:	Duan Jiong <duanj.fnst@...fujitsu.com>
Cc:	davem@...emloft.net, netdev@...r.kernel.org, vyasevic@...hat.com,
	dborkman@...hat.com, linux-sctp@...r.kernel.org
Subject: Re: [PATCH v2 1/6] ipv6: del the statements for updating route in  (dccp|tcp|sctp)_v6_err

[added linux-sctp, Daniel and Vlad + full quote]

On Fri, Sep 13, 2013 at 10:58:55AM +0800, Duan Jiong wrote:
> From: Duan Jiong <duanj.fnst@...fujitsu.com>
> 
> Because we will do route updating for redirect in nidsc layer. And
> when dealing with redirect message, the dccp and sctp should like
> tcp return directly.
> 
> Signed-off-by: Duan Jiong <duanj.fnst@...fujitsu.com>
> ---
>  net/dccp/ipv6.c     | 10 +++-------
>  net/ipv6/tcp_ipv6.c | 12 ++++--------
>  net/sctp/input.c    | 12 ------------
>  net/sctp/ipv6.c     |  6 +++---
>  4 files changed, 10 insertions(+), 30 deletions(-)
> 
> diff --git a/net/dccp/ipv6.c b/net/dccp/ipv6.c
> index 9c61f9c..300840c 100644
> --- a/net/dccp/ipv6.c
> +++ b/net/dccp/ipv6.c
> @@ -98,6 +98,9 @@ static void dccp_v6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
>  		return;
>  	}
>  
> +	if (type == NDISC_REDIRECT)
> +		return;
> +

Minor: Could you move this above the if (...) ICMP6_INC_STATS_BH. We don't
want to count errors here under no circumstance. These are in fact no errors.
(This is also the scheme in the other *_err functions).

Please Cc linux-sctp@...r.kernel.org on the sctp bits in the next round,
too.

Otherwise I looked at all patches and they seemed fine to me.

Duan, I would suggest the following:

I cannot judge if the patch Daniel proposed regarding EPROTO in sk->sk_err
on redirects should go to stable. If it should, perhaps this patch should
go in first and you could later on rebase this series as soon as Daniel's
patch has landed in the net repo? Only some minor edits should be needed
then. This way there is a clean patch for stable and David can consider
taking this in for net or net-next (this fixes a glitch where we do not
apply redirects generated by packets of ipv6 tunnels, otherwise a bit of
code removal).

Thanks,

  Hannes

>  	sk = inet6_lookup(net, &dccp_hashinfo,
>  			&hdr->daddr, dh->dccph_dport,
>  			&hdr->saddr, dh->dccph_sport, inet6_iif(skb));
> @@ -130,13 +133,6 @@ static void dccp_v6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
>  
>  	np = inet6_sk(sk);
>  
> -	if (type == NDISC_REDIRECT) {
> -		struct dst_entry *dst = __sk_dst_check(sk, np->dst_cookie);
> -
> -		if (dst)
> -			dst->ops->redirect(dst, sk, skb);
> -	}
> -
>  	if (type == ICMPV6_PKT_TOOBIG) {
>  		struct dst_entry *dst = NULL;
>  
> diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
> index 5c71501..d3ca8a4 100644
> --- a/net/ipv6/tcp_ipv6.c
> +++ b/net/ipv6/tcp_ipv6.c
> @@ -346,6 +346,10 @@ static void tcp_v6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
>  	__u32 seq;
>  	struct net *net = dev_net(skb->dev);
>  
> +
> +	if (type == NDISC_REDIRECT)
> +		return;
> +
>  	sk = inet6_lookup(net, &tcp_hashinfo, &hdr->daddr,
>  			th->dest, &hdr->saddr, th->source, skb->dev->ifindex);
>  
> @@ -382,14 +386,6 @@ static void tcp_v6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
>  
>  	np = inet6_sk(sk);
>  
> -	if (type == NDISC_REDIRECT) {
> -		struct dst_entry *dst = __sk_dst_check(sk, np->dst_cookie);
> -
> -		if (dst)
> -			dst->ops->redirect(dst, sk, skb);
> -		goto out;
> -	}
> -
>  	if (type == ICMPV6_PKT_TOOBIG) {
>  		/* We are not interested in TCP_LISTEN and open_requests
>  		 * (SYN-ACKs send out by Linux are always <576bytes so
> diff --git a/net/sctp/input.c b/net/sctp/input.c
> index 5f20686..0d2d4b7 100644
> --- a/net/sctp/input.c
> +++ b/net/sctp/input.c
> @@ -413,18 +413,6 @@ void sctp_icmp_frag_needed(struct sock *sk, struct sctp_association *asoc,
>  	sctp_retransmit(&asoc->outqueue, t, SCTP_RTXR_PMTUD);
>  }
>  
> -void sctp_icmp_redirect(struct sock *sk, struct sctp_transport *t,
> -			struct sk_buff *skb)
> -{
> -	struct dst_entry *dst;
> -
> -	if (!t)
> -		return;
> -	dst = sctp_transport_dst_check(t);
> -	if (dst)
> -		dst->ops->redirect(dst, sk, skb);
> -}
> -
>  /*
>   * SCTP Implementer's Guide, 2.37 ICMP handling procedures
>   *
> diff --git a/net/sctp/ipv6.c b/net/sctp/ipv6.c
> index da613ce..ee12d87 100644
> --- a/net/sctp/ipv6.c
> +++ b/net/sctp/ipv6.c
> @@ -151,6 +151,9 @@ static void sctp_v6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
>  	int err;
>  	struct net *net = dev_net(skb->dev);
>  
> +	if (type == NDISC_REDIRECT)
> +		return;
> +
>  	idev = in6_dev_get(skb->dev);
>  
>  	/* Fix up skb to look at the embedded net header. */
> @@ -181,9 +184,6 @@ static void sctp_v6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
>  			goto out_unlock;
>  		}
>  		break;
> -	case NDISC_REDIRECT:
> -		sctp_icmp_redirect(sk, transport, skb);
> -		break;
>  	default:
>  		break;
>  	}
> -- 
> 1.8.3.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

-- 
gruss,

  Hannes

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ