From: Eric Biggers <ebiggers@kernel.org> To: Satya Tangirala <satyat@google.com> Cc: linux-fscrypt@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net, linux-ext4@vger.kernel.org, Jaegeuk Kim <jaegeuk@kernel.org> Subject: Re: [PATCH v2 2/4] fscrypt: add inline encryption support Date: Mon, 29 Jun 2020 11:22:50 -0700 [thread overview] Message-ID: <20200629182250.GD20492@sol.localdomain> (raw) In-Reply-To: <20200629120405.701023-3-satyat@google.com> On Mon, Jun 29, 2020 at 12:04:03PM +0000, Satya Tangirala via Linux-f2fs-devel wrote: > diff --git a/fs/crypto/bio.c b/fs/crypto/bio.c > index 4fa18fff9c4e..1ea9369a7688 100644 > --- a/fs/crypto/bio.c > +++ b/fs/crypto/bio.c > @@ -41,6 +41,52 @@ void fscrypt_decrypt_bio(struct bio *bio) > } > EXPORT_SYMBOL(fscrypt_decrypt_bio); > > +static int fscrypt_zeroout_range_inline_crypt(const struct inode *inode, > + pgoff_t lblk, sector_t pblk, > + unsigned int len) > +{ > + const unsigned int blockbits = inode->i_blkbits; > + const unsigned int blocks_per_page = 1 << (PAGE_SHIFT - blockbits); > + struct bio *bio; > + int ret, err = 0; > + int num_pages = 0; > + > + /* This always succeeds since __GFP_DIRECT_RECLAIM is set. */ > + bio = bio_alloc(GFP_NOFS, BIO_MAX_PAGES); > + > + while (len) { > + unsigned int blocks_this_page = min(len, blocks_per_page); > + unsigned int bytes_this_page = blocks_this_page << blockbits; > + > + if (num_pages == 0) { > + fscrypt_set_bio_crypt_ctx(bio, inode, lblk, GFP_NOFS); > + bio_set_dev(bio, inode->i_sb->s_bdev); > + bio->bi_iter.bi_sector = > + pblk << (blockbits - SECTOR_SHIFT); > + bio_set_op_attrs(bio, REQ_OP_WRITE, 0); > + } > + ret = bio_add_page(bio, ZERO_PAGE(0), bytes_this_page, 0); > + if (WARN_ON(ret != bytes_this_page)) { > + err = -EIO; > + goto out; > + } > + num_pages++; > + len -= blocks_this_page; > + lblk += blocks_this_page; > + pblk += blocks_this_page; > + if (num_pages == BIO_MAX_PAGES || !len) { > + err = submit_bio_wait(bio); > + if (err) > + goto out; > + bio_reset(bio); > + num_pages = 0; > + } > + } > +out: > + bio_put(bio); > + return err; > +} I just realized we missed something. With the new IV_INO_LBLK_32 IV generation strategy, logically contiguous blocks don't necessarily have contiguous IVs. So we need to check fscrypt_mergeable_bio() here. Also it *should* be checked once per block, not once per page. However, that means that ext4_mpage_readpages() and f2fs_mpage_readpages() are wrong too, since they only check fscrypt_mergeable_bio() once per page. Given that difficulty, and the fact that IV_INO_LBLK_32 only has limited use cases on specific hardware, I suggest that for now we simply restrict inline encryption with IV_INO_LBLK_32 to the blocksize == PAGE_SIZE case. (Checking fscrypt_mergeable_bio() once per page is still needed.) I.e., on top of this patch: diff --git a/fs/crypto/bio.c b/fs/crypto/bio.c index 1ea9369a7688..b048a0e38516 100644 --- a/fs/crypto/bio.c +++ b/fs/crypto/bio.c @@ -74,7 +74,8 @@ static int fscrypt_zeroout_range_inline_crypt(const struct inode *inode, len -= blocks_this_page; lblk += blocks_this_page; pblk += blocks_this_page; - if (num_pages == BIO_MAX_PAGES || !len) { + if (num_pages == BIO_MAX_PAGES || !len || + !fscrypt_mergeable_bio(bio, inode, lblk)) { err = submit_bio_wait(bio); if (err) goto out; diff --git a/fs/crypto/inline_crypt.c b/fs/crypto/inline_crypt.c index ec514bc8ee86..097c5a565a21 100644 --- a/fs/crypto/inline_crypt.c +++ b/fs/crypto/inline_crypt.c @@ -84,6 +84,19 @@ int fscrypt_select_encryption_impl(struct fscrypt_info *ci) if (!(sb->s_flags & SB_INLINECRYPT)) return 0; + /* + * When a page contains multiple logically contiguous filesystem blocks, + * some filesystem code only calls fscrypt_mergeable_bio() for the first + * block in the page. This is fine for most of fscrypt's IV generation + * strategies, where contiguous blocks imply contiguous IVs. But it + * doesn't work with IV_INO_LBLK_32. For now, simply exclude + * IV_INO_LBLK_32 with blocksize != PAGE_SIZE from inline encryption. + */ + if ((fscrypt_policy_flags(&ci->ci_policy) & + FSCRYPT_POLICY_FLAG_IV_INO_LBLK_32) && + sb->s_blocksize != PAGE_SIZE) + return 0; +
WARNING: multiple messages have this Message-ID (diff)
From: Eric Biggers <ebiggers@kernel.org> To: Satya Tangirala <satyat@google.com> Cc: linux-fsdevel@vger.kernel.org, Jaegeuk Kim <jaegeuk@kernel.org>, linux-fscrypt@vger.kernel.org, linux-ext4@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net Subject: Re: [f2fs-dev] [PATCH v2 2/4] fscrypt: add inline encryption support Date: Mon, 29 Jun 2020 11:22:50 -0700 [thread overview] Message-ID: <20200629182250.GD20492@sol.localdomain> (raw) In-Reply-To: <20200629120405.701023-3-satyat@google.com> On Mon, Jun 29, 2020 at 12:04:03PM +0000, Satya Tangirala via Linux-f2fs-devel wrote: > diff --git a/fs/crypto/bio.c b/fs/crypto/bio.c > index 4fa18fff9c4e..1ea9369a7688 100644 > --- a/fs/crypto/bio.c > +++ b/fs/crypto/bio.c > @@ -41,6 +41,52 @@ void fscrypt_decrypt_bio(struct bio *bio) > } > EXPORT_SYMBOL(fscrypt_decrypt_bio); > > +static int fscrypt_zeroout_range_inline_crypt(const struct inode *inode, > + pgoff_t lblk, sector_t pblk, > + unsigned int len) > +{ > + const unsigned int blockbits = inode->i_blkbits; > + const unsigned int blocks_per_page = 1 << (PAGE_SHIFT - blockbits); > + struct bio *bio; > + int ret, err = 0; > + int num_pages = 0; > + > + /* This always succeeds since __GFP_DIRECT_RECLAIM is set. */ > + bio = bio_alloc(GFP_NOFS, BIO_MAX_PAGES); > + > + while (len) { > + unsigned int blocks_this_page = min(len, blocks_per_page); > + unsigned int bytes_this_page = blocks_this_page << blockbits; > + > + if (num_pages == 0) { > + fscrypt_set_bio_crypt_ctx(bio, inode, lblk, GFP_NOFS); > + bio_set_dev(bio, inode->i_sb->s_bdev); > + bio->bi_iter.bi_sector = > + pblk << (blockbits - SECTOR_SHIFT); > + bio_set_op_attrs(bio, REQ_OP_WRITE, 0); > + } > + ret = bio_add_page(bio, ZERO_PAGE(0), bytes_this_page, 0); > + if (WARN_ON(ret != bytes_this_page)) { > + err = -EIO; > + goto out; > + } > + num_pages++; > + len -= blocks_this_page; > + lblk += blocks_this_page; > + pblk += blocks_this_page; > + if (num_pages == BIO_MAX_PAGES || !len) { > + err = submit_bio_wait(bio); > + if (err) > + goto out; > + bio_reset(bio); > + num_pages = 0; > + } > + } > +out: > + bio_put(bio); > + return err; > +} I just realized we missed something. With the new IV_INO_LBLK_32 IV generation strategy, logically contiguous blocks don't necessarily have contiguous IVs. So we need to check fscrypt_mergeable_bio() here. Also it *should* be checked once per block, not once per page. However, that means that ext4_mpage_readpages() and f2fs_mpage_readpages() are wrong too, since they only check fscrypt_mergeable_bio() once per page. Given that difficulty, and the fact that IV_INO_LBLK_32 only has limited use cases on specific hardware, I suggest that for now we simply restrict inline encryption with IV_INO_LBLK_32 to the blocksize == PAGE_SIZE case. (Checking fscrypt_mergeable_bio() once per page is still needed.) I.e., on top of this patch: diff --git a/fs/crypto/bio.c b/fs/crypto/bio.c index 1ea9369a7688..b048a0e38516 100644 --- a/fs/crypto/bio.c +++ b/fs/crypto/bio.c @@ -74,7 +74,8 @@ static int fscrypt_zeroout_range_inline_crypt(const struct inode *inode, len -= blocks_this_page; lblk += blocks_this_page; pblk += blocks_this_page; - if (num_pages == BIO_MAX_PAGES || !len) { + if (num_pages == BIO_MAX_PAGES || !len || + !fscrypt_mergeable_bio(bio, inode, lblk)) { err = submit_bio_wait(bio); if (err) goto out; diff --git a/fs/crypto/inline_crypt.c b/fs/crypto/inline_crypt.c index ec514bc8ee86..097c5a565a21 100644 --- a/fs/crypto/inline_crypt.c +++ b/fs/crypto/inline_crypt.c @@ -84,6 +84,19 @@ int fscrypt_select_encryption_impl(struct fscrypt_info *ci) if (!(sb->s_flags & SB_INLINECRYPT)) return 0; + /* + * When a page contains multiple logically contiguous filesystem blocks, + * some filesystem code only calls fscrypt_mergeable_bio() for the first + * block in the page. This is fine for most of fscrypt's IV generation + * strategies, where contiguous blocks imply contiguous IVs. But it + * doesn't work with IV_INO_LBLK_32. For now, simply exclude + * IV_INO_LBLK_32 with blocksize != PAGE_SIZE from inline encryption. + */ + if ((fscrypt_policy_flags(&ci->ci_policy) & + FSCRYPT_POLICY_FLAG_IV_INO_LBLK_32) && + sb->s_blocksize != PAGE_SIZE) + return 0; + _______________________________________________ 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:[~2020-06-29 21:40 UTC|newest] Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-06-29 12:04 [PATCH v2 0/4] Inline Encryption Support for fscrypt Satya Tangirala 2020-06-29 12:04 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2020-06-29 12:04 ` [PATCH v2 1/4] fs: introduce SB_INLINECRYPT Satya Tangirala 2020-06-29 12:04 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2020-06-29 18:24 ` Eric Biggers 2020-06-29 18:24 ` [f2fs-dev] " Eric Biggers 2020-06-29 12:04 ` [PATCH v2 2/4] fscrypt: add inline encryption support Satya Tangirala 2020-06-29 12:04 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2020-06-29 18:22 ` Eric Biggers [this message] 2020-06-29 18:22 ` Eric Biggers 2020-06-29 12:04 ` [PATCH v2 3/4] f2fs: " Satya Tangirala 2020-06-29 12:04 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2020-06-29 18:53 ` Eric Biggers 2020-06-29 18:53 ` [f2fs-dev] " Eric Biggers 2020-06-29 12:04 ` [PATCH v2 4/4] ext4: " Satya Tangirala 2020-06-29 12:04 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2020-06-29 18:44 ` Eric Biggers 2020-06-29 18:44 ` [f2fs-dev] " Eric Biggers
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=20200629182250.GD20492@sol.localdomain \ --to=ebiggers@kernel.org \ --cc=jaegeuk@kernel.org \ --cc=linux-ext4@vger.kernel.org \ --cc=linux-f2fs-devel@lists.sourceforge.net \ --cc=linux-fscrypt@vger.kernel.org \ --cc=linux-fsdevel@vger.kernel.org \ --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.