[PATCH] Memory leak in smpp_pdu.c

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] Memory leak in smpp_pdu.c

Donald Jackson-6
Hi all,

I have been assisting with a KSMPPD implementation and couldn't figure out why the application was leaking memory.

After a long search I found that an SMPP client was sending the same TLV multiple times which was causing lost pointers/memory leaks.

I believe this would effect all Kannel derivatives (OpenSMPPBox, Commercial SMPPBox and Bearerbox) in the cases where an incorrectly implemented (or malicious?) client or gateway could be the cause of memory leaks.

Herewith patch to fix.

Thanks,
Donald

smpp_pdu_tlv_memory_leak.patch (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

RE: [PATCH] Memory leak in smpp_pdu.c

Rene Kluwen

+1 from me.

 

Van: devel [mailto:[hidden email]] Namens Donald Jackson
Verzonden: zaterdag 24 september 2016 12:00
Aan: kannel_dev_mailinglist <[hidden email]>
Onderwerp: [PATCH] Memory leak in smpp_pdu.c

 

Hi all,

 

I have been assisting with a KSMPPD implementation and couldn't figure out why the application was leaking memory.

 

After a long search I found that an SMPP client was sending the same TLV multiple times which was causing lost pointers/memory leaks.

 

I believe this would effect all Kannel derivatives (OpenSMPPBox, Commercial SMPPBox and Bearerbox) in the cases where an incorrectly implemented (or malicious?) client or gateway could be the cause of memory leaks.

 

Herewith patch to fix.

 

Thanks,

Donald

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Memory leak in smpp_pdu.c

Stipe Tolj-2
Am 24.09.2016 13:55, schrieb Rene Kluwen:
> +1 from me.

yep, good catch Donald.

+1 for it.

If no objections, will commit.

--
Best Regards,
Stipe Tolj

-------------------------------------------------------------------
Düsseldorf, NRW, Germany

Kannel Foundation                 tolj.org system architecture
http://www.kannel.org/            http://www.tolj.org/

stolj at kannel.org               st at tolj.org
-------------------------------------------------------------------

Reply | Threaded
Open this post in threaded view
|

RE: [PATCH] Memory leak in smpp_pdu.c

amalysh
Hi,

good catch! But there is a question: should we really accept such wrong PDUs or reject them?
IMHO rejecting those would be the correct behavior but I don't see in SMPP spec that case described.

Thanks,
Alexander Malysh
 

From: [hidden email]
Sent: Monday, September 26, 2016 07:24 PM
To:
Cc: [hidden email]
Subject: Re: [PATCH] Memory leak in smpp_pdu.c
Am 24.09.2016 13:55, schrieb Rene Kluwen:
> +1 from me.

yep, good catch Donald.

+1 for it.

If no objections, will commit.

--
Best Regards,
Stipe Tolj

-------------------------------------------------------------------
Düsseldorf, NRW, Germany

Kannel Foundation tolj.org system architecture
http://www.kannel.org/ http://www.tolj.org/

stolj at kannel.org st at tolj.org
-------------------------------------------------------------------
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Memory leak in smpp_pdu.c

Donald Jackson-4
Hi Alex,

Personally I think we should not reject PDU's that have this case. While it is pointless to send the same TLV multiple times in our case, we have established that this behavior is implicitly 'ok' by having Kannel accept these since the meta data patch (+- 7 years now). Changing this behavior would potentially cause issues for users with no real gain.

Thanks,
Donald

On 18 October 2016 at 13:18, Alexander Malysh <[hidden email]> wrote:
Hi,

good catch! But there is a question: should we really accept such wrong PDUs or reject them?
IMHO rejecting those would be the correct behavior but I don't see in SMPP spec that case described.

Thanks,
Alexander Malysh
 

From: [hidden email]
Sent: Monday, September 26, 2016 07:24 PM
To:
Cc: [hidden email]
Subject: Re: [PATCH] Memory leak in smpp_pdu.c
Am 24.09.2016 13:55, schrieb Rene Kluwen:
> +1 from me.

yep, good catch Donald.

+1 for it.

If no objections, will commit.

--
Best Regards,
Stipe Tolj

-------------------------------------------------------------------
Düsseldorf, NRW, Germany

Kannel Foundation tolj.org system architecture
http://www.kannel.org/ http://www.tolj.org/

stolj at kannel.org st at tolj.org
-------------------------------------------------------------------



--
Donald Jackson
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Memory leak in smpp_pdu.c

amalysh
Hi,

make sense, commited to svn.

Alex

Am 18.10.2016 um 22:37 schrieb Donald Jackson <[hidden email]>:

Hi Alex,

Personally I think we should not reject PDU's that have this case. While it is pointless to send the same TLV multiple times in our case, we have established that this behavior is implicitly 'ok' by having Kannel accept these since the meta data patch (+- 7 years now). Changing this behavior would potentially cause issues for users with no real gain.

Thanks,
Donald

On 18 October 2016 at 13:18, Alexander Malysh <[hidden email]> wrote:
Hi,

good catch! But there is a question: should we really accept such wrong PDUs or reject them?
IMHO rejecting those would be the correct behavior but I don't see in SMPP spec that case described.

Thanks,
Alexander Malysh
 

From: [hidden email]
Sent: Monday, September 26, 2016 07:24 PM
To:
Cc: [hidden email]
Subject: Re: [PATCH] Memory leak in smpp_pdu.c
Am 24.09.2016 13:55, schrieb Rene Kluwen:
> +1 from me.

yep, good catch Donald.

+1 for it.

If no objections, will commit.

--
Best Regards,
Stipe Tolj

-------------------------------------------------------------------
Düsseldorf, NRW, Germany

Kannel Foundation tolj.org system architecture
http://www.kannel.org/ http://www.tolj.org/

stolj at kannel.org st at tolj.org
-------------------------------------------------------------------



--
Donald Jackson