From: Eric Biggers <ebiggers@kernel.org> To: "Martin K. Petersen" <martin.petersen@oracle.com> Cc: "Darrick J. Wong" <darrick.wong@oracle.com>, Satya Tangirala <satyat@google.com>, linux-block@vger.kernel.org, linux-scsi@vger.kernel.org, linux-fscrypt@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net, Barani Muthukumaran <bmuthuku@qti.qualcomm.com>, Kuohong Wang <kuohong.wang@mediatek.com>, Kim Boojin <boojin.kim@samsung.com> Subject: Re: [PATCH v6 2/9] block: Add encryption context to struct bio Date: Thu, 19 Dec 2019 19:52:37 -0800 [thread overview] Message-ID: <20191220035237.GB718@sol.localdomain> (raw) In-Reply-To: <yq1fthhdttv.fsf@oracle.com> On Wed, Dec 18, 2019 at 07:47:56PM -0500, Martin K. Petersen wrote: > > Eric, > > > There's not really any such thing as "use the bio integrity plumbing". > > blk-integrity just does blk-integrity; it's not a plumbing layer that > > allows other features to be supported. Well, in theory we could > > refactor and rename all the hooks to "blk-extra" and make them > > delegate to either blk-integrity or blk-crypto, but I think that would > > be overkill. > > I certainly don't expect your crypto stuff to plug in without any > modification to what we currently have. I'm just observing that the > existing plumbing is designed to have pluggable functions that let > filesystems attach additional information to bios on writes and process > additional attached information on reads. And the block layer already > handles slicing and dicing these attachments as the I/O traverses the > stack. > > There's also other stuff that probably won't be directly applicable or > interesting for your use case. It just seems like identifying actual > commonalities and differences would be worthwhile. > > Note that substantial changes to the integrity code would inevitably > lead to a lot of pain and suffering for me. So from that perspective I > am very happy if you leave it alone. From an architectural viewpoint, > however, it seems that there are more similarities than differences > between crypto and integrity. And we should avoid duplication where > possible. That's all. There are some similarities, like both being optional features that need extra per-bio information and hooks for bio merging, freeing, cloning, and advancing. However, the nature of the per-bio information is very different. Most of the complexity in blk-integrity is around managing of a separate integrity scatterlist for each bio, alongside the regular data scatterlist. That's not something we need or want for inline encryption. For each bio we just need a key, algorithm, data unit number, and data unit size. Since the data unit number (IV) is automatically incremented for each sector and the encryption is length-preserving, there's no per-sector data. (Granted, from a crypto perspective ideally one would use authenticated encryption, which does require per-sector data. However, no one seems interested in building hardware that supports it. So for the forseeable future, only length-preserving encryption is in scope for this.) Also, blk-crypto actually transforms the data whereas blk-integrity does not. > > What we could do, though, is say that at most one of blk-crypto and > > blk-integrity can be used at once on a given bio, and put the > > bi_integrity and bi_crypt_context pointers in union. (That would > > require allocating a REQ_INLINECRYPT bit so that we can tell what the > > pointer points to.) > > Absolutely. That's why it's a union. Putting your stuff there is a > prerequisite as far as I'm concerned. No need to grow the bio when the > two features are unlikely to coexist. We can revisit that later should > the need arise. There are some ways the two features could be supported simultaneously without using more space, like making the pointer point to a linked list of tagged structs, or making the struct contain both a bio_crypt_ctx and bio_integrity_payload (or whichever combination is enabled in kconfig). But it would be painful and I don't think people need this for now. So if people really aren't willing to accept the extra 8 bytes per bio even behind a kconfig option, my vote is we that we put bi_crypt_context in the union with bi_integrity, and add a flag REQ_INLINECRYPT (like REQ_INTEGRITY) that indicates that the bi_crypt_context member of the union is valid. We'd also need some error-handling to prevent the two features from actually being used together. It looks like there are several cases to consider. One of them is what happens if bio_crypt_set_ctx() is called when blk-integrity verification or generation is enabled for the disk. I suppose it could either return an error, or we could make blk-crypto use the crypto API fallback provided that it was modified to make the decryption stop relying on ->bi_crypt_context, which could be done by cloning the bio and using ->bi_private instead. - Eric
WARNING: multiple messages have this Message-ID (diff)
From: Eric Biggers <ebiggers@kernel.org> To: "Martin K. Petersen" <martin.petersen@oracle.com> Cc: linux-scsi@vger.kernel.org, "Darrick J. Wong" <darrick.wong@oracle.com>, Kuohong Wang <kuohong.wang@mediatek.com>, Kim Boojin <boojin.kim@samsung.com>, Barani Muthukumaran <bmuthuku@qti.qualcomm.com>, Satya Tangirala <satyat@google.com>, linux-block@vger.kernel.org, linux-fscrypt@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net Subject: Re: [f2fs-dev] [PATCH v6 2/9] block: Add encryption context to struct bio Date: Thu, 19 Dec 2019 19:52:37 -0800 [thread overview] Message-ID: <20191220035237.GB718@sol.localdomain> (raw) In-Reply-To: <yq1fthhdttv.fsf@oracle.com> On Wed, Dec 18, 2019 at 07:47:56PM -0500, Martin K. Petersen wrote: > > Eric, > > > There's not really any such thing as "use the bio integrity plumbing". > > blk-integrity just does blk-integrity; it's not a plumbing layer that > > allows other features to be supported. Well, in theory we could > > refactor and rename all the hooks to "blk-extra" and make them > > delegate to either blk-integrity or blk-crypto, but I think that would > > be overkill. > > I certainly don't expect your crypto stuff to plug in without any > modification to what we currently have. I'm just observing that the > existing plumbing is designed to have pluggable functions that let > filesystems attach additional information to bios on writes and process > additional attached information on reads. And the block layer already > handles slicing and dicing these attachments as the I/O traverses the > stack. > > There's also other stuff that probably won't be directly applicable or > interesting for your use case. It just seems like identifying actual > commonalities and differences would be worthwhile. > > Note that substantial changes to the integrity code would inevitably > lead to a lot of pain and suffering for me. So from that perspective I > am very happy if you leave it alone. From an architectural viewpoint, > however, it seems that there are more similarities than differences > between crypto and integrity. And we should avoid duplication where > possible. That's all. There are some similarities, like both being optional features that need extra per-bio information and hooks for bio merging, freeing, cloning, and advancing. However, the nature of the per-bio information is very different. Most of the complexity in blk-integrity is around managing of a separate integrity scatterlist for each bio, alongside the regular data scatterlist. That's not something we need or want for inline encryption. For each bio we just need a key, algorithm, data unit number, and data unit size. Since the data unit number (IV) is automatically incremented for each sector and the encryption is length-preserving, there's no per-sector data. (Granted, from a crypto perspective ideally one would use authenticated encryption, which does require per-sector data. However, no one seems interested in building hardware that supports it. So for the forseeable future, only length-preserving encryption is in scope for this.) Also, blk-crypto actually transforms the data whereas blk-integrity does not. > > What we could do, though, is say that at most one of blk-crypto and > > blk-integrity can be used at once on a given bio, and put the > > bi_integrity and bi_crypt_context pointers in union. (That would > > require allocating a REQ_INLINECRYPT bit so that we can tell what the > > pointer points to.) > > Absolutely. That's why it's a union. Putting your stuff there is a > prerequisite as far as I'm concerned. No need to grow the bio when the > two features are unlikely to coexist. We can revisit that later should > the need arise. There are some ways the two features could be supported simultaneously without using more space, like making the pointer point to a linked list of tagged structs, or making the struct contain both a bio_crypt_ctx and bio_integrity_payload (or whichever combination is enabled in kconfig). But it would be painful and I don't think people need this for now. So if people really aren't willing to accept the extra 8 bytes per bio even behind a kconfig option, my vote is we that we put bi_crypt_context in the union with bi_integrity, and add a flag REQ_INLINECRYPT (like REQ_INTEGRITY) that indicates that the bi_crypt_context member of the union is valid. We'd also need some error-handling to prevent the two features from actually being used together. It looks like there are several cases to consider. One of them is what happens if bio_crypt_set_ctx() is called when blk-integrity verification or generation is enabled for the disk. I suppose it could either return an error, or we could make blk-crypto use the crypto API fallback provided that it was modified to make the decryption stop relying on ->bi_crypt_context, which could be done by cloning the bio and using ->bi_private instead. - Eric _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
next prev parent reply other threads:[~2019-12-20 3:52 UTC|newest] Thread overview: 112+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-12-18 14:51 [PATCH v6 0/9] Inline Encryption Support Satya Tangirala 2019-12-18 14:51 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2019-12-18 14:51 ` [PATCH v6 1/9] block: Keyslot Manager for Inline Encryption Satya Tangirala 2019-12-18 14:51 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2019-12-18 20:13 ` Eric Biggers 2019-12-18 20:13 ` [f2fs-dev] " Eric Biggers 2020-01-17 9:10 ` Christoph Hellwig 2020-01-17 9:10 ` [f2fs-dev] " Christoph Hellwig 2019-12-18 14:51 ` [PATCH v6 2/9] block: Add encryption context to struct bio Satya Tangirala 2019-12-18 14:51 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2019-12-18 21:10 ` Eric Biggers 2019-12-18 21:10 ` [f2fs-dev] " Eric Biggers 2019-12-18 21:21 ` Darrick J. Wong 2019-12-18 21:21 ` [f2fs-dev] " Darrick J. Wong 2019-12-18 21:25 ` Martin K. Petersen 2019-12-18 21:25 ` [f2fs-dev] " Martin K. Petersen 2019-12-18 22:27 ` Eric Biggers 2019-12-18 22:27 ` [f2fs-dev] " Eric Biggers 2019-12-19 0:47 ` Martin K. Petersen 2019-12-19 0:47 ` [f2fs-dev] " Martin K. Petersen 2019-12-20 3:52 ` Eric Biggers [this message] 2019-12-20 3:52 ` Eric Biggers 2020-01-07 4:35 ` Martin K. Petersen 2020-01-07 4:35 ` [f2fs-dev] " Martin K. Petersen 2020-01-08 14:07 ` Christoph Hellwig 2020-01-08 14:07 ` [f2fs-dev] " Christoph Hellwig 2020-01-08 17:26 ` Eric Biggers 2020-01-08 17:26 ` [f2fs-dev] " Eric Biggers 2020-01-17 8:32 ` Christoph Hellwig 2020-01-17 8:32 ` [f2fs-dev] " Christoph Hellwig 2020-01-18 5:11 ` Eric Biggers 2020-01-18 5:11 ` [f2fs-dev] " Eric Biggers 2020-01-21 22:05 ` Satya Tangirala 2020-01-21 22:05 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2020-01-09 3:40 ` Martin K. Petersen 2020-01-09 3:40 ` [f2fs-dev] " Martin K. Petersen 2020-01-14 21:24 ` Eric Biggers 2020-01-14 21:24 ` [f2fs-dev] " Eric Biggers 2019-12-18 14:51 ` [PATCH v6 3/9] block: blk-crypto for Inline Encryption Satya Tangirala 2019-12-18 14:51 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2019-12-20 3:14 ` Eric Biggers 2019-12-20 3:14 ` [f2fs-dev] " Eric Biggers 2019-12-20 5:10 ` Eric Biggers 2019-12-20 5:10 ` [f2fs-dev] " Eric Biggers 2020-01-14 21:22 ` Eric Biggers 2020-01-14 21:22 ` [f2fs-dev] " Eric Biggers 2019-12-18 14:51 ` [PATCH v6 4/9] scsi: ufs: UFS driver v2.1 spec crypto additions Satya Tangirala 2019-12-18 14:51 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2020-01-17 12:31 ` Christoph Hellwig 2020-01-17 12:31 ` [f2fs-dev] " Christoph Hellwig 2019-12-18 14:51 ` [PATCH v6 5/9] scsi: ufs: UFS crypto API Satya Tangirala 2019-12-18 14:51 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2019-12-20 4:48 ` Eric Biggers 2019-12-20 4:48 ` [f2fs-dev] " Eric Biggers 2020-01-14 21:16 ` Eric Biggers 2020-01-14 21:16 ` [f2fs-dev] " Eric Biggers 2020-01-17 13:51 ` Christoph Hellwig 2020-01-17 13:51 ` [f2fs-dev] " Christoph Hellwig 2019-12-18 14:51 ` [PATCH v6 6/9] scsi: ufs: Add inline encryption support to UFS Satya Tangirala 2019-12-18 14:51 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2019-12-20 5:44 ` Eric Biggers 2019-12-20 5:44 ` [f2fs-dev] " Eric Biggers 2020-01-17 13:58 ` Christoph Hellwig 2020-01-17 13:58 ` [f2fs-dev] " Christoph Hellwig 2020-01-18 5:27 ` Eric Biggers 2020-01-18 5:27 ` [f2fs-dev] " Eric Biggers 2020-02-05 18:07 ` Christoph Hellwig 2020-02-05 18:07 ` [f2fs-dev] " Christoph Hellwig 2020-01-18 3:58 ` Eric Biggers 2020-01-18 3:58 ` [f2fs-dev] " Eric Biggers 2020-02-05 20:47 ` Eric Biggers 2020-02-05 20:47 ` [f2fs-dev] " Eric Biggers 2019-12-18 14:51 ` [PATCH v6 7/9] fscrypt: add inline encryption support Satya Tangirala 2019-12-18 14:51 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2020-01-14 21:12 ` Eric Biggers 2020-01-14 21:12 ` [f2fs-dev] " Eric Biggers 2019-12-18 14:51 ` [PATCH v6 8/9] f2fs: " Satya Tangirala 2019-12-18 14:51 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2019-12-20 4:23 ` Eric Biggers 2019-12-20 4:23 ` [f2fs-dev] " Eric Biggers 2019-12-18 14:51 ` [PATCH v6 9/9] ext4: " Satya Tangirala 2019-12-18 14:51 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2019-12-19 0:12 ` Eric Biggers 2019-12-19 0:12 ` [f2fs-dev] " Eric Biggers 2019-12-19 0:31 ` Satya Tangirala 2019-12-19 0:31 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2019-12-22 0:16 ` Eric Biggers 2019-12-22 0:16 ` [f2fs-dev] " Eric Biggers 2020-01-08 14:05 ` [PATCH v6 0/9] Inline Encryption Support Christoph Hellwig 2020-01-08 14:05 ` [f2fs-dev] " Christoph Hellwig 2020-01-08 18:43 ` Satya Tangirala 2020-01-08 18:43 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2020-01-17 8:52 ` Christoph Hellwig 2020-01-17 8:52 ` [f2fs-dev] " Christoph Hellwig 2020-02-01 0:53 ` Satya Tangirala 2020-02-01 0:53 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2020-02-03 9:15 ` Christoph Hellwig 2020-02-03 9:15 ` [f2fs-dev] " Christoph Hellwig 2020-02-04 3:39 ` Satya Tangirala 2020-02-04 3:39 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2020-02-04 14:58 ` Christoph Hellwig 2020-02-04 14:58 ` [f2fs-dev] " Christoph Hellwig 2020-02-04 21:21 ` Eric Biggers 2020-02-04 21:21 ` [f2fs-dev] " Eric Biggers 2020-02-05 7:36 ` Eric Biggers 2020-02-05 7:36 ` [f2fs-dev] " Eric Biggers 2020-02-05 18:05 ` Christoph Hellwig 2020-02-05 18:05 ` [f2fs-dev] " Christoph Hellwig 2020-02-21 12:30 ` Satya Tangirala 2020-02-21 12:30 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2020-02-21 14:20 ` Christoph Hellwig 2020-02-21 14:20 ` [f2fs-dev] " Christoph Hellwig
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20191220035237.GB718@sol.localdomain \ --to=ebiggers@kernel.org \ --cc=bmuthuku@qti.qualcomm.com \ --cc=boojin.kim@samsung.com \ --cc=darrick.wong@oracle.com \ --cc=kuohong.wang@mediatek.com \ --cc=linux-block@vger.kernel.org \ --cc=linux-f2fs-devel@lists.sourceforge.net \ --cc=linux-fscrypt@vger.kernel.org \ --cc=linux-fsdevel@vger.kernel.org \ --cc=linux-scsi@vger.kernel.org \ --cc=martin.petersen@oracle.com \ --cc=satyat@google.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.