Messages in this thread | | | Subject | Re: [PATCH v4] ceph: mark directory as non-complete complete after loading key | From | Xiubo Li <> | Date | Wed, 30 Nov 2022 21:02:59 +0800 |
| |
On 30/11/2022 18:11, Jeff Layton wrote: > On Wed, 2022-11-30 at 16:25 +0800, Xiubo Li wrote: >> On 30/11/2022 14:54, Gregory Farnum wrote: >>> On Tue, Nov 29, 2022 at 7:21 AM Ilya Dryomov <idryomov@gmail.com> wrote: >>>> On Tue, Nov 29, 2022 at 3:50 PM Xiubo Li <xiubli@redhat.com> wrote: >>>>> On 29/11/2022 22:32, Ilya Dryomov wrote: >>>>>> On Tue, Nov 29, 2022 at 3:15 PM Xiubo Li <xiubli@redhat.com> wrote: >>>>>>> On 29/11/2022 18:39, Luís Henriques wrote: >>>>>>>> When setting a directory's crypt context, ceph_dir_clear_complete() needs to >>>>>>>> be called otherwise if it was complete before, any existing (old) dentry will >>>>>>>> still be valid. >>>>>>>> >>>>>>>> This patch adds a wrapper around __fscrypt_prepare_readdir() which will >>>>>>>> ensure a directory is marked as non-complete if key status changes. >>>>>>>> >>>>>>>> Signed-off-by: Luís Henriques <lhenriques@suse.de> >>>>>>>> --- >>>>>>>> Hi Xiubo, >>>>>>>> >>>>>>>> Here's a rebase of this patch. I did some testing but since this branch >>>>>>>> doesn't really have full fscrypt support, I couldn't even reproduce the >>>>>>>> bug. So, my testing was limited. >>>>>>> I'm planing not to update the wip-fscrypt branch any more, except the IO >>>>>>> path related fixes, which may introduce potential bugs each time as before. >>>>>>> >>>>>>> Since the qa tests PR has finished and the tests have passed, so we are >>>>>>> planing to merge the first none IO part, around 27 patches. And then >>>>>>> pull the reset patches from wip-fscrypt branch. >>>>>> I'm not sure if merging metadata and I/O path patches separately >>>>>> makes sense. What would a user do with just filename encryption? >>>>> Hi Ilya, >>>>> >>>>> I think the IO ones should be followed soon. >>>>> >>>>> Currently the filename ones have been well testes. And the contents will >>>>> be by passed for now. >>>>> >>>>> Since this is just for Dev Preview feature IMO it should be okay (?) >>>> I don't think there is such a thing as a Dev Preview feature when it >>>> comes to the mainline kernel, particularly in the area of filesystems >>>> and storage. It should be ready for users at least to some extent. So >>>> my question stands: what would a user do with just filename encryption? >>> I think how this merges is up to you guys and the kernel practices. >>> Merging only the filename encryption is definitely of *limited* >>> utility, but I don't think it's totally pointless -- the data versus >>> metadata paths are different and you are protecting against somewhat >>> different vulnerabilities and threat models with them. For instance, >>> MDS logs dump filenames, but OSD logs do not dump object data. There's >>> some obvious utility there even if you basically trust your provider, >>> or run your own cluster but want to be more secure about sending logs >>> via ceph-post-file. >> Hi Greg, >> >> Sounds reasonable to me. >> >> I will leave this to Ilya. >> >> Thanks! > For the record, the only reason I proposed merging them in multiple sets > was that it is a large set of changes and I was leery of regressions. I > don't see a lot of value in enabling just filename encryption without > the content piece. > > I'd be fine with merging it all en-masse, though it's a bit more to wade > through if we end up having to bisect to track down a bug. >
Hi Jeff,
After Ilya reviewing the testing's non-encrypt patches to make sure I won't do the rebase again and again for a large number of patches, I will begin to pick the contents patches from wip-fscrypt branch.
And then run the qa test again.
Thanks all :-)
- Xiubo
| |