Re: [RFC][PATCH v3 6/6] eCryptfs: added support for the encrypted key type

Roberto Sassu
2011-01-27T18:59:47Z

Hi David

thanks for the review!
I just need a clarification about the comment of this patch.
Regarding the locking issue, this also applies for the unpatched
version of eCryptfs, because the 'user' key type has the
'update' method defined. However, i think the read lock is not
enough because eCryptfs directly writes on the key's payload
in order to update the state of the authentication token.

I've developed a small patch, i will post shortly with Tyler Hicks
in CC, that locks requested keys for writing.

Roberto Sassu

On Wednesday, January 26, 2011 12:28:36 pm David Howells wrote:
> Roberto Sassu <roberto.sassu@polito.it> wrote:
>
> > (*auth_tok_key) = request_key(&key_type_user, sig, NULL);
> > if (!(*auth_tok_key) || IS_ERR(*auth_tok_key)) {
> > - printk(KERN_ERR "Could not find key with description: [%s]\n",
> > - sig);
> > - rc = process_request_key_err(PTR_ERR(*auth_tok_key));
> > - goto out;
> > + (*auth_tok_key) = ecryptfs_get_encrypted_key(sig);
> > + if (!(*auth_tok_key) || IS_ERR(*auth_tok_key)) {
> > + printk(KERN_ERR "Could not find key with description: [%s]\n",
> > + sig);
> > + rc = process_request_key_err(PTR_ERR(*auth_tok_key));
> > + goto out;
> > + }
> > }
> > (*auth_tok) = ecryptfs_get_key_payload_data(*auth_tok_key);
> > if (ecryptfs_verify_version((*auth_tok)->version)) {
>
> You need some locking here, even if it's only use of the RCU read lock.
> Encrypted-type keys have an update method and so may change under you.
>
> David
> ---
>