Jeff Layton <jlayton@xxxxxxxxxx> writes:
> On Wed, 2022-04-27 at 15:13 -0400, Jeff Layton wrote:
>> Allow writepage to issue encrypted writes. Extend out the requested size
>> and offset to cover complete blocks, and then encrypt and write them to
>> the OSDs.
>>
>> Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
>> ---
>> fs/ceph/addr.c | 34 +++++++++++++++++++++++++++-------
>> 1 file changed, 27 insertions(+), 7 deletions(-)
>>
>> diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c
>> index d65d431ec933..f54940fc96ee 100644
>> --- a/fs/ceph/addr.c
>> +++ b/fs/ceph/addr.c
>> @@ -586,10 +586,12 @@ static int writepage_nounlock(struct page *page, struct writeback_control *wbc)
>> loff_t page_off = page_offset(page);
>> int err;
>> loff_t len = thp_size(page);
>> + loff_t wlen;
>> struct ceph_writeback_ctl ceph_wbc;
>> struct ceph_osd_client *osdc = &fsc->client->osdc;
>> struct ceph_osd_request *req;
>> bool caching = ceph_is_cache_enabled(inode);
>> + struct page *bounce_page = NULL;
>>
>> dout("writepage %p idx %lu\n", page, page->index);
>>
>> @@ -621,6 +623,8 @@ static int writepage_nounlock(struct page *page, struct writeback_control *wbc)
>>
>> if (ceph_wbc.i_size < page_off + len)
>> len = ceph_wbc.i_size - page_off;
>> + if (IS_ENCRYPTED(inode))
>> + wlen = round_up(len, CEPH_FSCRYPT_BLOCK_SIZE);
>>
>
> The above is buggy. We're only setting "wlen" in the encrypted case. You
> would think that the compiler would catch that, but next usage of wlen
> just passes a pointer to it to another function and that cloaks the
> warning.
Yikes! That's indeed the sort of things we got used to have compilers
complaining about. That must have been fun to figure this out. Nice ;-)
Cheers,
--
Luís
>
> Fixed in the wip-fscrypt branch, but I'll hold off on re-posting the
> series in case any other new bugs turn up.
>
>> dout("writepage %p page %p index %lu on %llu~%llu snapc %p seq %lld\n",
>> inode, page, page->index, page_off, len, snapc, snapc->seq);
>> @@ -629,24 +633,39 @@ static int writepage_nounlock(struct page *page, struct writeback_control *wbc)
>> CONGESTION_ON_THRESH(fsc->mount_options->congestion_kb))
>> fsc->write_congested = true;
>>
>> - req = ceph_osdc_new_request(osdc, &ci->i_layout, ceph_vino(inode), page_off, &len, 0, 1,
>> - CEPH_OSD_OP_WRITE, CEPH_OSD_FLAG_WRITE, snapc,
>> - ceph_wbc.truncate_seq, ceph_wbc.truncate_size,
>> - true);
>> + req = ceph_osdc_new_request(osdc, &ci->i_layout, ceph_vino(inode),
>> + page_off, &wlen, 0, 1, CEPH_OSD_OP_WRITE,
>> + CEPH_OSD_FLAG_WRITE, snapc,
>> + ceph_wbc.truncate_seq,
>> + ceph_wbc.truncate_size, true);
>> if (IS_ERR(req)) {
>> redirty_page_for_writepage(wbc, page);
>> return PTR_ERR(req);
>> }
>>
>> + if (wlen < len)
>> + len = wlen;
>> +
>> set_page_writeback(page);
>> if (caching)
>> ceph_set_page_fscache(page);
>> ceph_fscache_write_to_cache(inode, page_off, len, caching);
>>
>> + if (IS_ENCRYPTED(inode)) {
>> + bounce_page = fscrypt_encrypt_pagecache_blocks(page, CEPH_FSCRYPT_BLOCK_SIZE,
>> + 0, GFP_NOFS);
>> + if (IS_ERR(bounce_page)) {
>> + err = PTR_ERR(bounce_page);
>> + goto out;
>> + }
>> + }
>> /* it may be a short write due to an object boundary */
>> WARN_ON_ONCE(len > thp_size(page));
>> - osd_req_op_extent_osd_data_pages(req, 0, &page, len, 0, false, false);
>> - dout("writepage %llu~%llu (%llu bytes)\n", page_off, len, len);
>> + osd_req_op_extent_osd_data_pages(req, 0,
>> + bounce_page ? &bounce_page : &page, wlen, 0,
>> + false, false);
>> + dout("writepage %llu~%llu (%llu bytes, %sencrypted)\n",
>> + page_off, len, wlen, IS_ENCRYPTED(inode) ? "" : "not ");
>>
>> req->r_mtime = inode->i_mtime;
>> err = ceph_osdc_start_request(osdc, req, true);
>> @@ -655,7 +674,8 @@ static int writepage_nounlock(struct page *page, struct writeback_control *wbc)
>>
>> ceph_update_write_metrics(&fsc->mdsc->metric, req->r_start_latency,
>> req->r_end_latency, len, err);
>> -
>> + fscrypt_free_bounce_page(bounce_page);
>> +out:
>> ceph_osdc_put_request(req);
>> if (err == 0)
>> err = len;
>
> --
> Jeff Layton <jlayton@xxxxxxxxxx>
