On Fri, 2022-06-03 at 10:17 +0100, Luís Henriques wrote:
> 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 ;-)
>
Yeah. I remember that some older versions of gcc would complain about
uninitialized vars when you passed a pointer to it to another function.
That went away a while back, which was good since it often fired on
false positives.
What would have been nice here would be for the compiler to notice that
wlen was inconsistently initialized before we passed the pointer to the
function. Not sure how hard that would be to catch though.
--
Jeff Layton <jlayton@xxxxxxxxxx>
