Hi Samuel,
For "debug" function, I think you're right, it's not really the good
correction,
the backtrace gives me following results:
-------- backtrace --------
[0]: call_result_func() [gweb.c:436]
[1]: received_data() [gweb.c:821]
[2]: main() [main.c:373]
[3]: _start() [start.S:122]
---------------------------
It is under old backtrace format cause my little target has not addr2line and
other binutils stuff, and the
line numbers are the ones after applying my proposed patch. Translated to the
last git commit it should give:
[0]: call_result_func() [gweb.c:432]
[1]: received_data() [gweb.c:813]
After reading the code again, I'm not confortable with that trace which is
showing the error on l.813 in "received_data"
on the "debug" called from "call_result_func" while l.807 a direct call should
raise an error before:
807 debug(session->web, "bytes read %zu", bytes_read);
808
809 if (status != G_IO_STATUS_NORMAL && status != G_IO_STATUS_AGAIN) {
810 session->transport_watch = 0;
811 session->result.buffer = NULL;
812 session->result.length = 0;
813 call_result_func(session, 0);
814 return FALSE;
815 }
I'm going to do some more tests to track the pointer value of "session->web"
and keep you informed if I
find something relevant
regards
Le 09/12/2011 14:35, Samuel Ortiz a écrit :
> Hi Thierry,
>
> On Sun, Dec 04, 2011 at 07:43:47PM +0100, Thierry Boureille wrote:
>> Fix null pointer derefencing in free_session function.
>> Adding null pointer check in debug and process_send_buffer functions
> I'm usually suspicious about these kind of fixes. Have you checked how we can
> end up with e.g. debug() being called with a NULL argument ? It seems like
> there is a more fundamental issue here...
>
>> @@ -137,13 +139,14 @@ static inline void debug(GWeb *web, const char
>> *format, ...)
>>
>> static void free_session(struct web_session *session)
>> {
>> - GWeb *web = session->web;
>> + GWeb *web;
>>
>> if (session == NULL)
>> return;
>>
>> g_free(session->request);
>>
>> + web = session->web;
> That fix is actually fine.
>
>
>> if (session->resolv_action > 0)
>> g_resolv_cancel_lookup(web->resolv, session->resolv_action);
>>
>> @@ -421,6 +424,9 @@ static inline void call_result_func(struct web_session
>> *session, guint16 status)
>> {
>> gboolean result;
>>
>> + if (session == NULL)
>> + return;
>> +
>> if (session->result_func == NULL)
>> return;
>>
>> @@ -435,10 +441,14 @@ static inline void call_result_func(struct web_session
>> *session, guint16 status)
>>
>> static gboolean process_send_buffer(struct web_session *session)
>> {
>> - GString *buf = session->send_buffer;
>> + GString *buf;
>> gsize count, bytes_written;
>> GIOStatus status;
>>
>> + if (session == NULL)
>> + return;
>> +
>> + buf = session->send_buffer;
>> count = buf->len;
> Same here.
> Cheers,
> Samuel.
_______________________________________________
connman mailing list
[email protected]
http://lists.connman.net/listinfo/connman
