Re: [PATCH 1/1 v5] Add CryptoAPI User Interface Patch v5

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi.

Sorry for late reply.

On Wed, Aug 27, 2008 at 02:23:31PM -0700, Shasi Pulijala (spulijala@xxxxxxxx) wrote:
> Hi Evgeniy,
>  This is Linux CryptoAPI user space interface support patch v5. This version adds:
> -Preventing race conditions in session creation.
> -Crypto operations are now issued from user space using writev(sync) and AIO write(async) instead of ioctls.

There is number of issues I found with this version.
1. It does not apply since your mailer damaged the patch.
2. It does not follow codying style in several places (like spaces,
empty lines and other similar small stuff).
3. There are way _too_ many allocations. I think it does not matter too
much in case of expensive crypto processing, but I think it can be
reduced.
4. You should not pass double pointer to csession in allocation path.
Instead return allocated structure as a result value, and use ERR_PTR,
PTR_ERR and IS_ERR functions to check that returned pointer is valid or
error.
5. Main issue: it is not asynchronous crypto layer. I did not check
aio_write() processing, but otherwise you wait in the submission fuction
for the operation to complete. Also I'm not sure it works as expected,
since waiting in wait_for_completion_interruptible(&result->crypto_completion)
can be interrupted and storage freed (tmp variable and thus result
variable), so eventually when crypto core will invoke async completion
callback, it will oops.

-- 
	Evgeniy Polyakov
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Kernel]     [Gnu Classpath]     [Gnu Crypto]     [DM Crypt]     [Netfilter]     [Bugtraq]

  Powered by Linux