[PATCH net] pds_core: Fix potential invalid stack memory access

[PATCH net] pds_core: Fix potential invalid stack memory access

[Eric Joyner]

pds_adminq_post() and pds_process_adminq() can run concurrently
together; either one can check if the current request is complete:
pds_adminq_post() does it after the loop inside it times out waiting
for the completion, and pds_process_adminq() when it sees the
uncompleted request on the ring.

However, since pds_adminq_post() does not do any synchronization
around checking for an incomplete command after it reaches its timeout
and marking the command as complete, there is a small window where both
pds_process_adminq() and pds_adminq_post() can execute their
if(!completion_done()) clauses when the completion_done() returns
false; if pdsc_adminq_post() finishes first and its callers return,
then pdsc_process_adminq() will attempt to access q_info->dest after
the address in there points to an invalid location.

(q_info->dest will be invalid after pdsc_adminq_post()'s call chain
exits because it is pointing to a stack variable "comp" in
pdsc_fw_rpc())

Fix this by locking around the completion_done() check and the
complete() contained in the if-statement with the same adminq_lock that
pds_process_adminq() uses, which will prevent this synchronization
issue from occurring.

Fixes: 3f77c3dfffc7 ("pds_core: make wait_context part of q_info")
Signed-off-by: Eric Joyner <eric.joyner@amd.com>
---
 drivers/net/ethernet/amd/pds_core/adminq.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/amd/pds_core/adminq.c b/drivers/net/ethernet/amd/pds_core/adminq.c
index 097bb092bdb8..6db7d29cdf5c 100644
--- a/drivers/net/ethernet/amd/pds_core/adminq.c
+++ b/drivers/net/ethernet/amd/pds_core/adminq.c
@@ -283,9 +283,15 @@ int pdsc_adminq_post(struct pdsc *pdsc,
 		__func__, jiffies_to_msecs(time_done - time_start));
 
 	/* Check the results and clear an un-completed timeout */
-	if (time_after_eq(time_done, time_limit) && !completion_done(wc)) {
-		err = -ETIMEDOUT;
-		complete(wc);
+	if (time_after_eq(time_done, time_limit)) {
+		unsigned long irqflags;
+
+		spin_lock_irqsave(&pdsc->adminq_lock, irqflags);
+		if (!completion_done(wc)) {
+			err = -ETIMEDOUT;
+			complete(wc);
+		}
+		spin_unlock_irqrestore(&pdsc->adminq_lock, irqflags);
 	}
 
 	dev_dbg(pdsc->dev, "read admin queue completion idx %d:\n", index);

base-commit: e728258debd553c95d2e70f9cd97c9fde27c7130
-- 
2.17.1