[rdma-next,v1,2/2] RDMA/core: Check that process is still alive before sending it to the users
diff mbox series

Message ID 20191010071105.25538-3-leon@kernel.org
State Accepted
Delegated to: Jason Gunthorpe
Headers show
Series
  • Remove PID namespaces support from restrack
Related show

Commit Message

Leon Romanovsky Oct. 10, 2019, 7:11 a.m. UTC
From: Leon Romanovsky <leonro@mellanox.com>

The PID information can disappear asynchronically because task can be
killed and moved to zombie state. In such case, PID will be zero in
similar way to the kernel tasks. Recognize such situation where
we are asking to return orphaned object and simply skip filling
PID attribute.

As part of this change, document the same scenario in counter.c code.

Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
 drivers/infiniband/core/counters.c | 14 ++++++++++++--
 drivers/infiniband/core/nldev.c    | 28 +++++++++++++++++++++-------
 2 files changed, 33 insertions(+), 9 deletions(-)

Patch
diff mbox series

diff --git a/drivers/infiniband/core/counters.c b/drivers/infiniband/core/counters.c
index 12ba2685abcf..47c551a0bcb0 100644
--- a/drivers/infiniband/core/counters.c
+++ b/drivers/infiniband/core/counters.c
@@ -149,8 +149,18 @@  static bool auto_mode_match(struct ib_qp *qp, struct rdma_counter *counter,
 	struct auto_mode_param *param = &counter->mode.param;
 	bool match = true;
 
-	/* Ensure that counter belongs to the right PID */
-	if (task_pid_nr(counter->res.task) != task_pid_nr(qp->res.task))
+	/*
+	 * Ensure that counter belongs to the right PID.
+	 * This operation can race with user space which kills
+	 * the process and leaves QP and counters orphans.
+	 *
+	 * It is not a big deal because exitted task will leave both
+	 * QP and counter in the same bucket of zombie process. Just ensure
+	 * that process is still alive before procedding.
+	 *
+	 */
+	if (task_pid_nr(counter->res.task) != task_pid_nr(qp->res.task) ||
+	    !task_pid_nr(qp->res.task))
 		return false;
 
 	if (auto_mask & RDMA_COUNTER_MASK_QP_TYPE)
diff --git a/drivers/infiniband/core/nldev.c b/drivers/infiniband/core/nldev.c
index 71bc08510064..0ebe95c79ae0 100644
--- a/drivers/infiniband/core/nldev.c
+++ b/drivers/infiniband/core/nldev.c
@@ -399,20 +399,34 @@  static int fill_res_info(struct sk_buff *msg, struct ib_device *device)
 static int fill_res_name_pid(struct sk_buff *msg,
 			     struct rdma_restrack_entry *res)
 {
+	int err = 0;
+
 	/*
 	 * For user resources, user is should read /proc/PID/comm to get the
 	 * name of the task file.
 	 */
 	if (rdma_is_kernel_res(res)) {
-		if (nla_put_string(msg, RDMA_NLDEV_ATTR_RES_KERN_NAME,
-		    res->kern_name))
-			return -EMSGSIZE;
+		err = nla_put_string(msg, RDMA_NLDEV_ATTR_RES_KERN_NAME,
+				     res->kern_name);
 	} else {
-		if (nla_put_u32(msg, RDMA_NLDEV_ATTR_RES_PID,
-		    task_pid_vnr(res->task)))
-			return -EMSGSIZE;
+		pid_t pid;
+
+		pid = task_pid_vnr(res->task);
+		/*
+		 * Task is dead and in zombie state.
+		 * There is no need to print PID anymore.
+		 */
+		if (pid)
+			/*
+			 * This part is racy, task can be killed and PID will
+			 * be zero right here but it is ok, next query won't
+			 * return PID. We don't promise real-time reflection
+			 * of SW objects.
+			 */
+			err = nla_put_u32(msg, RDMA_NLDEV_ATTR_RES_PID, pid);
 	}
-	return 0;
+
+	return err ? -EMSGSIZE : 0;
 }
 
 static bool fill_res_entry(struct ib_device *dev, struct sk_buff *msg,