Re: [PATCH 03/04] Add encryption ops to the keyctl syscall

[David Howells <dhowells@redhat.com>]

David Härdeman <david@2gen.com> wrote:

> Changes the keyctl syscall to accept six arguments (is it valid to do so?)
> and adds encryption as one of the supported ops for in-kernel keys.

I tried to avoid doing that for it required arch support as I recall, but I'm
not sure which arch I was thinking of. It looks like it ought to be okay...
it's no worse than mmap.

> +	 * - should return the amount of data that would be returned from the
> +         *   encryption even if the buffer is NULL
> +         * - expects the output buffer to be able to hold the result
> +	 */

Can you use TAB chars here please.

>  #define KEYCTL_ASSUME_AUTHORITY		16	/* assume request_key() authorisation */
> +#define KEYCTL_ENCRYPT                  17      /* encrypt a chunk of data using key */

And here.

> +	key = key_ref_to_ptr(key_ref);
> +
> +	/* see if we can read it directly */
> +	ret = key_permission(key_ref, KEY_READ);

You don't actually need to calculate key until after you've done all those
checks, so I'd move it further down the file. You can use the function to
release key references in the error handling or have a separate error handling
return path.

> +			down_read(&key->sem);
> +			ret = key->type->encrypt(key, data, dlen, result, rlen);
> +			up_read(&key->sem);

Do we really want to restrict the key type implementor to using the r/w
semaphore. Would it be better to let the type decide whether it wants to use
the semaphore or let it use RCU if it so desires?

David
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/