Notes and questions about remaining todo's and FIXME's marked 1.0

[John (J5) Palmieri]

The following are notes and questions on the remaining todo items.

********************************************************************
dbus_server_listen
/* FIXME 1.0 - we will unconditionally unlink() the path if
               * we don't support abstract namespace.  unlink() does
               * not follow symlinks, but would like independent
               * confirmation this is safe enough. See also
               * _dbus_listen_unix_socket() and comments therein.
               */
               
_dbus_listen_unix_socket
/* FIXME discussed security implications of this with Nalin,
       * and we couldn't think of where it would kick our ass, but
       * it still seems a bit sucky. It also has non-security suckage;
       * really we'd prefer to exit if the socket is already in use.
       * But there doesn't seem to be a good way to do this.
       *
       * Just to be extra careful, I threw in the stat() - clearly
       * the stat() can't *fix* any security issue, but it at least
       * avoids inadvertent/accidental data loss.

mclausen and I looked over this.  Since you can only unlink sockets you
have permission to there is no huge implication here.  You could unlink
another D-Bus socket but in practice this would be very unlikely.
Proposal is to remove the 1st FIXME and change the second FIXME to just
a comment.

***********************************************************************
_dbus_change_identity
/* setgroups() only works if we are a privileged process,
   * so we don't return error on failure; the only possible
   * failure is that we don't have perms to do it.
   * FIXME 1.0 not sure this is right, maybe if setuid()
   * is going to work then setgroups() should also work.
   */
  if (setgroups (0, NULL) < 0)
    _dbus_warn ("Failed to drop supplementary groups: %s\n",
                _dbus_strerror (errno));

  /* Set GID first, or the setuid may remove our permission
   * to change the GID
   */
  if (setgid (gid) < 0)
    {
      dbus_set_error (error, _dbus_error_from_errno (errno),
                      "Failed to set GID to %lu: %s", gid,
                      _dbus_strerror (errno));
      return FALSE;
    }

Should we change this to return an error or not?

**********************************************************************
bus_connection_disconnected
/* Drop any service ownership. FIXME 1.0? Unfortunately, this requires
   * memory allocation and there doesn't seem to be a good way to
   * handle it other than sleeping; we can't "fail" the operation of
   * disconnecting a client, and preallocating a broadcast "service is
   * now gone" message for every client-service pair seems kind of
   * involved. Probably we need to do that though.
   */
   
Is this important for 1.0?  How would I go about preallocating "service
is now gone" message for every client-service pair broadcast?

***********************************************************************
try_send_activation_failure
/**
 * FIXME @todo 1.0? the error messages here would ideally be
preallocated
 * so we don't need to allocate memory to send them.
 * Using the usual tactic, prealloc an OOM message, then
 * if we can't alloc the real error send the OOM error instead.
 */

This looks involved because even if you had a preallocated message
you would need to set the destination which could throw an OOM error.
Is it important for 1.0?

**********************************************************************
DBusKeyring
* @todo 1.0? there's a memory leak on some codepath in here, I saw it
once
 * when running make check - probably some specific initial cookies
 * present in the cookie file, then depending on what we do with them.

Any clue if this is still an issue.  Can it be tracked down? 

**********************************************************************
_dbus_setenv
 * @todo 1.0 if someone can verify it's safe, we could avoid the
 * memleak when doing an unset.

Nalin seems to think there is no safe way of doing this

**********************************************************************
_dbus_connection_queue_synthesized_message_link
* @todo 1.0? This needs to wake up the mainloop if it is in
* a poll/select and this is a multithreaded app.

Not sure what this todo is getting at.  We already do
_dbus_connection_wakeup_mainloop (connection); in this function.

_dbus_auth_decode_data
* @todo 1.0? We need to be able to distinguish "out of memory" error
* from "the data is hosed" error.

The problem here is we call into virtual methods:
if (DBUS_AUTH_IS_CLIENT (auth))
        return (* auth->mech->client_decode_func) (auth, encoded,
plaintext);
      else
        return (* auth->mech->server_decode_func) (auth, encoded,
plaintext);
        
Can those methods return OOM?  If so the solution would be to pass a
DBusError to them.  Where are the implementations for client_decode_func
and server_decode_func?

*************************************************************************
dbus_pending_call_block
 * @todo 1.0? when you start blocking, the timeout is reset, but it
should
 * really only use time remaining since the pending call was created.

This is a bit more involved than I first thought.  Timeouts are done in
intervals and not based on time stamps.  Not a big issue so I think we
should punt this for now.  It can be fixed later.

-- 
John (J5) Palmieri <johnp at redhat.com>