Re: [PATCH 8/9] nvmet-tcp: support secure channel concatenation

[Sagi Grimberg <sagi@xxxxxxxxxxx>]

> > > diff --git a/drivers/nvme/target/tcp.c b/drivers/nvme/target/tcp.c
> > > index 7c51c2a8c109..671600b5c64b 100644
> > > --- a/drivers/nvme/target/tcp.c
> > > +++ b/drivers/nvme/target/tcp.c
> > > @@ -1073,10 +1073,11 @@ static int nvmet_tcp_done_recv_pdu(struct 
> > > nvmet_tcp_queue *queue)
> > >       if (unlikely(!nvmet_req_init(req, &queue->nvme_cq,
> > >               &queue->nvme_sq, &nvmet_tcp_ops))) {
> > > -        pr_err("failed cmd %p id %d opcode %d, data_len: %d\n",
> > > +        pr_err("failed cmd %p id %d opcode %d, data_len: %d, 
> > > status: %04x\n",
> > >               req->cmd, req->cmd->common.command_id,
> > >               req->cmd->common.opcode,
> > > - le32_to_cpu(req->cmd->common.dptr.sgl.length));
> > > + le32_to_cpu(req->cmd->common.dptr.sgl.length),
> > > +               le16_to_cpu(req->cqe->status));
> > >           nvmet_tcp_handle_req_failure(queue, queue->cmd, req);
> > >           return 0;
> > > @@ -1602,6 +1603,7 @@ static void 
> > > nvmet_tcp_release_queue_work(struct work_struct *w)
> > >       /* stop accepting incoming data */
> > >       queue->rcv_state = NVMET_TCP_RECV_ERR;
> > > +    nvmet_sq_put_tls_key(&queue->nvme_sq);
> > >       nvmet_tcp_uninit_data_in_cmds(queue);
> > >       nvmet_sq_destroy(&queue->nvme_sq);
> > >       cancel_work_sync(&queue->io_work);
> > > @@ -1807,6 +1809,23 @@ static void nvmet_tcp_tls_handshake_done(void 
> > > *data, int status,
> > >       spin_unlock_bh(&queue->state_lock);
> > > cancel_delayed_work_sync(&queue->tls_handshake_tmo_work);
> > > +
> > > +    if (!status) {
> > > +        struct key *tls_key = nvme_tls_key_lookup(peerid);
> > > +
> > > +        if (IS_ERR(tls_key)) {
> >
> > It is not clear to me how this can happen. Can you explain?
> Passing key information between kernel and handshake daemon is
> done purely by key IDs (not the keys itself).
> So we will be getting a key ID back from the handshaked daemon,
> and we need to validate that; the user (or admin software) could
> have invalidated the key while the handshake was running, or before
> we had a chance to process the reply from the handshake daemon.

Got it. thanks.