* Bug in sms @ 2009-09-16 8:22 Gu, Yang 2009-09-16 9:22 ` Dennis.Yxun 2009-09-16 12:14 ` Andrzej Zaborowski 0 siblings, 2 replies; 6+ messages in thread From: Gu, Yang @ 2009-09-16 8:22 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 359 bytes --] Hi, Today I tried oFono with my cell phone, but it crashed when starting up. The problem happens in function at_cmgl_notify() of file drivers/atmodem/sms.c. In my case, strlen(hexpdu) == 338, but the buffer "pdu" has maximum size of 164. So after decode_hex_own_buf(), some memory was written unexpectedly. Call for a fix, please. Regards, -Yang ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Bug in sms 2009-09-16 8:22 Bug in sms Gu, Yang @ 2009-09-16 9:22 ` Dennis.Yxun 2009-09-16 9:49 ` Gu, Yang 2009-09-16 12:14 ` Andrzej Zaborowski 1 sibling, 1 reply; 6+ messages in thread From: Dennis.Yxun @ 2009-09-16 9:22 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 822 bytes --] HI Yang: Mind I ask you a question? maybe off-topic. Which hardware(the GSM modem?) are you testing? Is that possible for a developer to get an open hardware, So they can put their hands in Thanks. Dennis On Wed, Sep 16, 2009 at 8:22 AM, Gu, Yang <yang.gu@intel.com> wrote: > Hi, > Today I tried oFono with my cell phone, but it crashed when starting > up. The problem happens in function at_cmgl_notify() of file > drivers/atmodem/sms.c. In my case, strlen(hexpdu) == 338, but the buffer > "pdu" has maximum size of 164. So after decode_hex_own_buf(), some memory > was written unexpectedly. Call for a fix, please. > > > Regards, > -Yang > > > _______________________________________________ > ofono mailing list > ofono(a)ofono.org > http://lists.ofono.org/listinfo/ofono > [-- Attachment #2: attachment.html --] [-- Type: text/html, Size: 1225 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: Bug in sms 2009-09-16 9:22 ` Dennis.Yxun @ 2009-09-16 9:49 ` Gu, Yang 0 siblings, 0 replies; 6+ messages in thread From: Gu, Yang @ 2009-09-16 9:49 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 1624 bytes --] Hi Dennis, I am just using my cell phone (Sony Ericsson T618, GSM). Not sure what's your meaning by "open hardware". oFono is gradually adding support for some modems, such as HTC G1, Ericsson MBM, TI Calypso, etc. To my understanding, different modems have different implementation, in which some follows the spec, while others not. I think oFono's target is to support modems as many as possible, if they follow the spec well. For this problem, I don't know if my modem is a good one to follow spec. However, oFono needs more error handling to avoid segment fault. Regards, -Yang From: ofono-bounces(a)ofono.org [mailto:ofono-bounces(a)ofono.org] On Behalf Of Dennis.Yxun Sent: Wednesday, September 16, 2009 5:22 PM To: ofono(a)ofono.org Subject: Re: Bug in sms HI Yang: Mind I ask you a question? maybe off-topic. Which hardware(the GSM modem?) are you testing? Is that possible for a developer to get an open hardware, So they can put their hands in Thanks. Dennis On Wed, Sep 16, 2009 at 8:22 AM, Gu, Yang <yang.gu<http://yang.gu>@intel.com<http://intel.com>> wrote: Hi, Today I tried oFono with my cell phone, but it crashed when starting up. The problem happens in function at_cmgl_notify() of file drivers/atmodem/sms.c. In my case, strlen(hexpdu) == 338, but the buffer "pdu" has maximum size of 164. So after decode_hex_own_buf(), some memory was written unexpectedly. Call for a fix, please. Regards, -Yang _______________________________________________ ofono mailing list ofono(a)ofono.org<mailto:ofono@ofono.org> http://lists.ofono.org/listinfo/ofono [-- Attachment #2: attachment.html --] [-- Type: text/html, Size: 8294 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Bug in sms 2009-09-16 8:22 Bug in sms Gu, Yang 2009-09-16 9:22 ` Dennis.Yxun @ 2009-09-16 12:14 ` Andrzej Zaborowski 2009-09-16 12:15 ` Andrzej Zaborowski 1 sibling, 1 reply; 6+ messages in thread From: Andrzej Zaborowski @ 2009-09-16 12:14 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 558 bytes --] Hi, 2009/9/16 Gu, Yang <yang.gu@intel.com>: > Today I tried oFono with my cell phone, but it crashed when starting up. > The problem happens in function at_cmgl_notify() of file drivers/atmodem/sms.c. > In my case, strlen(hexpdu) == 338, but the buffer "pdu" has maximum size of > 164. So after decode_hex_own_buf(), some memory was written unexpectedly. The attached patch adds length check everywehere decode_hex_own_buf() is used with a static buffer and also enlarges the buffers to account for SMSC included in a PDU. Regards ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Bug in sms 2009-09-16 12:14 ` Andrzej Zaborowski @ 2009-09-16 12:15 ` Andrzej Zaborowski 2009-09-16 10:07 ` Denis Kenzior 0 siblings, 1 reply; 6+ messages in thread From: Andrzej Zaborowski @ 2009-09-16 12:15 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 25 bytes --] Oops, forgot to attach. [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0002-Check-received-PDUs-fit-in-the-buffer-fix-buffer-si.patch --] [-- Type: text/x-patch, Size: 2269 bytes --] From b5ed486ad94fa549d8f30ad054e7edcb52560301 Mon Sep 17 00:00:00 2001 From: Andrzej Zaborowski <andrew.zaborowski@intel.com> Date: Wed, 16 Sep 2009 16:03:50 +0200 Subject: [PATCH] Check received PDUs fit in the buffer, fix buffer size. --- drivers/atmodem/sms.c | 17 ++++++++++++++--- 1 files changed, 14 insertions(+), 3 deletions(-) diff --git a/drivers/atmodem/sms.c b/drivers/atmodem/sms.c index 3b7e9e4..d425818 100644 --- a/drivers/atmodem/sms.c +++ b/drivers/atmodem/sms.c @@ -314,7 +314,7 @@ static void at_cmt_notify(GAtResult *result, gpointer user_data) const char *hexpdu; long pdu_len; int tpdu_len; - unsigned char pdu[164]; + unsigned char pdu[176]; char buf[256]; dump_response("at_cmt_notify", TRUE, result); @@ -324,6 +324,11 @@ static void at_cmt_notify(GAtResult *result, gpointer user_data) return; } + if (strlen(hexpdu) > sizeof(pdu) * 2) { + ofono_error("Bad PDU length in CMT notification"); + return; + } + ofono_debug("Got new SMS Deliver PDU via CMT: %s, %d", hexpdu, tpdu_len); decode_hex_own_buf(hexpdu, -1, &pdu_len, 0, pdu); @@ -344,7 +349,7 @@ static void at_cmgr_notify(GAtResult *result, gpointer user_data) struct ofono_sms *sms = user_data; GAtResultIter iter; const char *hexpdu; - unsigned char pdu[164]; + unsigned char pdu[176]; long pdu_len; int tpdu_len; @@ -366,6 +371,9 @@ static void at_cmgr_notify(GAtResult *result, gpointer user_data) hexpdu = g_at_result_pdu(result); + if (strlen(hexpdu) > sizeof(pdu) * 2) + goto err; + ofono_debug("Got PDU: %s, with len: %d", hexpdu, tpdu_len); decode_hex_own_buf(hexpdu, -1, &pdu_len, 0, pdu); @@ -485,7 +493,7 @@ static void at_cmgl_notify(GAtResult *result, gpointer user_data) struct sms_data *data = ofono_sms_get_data(sms); GAtResultIter iter; const char *hexpdu; - unsigned char pdu[164]; + unsigned char pdu[176]; long pdu_len; int tpdu_len; int index; @@ -518,6 +526,9 @@ static void at_cmgl_notify(GAtResult *result, gpointer user_data) ofono_debug("Found an old SMS PDU: %s, with len: %d", hexpdu, tpdu_len); + if (strlen(hexpdu) > sizeof(pdu) * 2) + continue; + decode_hex_own_buf(hexpdu, -1, &pdu_len, 0, pdu); ofono_sms_deliver_notify(sms, pdu, pdu_len, tpdu_len); -- 1.6.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: Bug in sms 2009-09-16 12:15 ` Andrzej Zaborowski @ 2009-09-16 10:07 ` Denis Kenzior 0 siblings, 0 replies; 6+ messages in thread From: Denis Kenzior @ 2009-09-16 10:07 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 83 bytes --] > Oops, forgot to attach. Patch has been applied. Thanks. Regards, -Denis ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2009-09-16 12:15 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2009-09-16 8:22 Bug in sms Gu, Yang 2009-09-16 9:22 ` Dennis.Yxun 2009-09-16 9:49 ` Gu, Yang 2009-09-16 12:14 ` Andrzej Zaborowski 2009-09-16 12:15 ` Andrzej Zaborowski 2009-09-16 10:07 ` Denis Kenzior
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.