All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bastien Philbert <bastienphilbert@gmail.com>
To: Daniel Borkmann <daniel@iogearbox.net>, vyasevich@gmail.com
Cc: nhorman@tuxdriver.com, davem@davemloft.net,
	linux-sctp@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] sctp: Fix error handling for switch statement case in the function sctp_cmd_interprete
Date: Tue, 05 Apr 2016 21:58:38 +0000	[thread overview]
Message-ID: <5704350E.5080403@gmail.com> (raw)
In-Reply-To: <570433F0.6040506@iogearbox.net>



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;
>>
>>
> 

WARNING: multiple messages have this Message-ID (diff)
From: Bastien Philbert <bastienphilbert@gmail.com>
To: Daniel Borkmann <daniel@iogearbox.net>, vyasevich@gmail.com
Cc: nhorman@tuxdriver.com, davem@davemloft.net,
	linux-sctp@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] sctp: Fix error handling for switch statement case in the function sctp_cmd_interprete
Date: Tue, 5 Apr 2016 17:58:38 -0400	[thread overview]
Message-ID: <5704350E.5080403@gmail.com> (raw)
In-Reply-To: <570433F0.6040506@iogearbox.net>



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;
>>
>>
> 

  reply	other threads:[~2016-04-05 21:58 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5704350E.5080403@gmail.com \
    --to=bastienphilbert@gmail.com \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-sctp@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=nhorman@tuxdriver.com \
    --cc=vyasevich@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.