[PATCH 1/4] nvme-tcp: fix a possible use-after-free in controller reset during load

[Sagi Grimberg]

>>   	struct nvme_tcp_cmd_pdu *pdu = ctrl->async_req.pdu;
>>   	struct nvme_command *cmd = &pdu->cmd;
>>   	u8 hdgst = nvme_tcp_hdgst_len(queue);
>> +	bool queue_ready = test_bit(NVME_TCP_Q_LIVE, &queue->flags);
>> +
>> +	if (ctrl->ctrl.state != NVME_CTRL_LIVE || !queue_ready)
> 
> Why do we need the local variable?

I can remove the local variable, didn't want to have a multiline
if condition.

> Also what prevents the controller or queue state to change just after
> this check?

The driver will make sure to flush ctrl->async_event_work _after_
changing the controller state (it is flushed in nvme_stop_ctrl).
Only after that it will continue to free the admin queue. So if
this check passed, it is safe to submit the aer command.

I think that the ctrl->state check should be sufficient. In fact, I
think we can move it to the core instead of doing it in the drivers:
--
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index dd18861f77c0..d60a6e65ff75 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -4251,6 +4251,8 @@ static void nvme_async_event_work(struct 
work_struct *work)
                 container_of(work, struct nvme_ctrl, async_event_work);

         nvme_aen_uevent(ctrl);
+       if (ctrl->state != NVME_CTRL_LIVE)
+               return;
         ctrl->ops->submit_async_event(ctrl);
  }
--

I'll send a revised patch.