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@xxxxxxxxx> wrote: > > > On Tue, Nov 29, 2022 at 3:50 PM Xiubo Li <xiubli@xxxxxxxxxx> wrote: > > > > > > > > On 29/11/2022 22:32, Ilya Dryomov wrote: > > > > > On Tue, Nov 29, 2022 at 3:15 PM Xiubo Li <xiubli@xxxxxxxxxx> 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@xxxxxxx> > > > > > > > --- > > > > > > > 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. -- Jeff Layton <jlayton@xxxxxxxxxx>