netdev - Re: [PATCH v20 07/20] nvme-tcp: RX DDGST offload

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <253jzq0irx5.fsf@nvidia.com>
Date: Wed, 29 Nov 2023 15:56:54 +0200
From: Aurelien Aptel <aaptel@...dia.com>
To: Sagi Grimberg <sagi@...mberg.me>, linux-nvme@...ts.infradead.org,
 netdev@...r.kernel.org, hch@....de, kbusch@...nel.org, axboe@...com,
 chaitanyak@...dia.com, davem@...emloft.net, kuba@...nel.org
Cc: Yoray Zack <yorayz@...dia.com>, aurelien.aptel@...il.com,
 smalin@...dia.com, malin1024@...il.com, ogerlitz@...dia.com,
 borisp@...dia.com, galshalom@...dia.com, mgurtovoy@...dia.com
Subject: Re: [PATCH v20 07/20] nvme-tcp: RX DDGST offload

Sagi Grimberg <sagi@...mberg.me> writes:
>> +static void nvme_tcp_ddp_ddgst_recalc(struct ahash_request *hash,
>> +                                   struct request *rq,
>> +                                   __le32 *ddgst)
>> +{
>> +     struct nvme_tcp_request *req;
>> +
>> +     if (!rq)
>> +             return;
>
> How is this even possible? And what happens down the road if this is
> indeed a null rq?

You are correct, this isn't possible. We can see the req is fetched
earlier in nvme_tcp_recv_pdu() so it must exist.
We will remove it.

>> +
>> +     req = blk_mq_rq_to_pdu(rq);
>> +     ahash_request_set_crypt(hash, req->ddp.sg_table.sgl, (u8 *)ddgst,
>> +                             req->data_len);
>> +     crypto_ahash_digest(hash);
>> +}
>> +
>>   static bool nvme_tcp_resync_request(struct sock *sk, u32 seq, u32 flags);
>>   static void nvme_tcp_ddp_teardown_done(void *ddp_ctx);
>>   static const struct ulp_ddp_ulp_ops nvme_tcp_ddp_ulp_ops = {
>> @@ -430,6 +459,8 @@ static void nvme_tcp_setup_ddp(struct nvme_tcp_queue *queue,
>>   static int nvme_tcp_offload_socket(struct nvme_tcp_queue *queue)
>>   {
>>       struct ulp_ddp_config config = {.type = ULP_DDP_NVME};
>> +     bool offload_ddgst_rx = ulp_ddp_is_cap_active(queue->ctrl->ddp_netdev,
>> +                                                   ULP_DDP_CAP_NVME_TCP_DDGST_RX);
>
> Not sure a local variable is needed here.

Ok we will remove it.

Thanks