On Tue, Aug 31, 2021 at 09:50:32AM -0400, Jeff Layton wrote: > > > > > + /* It should never be re-set once set */ > > > > > + WARN_ON_ONCE(ci->fscrypt_auth); > > > > > + > > > > Maybe this should return -EEXIST if already set ? > > > > > > > I don't know. In general, once the context is set on an inode, we > > > shouldn't ever reset it. That said, I think we might need to allow > > > admins to override an existing context if it's corrupted. > > > > > > So, that's the rationale for the WARN_ON_ONCE. The admins should never > > > do this under normal circumstances but they do have the ability to > > > change it if needed (and we'll see a warning in the logs in that case). > > > > I may miss some code in the fs/crypto/ layer. > > > > I readed that once the directory/file has set the policy context, it > > will just return 0 if the new one matches the existence, if not match it > > will return -EEXIST, or will try to call ceph layer to set it. > > > > So once this is set, my understanding is that it shouldn't be here ? > > > > Where did you read that? If we have documented semantics we need to > follow here, then we should change it to comply with them. > That is how FS_IOC_SET_ENCRYPTION_POLICY behaves, but the check for an existing policy already happens in fscrypt_ioctl_set_policy(), so ->set_context doesn't need to worry about it. - Eric
