handle timeouts only handled for expected replies, not for queued messages

diff --git a/bus.h b/bus.h
index dfeea98dba56e1f2d4d1114571683b0c316049e2..1d58b4587e7f463a3ee8ed942210f11073081bd7 100644 (file)
--- a/bus.h
+++ b/bus.h
@@ -78,6 +78,5 @@ struct kdbus_bus *kdbus_bus_unref(struct kdbus_bus *bus);
 void kdbus_bus_disconnect(struct kdbus_bus *bus);
 
 bool kdbus_bus_uid_is_privileged(const struct kdbus_bus *bus);
-void kdbus_bus_scan_timeout_list(struct kdbus_bus *bus);
 struct kdbus_conn *kdbus_bus_find_conn_by_id(struct kdbus_bus *bus, u64 id);
 #endif

diff --git a/connection.c b/connection.c
index 2d13658819c666ffa2d7d79add017030f78f6a5c..08f8905f996c34ec169e0ab1bb6c698b3aa6a218 100644 (file)
--- a/connection.c
+++ b/connection.c
@@ -50,7 +50,6 @@
  * @fds:               Offset to array where to update the installed fd number
  * @fds_fp:            Array passed files queued up for this message
  * @fds_count:         Number of files
- * @deadline_ns:       Timeout for this message, used for replies
  * @src_id:            The ID of the sender
  * @cookie:            Message cookie, used for replies
  * @src_name_id:       The sequence number of the name this message is
@@ -69,7 +68,6 @@ struct kdbus_conn_queue {
        struct file **fds_fp;
        unsigned int fds_count;
 
-       u64 deadline_ns;
        u64 src_id;
        u64 cookie;
        u64 dst_name_id;
@@ -329,8 +327,7 @@ static void kdbus_conn_queue_cleanup(struct kdbus_conn_queue *queue)
 
 /* enqueue a message into the receiver's pool */
 static int kdbus_conn_queue_insert(struct kdbus_conn *conn,
-                                  struct kdbus_kmsg *kmsg,
-                                  u64 deadline_ns)
+                                  struct kdbus_kmsg *kmsg)
 {
        struct kdbus_conn_queue *queue;
        u64 msg_size;
@@ -352,7 +349,6 @@ static int kdbus_conn_queue_insert(struct kdbus_conn *conn,
                return -ENOMEM;
 
        /* copy message properties we need for the queue management */
-       queue->deadline_ns = deadline_ns;
        queue->src_id = kmsg->msg.src_id;
        queue->cookie = kmsg->msg.cookie;
 
@@ -507,11 +503,10 @@ exit_unlock:
 
 static void kdbus_conn_scan_timeout(struct kdbus_conn *conn)
 {
-       struct kdbus_conn_queue *queue, *queue_tmp;
        struct kdbus_conn_reply_entry *reply, *reply_tmp;
        LIST_HEAD(notify_list);
        LIST_HEAD(reply_list);
-       u64 deadline = -1;
+       u64 deadline = ~0ULL;
        struct timespec ts;
        u64 now;
 
@@ -519,34 +514,34 @@ static void kdbus_conn_scan_timeout(struct kdbus_conn *conn)
        now = timespec_to_ns(&ts);
 
        mutex_lock(&conn->lock);
-       list_for_each_entry_safe(queue, queue_tmp, &conn->msg_list, entry) {
-               if (queue->deadline_ns == 0)
-                       continue;
-
-               if (queue->deadline_ns <= now) {
-                       kdbus_pool_free_range(conn->pool, queue->off);
-                       list_del(&queue->entry);
-                       kdbus_conn_queue_cleanup(queue);
-                       continue;
-               }
+       list_for_each_entry_safe(reply, reply_tmp, &conn->reply_list, entry) {
+               if (reply->deadline_ns <= now) {
 
-               if (queue->deadline_ns < deadline)
-                       deadline = queue->deadline_ns;
-       }
+                       /*
+                        * Move to temporary cleanup list; we cannot unref and
+                        * possibly cleanup a connection that is holding a ref
+                        * back to us, while we are locking ourselves.
+                        */
+                       list_move_tail(&reply->entry, &reply_list);
 
-       list_for_each_entry_safe(reply, reply_tmp, &conn->reply_list, entry) {
-               if (reply->deadline_ns > now)
-                       continue;
+                       /*
+                        * A zero deadline means the connection died, was
+                        * cleaned up already and the notify sent.
+                        */
+                       if (reply->deadline_ns == 0)
+                               continue;
 
-               /*
-                * The connection died and cleared the timeout; the
-                * was already sent; just clean up.
-                */
-               if (reply->deadline_ns > 0)
                        kdbus_notify_reply_timeout(reply->conn->id,
-                                                  reply->cookie, &notify_list);
+                                                  reply->cookie,
+                                                  &notify_list);
+                       continue;
+               }
 
-               list_move_tail(&reply->entry, &reply_list);
+               /* remember next timeout */
+               if (reply->deadline_ns < deadline) {
+                       deadline = reply->deadline_ns;
+                       continue;
+               }
        }
        mutex_unlock(&conn->lock);
 
@@ -555,8 +550,10 @@ static void kdbus_conn_scan_timeout(struct kdbus_conn *conn)
        list_for_each_entry_safe(reply, reply_tmp, &reply_list, entry)
                kdbus_conn_reply_entry_free(reply);
 
-       if (deadline != -1) {
+       /* rearm timer with next timeout */
+       if (deadline != (~0ULL)) {
                u64 usecs = deadline - now;
+
                do_div(usecs, 1000ULL);
                mod_timer(&conn->timer, jiffies + usecs_to_jiffies(usecs));
        }
@@ -670,7 +667,6 @@ int kdbus_conn_kmsg_send(struct kdbus_ep *ep,
        const struct kdbus_msg *msg = &kmsg->msg;
        struct kdbus_conn *conn_dst = NULL;
        struct kdbus_conn *c;
-       u64 deadline_ns = 0;
        int ret;
 
        /* non-kernel senders append credentials/metadata */
@@ -710,7 +706,7 @@ int kdbus_conn_kmsg_send(struct kdbus_ep *ep,
                                kdbus_meta_append(kmsg->meta, conn_src,
                                                  conn_dst->attach_flags);
 
-                       kdbus_conn_queue_insert(conn_dst, kmsg, 0);
+                       kdbus_conn_queue_insert(conn_dst, kmsg);
                }
                mutex_unlock(&ep->bus->lock);
 
@@ -757,22 +753,10 @@ int kdbus_conn_kmsg_send(struct kdbus_ep *ep,
                }
        }
 
-       /*
-        * If the message has a timeout value set, calculate the deadline here,
-        * based on the current time. Note that this only takes care for the
-        * timeout of the actual message, and has nothing to do with a potential
-        * reply that may be expected.
-        */
-       if (msg->src_id != KDBUS_SRC_ID_KERNEL && msg->timeout_ns > 0) {
-               struct timespec ts;
-
-               ktime_get_ts(&ts);
-               deadline_ns = timespec_to_ns(&ts) + msg->timeout_ns;
-       }
-
        /* If the message expects a reply, add a kdbus_conn_reply_entry */
        if (conn_src && (msg->flags & KDBUS_MSG_FLAGS_EXPECT_REPLY)) {
                struct kdbus_conn_reply_entry *reply;
+               struct timespec ts;
 
                reply = kzalloc(sizeof(*reply), GFP_KERNEL);
                if (!reply) {
@@ -781,14 +765,19 @@ int kdbus_conn_kmsg_send(struct kdbus_ep *ep,
                }
 
                INIT_LIST_HEAD(&reply->entry);
-               reply->deadline_ns = deadline_ns;
                reply->conn = kdbus_conn_ref(conn_dst);
                atomic_inc(&reply->conn->reply_count);
                reply->cookie = msg->cookie;
 
+               /* calculate the deadline based on the current time */
+               ktime_get_ts(&ts);
+               reply->deadline_ns = timespec_to_ns(&ts) + msg->timeout_ns;
+
                mutex_lock(&conn_src->lock);
                list_add(&reply->entry, &conn_src->reply_list);
                mutex_unlock(&conn_src->lock);
+
+               kdbus_conn_timeout_schedule_scan(conn_dst);
        }
 
        if (conn_src) {
@@ -804,16 +793,13 @@ int kdbus_conn_kmsg_send(struct kdbus_ep *ep,
         */
        mutex_lock(&ep->bus->lock);
        list_for_each_entry(c, &ep->bus->monitors_list, monitor_entry)
-               kdbus_conn_queue_insert(c, kmsg, 0);
+               kdbus_conn_queue_insert(c, kmsg);
        mutex_unlock(&ep->bus->lock);
 
-       ret = kdbus_conn_queue_insert(conn_dst, kmsg, deadline_ns);
+       ret = kdbus_conn_queue_insert(conn_dst, kmsg);
        if (ret < 0)
                goto exit_unref;
 
-       if (deadline_ns > 0)
-               kdbus_conn_timeout_schedule_scan(conn_dst);
-
 exit_unref:
        /* conn_dst got an extra ref from kdbus_conn_get_conn_dst */
        kdbus_conn_unref(conn_dst);

diff --git a/kdbus.h b/kdbus.h
index 71bf86d063547ec5260ced38047473688c84fcb9..71cfe06eac1b37eace0f0b65ae8b8f05cf751087 100644 (file)
--- a/kdbus.h
+++ b/kdbus.h
@@ -345,21 +345,13 @@ enum kdbus_payload_type {
  * @payload_type:      Payload type (KDBUS_PAYLOAD_*)
  * @cookie:            Userspace-supplied cookie, for the connection
  *                     to identify its messages
- * @cookie_reply:      A reply to the message with the same cookie. The
- *                     reply itself has its own cookie, @cookie_reply
- *                     corresponds to the cookie of the request message
- * @timeout_ns:                For non-kernel-generated messages, this denotes the
- *                     message timeout in nanoseconds. A message has to be
- *                     received with KDBUS_CMD_MSG_RECV by the destination
- *                     connection within this time frame. For messages that
- *                     have KDBUS_MSG_FLAGS_EXPECT_REPLY set in @flags,
- *                     this value also denotes the timeout for the reply to
- *                     this message. If there is no reply, or the message is
- *                     not received in time by the other side, a
- *                     kernel-generated message with an attached
- *                     KDBUS_ITEM_REPLY_TIMEOUT item is sent to @src_id.
- *                     A 0-value is only valid if KDBUS_MSG_FLAGS_EXPECT_REPLY
- *                     is unset in @flags.
+ * @cookie_reply:      A reply to the requesting message with the same
+ *                     cookie. The requesting connection can match its
+ *                     request and the reply with this value
+ * @timeout_ns:                The time to wait for a message reply from the peer.
+ *                     If there is no reply, a kernel-generated message
+ *                     with an attached KDBUS_ITEM_REPLY_TIMEOUT item
+ *                     is sent to @src_id.
  * @items:             A list of kdbus_items containing the message payload
  */
 struct kdbus_msg {
@@ -369,8 +361,10 @@ struct kdbus_msg {
        __u64 src_id;
        __u64 payload_type;
        __u64 cookie;
-       __u64 cookie_reply;
-       __u64 timeout_ns;
+       union {
+               __u64 cookie_reply;
+               __u64 timeout_ns;
+       };
        struct kdbus_item items[0];
 } __attribute__((aligned(8)));
 

diff --git a/message.c b/message.c
index f6d3bc6444ef009903909c1d6ca2db3daba3ab15..2bbcc40a934471b7216542ee4c5b1679bce00091 100644 (file)
--- a/message.c
+++ b/message.c
@@ -219,7 +219,7 @@ static int kdbus_msg_scan_items(struct kdbus_conn *conn,
                        return -EBADMSG;
 
                /* timeouts are not allowed for broadcasts */
-               if (msg->timeout_ns)
+               if (msg->timeout_ns > 0)
                        return -ENOTUNIQ;
        }
 
@@ -276,13 +276,6 @@ int kdbus_kmsg_new_from_user(struct kdbus_conn *conn,
                goto exit_free;
        }
 
-       /* replies cannot request a reply themselves */
-       if (m->msg.flags & KDBUS_MSG_FLAGS_EXPECT_REPLY &&
-           m->msg.cookie_reply > 0) {
-               ret = -EINVAL;
-               goto exit_free;
-       }
-
        ret = kdbus_msg_scan_items(conn, m);
        if (ret < 0)
                goto exit_free;

diff --git a/test/kdbus-util.c b/test/kdbus-util.c
index 6e1b56558f438402aee2c1d370ce30b90c57837a..942ee8bf3cac4ead570460b3af4cad3f4a90a26f 100644 (file)
--- a/test/kdbus-util.c
+++ b/test/kdbus-util.c
@@ -212,12 +212,19 @@ void msg_dump(const struct conn *conn, const struct kdbus_msg *msg)
        const struct kdbus_item *item = msg->items;
        char buf_src[32];
        char buf_dst[32];
+       uint64_t timeout = 0;
+       uint64_t cookie_reply = 0;
 
-       printf("MESSAGE: %s (%llu bytes) flags=0x%08llx, %s → %s, cookie=%llu, timeout=%llu\n",
-               enum_PAYLOAD(msg->payload_type), (unsigned long long) msg->size,
-               (unsigned long long) msg->flags,
+       if (msg->flags & KDBUS_MSG_FLAGS_EXPECT_REPLY)
+               timeout = msg->timeout_ns;
+       else
+               cookie_reply = msg->cookie_reply;
+
+       printf("MESSAGE: %s (%llu bytes) flags=0x%08llx, %s → %s, cookie=%llu, timeout=%llu cookie_reply=%llu\n",
+               enum_PAYLOAD(msg->payload_type), (unsigned long long)msg->size,
+               (unsigned long long)msg->flags,
                msg_id(msg->src_id, buf_src), msg_id(msg->dst_id, buf_dst),
-               (unsigned long long) msg->cookie, (unsigned long long) msg->timeout_ns);
+               (unsigned long long)msg->cookie, (unsigned long long)timeout, (unsigned long long)cookie_reply);
 
        KDBUS_ITEM_FOREACH(item, msg, items) {
                if (item->size <= KDBUS_ITEM_HEADER_SIZE) {