All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net v2] sctp: fix ICMP processing if skb is non-linear
@ 2017-05-25 17:14 ` Davide Caratti
  0 siblings, 0 replies; 10+ messages in thread
From: Davide Caratti @ 2017-05-25 17:14 UTC (permalink / raw)
  To: netdev, linux-sctp; +Cc: Xin Long, Marcelo Ricardo Leitner, David S . Miller

sometimes ICMP replies to INIT chunks are ignored by the client, even if
the encapsulated SCTP headers match an open socket. This happens when the
ICMP packet is carried by a paged skb: use skb_header_pointer() to read
packet contents beyond the SCTP header, so that chunk header and initiate
tag are validated correctly.

v2:
- don't use skb_header_pointer() to read the transport header, since
  icmp_socket_deliver() already puts these 8 bytes in the linear area.
- change commit message to make specific reference to INIT chunks.

Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
 net/sctp/input.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/net/sctp/input.c b/net/sctp/input.c
index 0e06a27..ba9ad32 100644
--- a/net/sctp/input.c
+++ b/net/sctp/input.c
@@ -473,15 +473,14 @@ struct sock *sctp_err_lookup(struct net *net, int family, struct sk_buff *skb,
 			     struct sctp_association **app,
 			     struct sctp_transport **tpp)
 {
+	struct sctp_init_chunk *chunkhdr, _chunkhdr;
 	union sctp_addr saddr;
 	union sctp_addr daddr;
 	struct sctp_af *af;
 	struct sock *sk = NULL;
 	struct sctp_association *asoc;
 	struct sctp_transport *transport = NULL;
-	struct sctp_init_chunk *chunkhdr;
 	__u32 vtag = ntohl(sctphdr->vtag);
-	int len = skb->len - ((void *)sctphdr - (void *)skb->data);
 
 	*app = NULL; *tpp = NULL;
 
@@ -516,13 +515,16 @@ struct sock *sctp_err_lookup(struct net *net, int family, struct sk_buff *skb,
 	 * discard the packet.
 	 */
 	if (vtag == 0) {
-		chunkhdr = (void *)sctphdr + sizeof(struct sctphdr);
-		if (len < sizeof(struct sctphdr) + sizeof(sctp_chunkhdr_t)
-			  + sizeof(__be32) ||
+		/* chunk header + first 4 octects of init header */
+		chunkhdr = skb_header_pointer(skb, skb_transport_offset(skb) +
+					      sizeof(struct sctphdr),
+					      sizeof(struct sctp_chunkhdr) +
+					      sizeof(__be32), &_chunkhdr);
+		if (!chunkhdr ||
 		    chunkhdr->chunk_hdr.type != SCTP_CID_INIT ||
-		    ntohl(chunkhdr->init_hdr.init_tag) != asoc->c.my_vtag) {
+		    ntohl(chunkhdr->init_hdr.init_tag) != asoc->c.my_vtag)
 			goto out;
-		}
+
 	} else if (vtag != asoc->c.peer_vtag) {
 		goto out;
 	}
-- 
2.9.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH net v2] sctp: fix ICMP processing if skb is non-linear
@ 2017-05-25 17:14 ` Davide Caratti
  0 siblings, 0 replies; 10+ messages in thread
From: Davide Caratti @ 2017-05-25 17:14 UTC (permalink / raw)
  To: netdev, linux-sctp; +Cc: Xin Long, Marcelo Ricardo Leitner, David S . Miller

sometimes ICMP replies to INIT chunks are ignored by the client, even if
the encapsulated SCTP headers match an open socket. This happens when the
ICMP packet is carried by a paged skb: use skb_header_pointer() to read
packet contents beyond the SCTP header, so that chunk header and initiate
tag are validated correctly.

v2:
- don't use skb_header_pointer() to read the transport header, since
  icmp_socket_deliver() already puts these 8 bytes in the linear area.
- change commit message to make specific reference to INIT chunks.

Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
 net/sctp/input.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/net/sctp/input.c b/net/sctp/input.c
index 0e06a27..ba9ad32 100644
--- a/net/sctp/input.c
+++ b/net/sctp/input.c
@@ -473,15 +473,14 @@ struct sock *sctp_err_lookup(struct net *net, int family, struct sk_buff *skb,
 			     struct sctp_association **app,
 			     struct sctp_transport **tpp)
 {
+	struct sctp_init_chunk *chunkhdr, _chunkhdr;
 	union sctp_addr saddr;
 	union sctp_addr daddr;
 	struct sctp_af *af;
 	struct sock *sk = NULL;
 	struct sctp_association *asoc;
 	struct sctp_transport *transport = NULL;
-	struct sctp_init_chunk *chunkhdr;
 	__u32 vtag = ntohl(sctphdr->vtag);
-	int len = skb->len - ((void *)sctphdr - (void *)skb->data);
 
 	*app = NULL; *tpp = NULL;
 
@@ -516,13 +515,16 @@ struct sock *sctp_err_lookup(struct net *net, int family, struct sk_buff *skb,
 	 * discard the packet.
 	 */
 	if (vtag = 0) {
-		chunkhdr = (void *)sctphdr + sizeof(struct sctphdr);
-		if (len < sizeof(struct sctphdr) + sizeof(sctp_chunkhdr_t)
-			  + sizeof(__be32) ||
+		/* chunk header + first 4 octects of init header */
+		chunkhdr = skb_header_pointer(skb, skb_transport_offset(skb) +
+					      sizeof(struct sctphdr),
+					      sizeof(struct sctp_chunkhdr) +
+					      sizeof(__be32), &_chunkhdr);
+		if (!chunkhdr ||
 		    chunkhdr->chunk_hdr.type != SCTP_CID_INIT ||
-		    ntohl(chunkhdr->init_hdr.init_tag) != asoc->c.my_vtag) {
+		    ntohl(chunkhdr->init_hdr.init_tag) != asoc->c.my_vtag)
 			goto out;
-		}
+
 	} else if (vtag != asoc->c.peer_vtag) {
 		goto out;
 	}
-- 
2.9.4


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH net v2] sctp: fix ICMP processing if skb is non-linear
  2017-05-25 17:14 ` Davide Caratti
@ 2017-05-25 17:29   ` Marcelo Ricardo Leitner
  -1 siblings, 0 replies; 10+ messages in thread
From: Marcelo Ricardo Leitner @ 2017-05-25 17:29 UTC (permalink / raw)
  To: Davide Caratti; +Cc: netdev, linux-sctp, Xin Long, David S . Miller

On Thu, May 25, 2017 at 07:14:56PM +0200, Davide Caratti wrote:
> sometimes ICMP replies to INIT chunks are ignored by the client, even if
> the encapsulated SCTP headers match an open socket. This happens when the
> ICMP packet is carried by a paged skb: use skb_header_pointer() to read
> packet contents beyond the SCTP header, so that chunk header and initiate
> tag are validated correctly.
> 
> v2:
> - don't use skb_header_pointer() to read the transport header, since
>   icmp_socket_deliver() already puts these 8 bytes in the linear area.
> - change commit message to make specific reference to INIT chunks.
> 
> Signed-off-by: Davide Caratti <dcaratti@redhat.com>

LGTM
Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>

> ---
>  net/sctp/input.c | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/net/sctp/input.c b/net/sctp/input.c
> index 0e06a27..ba9ad32 100644
> --- a/net/sctp/input.c
> +++ b/net/sctp/input.c
> @@ -473,15 +473,14 @@ struct sock *sctp_err_lookup(struct net *net, int family, struct sk_buff *skb,
>  			     struct sctp_association **app,
>  			     struct sctp_transport **tpp)
>  {
> +	struct sctp_init_chunk *chunkhdr, _chunkhdr;
>  	union sctp_addr saddr;
>  	union sctp_addr daddr;
>  	struct sctp_af *af;
>  	struct sock *sk = NULL;
>  	struct sctp_association *asoc;
>  	struct sctp_transport *transport = NULL;
> -	struct sctp_init_chunk *chunkhdr;
>  	__u32 vtag = ntohl(sctphdr->vtag);
> -	int len = skb->len - ((void *)sctphdr - (void *)skb->data);
>  
>  	*app = NULL; *tpp = NULL;
>  
> @@ -516,13 +515,16 @@ struct sock *sctp_err_lookup(struct net *net, int family, struct sk_buff *skb,
>  	 * discard the packet.
>  	 */
>  	if (vtag == 0) {
> -		chunkhdr = (void *)sctphdr + sizeof(struct sctphdr);
> -		if (len < sizeof(struct sctphdr) + sizeof(sctp_chunkhdr_t)
> -			  + sizeof(__be32) ||
> +		/* chunk header + first 4 octects of init header */
> +		chunkhdr = skb_header_pointer(skb, skb_transport_offset(skb) +
> +					      sizeof(struct sctphdr),
> +					      sizeof(struct sctp_chunkhdr) +
> +					      sizeof(__be32), &_chunkhdr);
> +		if (!chunkhdr ||
>  		    chunkhdr->chunk_hdr.type != SCTP_CID_INIT ||
> -		    ntohl(chunkhdr->init_hdr.init_tag) != asoc->c.my_vtag) {
> +		    ntohl(chunkhdr->init_hdr.init_tag) != asoc->c.my_vtag)
>  			goto out;
> -		}
> +
>  	} else if (vtag != asoc->c.peer_vtag) {
>  		goto out;
>  	}
> -- 
> 2.9.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net v2] sctp: fix ICMP processing if skb is non-linear
@ 2017-05-25 17:29   ` Marcelo Ricardo Leitner
  0 siblings, 0 replies; 10+ messages in thread
From: Marcelo Ricardo Leitner @ 2017-05-25 17:29 UTC (permalink / raw)
  To: Davide Caratti; +Cc: netdev, linux-sctp, Xin Long, David S . Miller

On Thu, May 25, 2017 at 07:14:56PM +0200, Davide Caratti wrote:
> sometimes ICMP replies to INIT chunks are ignored by the client, even if
> the encapsulated SCTP headers match an open socket. This happens when the
> ICMP packet is carried by a paged skb: use skb_header_pointer() to read
> packet contents beyond the SCTP header, so that chunk header and initiate
> tag are validated correctly.
> 
> v2:
> - don't use skb_header_pointer() to read the transport header, since
>   icmp_socket_deliver() already puts these 8 bytes in the linear area.
> - change commit message to make specific reference to INIT chunks.
> 
> Signed-off-by: Davide Caratti <dcaratti@redhat.com>

LGTM
Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>

> ---
>  net/sctp/input.c | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/net/sctp/input.c b/net/sctp/input.c
> index 0e06a27..ba9ad32 100644
> --- a/net/sctp/input.c
> +++ b/net/sctp/input.c
> @@ -473,15 +473,14 @@ struct sock *sctp_err_lookup(struct net *net, int family, struct sk_buff *skb,
>  			     struct sctp_association **app,
>  			     struct sctp_transport **tpp)
>  {
> +	struct sctp_init_chunk *chunkhdr, _chunkhdr;
>  	union sctp_addr saddr;
>  	union sctp_addr daddr;
>  	struct sctp_af *af;
>  	struct sock *sk = NULL;
>  	struct sctp_association *asoc;
>  	struct sctp_transport *transport = NULL;
> -	struct sctp_init_chunk *chunkhdr;
>  	__u32 vtag = ntohl(sctphdr->vtag);
> -	int len = skb->len - ((void *)sctphdr - (void *)skb->data);
>  
>  	*app = NULL; *tpp = NULL;
>  
> @@ -516,13 +515,16 @@ struct sock *sctp_err_lookup(struct net *net, int family, struct sk_buff *skb,
>  	 * discard the packet.
>  	 */
>  	if (vtag = 0) {
> -		chunkhdr = (void *)sctphdr + sizeof(struct sctphdr);
> -		if (len < sizeof(struct sctphdr) + sizeof(sctp_chunkhdr_t)
> -			  + sizeof(__be32) ||
> +		/* chunk header + first 4 octects of init header */
> +		chunkhdr = skb_header_pointer(skb, skb_transport_offset(skb) +
> +					      sizeof(struct sctphdr),
> +					      sizeof(struct sctp_chunkhdr) +
> +					      sizeof(__be32), &_chunkhdr);
> +		if (!chunkhdr ||
>  		    chunkhdr->chunk_hdr.type != SCTP_CID_INIT ||
> -		    ntohl(chunkhdr->init_hdr.init_tag) != asoc->c.my_vtag) {
> +		    ntohl(chunkhdr->init_hdr.init_tag) != asoc->c.my_vtag)
>  			goto out;
> -		}
> +
>  	} else if (vtag != asoc->c.peer_vtag) {
>  		goto out;
>  	}
> -- 
> 2.9.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net v2] sctp: fix ICMP processing if skb is non-linear
  2017-05-25 17:14 ` Davide Caratti
@ 2017-05-25 17:58   ` Vlad Yasevich
  -1 siblings, 0 replies; 10+ messages in thread
From: Vlad Yasevich @ 2017-05-25 17:58 UTC (permalink / raw)
  To: Davide Caratti, netdev, linux-sctp
  Cc: Xin Long, Marcelo Ricardo Leitner, David S . Miller

On 05/25/2017 01:14 PM, Davide Caratti wrote:
> sometimes ICMP replies to INIT chunks are ignored by the client, even if
> the encapsulated SCTP headers match an open socket. This happens when the
> ICMP packet is carried by a paged skb: use skb_header_pointer() to read
> packet contents beyond the SCTP header, so that chunk header and initiate
> tag are validated correctly.
> 
> v2:
> - don't use skb_header_pointer() to read the transport header, since
>   icmp_socket_deliver() already puts these 8 bytes in the linear area.
> - change commit message to make specific reference to INIT chunks.
> 
> Signed-off-by: Davide Caratti <dcaratti@redhat.com>

Acked-by: Vlad Yasevich <vyasevich@gmail.com>

-vlad


> ---
>  net/sctp/input.c | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/net/sctp/input.c b/net/sctp/input.c
> index 0e06a27..ba9ad32 100644
> --- a/net/sctp/input.c
> +++ b/net/sctp/input.c
> @@ -473,15 +473,14 @@ struct sock *sctp_err_lookup(struct net *net, int family, struct sk_buff *skb,
>  			     struct sctp_association **app,
>  			     struct sctp_transport **tpp)
>  {
> +	struct sctp_init_chunk *chunkhdr, _chunkhdr;
>  	union sctp_addr saddr;
>  	union sctp_addr daddr;
>  	struct sctp_af *af;
>  	struct sock *sk = NULL;
>  	struct sctp_association *asoc;
>  	struct sctp_transport *transport = NULL;
> -	struct sctp_init_chunk *chunkhdr;
>  	__u32 vtag = ntohl(sctphdr->vtag);
> -	int len = skb->len - ((void *)sctphdr - (void *)skb->data);
>  
>  	*app = NULL; *tpp = NULL;
>  
> @@ -516,13 +515,16 @@ struct sock *sctp_err_lookup(struct net *net, int family, struct sk_buff *skb,
>  	 * discard the packet.
>  	 */
>  	if (vtag == 0) {
> -		chunkhdr = (void *)sctphdr + sizeof(struct sctphdr);
> -		if (len < sizeof(struct sctphdr) + sizeof(sctp_chunkhdr_t)
> -			  + sizeof(__be32) ||
> +		/* chunk header + first 4 octects of init header */
> +		chunkhdr = skb_header_pointer(skb, skb_transport_offset(skb) +
> +					      sizeof(struct sctphdr),
> +					      sizeof(struct sctp_chunkhdr) +
> +					      sizeof(__be32), &_chunkhdr);
> +		if (!chunkhdr ||
>  		    chunkhdr->chunk_hdr.type != SCTP_CID_INIT ||
> -		    ntohl(chunkhdr->init_hdr.init_tag) != asoc->c.my_vtag) {
> +		    ntohl(chunkhdr->init_hdr.init_tag) != asoc->c.my_vtag)
>  			goto out;
> -		}
> +
>  	} else if (vtag != asoc->c.peer_vtag) {
>  		goto out;
>  	}
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net v2] sctp: fix ICMP processing if skb is non-linear
@ 2017-05-25 17:58   ` Vlad Yasevich
  0 siblings, 0 replies; 10+ messages in thread
From: Vlad Yasevich @ 2017-05-25 17:58 UTC (permalink / raw)
  To: Davide Caratti, netdev, linux-sctp
  Cc: Xin Long, Marcelo Ricardo Leitner, David S . Miller

On 05/25/2017 01:14 PM, Davide Caratti wrote:
> sometimes ICMP replies to INIT chunks are ignored by the client, even if
> the encapsulated SCTP headers match an open socket. This happens when the
> ICMP packet is carried by a paged skb: use skb_header_pointer() to read
> packet contents beyond the SCTP header, so that chunk header and initiate
> tag are validated correctly.
> 
> v2:
> - don't use skb_header_pointer() to read the transport header, since
>   icmp_socket_deliver() already puts these 8 bytes in the linear area.
> - change commit message to make specific reference to INIT chunks.
> 
> Signed-off-by: Davide Caratti <dcaratti@redhat.com>

Acked-by: Vlad Yasevich <vyasevich@gmail.com>

-vlad


> ---
>  net/sctp/input.c | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/net/sctp/input.c b/net/sctp/input.c
> index 0e06a27..ba9ad32 100644
> --- a/net/sctp/input.c
> +++ b/net/sctp/input.c
> @@ -473,15 +473,14 @@ struct sock *sctp_err_lookup(struct net *net, int family, struct sk_buff *skb,
>  			     struct sctp_association **app,
>  			     struct sctp_transport **tpp)
>  {
> +	struct sctp_init_chunk *chunkhdr, _chunkhdr;
>  	union sctp_addr saddr;
>  	union sctp_addr daddr;
>  	struct sctp_af *af;
>  	struct sock *sk = NULL;
>  	struct sctp_association *asoc;
>  	struct sctp_transport *transport = NULL;
> -	struct sctp_init_chunk *chunkhdr;
>  	__u32 vtag = ntohl(sctphdr->vtag);
> -	int len = skb->len - ((void *)sctphdr - (void *)skb->data);
>  
>  	*app = NULL; *tpp = NULL;
>  
> @@ -516,13 +515,16 @@ struct sock *sctp_err_lookup(struct net *net, int family, struct sk_buff *skb,
>  	 * discard the packet.
>  	 */
>  	if (vtag = 0) {
> -		chunkhdr = (void *)sctphdr + sizeof(struct sctphdr);
> -		if (len < sizeof(struct sctphdr) + sizeof(sctp_chunkhdr_t)
> -			  + sizeof(__be32) ||
> +		/* chunk header + first 4 octects of init header */
> +		chunkhdr = skb_header_pointer(skb, skb_transport_offset(skb) +
> +					      sizeof(struct sctphdr),
> +					      sizeof(struct sctp_chunkhdr) +
> +					      sizeof(__be32), &_chunkhdr);
> +		if (!chunkhdr ||
>  		    chunkhdr->chunk_hdr.type != SCTP_CID_INIT ||
> -		    ntohl(chunkhdr->init_hdr.init_tag) != asoc->c.my_vtag) {
> +		    ntohl(chunkhdr->init_hdr.init_tag) != asoc->c.my_vtag)
>  			goto out;
> -		}
> +
>  	} else if (vtag != asoc->c.peer_vtag) {
>  		goto out;
>  	}
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net v2] sctp: fix ICMP processing if skb is non-linear
  2017-05-25 17:14 ` Davide Caratti
@ 2017-05-26  7:02   ` Xin Long
  -1 siblings, 0 replies; 10+ messages in thread
From: Xin Long @ 2017-05-26  7:02 UTC (permalink / raw)
  To: Davide Caratti
  Cc: network dev, linux-sctp, Marcelo Ricardo Leitner, David S . Miller

On Fri, May 26, 2017 at 1:14 AM, Davide Caratti <dcaratti@redhat.com> wrote:
> sometimes ICMP replies to INIT chunks are ignored by the client, even if
> the encapsulated SCTP headers match an open socket. This happens when the
> ICMP packet is carried by a paged skb: use skb_header_pointer() to read
> packet contents beyond the SCTP header, so that chunk header and initiate
> tag are validated correctly.
>
> v2:
> - don't use skb_header_pointer() to read the transport header, since
>   icmp_socket_deliver() already puts these 8 bytes in the linear area.
> - change commit message to make specific reference to INIT chunks.
>
> Signed-off-by: Davide Caratti <dcaratti@redhat.com>
Thanks.
Reviewed-by: Xin Long <lucien.xin@gmail.com>

> ---
>  net/sctp/input.c | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/net/sctp/input.c b/net/sctp/input.c
> index 0e06a27..ba9ad32 100644
> --- a/net/sctp/input.c
> +++ b/net/sctp/input.c
> @@ -473,15 +473,14 @@ struct sock *sctp_err_lookup(struct net *net, int family, struct sk_buff *skb,
>                              struct sctp_association **app,
>                              struct sctp_transport **tpp)
>  {
> +       struct sctp_init_chunk *chunkhdr, _chunkhdr;
>         union sctp_addr saddr;
>         union sctp_addr daddr;
>         struct sctp_af *af;
>         struct sock *sk = NULL;
>         struct sctp_association *asoc;
>         struct sctp_transport *transport = NULL;
> -       struct sctp_init_chunk *chunkhdr;
>         __u32 vtag = ntohl(sctphdr->vtag);
> -       int len = skb->len - ((void *)sctphdr - (void *)skb->data);
>
>         *app = NULL; *tpp = NULL;
>
> @@ -516,13 +515,16 @@ struct sock *sctp_err_lookup(struct net *net, int family, struct sk_buff *skb,
>          * discard the packet.
>          */
>         if (vtag == 0) {
> -               chunkhdr = (void *)sctphdr + sizeof(struct sctphdr);
> -               if (len < sizeof(struct sctphdr) + sizeof(sctp_chunkhdr_t)
> -                         + sizeof(__be32) ||
> +               /* chunk header + first 4 octects of init header */
> +               chunkhdr = skb_header_pointer(skb, skb_transport_offset(skb) +
> +                                             sizeof(struct sctphdr),
> +                                             sizeof(struct sctp_chunkhdr) +
> +                                             sizeof(__be32), &_chunkhdr);
> +               if (!chunkhdr ||
>                     chunkhdr->chunk_hdr.type != SCTP_CID_INIT ||
> -                   ntohl(chunkhdr->init_hdr.init_tag) != asoc->c.my_vtag) {
> +                   ntohl(chunkhdr->init_hdr.init_tag) != asoc->c.my_vtag)
>                         goto out;
> -               }
> +
>         } else if (vtag != asoc->c.peer_vtag) {
>                 goto out;
>         }
> --
> 2.9.4
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net v2] sctp: fix ICMP processing if skb is non-linear
@ 2017-05-26  7:02   ` Xin Long
  0 siblings, 0 replies; 10+ messages in thread
From: Xin Long @ 2017-05-26  7:02 UTC (permalink / raw)
  To: Davide Caratti
  Cc: network dev, linux-sctp, Marcelo Ricardo Leitner, David S . Miller

On Fri, May 26, 2017 at 1:14 AM, Davide Caratti <dcaratti@redhat.com> wrote:
> sometimes ICMP replies to INIT chunks are ignored by the client, even if
> the encapsulated SCTP headers match an open socket. This happens when the
> ICMP packet is carried by a paged skb: use skb_header_pointer() to read
> packet contents beyond the SCTP header, so that chunk header and initiate
> tag are validated correctly.
>
> v2:
> - don't use skb_header_pointer() to read the transport header, since
>   icmp_socket_deliver() already puts these 8 bytes in the linear area.
> - change commit message to make specific reference to INIT chunks.
>
> Signed-off-by: Davide Caratti <dcaratti@redhat.com>
Thanks.
Reviewed-by: Xin Long <lucien.xin@gmail.com>

> ---
>  net/sctp/input.c | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/net/sctp/input.c b/net/sctp/input.c
> index 0e06a27..ba9ad32 100644
> --- a/net/sctp/input.c
> +++ b/net/sctp/input.c
> @@ -473,15 +473,14 @@ struct sock *sctp_err_lookup(struct net *net, int family, struct sk_buff *skb,
>                              struct sctp_association **app,
>                              struct sctp_transport **tpp)
>  {
> +       struct sctp_init_chunk *chunkhdr, _chunkhdr;
>         union sctp_addr saddr;
>         union sctp_addr daddr;
>         struct sctp_af *af;
>         struct sock *sk = NULL;
>         struct sctp_association *asoc;
>         struct sctp_transport *transport = NULL;
> -       struct sctp_init_chunk *chunkhdr;
>         __u32 vtag = ntohl(sctphdr->vtag);
> -       int len = skb->len - ((void *)sctphdr - (void *)skb->data);
>
>         *app = NULL; *tpp = NULL;
>
> @@ -516,13 +515,16 @@ struct sock *sctp_err_lookup(struct net *net, int family, struct sk_buff *skb,
>          * discard the packet.
>          */
>         if (vtag = 0) {
> -               chunkhdr = (void *)sctphdr + sizeof(struct sctphdr);
> -               if (len < sizeof(struct sctphdr) + sizeof(sctp_chunkhdr_t)
> -                         + sizeof(__be32) ||
> +               /* chunk header + first 4 octects of init header */
> +               chunkhdr = skb_header_pointer(skb, skb_transport_offset(skb) +
> +                                             sizeof(struct sctphdr),
> +                                             sizeof(struct sctp_chunkhdr) +
> +                                             sizeof(__be32), &_chunkhdr);
> +               if (!chunkhdr ||
>                     chunkhdr->chunk_hdr.type != SCTP_CID_INIT ||
> -                   ntohl(chunkhdr->init_hdr.init_tag) != asoc->c.my_vtag) {
> +                   ntohl(chunkhdr->init_hdr.init_tag) != asoc->c.my_vtag)
>                         goto out;
> -               }
> +
>         } else if (vtag != asoc->c.peer_vtag) {
>                 goto out;
>         }
> --
> 2.9.4
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net v2] sctp: fix ICMP processing if skb is non-linear
  2017-05-25 17:14 ` Davide Caratti
@ 2017-05-26 18:42   ` David Miller
  -1 siblings, 0 replies; 10+ messages in thread
From: David Miller @ 2017-05-26 18:42 UTC (permalink / raw)
  To: dcaratti; +Cc: netdev, linux-sctp, lucien.xin, marcelo.leitner

From: Davide Caratti <dcaratti@redhat.com>
Date: Thu, 25 May 2017 19:14:56 +0200

> sometimes ICMP replies to INIT chunks are ignored by the client, even if
> the encapsulated SCTP headers match an open socket. This happens when the
> ICMP packet is carried by a paged skb: use skb_header_pointer() to read
> packet contents beyond the SCTP header, so that chunk header and initiate
> tag are validated correctly.
> 
> v2:
> - don't use skb_header_pointer() to read the transport header, since
>   icmp_socket_deliver() already puts these 8 bytes in the linear area.
> - change commit message to make specific reference to INIT chunks.
> 
> Signed-off-by: Davide Caratti <dcaratti@redhat.com>

Applied and queued up for -stable, thanks.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net v2] sctp: fix ICMP processing if skb is non-linear
@ 2017-05-26 18:42   ` David Miller
  0 siblings, 0 replies; 10+ messages in thread
From: David Miller @ 2017-05-26 18:42 UTC (permalink / raw)
  To: dcaratti; +Cc: netdev, linux-sctp, lucien.xin, marcelo.leitner

From: Davide Caratti <dcaratti@redhat.com>
Date: Thu, 25 May 2017 19:14:56 +0200

> sometimes ICMP replies to INIT chunks are ignored by the client, even if
> the encapsulated SCTP headers match an open socket. This happens when the
> ICMP packet is carried by a paged skb: use skb_header_pointer() to read
> packet contents beyond the SCTP header, so that chunk header and initiate
> tag are validated correctly.
> 
> v2:
> - don't use skb_header_pointer() to read the transport header, since
>   icmp_socket_deliver() already puts these 8 bytes in the linear area.
> - change commit message to make specific reference to INIT chunks.
> 
> Signed-off-by: Davide Caratti <dcaratti@redhat.com>

Applied and queued up for -stable, thanks.

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2017-05-26 18:42 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-25 17:14 [PATCH net v2] sctp: fix ICMP processing if skb is non-linear Davide Caratti
2017-05-25 17:14 ` Davide Caratti
2017-05-25 17:29 ` Marcelo Ricardo Leitner
2017-05-25 17:29   ` Marcelo Ricardo Leitner
2017-05-25 17:58 ` Vlad Yasevich
2017-05-25 17:58   ` Vlad Yasevich
2017-05-26  7:02 ` Xin Long
2017-05-26  7:02   ` Xin Long
2017-05-26 18:42 ` David Miller
2017-05-26 18:42   ` David Miller

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.