[PATCH] dbus_new_handlers: Don't send NULL to dbus_message_new_error

[Witold Sowa]

2010/10/6 Paul Stewart <pstew at google.com>

> On Wed, Oct 6, 2010 at 7:02 AM, Dan Williams <dcbw at redhat.com> wrote:
> > On Tue, 2010-10-05 at 12:07 -0700, Paul Stewart wrote:
> >> From: Paul Stewart <pstew at google.com>
> >>
> >> The new DBus API helper function wpas_dbus_error_unknown_error
> >> function can be called as a result of a failure within internal
> >> getter calls, which will call this function with a NULL message
> >> parameter.  However, dbus_message_new_error looks very unkindly
> >> (i.e, abort()) on a NULL message, so in this case, we should not
> >> call it.
> >>
> >> I've observed this course of events during a call to
> >> wpas_dbus_getter_bss_wpa with a faileld parse of the IE parameter.
> >> We got here through a call to fill_dict_with_properties which
> >> explicitly calls getters with a NULL message parameter.  Judging
> >> from the way it is called, this could easily occur if an AP sends
> >> out a malformed (or mis-received) probe response.  I usually run
> >> into this problem while driving through San Francisco, so I'm
> >> exposed to any number of base stations along this path.
> >
> > I'm thinking that instead, we should just pass the DBusMessage down to
> > fill_dict_with_properties() so that they can do something intelligent
> > with the error.  That would probably  mean changing the return type of
> > fill_dict_with_properties() to a DBusMessage* which would be NULL on
> > success, and an error message on error.
> >
> > Dan
> >
>
> The nature of fill_dict_with_properties is that it iterates over the
> properties and calls all getters.  The current behavior is tolerant of
> returned errors (continues if they occur) so that if there are any
> successfully retrieved properties, they are returned to the callers.
> Callers currently return their own error ("No readable properties in
> this interface") if no properties are returned.  I don't think
> immediately replying to the caller with an error about a single
> property element in an iteration makes sense in this case.
>
> Also, there are times, such as the case where I ran into this problem,
> when there wasn't actually an incoming message to respond to, and
> instead, this was an outgoing signal:
>
> wpas_dbus_getter_bss_wpa <- fill_dict_with_properties  <-
> wpa_dbus_get_object_properties <- wpas_dbus_signal_bss
>
> so, I believe that even if we resolved the first issue above, there
> are still legitimate cases where the reply_to would be NULL.
>
> --
> Paul
>

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.

Witek


> >> ---
> >>  wpa_supplicant/dbus/dbus_new_handlers.c |   10 ++++++++++
> >>  1 files changed, 10 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/wpa_supplicant/dbus/dbus_new_handlers.c
> b/wpa_supplicant/dbus/dbus_new_handlers.c
> >> index 8e87e20..b9f6a8d 100644
> >> --- a/wpa_supplicant/dbus/dbus_new_handlers.c
> >> +++ b/wpa_supplicant/dbus/dbus_new_handlers.c
> >> @@ -117,6 +117,16 @@ static char *
> wpas_dbus_new_decompose_object_path(const char *path,
> >>  DBusMessage * wpas_dbus_error_unknown_error(DBusMessage *message,
> >>                                           const char *arg)
> >>  {
> >> +     /*
> >> +      * This function can be called as a result of a failure
> >> +      * within internal getter calls, which will call this function
> >> +      * with a NULL message parameter.  However, dbus_message_new_error
> >> +      * looks very unkindly (i.e, abort()) on a NULL message, so
> >> +      * in this case, we should not call it.
> >> +      */
> >> +     if (message == NULL)
> >> +             return NULL;
> >> +
> >>       return dbus_message_new_error(message,
> WPAS_DBUS_ERROR_UNKNOWN_ERROR,
> >>                                     arg);
> >>  }
> >
> >
> >
> _______________________________________________
> HostAP mailing list
> HostAP at lists.shmoo.com
> http://lists.shmoo.com/mailman/listinfo/hostap
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.shmoo.com/pipermail/hostap/attachments/20101007/0b980815/attachment.htm