* [PATCH] sctp: Fix error handling for switch statement case in the function sctp_cmd_interprete
@ 2016-04-05 21:36 ` Bastien Philbert
0 siblings, 0 replies; 14+ messages in thread
From: Bastien Philbert @ 2016-04-05 21:36 UTC (permalink / raw)
To: vyasevich; +Cc: nhorman, davem, linux-sctp, netdev, linux-kernel
This fixes error handling for the switch statement case
SCTP_CMD_SEND_PKT by making the error value of the call
to sctp_packet_transmit equal the variable error due to
this function being able to fail with a error code. In
addition allow the call to sctp_ootb_pkt_free afterwards
to free up the no longer in use sctp packet even if the
call to the function sctp_packet_transmit fails in order
to avoid a memory leak here for not freeing the sctp
Signed-off-by: Bastien Philbert <bastienphilbert@gmail.com>
---
net/sctp/sm_sideeffect.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c
index 7fe56d0..f3a8b58 100644
--- a/net/sctp/sm_sideeffect.c
+++ b/net/sctp/sm_sideeffect.c
@@ -1434,7 +1434,7 @@ static int sctp_cmd_interpreter(sctp_event_t event_type,
case SCTP_CMD_SEND_PKT:
/* Send a full packet to our peer. */
packet = cmd->obj.packet;
- sctp_packet_transmit(packet, gfp);
+ error = sctp_packet_transmit(packet, gfp);
sctp_ootb_pkt_free(packet);
break;
--
2.5.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH] sctp: Fix error handling for switch statement case in the function sctp_cmd_interprete
@ 2016-04-05 21:36 ` Bastien Philbert
0 siblings, 0 replies; 14+ messages in thread
From: Bastien Philbert @ 2016-04-05 21:36 UTC (permalink / raw)
To: vyasevich; +Cc: nhorman, davem, linux-sctp, netdev, linux-kernel
This fixes error handling for the switch statement case
SCTP_CMD_SEND_PKT by making the error value of the call
to sctp_packet_transmit equal the variable error due to
this function being able to fail with a error code. In
addition allow the call to sctp_ootb_pkt_free afterwards
to free up the no longer in use sctp packet even if the
call to the function sctp_packet_transmit fails in order
to avoid a memory leak here for not freeing the sctp
Signed-off-by: Bastien Philbert <bastienphilbert@gmail.com>
---
net/sctp/sm_sideeffect.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c
index 7fe56d0..f3a8b58 100644
--- a/net/sctp/sm_sideeffect.c
+++ b/net/sctp/sm_sideeffect.c
@@ -1434,7 +1434,7 @@ static int sctp_cmd_interpreter(sctp_event_t event_type,
case SCTP_CMD_SEND_PKT:
/* Send a full packet to our peer. */
packet = cmd->obj.packet;
- sctp_packet_transmit(packet, gfp);
+ error = sctp_packet_transmit(packet, gfp);
sctp_ootb_pkt_free(packet);
break;
--
2.5.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] sctp: Fix error handling for switch statement case in the function sctp_cmd_interprete
2016-04-05 21:36 ` Bastien Philbert
@ 2016-04-05 21:53 ` Daniel Borkmann
-1 siblings, 0 replies; 14+ messages in thread
From: Daniel Borkmann @ 2016-04-05 21:53 UTC (permalink / raw)
To: Bastien Philbert, vyasevich
Cc: nhorman, davem, linux-sctp, netdev, linux-kernel
On 04/05/2016 11:36 PM, Bastien Philbert wrote:
> This fixes error handling for the switch statement case
> SCTP_CMD_SEND_PKT by making the error value of the call
> to sctp_packet_transmit equal the variable error due to
> this function being able to fail with a error code. In
What actual issue have you observed that you fix?
> addition allow the call to sctp_ootb_pkt_free afterwards
> to free up the no longer in use sctp packet even if the
> call to the function sctp_packet_transmit fails in order
> to avoid a memory leak here for not freeing the sctp
Not sure how this relates to your code?
> Signed-off-by: Bastien Philbert <bastienphilbert@gmail.com>
> ---
> net/sctp/sm_sideeffect.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c
> index 7fe56d0..f3a8b58 100644
> --- a/net/sctp/sm_sideeffect.c
> +++ b/net/sctp/sm_sideeffect.c
> @@ -1434,7 +1434,7 @@ static int sctp_cmd_interpreter(sctp_event_t event_type,
> case SCTP_CMD_SEND_PKT:
> /* Send a full packet to our peer. */
> packet = cmd->obj.packet;
> - sctp_packet_transmit(packet, gfp);
> + error = sctp_packet_transmit(packet, gfp);
> sctp_ootb_pkt_free(packet);
> break;
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] sctp: Fix error handling for switch statement case in the function sctp_cmd_interprete
@ 2016-04-05 21:53 ` Daniel Borkmann
0 siblings, 0 replies; 14+ messages in thread
From: Daniel Borkmann @ 2016-04-05 21:53 UTC (permalink / raw)
To: Bastien Philbert, vyasevich
Cc: nhorman, davem, linux-sctp, netdev, linux-kernel
On 04/05/2016 11:36 PM, Bastien Philbert wrote:
> This fixes error handling for the switch statement case
> SCTP_CMD_SEND_PKT by making the error value of the call
> to sctp_packet_transmit equal the variable error due to
> this function being able to fail with a error code. In
What actual issue have you observed that you fix?
> addition allow the call to sctp_ootb_pkt_free afterwards
> to free up the no longer in use sctp packet even if the
> call to the function sctp_packet_transmit fails in order
> to avoid a memory leak here for not freeing the sctp
Not sure how this relates to your code?
> Signed-off-by: Bastien Philbert <bastienphilbert@gmail.com>
> ---
> net/sctp/sm_sideeffect.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c
> index 7fe56d0..f3a8b58 100644
> --- a/net/sctp/sm_sideeffect.c
> +++ b/net/sctp/sm_sideeffect.c
> @@ -1434,7 +1434,7 @@ static int sctp_cmd_interpreter(sctp_event_t event_type,
> case SCTP_CMD_SEND_PKT:
> /* Send a full packet to our peer. */
> packet = cmd->obj.packet;
> - sctp_packet_transmit(packet, gfp);
> + error = sctp_packet_transmit(packet, gfp);
> sctp_ootb_pkt_free(packet);
> break;
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] sctp: Fix error handling for switch statement case in the function sctp_cmd_interprete
2016-04-05 21:53 ` Daniel Borkmann
@ 2016-04-05 21:58 ` Bastien Philbert
-1 siblings, 0 replies; 14+ messages in thread
From: Bastien Philbert @ 2016-04-05 21:58 UTC (permalink / raw)
To: Daniel Borkmann, vyasevich
Cc: nhorman, davem, linux-sctp, netdev, linux-kernel
On 2016-04-05 05:53 PM, Daniel Borkmann wrote:
> On 04/05/2016 11:36 PM, Bastien Philbert wrote:
>> This fixes error handling for the switch statement case
>> SCTP_CMD_SEND_PKT by making the error value of the call
>> to sctp_packet_transmit equal the variable error due to
>> this function being able to fail with a error code. In
>
> What actual issue have you observed that you fix?
>
The issue here is basically that sctp_packet_transmit
can return a error if it unsuccessfully transmit the
sk_buff as a parameter. Seems that we should signal
the user/caller(s) when a sctp packet transmission
fails here. If you would like I can resend with a better
commit message in a V2 if this explains the issue better.
Bastien
>> addition allow the call to sctp_ootb_pkt_free afterwards
>> to free up the no longer in use sctp packet even if the
>> call to the function sctp_packet_transmit fails in order
>> to avoid a memory leak here for not freeing the sctp
>
> Not sure how this relates to your code?
>
>> Signed-off-by: Bastien Philbert <bastienphilbert@gmail.com>
>> ---
>> net/sctp/sm_sideeffect.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c
>> index 7fe56d0..f3a8b58 100644
>> --- a/net/sctp/sm_sideeffect.c
>> +++ b/net/sctp/sm_sideeffect.c
>> @@ -1434,7 +1434,7 @@ static int sctp_cmd_interpreter(sctp_event_t event_type,
>> case SCTP_CMD_SEND_PKT:
>> /* Send a full packet to our peer. */
>> packet = cmd->obj.packet;
>> - sctp_packet_transmit(packet, gfp);
>> + error = sctp_packet_transmit(packet, gfp);
>> sctp_ootb_pkt_free(packet);
>> break;
>>
>>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] sctp: Fix error handling for switch statement case in the function sctp_cmd_interprete
@ 2016-04-05 21:58 ` Bastien Philbert
0 siblings, 0 replies; 14+ messages in thread
From: Bastien Philbert @ 2016-04-05 21:58 UTC (permalink / raw)
To: Daniel Borkmann, vyasevich
Cc: nhorman, davem, linux-sctp, netdev, linux-kernel
On 2016-04-05 05:53 PM, Daniel Borkmann wrote:
> On 04/05/2016 11:36 PM, Bastien Philbert wrote:
>> This fixes error handling for the switch statement case
>> SCTP_CMD_SEND_PKT by making the error value of the call
>> to sctp_packet_transmit equal the variable error due to
>> this function being able to fail with a error code. In
>
> What actual issue have you observed that you fix?
>
The issue here is basically that sctp_packet_transmit
can return a error if it unsuccessfully transmit the
sk_buff as a parameter. Seems that we should signal
the user/caller(s) when a sctp packet transmission
fails here. If you would like I can resend with a better
commit message in a V2 if this explains the issue better.
Bastien
>> addition allow the call to sctp_ootb_pkt_free afterwards
>> to free up the no longer in use sctp packet even if the
>> call to the function sctp_packet_transmit fails in order
>> to avoid a memory leak here for not freeing the sctp
>
> Not sure how this relates to your code?
>
>> Signed-off-by: Bastien Philbert <bastienphilbert@gmail.com>
>> ---
>> net/sctp/sm_sideeffect.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c
>> index 7fe56d0..f3a8b58 100644
>> --- a/net/sctp/sm_sideeffect.c
>> +++ b/net/sctp/sm_sideeffect.c
>> @@ -1434,7 +1434,7 @@ static int sctp_cmd_interpreter(sctp_event_t event_type,
>> case SCTP_CMD_SEND_PKT:
>> /* Send a full packet to our peer. */
>> packet = cmd->obj.packet;
>> - sctp_packet_transmit(packet, gfp);
>> + error = sctp_packet_transmit(packet, gfp);
>> sctp_ootb_pkt_free(packet);
>> break;
>>
>>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] sctp: Fix error handling for switch statement case in the function sctp_cmd_interprete
2016-04-05 21:53 ` Daniel Borkmann
@ 2016-04-05 23:29 ` David Miller
-1 siblings, 0 replies; 14+ messages in thread
From: David Miller @ 2016-04-05 23:29 UTC (permalink / raw)
To: daniel
Cc: bastienphilbert, vyasevich, nhorman, linux-sctp, netdev,
linux-kernel
From: Daniel Borkmann <daniel@iogearbox.net>
Date: Tue, 05 Apr 2016 23:53:52 +0200
> On 04/05/2016 11:36 PM, Bastien Philbert wrote:
>> This fixes error handling for the switch statement case
>> SCTP_CMD_SEND_PKT by making the error value of the call
>> to sctp_packet_transmit equal the variable error due to
>> this function being able to fail with a error code. In
>
> What actual issue have you observed that you fix?
>
>> addition allow the call to sctp_ootb_pkt_free afterwards
>> to free up the no longer in use sctp packet even if the
>> call to the function sctp_packet_transmit fails in order
>> to avoid a memory leak here for not freeing the sctp
>
> Not sure how this relates to your code?
Bastien, I'm seeing a clear negative pattern with the bug fixes
you are submitting.
Just now you submitted the ICMP change which obviously was never
tested because it tried to take the RTNL mutex in atomic context,
and now this sctp thing.
If you don't start actually testing your changes and expalining
clearly what the problem actually is, how you discovered it,
and how you actually tested your patch, I will start completely
ignoring your patch submissions.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] sctp: Fix error handling for switch statement case in the function sctp_cmd_interprete
@ 2016-04-05 23:29 ` David Miller
0 siblings, 0 replies; 14+ messages in thread
From: David Miller @ 2016-04-05 23:29 UTC (permalink / raw)
To: daniel
Cc: bastienphilbert, vyasevich, nhorman, linux-sctp, netdev,
linux-kernel
From: Daniel Borkmann <daniel@iogearbox.net>
Date: Tue, 05 Apr 2016 23:53:52 +0200
> On 04/05/2016 11:36 PM, Bastien Philbert wrote:
>> This fixes error handling for the switch statement case
>> SCTP_CMD_SEND_PKT by making the error value of the call
>> to sctp_packet_transmit equal the variable error due to
>> this function being able to fail with a error code. In
>
> What actual issue have you observed that you fix?
>
>> addition allow the call to sctp_ootb_pkt_free afterwards
>> to free up the no longer in use sctp packet even if the
>> call to the function sctp_packet_transmit fails in order
>> to avoid a memory leak here for not freeing the sctp
>
> Not sure how this relates to your code?
Bastien, I'm seeing a clear negative pattern with the bug fixes
you are submitting.
Just now you submitted the ICMP change which obviously was never
tested because it tried to take the RTNL mutex in atomic context,
and now this sctp thing.
If you don't start actually testing your changes and expalining
clearly what the problem actually is, how you discovered it,
and how you actually tested your patch, I will start completely
ignoring your patch submissions.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] sctp: Fix error handling for switch statement case in the function sctp_cmd_interprete
2016-04-05 23:29 ` David Miller
@ 2016-04-05 23:34 ` Bastien Philbert
-1 siblings, 0 replies; 14+ messages in thread
From: Bastien Philbert @ 2016-04-05 23:34 UTC (permalink / raw)
To: David Miller, daniel; +Cc: vyasevich, nhorman, linux-sctp, netdev, linux-kernel
On 2016-04-05 07:29 PM, David Miller wrote:
> From: Daniel Borkmann <daniel@iogearbox.net>
> Date: Tue, 05 Apr 2016 23:53:52 +0200
>
>> On 04/05/2016 11:36 PM, Bastien Philbert wrote:
>>> This fixes error handling for the switch statement case
>>> SCTP_CMD_SEND_PKT by making the error value of the call
>>> to sctp_packet_transmit equal the variable error due to
>>> this function being able to fail with a error code. In
>>
>> What actual issue have you observed that you fix?
>>
>>> addition allow the call to sctp_ootb_pkt_free afterwards
>>> to free up the no longer in use sctp packet even if the
>>> call to the function sctp_packet_transmit fails in order
>>> to avoid a memory leak here for not freeing the sctp
>>
>> Not sure how this relates to your code?
>
> Bastien, I'm seeing a clear negative pattern with the bug fixes
> you are submitting.
>
> Just now you submitted the ICMP change which obviously was never
> tested because it tried to take the RTNL mutex in atomic context,
> and now this sctp thing.
>
> If you don't start actually testing your changes and expalining
> clearly what the problem actually is, how you discovered it,
> and how you actually tested your patch, I will start completely
> ignoring your patch submissions.
>
Ok sure I will be more careful with my future patches. Sorry about those
two patches :(.
Bastien
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] sctp: Fix error handling for switch statement case in the function sctp_cmd_interprete
@ 2016-04-05 23:34 ` Bastien Philbert
0 siblings, 0 replies; 14+ messages in thread
From: Bastien Philbert @ 2016-04-05 23:34 UTC (permalink / raw)
To: David Miller, daniel; +Cc: vyasevich, nhorman, linux-sctp, netdev, linux-kernel
On 2016-04-05 07:29 PM, David Miller wrote:
> From: Daniel Borkmann <daniel@iogearbox.net>
> Date: Tue, 05 Apr 2016 23:53:52 +0200
>
>> On 04/05/2016 11:36 PM, Bastien Philbert wrote:
>>> This fixes error handling for the switch statement case
>>> SCTP_CMD_SEND_PKT by making the error value of the call
>>> to sctp_packet_transmit equal the variable error due to
>>> this function being able to fail with a error code. In
>>
>> What actual issue have you observed that you fix?
>>
>>> addition allow the call to sctp_ootb_pkt_free afterwards
>>> to free up the no longer in use sctp packet even if the
>>> call to the function sctp_packet_transmit fails in order
>>> to avoid a memory leak here for not freeing the sctp
>>
>> Not sure how this relates to your code?
>
> Bastien, I'm seeing a clear negative pattern with the bug fixes
> you are submitting.
>
> Just now you submitted the ICMP change which obviously was never
> tested because it tried to take the RTNL mutex in atomic context,
> and now this sctp thing.
>
> If you don't start actually testing your changes and expalining
> clearly what the problem actually is, how you discovered it,
> and how you actually tested your patch, I will start completely
> ignoring your patch submissions.
>
Ok sure I will be more careful with my future patches. Sorry about those
two patches :(.
Bastien
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] sctp: Fix error handling for switch statement case in the function sctp_cmd_interprete
2016-04-05 21:36 ` Bastien Philbert
@ 2016-04-05 22:12 ` Marcelo Ricardo Leitner
-1 siblings, 0 replies; 14+ messages in thread
From: Marcelo Ricardo Leitner @ 2016-04-05 22:12 UTC (permalink / raw)
To: Bastien Philbert
Cc: vyasevich, nhorman, davem, linux-sctp, netdev, linux-kernel
On Tue, Apr 05, 2016 at 05:36:41PM -0400, Bastien Philbert wrote:
> This fixes error handling for the switch statement case
> SCTP_CMD_SEND_PKT by making the error value of the call
> to sctp_packet_transmit equal the variable error due to
> this function being able to fail with a error code. In
> addition allow the call to sctp_ootb_pkt_free afterwards
> to free up the no longer in use sctp packet even if the
> call to the function sctp_packet_transmit fails in order
> to avoid a memory leak here for not freeing the sctp
This leak shouldn't exist as sctp_packet_transmit() will free the packet
if it returns ENOMEM, through the nomem: handling.
But about making it visible to the user, that looks interesting to me
although I cannot foresee yet its effects, like the comment at the end
of sctp_packet_transmit() on not returning EHOSTUNREACH. Did you check
it?
>
> Signed-off-by: Bastien Philbert <bastienphilbert@gmail.com>
> ---
> net/sctp/sm_sideeffect.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c
> index 7fe56d0..f3a8b58 100644
> --- a/net/sctp/sm_sideeffect.c
> +++ b/net/sctp/sm_sideeffect.c
> @@ -1434,7 +1434,7 @@ static int sctp_cmd_interpreter(sctp_event_t event_type,
> case SCTP_CMD_SEND_PKT:
> /* Send a full packet to our peer. */
> packet = cmd->obj.packet;
> - sctp_packet_transmit(packet, gfp);
> + error = sctp_packet_transmit(packet, gfp);
> sctp_ootb_pkt_free(packet);
> break;
>
> --
> 2.5.0
>
> --
> 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] 14+ messages in thread
* Re: [PATCH] sctp: Fix error handling for switch statement case in the function sctp_cmd_interprete
@ 2016-04-05 22:12 ` Marcelo Ricardo Leitner
0 siblings, 0 replies; 14+ messages in thread
From: Marcelo Ricardo Leitner @ 2016-04-05 22:12 UTC (permalink / raw)
To: Bastien Philbert
Cc: vyasevich, nhorman, davem, linux-sctp, netdev, linux-kernel
On Tue, Apr 05, 2016 at 05:36:41PM -0400, Bastien Philbert wrote:
> This fixes error handling for the switch statement case
> SCTP_CMD_SEND_PKT by making the error value of the call
> to sctp_packet_transmit equal the variable error due to
> this function being able to fail with a error code. In
> addition allow the call to sctp_ootb_pkt_free afterwards
> to free up the no longer in use sctp packet even if the
> call to the function sctp_packet_transmit fails in order
> to avoid a memory leak here for not freeing the sctp
This leak shouldn't exist as sctp_packet_transmit() will free the packet
if it returns ENOMEM, through the nomem: handling.
But about making it visible to the user, that looks interesting to me
although I cannot foresee yet its effects, like the comment at the end
of sctp_packet_transmit() on not returning EHOSTUNREACH. Did you check
it?
>
> Signed-off-by: Bastien Philbert <bastienphilbert@gmail.com>
> ---
> net/sctp/sm_sideeffect.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c
> index 7fe56d0..f3a8b58 100644
> --- a/net/sctp/sm_sideeffect.c
> +++ b/net/sctp/sm_sideeffect.c
> @@ -1434,7 +1434,7 @@ static int sctp_cmd_interpreter(sctp_event_t event_type,
> case SCTP_CMD_SEND_PKT:
> /* Send a full packet to our peer. */
> packet = cmd->obj.packet;
> - sctp_packet_transmit(packet, gfp);
> + error = sctp_packet_transmit(packet, gfp);
> sctp_ootb_pkt_free(packet);
> break;
>
> --
> 2.5.0
>
> --
> 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] 14+ messages in thread
* Re: [PATCH] sctp: Fix error handling for switch statement case in the function sctp_cmd_interprete
2016-04-05 22:12 ` Marcelo Ricardo Leitner
@ 2016-04-05 23:03 ` Bastien Philbert
-1 siblings, 0 replies; 14+ messages in thread
From: Bastien Philbert @ 2016-04-05 23:03 UTC (permalink / raw)
To: Marcelo Ricardo Leitner
Cc: vyasevich, nhorman, davem, linux-sctp, netdev, linux-kernel
On 2016-04-05 06:12 PM, Marcelo Ricardo Leitner wrote:
> On Tue, Apr 05, 2016 at 05:36:41PM -0400, Bastien Philbert wrote:
>> This fixes error handling for the switch statement case
>> SCTP_CMD_SEND_PKT by making the error value of the call
>> to sctp_packet_transmit equal the variable error due to
>> this function being able to fail with a error code. In
>> addition allow the call to sctp_ootb_pkt_free afterwards
>> to free up the no longer in use sctp packet even if the
>> call to the function sctp_packet_transmit fails in order
>> to avoid a memory leak here for not freeing the sctp
>
> This leak shouldn't exist as sctp_packet_transmit() will free the packet
> if it returns ENOMEM, through the nomem: handling.
>
> But about making it visible to the user, that looks interesting to me
> although I cannot foresee yet its effects, like the comment at the end
> of sctp_packet_transmit() on not returning EHOSTUNREACH. Did you check
> it?
>
I was aware of the -EHOSTUNREACH issue but assumed that this needs to be
known to functions internal to the kernel. TO rephase does it matter if
the callers of this function known if sctp_packet_transmit or care if it
fails or is this just unnecessary as we do cleanup else where which is
enough so the new error check is not needed? Again if their is a certain
test would like me to run on this patch too to make sure it's OK I don't
mind, just let me known :).
Cheers,
Bastien
>>
>> Signed-off-by: Bastien Philbert <bastienphilbert@gmail.com>
>> ---
>> net/sctp/sm_sideeffect.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c
>> index 7fe56d0..f3a8b58 100644
>> --- a/net/sctp/sm_sideeffect.c
>> +++ b/net/sctp/sm_sideeffect.c
>> @@ -1434,7 +1434,7 @@ static int sctp_cmd_interpreter(sctp_event_t event_type,
>> case SCTP_CMD_SEND_PKT:
>> /* Send a full packet to our peer. */
>> packet = cmd->obj.packet;
>> - sctp_packet_transmit(packet, gfp);
>> + error = sctp_packet_transmit(packet, gfp);
>> sctp_ootb_pkt_free(packet);
>> break;
>>
>> --
>> 2.5.0
>>
>> --
>> 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] 14+ messages in thread
* Re: [PATCH] sctp: Fix error handling for switch statement case in the function sctp_cmd_interprete
@ 2016-04-05 23:03 ` Bastien Philbert
0 siblings, 0 replies; 14+ messages in thread
From: Bastien Philbert @ 2016-04-05 23:03 UTC (permalink / raw)
To: Marcelo Ricardo Leitner
Cc: vyasevich, nhorman, davem, linux-sctp, netdev, linux-kernel
On 2016-04-05 06:12 PM, Marcelo Ricardo Leitner wrote:
> On Tue, Apr 05, 2016 at 05:36:41PM -0400, Bastien Philbert wrote:
>> This fixes error handling for the switch statement case
>> SCTP_CMD_SEND_PKT by making the error value of the call
>> to sctp_packet_transmit equal the variable error due to
>> this function being able to fail with a error code. In
>> addition allow the call to sctp_ootb_pkt_free afterwards
>> to free up the no longer in use sctp packet even if the
>> call to the function sctp_packet_transmit fails in order
>> to avoid a memory leak here for not freeing the sctp
>
> This leak shouldn't exist as sctp_packet_transmit() will free the packet
> if it returns ENOMEM, through the nomem: handling.
>
> But about making it visible to the user, that looks interesting to me
> although I cannot foresee yet its effects, like the comment at the end
> of sctp_packet_transmit() on not returning EHOSTUNREACH. Did you check
> it?
>
I was aware of the -EHOSTUNREACH issue but assumed that this needs to be
known to functions internal to the kernel. TO rephase does it matter if
the callers of this function known if sctp_packet_transmit or care if it
fails or is this just unnecessary as we do cleanup else where which is
enough so the new error check is not needed? Again if their is a certain
test would like me to run on this patch too to make sure it's OK I don't
mind, just let me known :).
Cheers,
Bastien
>>
>> Signed-off-by: Bastien Philbert <bastienphilbert@gmail.com>
>> ---
>> net/sctp/sm_sideeffect.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c
>> index 7fe56d0..f3a8b58 100644
>> --- a/net/sctp/sm_sideeffect.c
>> +++ b/net/sctp/sm_sideeffect.c
>> @@ -1434,7 +1434,7 @@ static int sctp_cmd_interpreter(sctp_event_t event_type,
>> case SCTP_CMD_SEND_PKT:
>> /* Send a full packet to our peer. */
>> packet = cmd->obj.packet;
>> - sctp_packet_transmit(packet, gfp);
>> + error = sctp_packet_transmit(packet, gfp);
>> sctp_ootb_pkt_free(packet);
>> break;
>>
>> --
>> 2.5.0
>>
>> --
>> 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] 14+ messages in thread
end of thread, other threads:[~2016-04-05 23:34 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-04-05 21:36 [PATCH] sctp: Fix error handling for switch statement case in the function sctp_cmd_interprete Bastien Philbert
2016-04-05 21:36 ` Bastien Philbert
2016-04-05 21:53 ` Daniel Borkmann
2016-04-05 21:53 ` Daniel Borkmann
2016-04-05 21:58 ` Bastien Philbert
2016-04-05 21:58 ` Bastien Philbert
2016-04-05 23:29 ` David Miller
2016-04-05 23:29 ` David Miller
2016-04-05 23:34 ` Bastien Philbert
2016-04-05 23:34 ` Bastien Philbert
2016-04-05 22:12 ` Marcelo Ricardo Leitner
2016-04-05 22:12 ` Marcelo Ricardo Leitner
2016-04-05 23:03 ` Bastien Philbert
2016-04-05 23:03 ` Bastien Philbert
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.