<div class="gmail_quote">2010/10/6 Paul Stewart <span dir="ltr"><<a href="mailto:pstew@google.com">pstew@google.com</a>></span><br><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
<div class="im">On Wed, Oct 6, 2010 at 7:02 AM, Dan Williams <<a href="mailto:dcbw@redhat.com">dcbw@redhat.com</a>> wrote:<br>
> On Tue, 2010-10-05 at 12:07 -0700, Paul Stewart wrote:<br>
>> From: Paul Stewart <<a href="mailto:pstew@google.com">pstew@google.com</a>><br>
>><br>
>> The new DBus API helper function wpas_dbus_error_unknown_error<br>
>> function can be called as a result of a failure within internal<br>
>> getter calls, which will call this function with a NULL message<br>
>> parameter. However, dbus_message_new_error looks very unkindly<br>
>> (i.e, abort()) on a NULL message, so in this case, we should not<br>
>> call it.<br>
>><br>
>> I've observed this course of events during a call to<br>
>> wpas_dbus_getter_bss_wpa with a faileld parse of the IE parameter.<br>
>> We got here through a call to fill_dict_with_properties which<br>
>> explicitly calls getters with a NULL message parameter. Judging<br>
>> from the way it is called, this could easily occur if an AP sends<br>
>> out a malformed (or mis-received) probe response. I usually run<br>
>> into this problem while driving through San Francisco, so I'm<br>
>> exposed to any number of base stations along this path.<br>
><br>
> I'm thinking that instead, we should just pass the DBusMessage down to<br>
> fill_dict_with_properties() so that they can do something intelligent<br>
> with the error. That would probably mean changing the return type of<br>
> fill_dict_with_properties() to a DBusMessage* which would be NULL on<br>
> success, and an error message on error.<br>
><br>
> Dan<br>
><br>
<br>
</div>The nature of fill_dict_with_properties is that it iterates over the<br>
properties and calls all getters. The current behavior is tolerant of<br>
returned errors (continues if they occur) so that if there are any<br>
successfully retrieved properties, they are returned to the callers.<br>
Callers currently return their own error ("No readable properties in<br>
this interface") if no properties are returned. I don't think<br>
immediately replying to the caller with an error about a single<br>
property element in an iteration makes sense in this case.<br>
<br>
Also, there are times, such as the case where I ran into this problem,<br>
when there wasn't actually an incoming message to respond to, and<br>
instead, this was an outgoing signal:<br>
<br>
wpas_dbus_getter_bss_wpa <- fill_dict_with_properties <-<br>
wpa_dbus_get_object_properties <- wpas_dbus_signal_bss<br>
<br>
so, I believe that even if we resolved the first issue above, there<br>
are still legitimate cases where the reply_to would be NULL.<br>
<br>
--<br>
<font color="#888888">Paul<br></font></blockquote><div><br>I believe that the approach in that patch is fine but I would log the message coming in arg argument in case of NULL message so there would be any trace of the error left in logs.<br>
<br>Witek<br><br></div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;"><font color="#888888">
</font><div><div></div><div class="h5"><br>
>> ---<br>
>> wpa_supplicant/dbus/dbus_new_handlers.c | 10 ++++++++++<br>
>> 1 files changed, 10 insertions(+), 0 deletions(-)<br>
>><br>
>> diff --git a/wpa_supplicant/dbus/dbus_new_handlers.c b/wpa_supplicant/dbus/dbus_new_handlers.c<br>
>> index 8e87e20..b9f6a8d 100644<br>
>> --- a/wpa_supplicant/dbus/dbus_new_handlers.c<br>
>> +++ b/wpa_supplicant/dbus/dbus_new_handlers.c<br>
>> @@ -117,6 +117,16 @@ static char * wpas_dbus_new_decompose_object_path(const char *path,<br>
>> DBusMessage * wpas_dbus_error_unknown_error(DBusMessage *message,<br>
>> const char *arg)<br>
>> {<br>
>> + /*<br>
>> + * This function can be called as a result of a failure<br>
>> + * within internal getter calls, which will call this function<br>
>> + * with a NULL message parameter. However, dbus_message_new_error<br>
>> + * looks very unkindly (i.e, abort()) on a NULL message, so<br>
>> + * in this case, we should not call it.<br>
>> + */<br>
>> + if (message == NULL)<br>
>> + return NULL;<br>
>> +<br>
>> return dbus_message_new_error(message, WPAS_DBUS_ERROR_UNKNOWN_ERROR,<br>
>> arg);<br>
>> }<br>
><br>
><br>
><br>
_______________________________________________<br>
HostAP mailing list<br>
<a href="mailto:HostAP@lists.shmoo.com">HostAP@lists.shmoo.com</a><br>
<a href="http://lists.shmoo.com/mailman/listinfo/hostap" target="_blank">http://lists.shmoo.com/mailman/listinfo/hostap</a><br>
</div></div></blockquote></div><br>