[PATCH] [SCTP]: Do not enable peer IPv6 address support on PF_INET socket

[PATCH] [SCTP]: Do not enable peer IPv6 address support on PF_INET socket

[Vlad Yasevich]

From: Wei Yongjun <yjwei@cn.fujitsu.com>

If socket is create by PF_INET type, it can not used IPv6 address to
send/recv DATA, So we can not used IPv6 address even if peer tell us it
support IPv6 address.
This patch fix to only enabled peer IPv6 address support on PF_INET6 socket.

Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com>
Signed-off-by: Vlad Yasevich <vladislav.yasevich@hp.com>
---
 net/sctp/sm_make_chunk.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
index 81b6064..69a464f 100644
--- a/net/sctp/sm_make_chunk.c
+++ b/net/sctp/sm_make_chunk.c
@@ -2418,7 +2418,8 @@ static int sctp_process_param(struct sctp_association *asoc,
 				break;
 
 			case SCTP_PARAM_IPV6_ADDRESS:
-				asoc->peer.ipv6_address = 1;
+				if (PF_INET6 == asoc->base.sk->sk_family)
+					asoc->peer.ipv6_address = 1;
 				break;
 
 			case SCTP_PARAM_HOST_NAME_ADDRESS:
-- 
1.5.3.5

[PATCH] [SCTP]: Add address type check while process paramaters of ASCONF chunk

[Vlad Yasevich]

From: Wei Yongjun <yjwei@cn.fujitsu.com>

If socket is create by AF_INET type, add IPv6 address to asoc will cause
kernel panic while packet is transmitted on that transport.

This patch add address type check before process paramaters of ASCONF
chunk. If peer is not support this address type, return with error
invald parameter.

Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com>
Signed-off-by: Vlad Yasevich <vladislav.yasevich@hp.com>
---
 net/sctp/sm_make_chunk.c |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
index 69a464f..6eeee53 100644
--- a/net/sctp/sm_make_chunk.c
+++ b/net/sctp/sm_make_chunk.c
@@ -2827,6 +2827,19 @@ static __be16 sctp_process_asconf_param(struct sctp_association *asoc,
 	union sctp_addr	addr;
 	union sctp_addr_param *addr_param;
 
+	switch (addr_param->v4.param_hdr.type) {
+	case SCTP_PARAM_IPV6_ADDRESS:
+		if (!asoc->peer.ipv6_address)
+			return SCTP_ERROR_INV_PARAM;
+		break;
+	case SCTP_PARAM_IPV4_ADDRESS:
+		if (!asoc->peer.ipv4_address)
+			return SCTP_ERROR_INV_PARAM;
+		break;
+	default:
+		return SCTP_ERROR_INV_PARAM;
+	}
+
 	addr_param = (union sctp_addr_param *)
 			((void *)asconf_param + sizeof(sctp_addip_param_t));
 
-- 
1.5.3.5

Re: [PATCH] [SCTP]: Do not enable peer IPv6 address support on PF_INET socket

[David Miller]

From: Vlad Yasevich <vladislav.yasevich@hp.com>
Date: Fri,  9 May 2008 15:09:54 -0400

> If socket is create by PF_INET type, it can not used IPv6 address to
> send/recv DATA, So we can not used IPv6 address even if peer tell us it
> support IPv6 address.
> This patch fix to only enabled peer IPv6 address support on PF_INET6 socket.
> 
> Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com>
> Signed-off-by: Vlad Yasevich <vladislav.yasevich@hp.com>

Applied.

Re: [PATCH] [SCTP]: Add address type check while process paramaters of ASCONF chunk

[David Miller]

From: Vlad Yasevich <vladislav.yasevich@hp.com>
Date: Fri,  9 May 2008 15:09:55 -0400

> If socket is create by AF_INET type, add IPv6 address to asoc will cause
> kernel panic while packet is transmitted on that transport.
> 
> This patch add address type check before process paramaters of ASCONF
> chunk. If peer is not support this address type, return with error
> invald parameter.
> 
> Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com>
> Signed-off-by: Vlad Yasevich <vladislav.yasevich@hp.com>

Applied, thanks.

Re: [PATCH] [SCTP]: Add address type check while process paramaters of ASCONF chunk

[Patrick McHardy]

[-- Attachment #1: Type: text/plain, Size: 731 bytes --]

Vlad Yasevich wrote:
> @@ -2827,6 +2827,19 @@ static __be16 sctp_process_asconf_param(struct sctp_association *asoc,
>  	union sctp_addr	addr;
>  	union sctp_addr_param *addr_param;
>  
> +	switch (addr_param->v4.param_hdr.type) {

^^^ use of addr_param

> +	case SCTP_PARAM_IPV6_ADDRESS:
> +		if (!asoc->peer.ipv6_address)
> +			return SCTP_ERROR_INV_PARAM;
> +		break;
> +	case SCTP_PARAM_IPV4_ADDRESS:
> +		if (!asoc->peer.ipv4_address)
> +			return SCTP_ERROR_INV_PARAM;
> +		break;
> +	default:
> +		return SCTP_ERROR_INV_PARAM;
> +	}
> +
>  	addr_param = (union sctp_addr_param *)
>  			((void *)asconf_param + sizeof(sctp_addip_param_t));

^^^ initialization of addr_param

The patch is wrong and needs this fix on top.





[-- Attachment #2: x --]
[-- Type: text/plain, Size: 1465 bytes --]

commit 1beb4530b5bf180d8989993d3050fabdf21522cc
Author: Patrick McHardy <kaber@trash.net>
Date:   Tue May 13 13:27:22 2008 +0200

    [SCTP]: Fix use of uninitialized pointer
    
    Introduced by c4492586 (sctp: Add address type check while process
    paramaters of ASCONF chunk):
    
    net/sctp/sm_make_chunk.c: In function 'sctp_process_asconf':
    net/sctp/sm_make_chunk.c:2828: warning: 'addr_param' may be used uninitialized in this function
    net/sctp/sm_make_chunk.c:2828: note: 'addr_param' was declared here
    
    Signed-off-by: Patrick McHardy <kaber@trash.net>

diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
index 6eeee53..bbc7107 100644
--- a/net/sctp/sm_make_chunk.c
+++ b/net/sctp/sm_make_chunk.c
@@ -2827,6 +2827,9 @@ static __be16 sctp_process_asconf_param(struct sctp_association *asoc,
 	union sctp_addr	addr;
 	union sctp_addr_param *addr_param;
 
+	addr_param = (union sctp_addr_param *)
+			((void *)asconf_param + sizeof(sctp_addip_param_t));
+
 	switch (addr_param->v4.param_hdr.type) {
 	case SCTP_PARAM_IPV6_ADDRESS:
 		if (!asoc->peer.ipv6_address)
@@ -2840,9 +2843,6 @@ static __be16 sctp_process_asconf_param(struct sctp_association *asoc,
 		return SCTP_ERROR_INV_PARAM;
 	}
 
-	addr_param = (union sctp_addr_param *)
-			((void *)asconf_param + sizeof(sctp_addip_param_t));
-
 	af = sctp_get_af_specific(param_type2af(addr_param->v4.param_hdr.type));
 	if (unlikely(!af))
 		return SCTP_ERROR_INV_PARAM;

Re: [PATCH] [SCTP]: Add address type check while process paramaters of ASCONF chunk

[Vlad Yasevich]

Patrick McHardy wrote:
> Vlad Yasevich wrote:
>> @@ -2827,6 +2827,19 @@ static __be16 sctp_process_asconf_param(struct 
>> sctp_association *asoc,
>>      union sctp_addr    addr;
>>      union sctp_addr_param *addr_param;
>>  
>> +    switch (addr_param->v4.param_hdr.type) {
> 
> ^^^ use of addr_param
> 
>> +    case SCTP_PARAM_IPV6_ADDRESS:
>> +        if (!asoc->peer.ipv6_address)
>> +            return SCTP_ERROR_INV_PARAM;
>> +        break;
>> +    case SCTP_PARAM_IPV4_ADDRESS:
>> +        if (!asoc->peer.ipv4_address)
>> +            return SCTP_ERROR_INV_PARAM;
>> +        break;
>> +    default:
>> +        return SCTP_ERROR_INV_PARAM;
>> +    }
>> +
>>      addr_param = (union sctp_addr_param *)
>>              ((void *)asconf_param + sizeof(sctp_addip_param_t));
> 
> ^^^ initialization of addr_param
> 
> The patch is wrong and needs this fix on top.
> 

Ack.  David, please apply.  I missed this warning in the my builds...

Thanks
-vlad

Re: [PATCH] [SCTP]: Add address type check while process paramaters of ASCONF chunk

[David Miller]

From: Vlad Yasevich <vladislav.yasevich@hp.com>
Date: Tue, 13 May 2008 08:48:39 -0400

> I missed this warning in the my builds...

And in your testing of the patch too, right?

Re: [PATCH] [SCTP]: Add address type check while process paramaters of ASCONF chunk

[Wei Yongjun]

Hi David:

 Sorry for my mistake. This patch is create and tested by me.

 I tested it used a test tool which is created by me. And this patchset 
resolved the kernel panic problem. It seems that I need to do more 
review my patch and
add more test case to my tool. I will pay more attention to do the 
correct patchs.

 Thanks everyone.

David Miller wrote:
> From: Vlad Yasevich <vladislav.yasevich@hp.com>
> Date: Tue, 13 May 2008 08:48:39 -0400
>
>   
>> I missed this warning in the my builds...
>>     
>
> And in your testing of the patch too, right?
>
>   

Re: [PATCH] [SCTP]: Add address type check while process paramaters of ASCONF chunk

[David Miller]

From: Wei Yongjun <yjwei@cn.fujitsu.com>
Date: Wed, 14 May 2008 08:56:21 +0800

> Hi David:
> 
>  Sorry for my mistake. This patch is create and tested by me.
> 
>  I tested it used a test tool which is created by me. And this patchset 
> resolved the kernel panic problem. It seems that I need to do more 
> review my patch and
> add more test case to my tool. I will pay more attention to do the 
> correct patchs.

It dereferences an uninitialized pointer, unconditionally.  Therefore,
the patch was definitely completely untested.  This function never ran
during your tests.

Please, no more excuses.

You didn't test the patch.
The end.

Re: [PATCH] [SCTP]: Add address type check while process paramaters of ASCONF chunk

[Wei Yongjun]

David Miller wrote:
> From: Wei Yongjun <yjwei@cn.fujitsu.com>
> Date: Wed, 14 May 2008 08:56:21 +0800
>
>   
>> Hi David:
>>
>>  Sorry for my mistake. This patch is create and tested by me.
>>
>>  I tested it used a test tool which is created by me. And this patchset 
>> resolved the kernel panic problem. It seems that I need to do more 
>> review my patch and
>> add more test case to my tool. I will pay more attention to do the 
>> correct patchs.
>>     
>
> It dereferences an uninitialized pointer, unconditionally.  Therefore,
> the patch was definitely completely untested.  This function never ran
> during your tests.
>
> Please, no more excuses.
>
> You didn't test the patch.
> The end.
>   

Are you sure you are right?

First I am sorry for stupid patch, second I said again, I do test it, 
but do not enough test.

uninitialized pointer is not NULL pointer, so it not kernel panic when 
use "addr_param->v4.param_hdr.type", this patch cause function alaways 
return SCTP_ERROR_INV_PARAM error, so even if I add IPv6 address to peer 
which not supported IPv6 address or add IPv4 address to peer which not 
supported IPv4 address is returned SCTP_ERROR_INV_PARAM, and passed my 
test case.
I not do test add IPv4/IPv6 address to peer which supported IPv4/IPv6 
address, this is my fault .

Re: [PATCH] [SCTP]: Add address type check while process paramaters of ASCONF chunk

[David Miller]

From: Vlad Yasevich <vladislav.yasevich@hp.com>
Date: Tue, 13 May 2008 08:48:39 -0400

> Patrick McHardy wrote:
> > The patch is wrong and needs this fix on top.
> > 
> 
> Ack.  David, please apply.

Done, thanks everyone.