metadata: major overhaul to fix several bugs
authorDavid Herrmann <dh.herrmann@gmail.com>
Sun, 11 Jan 2015 01:16:19 +0000 (02:16 +0100)
committerDavid Herrmann <dh.herrmann@gmail.com>
Sun, 11 Jan 2015 01:16:19 +0000 (02:16 +0100)
commit8d411f94a321e109d3c9ed27659f021fba619793
treee746ce1e8ffbe469b4a041acf842c9713a8afc5c
parent9b5e106d113aafe9005ff24246ebb244e3fd0b80
metadata: major overhaul to fix several bugs

The current metadata implementation suffers from several bugs:

1) If we run GET_CONN_INFO, we take the metadata object of the connection
and try to augment it with owned-names and the connection-description. We
do this unlocked on a _shared_ metadata object. This breaks parallel
GET_CONN_INFO calls as someone else might be calling kdbus_meta_export()
in parallel.

2) If we send a broadcast, we create a new metadata object that we fill
with information. Then, for each target, we collect further information
and queue it on the target. However, if a message is queued on target A
and it already tries to dequeue it while the broadcast still continues on
target B, we again get a collect vs. export race.

As I assumed we use faked-metadata as base for messages, too, I started to
split the metadata object into meta_proc and meta_conn. This turned out to
be not needed, but I thought it's a nice split and allows us to reduce
bus->meta and conn->meta to meta_proc, instead of meta_conn. If people
don't like it, we can revert it again. But the added code is minimal.

Anyway, the real fixes of this commit are:
 * meta objects have a lock now. This lock is held during updates *only*.
   During export, we only retrieve the "collected" flag and then can be
   sure that we can access the properly collected fields without races and
   without holding the lock.
 * meta objects distinguish "collected" and "valid" flags now. This fixes
   a race where we try to add owned-names, but a connection doesn't own
   names. On the next broadcast target, we try again and at this time the
   connection owns names. Thus, we would send the message with different
   information to different targets. To avoid this, we now set "collected"
   when we collected a flag without errors, but we only set "valid" if we
   collected it *AND* it is non-empty.
   This way, we will never collect a field twice, even if it was empty the
   first time.
 * get-conn-info now uses a temporary kdbus_meta_conn object to collect
   the connection related metadata. This is a one-time object, as the data
   is no longer valid afterwards.
 * Lots of random error-path fixes I was too lazy to commit separately (as
   they were mostly overwritten by further rewrites).
 * Reduce "struct file *exe" to "struct path exe". We really only ever
   accessed exe->f_path, so no need to pin the whole file. It's enough to
   pin the underlying inode via the path.

Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
bus.c
bus.h
connection.c
connection.h
message.c
message.h
metadata.c
metadata.h
notify.c
queue.c
queue.h