linux-kernel - RE:(2) [PATCH v2 02/14] block: bio-integrity: modify bio_integrity_add

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]

Date:   Tue, 16 May 2023 21:54:07 +0900
From:   Jinyoung CHOI <j-young.choi@...sung.com>
To:     "hch@....de" <hch@....de>
CC:     "axboe@...nel.dk" <axboe@...nel.dk>,
        "kbusch@...nel.org" <kbusch@...nel.org>,
        "sagi@...mberg.me" <sagi@...mberg.me>,
        "jejb@...ux.ibm.com" <jejb@...ux.ibm.com>,
        "martin.petersen@...cle.com" <martin.petersen@...cle.com>,
        "johannes.thumshirn@....com" <johannes.thumshirn@....com>,
        "kch@...dia.com" <kch@...dia.com>,
        "willy@...radead.org" <willy@...radead.org>,
        "linux-block@...r.kernel.org" <linux-block@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE:(2) [PATCH v2 02/14] block: bio-integrity: modify
 bio_integrity_add_page()

>Hi Jinyoung,
>
>can you work a bit on the commit log and especially the subject line?
>
>I'd word this as something like:
>
>"Subject: bio-integrity: create multi-page bvecs in bio_integrity_add_page()
>
>Allow bio_integrity_add_page to create multi-page bvecs, just like
>the bio payloads.  This simplifies adding larger payloads, and fixes
>support for non-tiny workloads with nvme, which stopped using scatterlist
>for metadata a while ago"
>

Hi. Christoph. I checked late.

OK. I will revise it like that.

>It should probably also mentioned somewhere that you did an audit to
>ensure all drivers and the core code is fine with these multi-page
>segments.  If it's not, this patch should only be added after that
>has been made the case.

Regardless of a single-page or a multi-page configuration,
block_rq_count_integrity_sg() and block_rq_map_integry_sg() check
the pages included in the bip to create a segment for sg.
So... there doesn't seem to be a problem yet.

First, the nvme device that supports fips was tested using the above
functions, and it was also checked with the dix of scsi_debug turned on.
I am also trying to test the sas device that supports sed. It seems that
a problem occurs while going through the laid controller (mpt3sas driver).
It works normally in DIF mode, but protection error occurs when DIX mode
is forcibly activated. Of course, the same is happening in the original code.
I will check additionally and be careful not to cause any problems.

>
>I think the extra arguments struct is a bit overcompliated, and mostly
>due to me making the existing code to weird things in the low-level
>helpers.  With the "rationalize the flow in bio_add_page and friends"
>series I just sent out, I think we can drop the previous patch and
>simplify this one down to:

I tried not to make a big change from the existing logic. So I added
struct unnecessarily. I thought it would be a problem if I added 
conditions to the callers and made them call the common code.
Thank you for solving this part.
If your code merges, I will modify it accordingly and proceed.

Best Regards,
Jinyoung.