[PATCH CLEANUP] sctp: fix the comment of sctp_sf_violation_paramlen()

[PATCH CLEANUP] sctp: fix the comment of sctp_sf_violation_paramlen()

[Shan Wei]

The comment about sctp_sf_violation_paramlen() is wrong.

Invalid length should be identified as it does not equal actual length
of a given parameter. The actual length of a given parameter is not including
the padding part.


Signed-off-by: Shan Wei <shanwei@cn.fujitsu.com>
---
 net/sctp/sm_statefuns.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
index edb71e6..8774a6e 100644
--- a/net/sctp/sm_statefuns.c
+++ b/net/sctp/sm_statefuns.c
@@ -4355,8 +4355,8 @@ static sctp_disposition_t sctp_sf_violation_chunklen(
 
 /*
  * Handle a protocol violation when the parameter length is invalid.
- * "Invalid" length is identified as smaller than the minimal length a
- * given parameter can be.
+ * "Invalid" length is identified as it does not equal actual length
+ * of a given parameter.
  */
 static sctp_disposition_t sctp_sf_violation_paramlen(
 				     const struct sctp_endpoint *ep,
-- 
1.6.3.3

Re: [PATCH CLEANUP] sctp: fix the comment of sctp_sf_violation_paramlen()

[Wei Yongjun]

于 2010年05月17日 14:08, Shan Wei 写道:
> The comment about sctp_sf_violation_paramlen() is wrong.
>
> Invalid length should be identified as it does not equal actual length
> of a given parameter. The actual length of a given parameter is not including
> the padding part.
>   

Not right. If the param length is larger then what is request, we
also treat it as correct. And for unkonw params, we just check
the length larger then the minimal length.

>
> Signed-off-by: Shan Wei <shanwei@cn.fujitsu.com>
> ---
>  net/sctp/sm_statefuns.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
> index edb71e6..8774a6e 100644
> --- a/net/sctp/sm_statefuns.c
> +++ b/net/sctp/sm_statefuns.c
> @@ -4355,8 +4355,8 @@ static sctp_disposition_t sctp_sf_violation_chunklen(
>  
>  /*
>   * Handle a protocol violation when the parameter length is invalid.
> - * "Invalid" length is identified as smaller than the minimal length a
> - * given parameter can be.
> + * "Invalid" length is identified as it does not equal actual length
> + * of a given parameter.
>   */
>  static sctp_disposition_t sctp_sf_violation_paramlen(
>  				     const struct sctp_endpoint *ep,
>   

Re: [PATCH CLEANUP] sctp: fix the comment of sctp_sf_violation_paramlen()

[Shan Wei]

Wei Yongjun wrote, at 05/17/2010 02:14 PM:
> 
> 于 2010年05月17日 14:08, Shan Wei 写道:
>> The comment about sctp_sf_violation_paramlen() is wrong.
>>
>> Invalid length should be identified as it does not equal actual length
>> of a given parameter. The actual length of a given parameter is not including
>> the padding part.
>>   
> 
> Not right. If the param length is larger then what is request, we
> also treat it as correct. And for unkonw params, we just check
> the length larger then the minimal length.

From the code of sctp_verify_asconf(), I saw that if length value of any parameter in chunk
is not equal true length, sctp_verify_asconf() return 0.

@@sctp_verify_asconf()
3045                 if (param.v > chunk_end - length ||
3046                     length < sizeof(sctp_paramhdr_t))
3047                         return 0;


-- 
Best Regards
-----
Shan Wei

Re: [PATCH CLEANUP] sctp: fix the comment of sctp_sf_violation_paramlen()

[Vlad Yasevich]

Shan Wei wrote:
> Wei Yongjun wrote, at 05/17/2010 02:14 PM:
>> 于 2010年05月17日 14:08, Shan Wei 写道:
>>> The comment about sctp_sf_violation_paramlen() is wrong.
>>>
>>> Invalid length should be identified as it does not equal actual length
>>> of a given parameter. The actual length of a given parameter is not including
>>> the padding part.
>>>   
>> Not right. If the param length is larger then what is request, we
>> also treat it as correct. And for unkonw params, we just check
>> the length larger then the minimal length.
> 
> From the code of sctp_verify_asconf(), I saw that if length value of any parameter in chunk
> is not equal true length, sctp_verify_asconf() return 0.
> 
> @@sctp_verify_asconf()
> 3045                 if (param.v > chunk_end - length ||
> 3046                     length < sizeof(sctp_paramhdr_t))
> 3047                         return 0;
> 

This doesn't check the 'true' length of the parameter.  It makes sure that
the length specified is at least as long as sctp_paramhdr_t (minimum length),
and not longer then the end of the chunk.  It makes no assumptions about
the actual length of the parameter.

-vlad

> 

Re: [PATCH CLEANUP] sctp: fix the comment of sctp_sf_violation_paramlen()

[Shan Wei]

Vlad Yasevich wrote, at 05/17/2010 10:30 PM:
>> @@sctp_verify_asconf()
>> 3045                 if (param.v > chunk_end - length ||
>> 3046                     length < sizeof(sctp_paramhdr_t))
>> 3047                         return 0;
>>
> 
> This doesn't check the 'true' length of the parameter.  It makes sure that
> the length specified is at least as long as sctp_paramhdr_t (minimum length),
> and not longer then the end of the chunk.  It makes no assumptions about
> the actual length of the parameter.

Thanks for your exact description.

How about fixing like this?

diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
index 565a690..32e2c34 100644
--- a/net/sctp/sm_statefuns.c
+++ b/net/sctp/sm_statefuns.c
@@ -4329,8 +4329,9 @@ static sctp_disposition_t sctp_sf_violation_chunklen(
 
 /*
  * Handle a protocol violation when the parameter length is invalid.
- * "Invalid" length is identified as smaller than the minimal length a
- * given parameter can be.
+ * If the length is smaller than the minimum length of a given parameter,
+ * or accumulated length in multi parameters exceeds the end of the chunk,
+ * the length is considered as invalid.
  */
 static sctp_disposition_t sctp_sf_violation_paramlen(
 				     const struct sctp_endpoint *ep,

-- 
Best Regards
-----
Shan Wei


> 
> -vlad
> 
> 
>