diff mbox

sctp: verify length provided in heartbeat information parameter

Message ID 20121130121627.GG30697@casper.infradead.org
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Thomas Graf Nov. 30, 2012, 12:16 p.m. UTC 
If the variable parameter length provided in the mandatory
heartbeat information parameter exceeds the calculated payload
length the packet has been corrupted. Reply with a parameter
length protocol violation message.

Signed-off-by: Thomas Graf <tgraf@suug.ch>
---
 net/sctp/sm_statefuns.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

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

Comments

Neil Horman Nov. 30, 2012, 4:01 p.m. UTC | #1 
On Fri, Nov 30, 2012 at 12:16:27PM +0000, Thomas Graf wrote:
> If the variable parameter length provided in the mandatory
> heartbeat information parameter exceeds the calculated payload
> length the packet has been corrupted. Reply with a parameter
> length protocol violation message.
> 
> Signed-off-by: Thomas Graf <tgraf@suug.ch>
> ---
>  net/sctp/sm_statefuns.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
> index b6adef8..e92079d 100644
> --- a/net/sctp/sm_statefuns.c
> +++ b/net/sctp/sm_statefuns.c
> @@ -1055,6 +1055,7 @@ sctp_disposition_t sctp_sf_beat_8_3(struct net *net,
>  				    void *arg,
>  				    sctp_cmd_seq_t *commands)
>  {
> +	sctp_paramhdr_t *param_hdr;
>  	struct sctp_chunk *chunk = arg;
>  	struct sctp_chunk *reply;
>  	size_t paylen = 0;
> @@ -1072,12 +1073,17 @@ sctp_disposition_t sctp_sf_beat_8_3(struct net *net,
>  	 * Information field copied from the received HEARTBEAT chunk.
>  	 */
>  	chunk->subh.hb_hdr = (sctp_heartbeathdr_t *) chunk->skb->data;
> +	param_hdr = (sctp_paramhdr_t *) chunk->subh.hb_hdr;
>  	paylen = ntohs(chunk->chunk_hdr->length) - sizeof(sctp_chunkhdr_t);
> +
> +	if (ntohs(param_hdr->length) > paylen)
> +		return sctp_sf_violation_paramlen(net, ep, asoc, type, arg,
> +						  param_hdr, commands);
> +
>  	if (!pskb_pull(chunk->skb, paylen))
>  		goto nomem;
>  
> -	reply = sctp_make_heartbeat_ack(asoc, chunk,
> -					chunk->subh.hb_hdr, paylen);
> +	reply = sctp_make_heartbeat_ack(asoc, chunk, param_hdr, paylen);
>  	if (!reply)
>  		goto nomem;
>  
> --
> 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
> 
Looks good, thanks Thomas.
Acked-by: Neil Horman <nhorman@tuxdriver.com>

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller Nov. 30, 2012, 5:26 p.m. UTC | #2 
From: Neil Horman <nhorman@tuxdriver.com>
Date: Fri, 30 Nov 2012 11:01:57 -0500

> On Fri, Nov 30, 2012 at 12:16:27PM +0000, Thomas Graf wrote:
>> If the variable parameter length provided in the mandatory
>> heartbeat information parameter exceeds the calculated payload
>> length the packet has been corrupted. Reply with a parameter
>> length protocol violation message.
>> 
>> Signed-off-by: Thomas Graf <tgraf@suug.ch>
...
>> 
> Looks good, thanks Thomas.
> Acked-by: Neil Horman <nhorman@tuxdriver.com>
> 

Applied, thanks.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Vladislav Yasevich Nov. 30, 2012, 5:34 p.m. UTC | #3 
On 11/30/2012 07:16 AM, Thomas Graf wrote:
> If the variable parameter length provided in the mandatory
> heartbeat information parameter exceeds the calculated payload
> length the packet has been corrupted. Reply with a parameter
> length protocol violation message.
>
> Signed-off-by: Thomas Graf <tgraf@suug.ch>

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

-vlad

> ---
>   net/sctp/sm_statefuns.c | 10 ++++++++--
>   1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
> index b6adef8..e92079d 100644
> --- a/net/sctp/sm_statefuns.c
> +++ b/net/sctp/sm_statefuns.c
> @@ -1055,6 +1055,7 @@ sctp_disposition_t sctp_sf_beat_8_3(struct net *net,
>   				    void *arg,
>   				    sctp_cmd_seq_t *commands)
>   {
> +	sctp_paramhdr_t *param_hdr;
>   	struct sctp_chunk *chunk = arg;
>   	struct sctp_chunk *reply;
>   	size_t paylen = 0;
> @@ -1072,12 +1073,17 @@ sctp_disposition_t sctp_sf_beat_8_3(struct net *net,
>   	 * Information field copied from the received HEARTBEAT chunk.
>   	 */
>   	chunk->subh.hb_hdr = (sctp_heartbeathdr_t *) chunk->skb->data;
> +	param_hdr = (sctp_paramhdr_t *) chunk->subh.hb_hdr;
>   	paylen = ntohs(chunk->chunk_hdr->length) - sizeof(sctp_chunkhdr_t);
> +
> +	if (ntohs(param_hdr->length) > paylen)
> +		return sctp_sf_violation_paramlen(net, ep, asoc, type, arg,
> +						  param_hdr, commands);
> +
>   	if (!pskb_pull(chunk->skb, paylen))
>   		goto nomem;
>
> -	reply = sctp_make_heartbeat_ack(asoc, chunk,
> -					chunk->subh.hb_hdr, paylen);
> +	reply = sctp_make_heartbeat_ack(asoc, chunk, param_hdr, paylen);
>   	if (!reply)
>   		goto nomem;
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

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

Patch

diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
index b6adef8..e92079d 100644
--- a/net/sctp/sm_statefuns.c
+++ b/net/sctp/sm_statefuns.c
@@ -1055,6 +1055,7 @@  sctp_disposition_t sctp_sf_beat_8_3(struct net *net,
 				    void *arg,
 				    sctp_cmd_seq_t *commands)
 {
+	sctp_paramhdr_t *param_hdr;
 	struct sctp_chunk *chunk = arg;
 	struct sctp_chunk *reply;
 	size_t paylen = 0;
@@ -1072,12 +1073,17 @@  sctp_disposition_t sctp_sf_beat_8_3(struct net *net,
 	 * Information field copied from the received HEARTBEAT chunk.
 	 */
 	chunk->subh.hb_hdr = (sctp_heartbeathdr_t *) chunk->skb->data;
+	param_hdr = (sctp_paramhdr_t *) chunk->subh.hb_hdr;
 	paylen = ntohs(chunk->chunk_hdr->length) - sizeof(sctp_chunkhdr_t);
+
+	if (ntohs(param_hdr->length) > paylen)
+		return sctp_sf_violation_paramlen(net, ep, asoc, type, arg,
+						  param_hdr, commands);
+
 	if (!pskb_pull(chunk->skb, paylen))
 		goto nomem;
 
-	reply = sctp_make_heartbeat_ack(asoc, chunk,
-					chunk->subh.hb_hdr, paylen);
+	reply = sctp_make_heartbeat_ack(asoc, chunk, param_hdr, paylen);
 	if (!reply)
 		goto nomem;