SE-DBUS updates

[Havoc Pennington]

On Mon, 2004-07-26 at 16:03, Matthew Rickard wrote:
> 
> Now this one I'm not quite sure the cause.  In 
> >  bus_selinux_id_table_free_value (BusSELinuxID *sid)
> It seems that sometimes the sid passed in is NULL, which causes
> bus_selinux_id_unref to throw an assertion.  I solved this by simply
> testing for NULL before calling bus_selinux_id_unref.  However, I'm
> still not sure why this is ever being passed NULL, so I don't know if
> this is the correct solution.

This is just an oddity of how DBusHashTable works, checking for NULL is
the right fix.

> >  bus_selinux_allows_send (DBusConnection     *sender,
> >                           DBusConnection     *proposed_recipient)
> proposed_recipient can be NULL to represent the bus driver.  The
> bus_selinux_allows_send was not testing this, and therefore throwing an
> assertion in bus_connection_get_selinux_id().  Adding a simple check
> here that uses the bus_sid on a NULL proposed_recipient corrected this.

Sounds sensible.

> For the 
> >  _dbus_assert_not_reached ("FIXME the avc_context_to_sid() error 
> >  handling");
> The only error avc_context_to_sid should fail on is ENOMEM, so I think
> it should be OK to just return NULL.

Wouldn't hurt to put in a _dbus_assert (errno == ENOMEM), which also
acts as a comment indicating that we thought about this.

> However, in
> >  _dbus_assert_not_reached ("bus_selinux_init_id doesn't properly
> >  indicate OOM");
> it can fail on errors other than ENOMEM (like failing to get the
> connection context).  How should we handle this?

Just need to figure out what to do in each case. On ENOMEM usually what
we want to do is try again. If a connection has no context, then we'd
want to have a NULL context probably - but we have to be able to
distinguish no context from OOM. Typically DBusError is used for this.

> I also noticed that rather than using security_context_t for context
> types you are simply using char* instead.  Is there any particular
> reason for this (since we do have the SELinux headers available in
> selinux.c)?

IIRC there were various spots that assumed it was a char* (copying it
with strdup, printing it out, assigning NULL to it) so I thought it was
clearer to just use char* - the type isn't genuinely opaque. Also
matches the general dbus and glib convention, we never typedef char* to
anything.

> I've attached a diff that applies on top of the patch you sent to show
> the few changes that I made.  Hopefully we can get this ready in time to
> be included in the next D-BUS release.

Perfect, this is great. Do you want to fix the remaining issues above or
should I? We can commit after that I think, unless someone else sees
bugs.

Havoc