Re: [RFC PATCH v8 09/24] ceph: add ability to set fscrypt_auth via setattr

[Xiubo Li <xiubli@redhat.com>]

On 9/1/21 1:54 AM, Eric Biggers wrote:
> 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.

Yeah, the FS_IOC_SET_ENCRYPTION_POLICY section in 
"Documentation/filesystems/fscrypt.rst".


> - Eric
>