* Re: [PATCH 5/9] block: support diskcipher [not found] <CGME20190827083334epcas2p115d479190b9a72c886f66569add78203@epcas2p1.samsung.com> @ 2019-08-27 8:33 ` boojin.kim 2019-08-27 16:40 ` Theodore Y. Ts'o 0 siblings, 1 reply; 7+ messages in thread From: boojin.kim @ 2019-08-27 8:33 UTC (permalink / raw) To: 'Satya Tangirala' Cc: 'Herbert Xu', 'David S. Miller', 'Eric Biggers', 'Theodore Y. Ts'o', 'Chao Yu', 'Jaegeuk Kim', 'Andreas Dilger', 'Theodore Ts'o', dm-devel, 'Mike Snitzer', 'Alasdair Kergon', 'Jens Axboe', 'Krzysztof Kozlowski', 'Kukjin Kim', 'Jaehoon Chung', 'Ulf Hansson', linux-crypto, linux-kernel, linux-fscrypt, linux-mmc, linux-samsung-soc, linux-block, linux-ext4, linux-f2fs-devel, linux-samsung-soc, linux-arm-kernel, linux-fsdevel On Wed, Aug 21, 2019 at 5:10 AM Satya Tangirala <satyat@kernel.dk> wrote: > > Hi Boojin, > > We're very keen to make sure that our approach to inline encryption can > work with diverse hardware, including Samsung's FMP hardware; if you > can see any issues with using our approach with your hardware please > let us know. > > We understand that a possible concern for getting FMP working with our > patch series for Inline Encryption Support at > > https://lore.kernel.org/linux-block/20190821075714.65140-1-satyat@google.com / > > is that unlike some inline encryption hardware (and also unlike the JEDEC > UFS v2.1 spec), FMP doesn't have the concept of a limited number of > keyslots - to address that difference we have a "passthrough keyslot > manager", which we put up on top of our patch series for inline encryption > support at > > https://android-review.googlesource.com/c/kernel/common/+/980137/2 > > Setting up a passthrough keyslot manager in the request queue of a > device allows the device to receive a bio's encryption context as-is with > the bio, which is what FMP would prefer. Are there any issues with > using the passthrough keyslot manager for FMP? > > Thanks! > Satya Dear Satya. Keyslot manager is a good solution for ICE. And probably no issue for FMP. But, I think it's complicated for FMP because FMP doesn't need any keyslot control. Crypto API that FMP's using is simply, stable, and supports test. FMP has been mass producing and certificating using crypto APIs for several years. So I wants to keep our current crypto API solution. But, I'm looking at your patch. And I will keep examining at your patch because our goal is to run the FMP on the mainline kernel. Thanks for your reply. Boojin Kim. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 5/9] block: support diskcipher 2019-08-27 8:33 ` [PATCH 5/9] block: support diskcipher boojin.kim @ 2019-08-27 16:40 ` Theodore Y. Ts'o 0 siblings, 0 replies; 7+ messages in thread From: Theodore Y. Ts'o @ 2019-08-27 16:40 UTC (permalink / raw) To: boojin.kim Cc: 'Satya Tangirala', 'Herbert Xu', 'David S. Miller', 'Eric Biggers', 'Chao Yu', 'Jaegeuk Kim', 'Andreas Dilger', dm-devel, 'Mike Snitzer', 'Alasdair Kergon', 'Jens Axboe', 'Krzysztof Kozlowski', 'Kukjin Kim', 'Jaehoon Chung', 'Ulf Hansson', linux-crypto, linux-kernel, linux-fscrypt, linux-mmc, linux-samsung-soc, linux-block, linux-ext4, linux-f2fs-devel, linux-arm-kernel, linux-fsdevel On Tue, Aug 27, 2019 at 05:33:33PM +0900, boojin.kim wrote: > > Dear Satya. > Keyslot manager is a good solution for ICE. And probably no issue for FMP. > But, I think it's complicated for FMP because FMP doesn't need > any keyslot control. Hi Boojin, I think the important thing to realize here is that there are a large number of hardware devices for which the keyslot manager *is* needed. And from the upstream kernel's perspective, supporting two different schemes for supporting the inline encryption feature is more complexity than just supporting one which is general enough to support a wider variety of hardware devices. If you want somethig which is only good for the hardware platform you are charged to support, that's fine if it's only going to be in a Samsung-specific kernel. But if your goal is to get something that works upstream, especially if it requires changes in core layers of the kernel, it's important that it's general enough to support most, if not all, if the hardware devices in the industry. Regards, - Ted ^ permalink raw reply [flat|nested] 7+ messages in thread
[parent not found: <CGME20190828022055epcas2p25525077d0a5a3fa5a2027bac06a10bc1@epcas2p2.samsung.com>]
* Re: [PATCH 5/9] block: support diskcipher [not found] <CGME20190828022055epcas2p25525077d0a5a3fa5a2027bac06a10bc1@epcas2p2.samsung.com> @ 2019-08-28 2:20 ` boojin.kim 0 siblings, 0 replies; 7+ messages in thread From: boojin.kim @ 2019-08-28 2:20 UTC (permalink / raw) To: 'Theodore Y. Ts'o' Cc: 'Herbert Xu', 'David S. Miller', 'Eric Biggers', 'Theodore Y. Ts'o', 'Chao Yu', 'Jaegeuk Kim', 'Andreas Dilger', 'Theodore Ts'o', dm-devel, 'Mike Snitzer', 'Alasdair Kergon', 'Jens Axboe', 'Krzysztof Kozlowski', 'Kukjin Kim', 'Jaehoon Chung', 'Ulf Hansson', linux-crypto, linux-kernel, linux-fscrypt, linux-mmc, linux-samsung-soc, linux-block, linux-ext4, linux-f2fs-devel, linux-samsung-soc, linux-arm-kernel, linux-fsdevel On Tue, Aug 27, 2019 at 05:33:33PM +0900, boojin.kim wrote: > > Hi Boojin, > > I think the important thing to realize here is that there are a large > number of hardware devices for which the keyslot manager *is* needed. > And from the upstream kernel's perspective, supporting two different > schemes for supporting the inline encryption feature is more > complexity than just supporting one which is general enough to support > a wider variety of hardware devices. > > If you want somethig which is only good for the hardware platform you > are charged to support, that's fine if it's only going to be in a > Samsung-specific kernel. But if your goal is to get something that > works upstream, especially if it requires changes in core layers of > the kernel, it's important that it's general enough to support most, > if not all, if the hardware devices in the industry. > > Regards, I understood your reply. But, Please consider the diskcipher isn't just for FMP. The UFS in Samsung SoC also has UFS ICE. This UFS ICE can be registered as an algorithm of diskcipher like FMP. Following is my opinion to introduce diskcipher. I think the common feature of ICE like FMP and UFS ICE, is 'exposing cipher text to storage". And, Crypto test is also important for ICE. Diskcipher supports the common feature of ICE. I think specific functions for each ICE such as the key control of UFS ICE and the writing crypto table of FMP can be processed at algorithm level. Thanks for your reply. Boojin Kim. ^ permalink raw reply [flat|nested] 7+ messages in thread
[parent not found: <CGME20190822005438epcas2p337aba06b328cdcdd1549395f0bbcfdbc@epcas2p3.samsung.com>]
* Re: [PATCH 5/9] block: support diskcipher [not found] <CGME20190822005438epcas2p337aba06b328cdcdd1549395f0bbcfdbc@epcas2p3.samsung.com> @ 2019-08-22 0:54 ` boojin.kim 0 siblings, 0 replies; 7+ messages in thread From: boojin.kim @ 2019-08-22 0:54 UTC (permalink / raw) To: axboe, linux-block, linux-kernel Cc: 'Herbert Xu', 'David S. Miller', 'Eric Biggers', 'Theodore Y. Ts'o', 'Chao Yu', 'Jaegeuk Kim', 'Andreas Dilger', 'Theodore Ts'o', dm-devel, 'Mike Snitzer', 'Alasdair Kergon', 'Jens Axboe', 'Krzysztof Kozlowski', 'Kukjin Kim', 'Jaehoon Chung', 'Ulf Hansson', linux-crypto, linux-fscrypt, linux-mmc, linux-samsung-soc, linux-ext4, linux-f2fs-devel, linux-samsung-soc, linux-arm-kernel, linux-fsdevel On 8/21/19 21:09 AM, Jens Axboe wrote: > This isn't going to happen. With this, and the inline encryption > proposed by Google, we'll bloat the bio even more. At least the Google > approach didn't include bio iter changes as well. > Please work it out between yourselves so we can have a single, clean > abstraction that works for both. I'm looking at inline encryption by Google. I will find compatibility with inline encryption to avoid conflicts in BIO/F2FS. And changing bio iter has a benefit for diskcipher. Without changing bio iter, diskcipher should control(alloc/free) a buffer to hold a 'bi_dun' variable for every bio. But changing bio iter is difficult to accept for block layer, I will modify the diskcipher. And, as you mentioned, inline encryption by Google has this control. So I might be able to use it. Thanks. Boojin Kim. ^ permalink raw reply [flat|nested] 7+ messages in thread
[parent not found: <CGME20190821064226epcas2p2835b8a9084988b79107e54abfc5e7dab@epcas2p2.samsung.com>]
* [PATCH 5/9] block: support diskcipher [not found] <CGME20190821064226epcas2p2835b8a9084988b79107e54abfc5e7dab@epcas2p2.samsung.com> @ 2019-08-21 6:42 ` boojin.kim 2019-08-21 12:09 ` Jens Axboe 0 siblings, 1 reply; 7+ messages in thread From: boojin.kim @ 2019-08-21 6:42 UTC (permalink / raw) To: 'Jens Axboe', linux-block, linux-kernel Cc: 'Herbert Xu', 'David S. Miller', 'Eric Biggers', 'Theodore Y. Ts'o', 'Chao Yu', 'Jaegeuk Kim', 'Andreas Dilger', 'Theodore Ts'o', dm-devel, 'Mike Snitzer', 'Alasdair Kergon', 'Jens Axboe', 'Krzysztof Kozlowski', 'Kukjin Kim', 'Jaehoon Chung', 'Ulf Hansson', linux-crypto, linux-kernel, linux-fscrypt, linux-mmc, linux-samsung-soc, linux-block, linux-ext4, linux-f2fs-devel, linux-samsung-soc, linux-arm-kernel, linux-fsdevel This patch supports crypto information to be maintained via BIO and passed to the storage driver. To do this, 'bi_aux_private', 'REQ_CYPTE' and 'bi_dun' are added to the block layer. 'bi_aux_private' is added for loading additional private information into BIO. 'REQ_CRYPT' is added to distinguish that bi_aux_private is being used for diskcipher. F2FS among encryption users uses DUN(device unit number) as the IV(initial vector) for cryptographic operations. DUN is stored in 'bi_dun' of bi_iter as a specific value for each BIO. Before attempting to merge the two BIOs, the operation is also added to verify that the crypto information contained in two BIOs is consistent. Cc: Jens Axboe <axboe@kernel.dk> Signed-off-by: Boojin Kim <boojin.kim@samsung.com> --- block/bio.c | 1 + block/blk-merge.c | 19 +++++++++++++++++-- block/bounce.c | 5 ++++- include/linux/bio.h | 10 ++++++++++ include/linux/blk_types.h | 4 ++++ include/linux/bvec.h | 3 +++ 6 files changed, 39 insertions(+), 3 deletions(-) diff --git a/block/bio.c b/block/bio.c index 5476965..c60eb8e 100644 --- a/block/bio.c +++ b/block/bio.c @@ -588,6 +588,7 @@ void __bio_clone_fast(struct bio *bio, struct bio *bio_src) bio->bi_write_hint = bio_src->bi_write_hint; bio->bi_iter = bio_src->bi_iter; bio->bi_io_vec = bio_src->bi_io_vec; + bio->bi_aux_private = bio_src->bi_aux_private; bio_clone_blkg_association(bio, bio_src); blkcg_bio_issue_init(bio); diff --git a/block/blk-merge.c b/block/blk-merge.c index 48e6725..d031257 100644 --- a/block/blk-merge.c +++ b/block/blk-merge.c @@ -7,6 +7,7 @@ #include <linux/bio.h> #include <linux/blkdev.h> #include <linux/scatterlist.h> +#include <crypto/diskcipher.h> #include <trace/events/block.h> @@ -576,6 +577,8 @@ int ll_back_merge_fn(struct request *req, struct bio *bio, unsigned int nr_segs) if (blk_integrity_rq(req) && integrity_req_gap_back_merge(req, bio)) return 0; + if (blk_try_merge(req, bio) != ELEVATOR_BACK_MERGE) + return 0; if (blk_rq_sectors(req) + bio_sectors(bio) > blk_rq_get_max_sectors(req, blk_rq_pos(req))) { req_set_nomerge(req->q, req); @@ -592,6 +595,8 @@ int ll_front_merge_fn(struct request *req, struct bio *bio, unsigned int nr_segs if (blk_integrity_rq(req) && integrity_req_gap_front_merge(req, bio)) return 0; + if (blk_try_merge(req, bio) != ELEVATOR_FRONT_MERGE) + return 0; if (blk_rq_sectors(req) + bio_sectors(bio) > blk_rq_get_max_sectors(req, bio->bi_iter.bi_sector)) { req_set_nomerge(req->q, req); @@ -738,6 +743,9 @@ static struct request *attempt_merge(struct request_queue *q, !blk_write_same_mergeable(req->bio, next->bio)) return NULL; + if (!crypto_diskcipher_blk_mergeble(req->bio, next->bio)) + return NULL; + /* * Don't allow merge of different write hints, or for a hint with * non-hint IO. @@ -887,9 +895,16 @@ enum elv_merge blk_try_merge(struct request *rq, struct bio *bio) { if (blk_discard_mergable(rq)) return ELEVATOR_DISCARD_MERGE; - else if (blk_rq_pos(rq) + blk_rq_sectors(rq) == bio->bi_iter.bi_sector) + else if (blk_rq_pos(rq) + blk_rq_sectors(rq) == + bio->bi_iter.bi_sector) { + if (!crypto_diskcipher_blk_mergeble(rq->bio, bio)) + return ELEVATOR_NO_MERGE; return ELEVATOR_BACK_MERGE; - else if (blk_rq_pos(rq) - bio_sectors(bio) == bio->bi_iter.bi_sector) + } else if (blk_rq_pos(rq) - bio_sectors(bio) == + bio->bi_iter.bi_sector) { + if (!crypto_diskcipher_blk_mergeble(bio, rq->bio)) + return ELEVATOR_NO_MERGE; return ELEVATOR_FRONT_MERGE; + } return ELEVATOR_NO_MERGE; } diff --git a/block/bounce.c b/block/bounce.c index f8ed677..720b065 100644 --- a/block/bounce.c +++ b/block/bounce.c @@ -252,7 +252,10 @@ static struct bio *bounce_clone_bio(struct bio *bio_src, gfp_t gfp_mask, bio->bi_write_hint = bio_src->bi_write_hint; bio->bi_iter.bi_sector = bio_src->bi_iter.bi_sector; bio->bi_iter.bi_size = bio_src->bi_iter.bi_size; - + bio->bi_aux_private = bio_src->bi_aux_private; +#ifdef CONFIG_CRYPTO_DISKCIPHER + bio->bi_iter.bi_dun = bio_src->bi_iter.bi_dun; +#endif switch (bio_op(bio)) { case REQ_OP_DISCARD: case REQ_OP_SECURE_ERASE: diff --git a/include/linux/bio.h b/include/linux/bio.h index 3cdb84c..351e65e 100644 --- a/include/linux/bio.h +++ b/include/linux/bio.h @@ -49,6 +49,12 @@ #define bio_sectors(bio) bvec_iter_sectors((bio)->bi_iter) #define bio_end_sector(bio) bvec_iter_end_sector((bio)->bi_iter) +#ifdef CONFIG_CRYPTO_DISKCIPHER +#define bio_dun(bio) ((bio)->bi_iter.bi_dun) +#define bio_duns(bio) (bio_sectors(bio) >> 3) /* 4KB unit */ +#define bio_end_dun(bio) (bio_dun(bio) + bio_duns(bio)) +#endif + /* * Return the data direction, READ or WRITE. */ @@ -143,6 +149,10 @@ static inline void bio_advance_iter(struct bio *bio, struct bvec_iter *iter, { iter->bi_sector += bytes >> 9; +#ifdef CONFIG_CRYPTO_DISKCIPHER + if (iter->bi_dun) + iter->bi_dun += bytes >> 12; +#endif if (bio_no_advance_iter(bio)) iter->bi_size -= bytes; else diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h index 75059c1..117119a 100644 --- a/include/linux/blk_types.h +++ b/include/linux/blk_types.h @@ -160,6 +160,8 @@ struct bio { bio_end_io_t *bi_end_io; void *bi_private; + void *bi_aux_private; + #ifdef CONFIG_BLK_CGROUP /* * Represents the association of the css and request_queue for the bio. @@ -311,6 +313,7 @@ enum req_flag_bits { __REQ_INTEGRITY, /* I/O includes block integrity payload */ __REQ_FUA, /* forced unit access */ __REQ_PREFLUSH, /* request for cache flush */ + __REQ_CRYPT, /* request inline crypt */ __REQ_RAHEAD, /* read ahead, can fail anytime */ __REQ_BACKGROUND, /* background IO */ __REQ_NOWAIT, /* Don't wait if request will block */ @@ -343,6 +346,7 @@ enum req_flag_bits { #define REQ_NOMERGE (1ULL << __REQ_NOMERGE) #define REQ_IDLE (1ULL << __REQ_IDLE) #define REQ_INTEGRITY (1ULL << __REQ_INTEGRITY) +#define REQ_CRYPT (1ULL << __REQ_CRYPT) #define REQ_FUA (1ULL << __REQ_FUA) #define REQ_PREFLUSH (1ULL << __REQ_PREFLUSH) #define REQ_RAHEAD (1ULL << __REQ_RAHEAD) diff --git a/include/linux/bvec.h b/include/linux/bvec.h index a032f01..5f89641 100644 --- a/include/linux/bvec.h +++ b/include/linux/bvec.h @@ -30,6 +30,9 @@ struct bvec_iter { unsigned int bi_bvec_done; /* number of bytes completed in current bvec */ +#ifdef CONFIG_CRYPTO_DISKCIPHER + u64 bi_dun; +#endif }; struct bvec_iter_all { -- 2.7.4 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 5/9] block: support diskcipher 2019-08-21 6:42 ` boojin.kim @ 2019-08-21 12:09 ` Jens Axboe 2019-08-23 2:35 ` Satya Tangirala 0 siblings, 1 reply; 7+ messages in thread From: Jens Axboe @ 2019-08-21 12:09 UTC (permalink / raw) To: boojin.kim, linux-block, linux-kernel Cc: 'Herbert Xu', 'David S. Miller', 'Eric Biggers', 'Theodore Y. Ts'o', 'Chao Yu', 'Jaegeuk Kim', 'Andreas Dilger', dm-devel, 'Mike Snitzer', 'Alasdair Kergon', 'Krzysztof Kozlowski', 'Kukjin Kim', 'Jaehoon Chung', 'Ulf Hansson', linux-crypto, linux-fscrypt, linux-mmc, linux-samsung-soc, linux-ext4, linux-f2fs-devel, linux-arm-kernel, linux-fsdevel On 8/21/19 12:42 AM, boojin.kim wrote: > This patch supports crypto information to be maintained via BIO > and passed to the storage driver. > > To do this, 'bi_aux_private', 'REQ_CYPTE' and 'bi_dun' are added > to the block layer. > > 'bi_aux_private' is added for loading additional private information into > BIO. > 'REQ_CRYPT' is added to distinguish that bi_aux_private is being used > for diskcipher. > F2FS among encryption users uses DUN(device unit number) as > the IV(initial vector) for cryptographic operations. > DUN is stored in 'bi_dun' of bi_iter as a specific value for each BIO. > > Before attempting to merge the two BIOs, the operation is also added to > verify that the crypto information contained in two BIOs is consistent. This isn't going to happen. With this, and the inline encryption proposed by Google, we'll bloat the bio even more. At least the Google approach didn't include bio iter changes as well. Please work it out between yourselves so we can have a single, clean abstraction that works for both. -- Jens Axboe ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 5/9] block: support diskcipher 2019-08-21 12:09 ` Jens Axboe @ 2019-08-23 2:35 ` Satya Tangirala 0 siblings, 0 replies; 7+ messages in thread From: Satya Tangirala @ 2019-08-23 2:35 UTC (permalink / raw) To: Jens Axboe, boojin.kim Cc: linux-block, linux-kernel, Herbert Xu, David S. Miller, Eric Biggers, Theodore Y. Ts'o, Chao Yu, Jaegeuk Kim, Andreas Dilger, dm-devel, Mike Snitzer, Alasdair Kergon, Krzysztof Kozlowski, Kukjin Kim, Jaehoon Chung, Ulf Hansson, linux-crypto, linux-fscrypt, linux-mmc, linux-samsung-soc, linux-ext4, linux-f2fs-devel, linux-arm-kernel, linux-fsdevel On Wed, Aug 21, 2019 at 5:10 AM Jens Axboe <axboe@kernel.dk> wrote: > > On 8/21/19 12:42 AM, boojin.kim wrote: > > This patch supports crypto information to be maintained via BIO > > and passed to the storage driver. > > > > To do this, 'bi_aux_private', 'REQ_CYPTE' and 'bi_dun' are added > > to the block layer. > > > > 'bi_aux_private' is added for loading additional private information into > > BIO. > > 'REQ_CRYPT' is added to distinguish that bi_aux_private is being used > > for diskcipher. > > F2FS among encryption users uses DUN(device unit number) as > > the IV(initial vector) for cryptographic operations. > > DUN is stored in 'bi_dun' of bi_iter as a specific value for each BIO. > > > > Before attempting to merge the two BIOs, the operation is also added to > > verify that the crypto information contained in two BIOs is consistent. > > This isn't going to happen. With this, and the inline encryption > proposed by Google, we'll bloat the bio even more. At least the Google > approach didn't include bio iter changes as well. > > Please work it out between yourselves so we can have a single, clean > abstraction that works for both. > > -- > Jens Axboe > Hi Boojin, We're very keen to make sure that our approach to inline encryption can work with diverse hardware, including Samsung's FMP hardware; if you can see any issues with using our approach with your hardware please let us know. We understand that a possible concern for getting FMP working with our patch series for Inline Encryption Support at https://lore.kernel.org/linux-block/20190821075714.65140-1-satyat@google.com/ is that unlike some inline encryption hardware (and also unlike the JEDEC UFS v2.1 spec), FMP doesn't have the concept of a limited number of keyslots - to address that difference we have a "passthrough keyslot manager", which we put up on top of our patch series for inline encryption support at https://android-review.googlesource.com/c/kernel/common/+/980137/2 Setting up a passthrough keyslot manager in the request queue of a device allows the device to receive a bio's encryption context as-is with the bio, which is what FMP would prefer. Are there any issues with using the passthrough keyslot manager for FMP? Thanks! Satya ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-08-28 2:21 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <CGME20190827083334epcas2p115d479190b9a72c886f66569add78203@epcas2p1.samsung.com> 2019-08-27 8:33 ` [PATCH 5/9] block: support diskcipher boojin.kim 2019-08-27 16:40 ` Theodore Y. Ts'o [not found] <CGME20190828022055epcas2p25525077d0a5a3fa5a2027bac06a10bc1@epcas2p2.samsung.com> 2019-08-28 2:20 ` boojin.kim [not found] <CGME20190822005438epcas2p337aba06b328cdcdd1549395f0bbcfdbc@epcas2p3.samsung.com> 2019-08-22 0:54 ` boojin.kim [not found] <CGME20190821064226epcas2p2835b8a9084988b79107e54abfc5e7dab@epcas2p2.samsung.com> 2019-08-21 6:42 ` boojin.kim 2019-08-21 12:09 ` Jens Axboe 2019-08-23 2:35 ` Satya Tangirala
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).