[LEDE-DEV] ubus: ubusd segfault on lookup

[Felix Fietkau]

On 2016-08-18 12:19, Delio Brignoli wrote:
> Hello Felix,
> 
> ubusd_handle_remove_object() currently sends object removal event
> followed by a reply to the peer’s remove object request. However the
> payload of the reply is the same as the object removal event. This
> results in the peer not clearing the type id in struct ubus_object_type
> because UBUS_ATTR_OBJTYPE is missing form the reply. If the peer
> attempts to add the object again, the object will end up having a NULL
> type field and a lookup will trigger a NULL pointer dereference in
> used_prot.c:131
> 
> Swapping the order of ubus_proto_send_msg_from_blob() and
> ubusd_free_object() (see diff below) the peer sees the expected
> messages. Is this an acceptable fix for the issue? It looks like
> ubusd_create_object() allows object with NULL type to be created,
> probably because they are used internally, but shouldn’t it reply with
> an error when a peer tries to add an object with a non-existent type id?
> 
> Thank you
> —
> Delio
> 
> 
> diff --git a/ubusd_proto.c b/ubusd_proto.c
> index 0af11f2..d0cfa10 100644
> --- a/ubusd_proto.c
> +++ b/ubusd_proto.c
> @@ -130,8 +130,8 @@ static int ubusd_handle_remove_object(struct ubus_client *cl, struct ubus_msg_bu
>         if (obj->type && obj->type->refcount == 1)
>                 blob_put_int32(&b, UBUS_ATTR_OBJTYPE, obj->type->id.id);
> 
> -       ubusd_free_object(obj);
>         ubus_proto_send_msg_from_blob(cl, ub, UBUS_MSG_DATA);
> +       ubusd_free_object(obj);
I've added some fixes for NULL pointer dereference issues, however I
don't quite understand how this patch changes anything. As far as I can
tell, ubus_proto_send_msg_from_blob does not access obj.
Could you please provide some more context for your proposed fix?

Thanks,

- Felix