* [PATCH net v4] sctp: asconf's process should verify address parameter is in the beginning
@ 2015-08-26 21:03 Xin Long
2015-08-26 21:33 ` Vlad Yasevich
0 siblings, 1 reply; 6+ messages in thread
From: Xin Long @ 2015-08-26 21:03 UTC (permalink / raw)
To: network dev; +Cc: mleitner, davem, vyasevic
in sctp_process_asconf(), we get address parameter from the beginning of
the addip params. but we never check if it's really there. if the addr
param is not there, it still can pass sctp_verify_asconf(), then to be
handled by sctp_process_asconf(), it will not be safe.
so add detection in sctp_verify_asconf() to check the address parameter is in
the beginning, or return false to send abort.
Signed-off-by: Xin Long <lucien.xin@gmail.com>
Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
---
net/sctp/sm_make_chunk.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
index 06320c8..f3fc881 100644
--- a/net/sctp/sm_make_chunk.c
+++ b/net/sctp/sm_make_chunk.c
@@ -3132,12 +3132,14 @@ bool sctp_verify_asconf(const struct sctp_association *asoc,
case SCTP_PARAM_IPV4_ADDRESS:
if (length != sizeof(sctp_ipv4addr_param_t))
return false;
- addr_param_seen = true;
+ if (param.v == addip->addip_hdr.params)
+ addr_param_seen = true;
break;
case SCTP_PARAM_IPV6_ADDRESS:
if (length != sizeof(sctp_ipv6addr_param_t))
return false;
- addr_param_seen = true;
+ if (param.v == addip->addip_hdr.params)
+ addr_param_seen = true;
break;
case SCTP_PARAM_ADD_IP:
case SCTP_PARAM_DEL_IP:
--
2.1.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net v4] sctp: asconf's process should verify address parameter is in the beginning
2015-08-26 21:03 [PATCH net v4] sctp: asconf's process should verify address parameter is in the beginning Xin Long
@ 2015-08-26 21:33 ` Vlad Yasevich
2015-08-27 8:25 ` lucien xin
0 siblings, 1 reply; 6+ messages in thread
From: Vlad Yasevich @ 2015-08-26 21:33 UTC (permalink / raw)
To: Xin Long, network dev; +Cc: mleitner, davem
On 08/26/2015 05:03 PM, Xin Long wrote:
> in sctp_process_asconf(), we get address parameter from the beginning of
> the addip params. but we never check if it's really there. if the addr
> param is not there, it still can pass sctp_verify_asconf(), then to be
> handled by sctp_process_asconf(), it will not be safe.
>
> so add detection in sctp_verify_asconf() to check the address parameter is in
> the beginning, or return false to send abort.
>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
Acked-by: Vlad Yasevich <vyasevich@gmail.com>
-vlad
> ---
> net/sctp/sm_make_chunk.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
> index 06320c8..f3fc881 100644
> --- a/net/sctp/sm_make_chunk.c
> +++ b/net/sctp/sm_make_chunk.c
> @@ -3132,12 +3132,14 @@ bool sctp_verify_asconf(const struct sctp_association *asoc,
> case SCTP_PARAM_IPV4_ADDRESS:
> if (length != sizeof(sctp_ipv4addr_param_t))
> return false;
> - addr_param_seen = true;
> + if (param.v == addip->addip_hdr.params)
> + addr_param_seen = true;
> break;
> case SCTP_PARAM_IPV6_ADDRESS:
> if (length != sizeof(sctp_ipv6addr_param_t))
> return false;
> - addr_param_seen = true;
> + if (param.v == addip->addip_hdr.params)
> + addr_param_seen = true;
> break;
> case SCTP_PARAM_ADD_IP:
> case SCTP_PARAM_DEL_IP:
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net v4] sctp: asconf's process should verify address parameter is in the beginning
2015-08-26 21:33 ` Vlad Yasevich
@ 2015-08-27 8:25 ` lucien xin
0 siblings, 0 replies; 6+ messages in thread
From: lucien xin @ 2015-08-27 8:25 UTC (permalink / raw)
To: Vlad Yasevich; +Cc: network dev, Marcelo Ricardo Leitner, davem
On Thu, Aug 27, 2015 at 5:33 AM, Vlad Yasevich <vyasevic@redhat.com> wrote:
> On 08/26/2015 05:03 PM, Xin Long wrote:
>> in sctp_process_asconf(), we get address parameter from the beginning of
>> the addip params. but we never check if it's really there. if the addr
>> param is not there, it still can pass sctp_verify_asconf(), then to be
>> handled by sctp_process_asconf(), it will not be safe.
>>
>> so add detection in sctp_verify_asconf() to check the address parameter is in
>> the beginning, or return false to send abort.
>>
>> Signed-off-by: Xin Long <lucien.xin@gmail.com>
>> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
>
> Acked-by: Vlad Yasevich <vyasevich@gmail.com>
>
> -vlad
>
>> ---
>> net/sctp/sm_make_chunk.c | 6 ++++--
>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
>> index 06320c8..f3fc881 100644
>> --- a/net/sctp/sm_make_chunk.c
>> +++ b/net/sctp/sm_make_chunk.c
>> @@ -3132,12 +3132,14 @@ bool sctp_verify_asconf(const struct sctp_association *asoc,
>> case SCTP_PARAM_IPV4_ADDRESS:
>> if (length != sizeof(sctp_ipv4addr_param_t))
>> return false;
>> - addr_param_seen = true;
>> + if (param.v == addip->addip_hdr.params)
>> + addr_param_seen = true;
>> break;
>> case SCTP_PARAM_IPV6_ADDRESS:
>> if (length != sizeof(sctp_ipv6addr_param_t))
>> return false;
>> - addr_param_seen = true;
>> + if (param.v == addip->addip_hdr.params)
>> + addr_param_seen = true;
>> break;
>> case SCTP_PARAM_ADD_IP:
>> case SCTP_PARAM_DEL_IP:
>>
>
let's forget this one, I will repost.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH net v4] sctp: asconf's process should verify address parameter is in the beginning
@ 2015-08-27 8:26 Xin Long
2015-08-27 12:46 ` Vlad Yasevich
2015-08-27 21:00 ` David Miller
0 siblings, 2 replies; 6+ messages in thread
From: Xin Long @ 2015-08-27 8:26 UTC (permalink / raw)
To: network dev; +Cc: mleitner, davem, vyasevic
in sctp_process_asconf(), we get address parameter from the beginning of
the addip params. but we never check if it's really there. if the addr
param is not there, it still can pass sctp_verify_asconf(), then to be
handled by sctp_process_asconf(), it will not be safe.
so add a code in sctp_verify_asconf() to check the address parameter is in
the beginning, or return false to send abort.
note that this can also detect multiple address parameters, and reject it.
Signed-off-by: Xin Long <lucien.xin@gmail.com>
Signed-off-by: Marcelo Ricardo Leitner <mleitner@redhat.com>
---
net/sctp/sm_make_chunk.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
index 06320c8..a655ddc 100644
--- a/net/sctp/sm_make_chunk.c
+++ b/net/sctp/sm_make_chunk.c
@@ -3132,11 +3132,18 @@ bool sctp_verify_asconf(const struct sctp_association *asoc,
case SCTP_PARAM_IPV4_ADDRESS:
if (length != sizeof(sctp_ipv4addr_param_t))
return false;
+ /* ensure there is only one addr param and it's in the
+ * beginning of addip_hdr params, or we reject it.
+ */
+ if (param.v != addip->addip_hdr.params)
+ return false;
addr_param_seen = true;
break;
case SCTP_PARAM_IPV6_ADDRESS:
if (length != sizeof(sctp_ipv6addr_param_t))
return false;
+ if (param.v != addip->addip_hdr.params)
+ return false;
addr_param_seen = true;
break;
case SCTP_PARAM_ADD_IP:
--
2.1.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net v4] sctp: asconf's process should verify address parameter is in the beginning
2015-08-27 8:26 Xin Long
@ 2015-08-27 12:46 ` Vlad Yasevich
2015-08-27 21:00 ` David Miller
1 sibling, 0 replies; 6+ messages in thread
From: Vlad Yasevich @ 2015-08-27 12:46 UTC (permalink / raw)
To: Xin Long, network dev; +Cc: mleitner, davem
On 08/27/2015 04:26 AM, Xin Long wrote:
> in sctp_process_asconf(), we get address parameter from the beginning of
> the addip params. but we never check if it's really there. if the addr
> param is not there, it still can pass sctp_verify_asconf(), then to be
> handled by sctp_process_asconf(), it will not be safe.
>
> so add a code in sctp_verify_asconf() to check the address parameter is in
> the beginning, or return false to send abort.
>
> note that this can also detect multiple address parameters, and reject it.
>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> Signed-off-by: Marcelo Ricardo Leitner <mleitner@redhat.com>
Looks good to me.
Acked-by: Vlad Yasevich <vyasevich@gmail.com>
-vlad
> ---
> net/sctp/sm_make_chunk.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
> index 06320c8..a655ddc 100644
> --- a/net/sctp/sm_make_chunk.c
> +++ b/net/sctp/sm_make_chunk.c
> @@ -3132,11 +3132,18 @@ bool sctp_verify_asconf(const struct sctp_association *asoc,
> case SCTP_PARAM_IPV4_ADDRESS:
> if (length != sizeof(sctp_ipv4addr_param_t))
> return false;
> + /* ensure there is only one addr param and it's in the
> + * beginning of addip_hdr params, or we reject it.
> + */
> + if (param.v != addip->addip_hdr.params)
> + return false;
> addr_param_seen = true;
> break;
> case SCTP_PARAM_IPV6_ADDRESS:
> if (length != sizeof(sctp_ipv6addr_param_t))
> return false;
> + if (param.v != addip->addip_hdr.params)
> + return false;
> addr_param_seen = true;
> break;
> case SCTP_PARAM_ADD_IP:
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net v4] sctp: asconf's process should verify address parameter is in the beginning
2015-08-27 8:26 Xin Long
2015-08-27 12:46 ` Vlad Yasevich
@ 2015-08-27 21:00 ` David Miller
1 sibling, 0 replies; 6+ messages in thread
From: David Miller @ 2015-08-27 21:00 UTC (permalink / raw)
To: lucien.xin; +Cc: netdev, mleitner, vyasevic
From: Xin Long <lucien.xin@gmail.com>
Date: Thu, 27 Aug 2015 16:26:34 +0800
> in sctp_process_asconf(), we get address parameter from the beginning of
> the addip params. but we never check if it's really there. if the addr
> param is not there, it still can pass sctp_verify_asconf(), then to be
> handled by sctp_process_asconf(), it will not be safe.
>
> so add a code in sctp_verify_asconf() to check the address parameter is in
> the beginning, or return false to send abort.
>
> note that this can also detect multiple address parameters, and reject it.
>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> Signed-off-by: Marcelo Ricardo Leitner <mleitner@redhat.com>
Applied, thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-08-27 21:00 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-08-26 21:03 [PATCH net v4] sctp: asconf's process should verify address parameter is in the beginning Xin Long
2015-08-26 21:33 ` Vlad Yasevich
2015-08-27 8:25 ` lucien xin
-- strict thread matches above, loose matches on Subject: below --
2015-08-27 8:26 Xin Long
2015-08-27 12:46 ` Vlad Yasevich
2015-08-27 21:00 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).