[illumos-Developer] [REVIEW] 289 invalid padding when using java pkcs11 provider

[Gordon Ross]

On Fri, Oct 8, 2010 at 9:01 AM, Jason King <jason.brian.king at gmail.com> wrote:
> Webrev is at http://cr.illumos.org/view/madbjvqy/ (it worked this time! \o/)
>
> The synopsis is somewhat misleading.. The actual bug is that DH key
> generation is plain broken (just revealed by the java pkcs11 provider,
> anything else using pkcs11 to generate DH keys will be broken as
> well).

Hi Jason, great job figuring this out.

I'd like to suggest one change, which is sort of a "while we're here" type.
The kmem_free call on 380-381 recomputes the size to be free'd.
I'd much prefer the introduction of a new local variable to hold the
allocated size, and use that in both kmem_alloc and kmem_free.  i.e.

/* line 249 */
	size_t s_size;

/* lines 318-324 */
	s_size = P2ROUNDUP_TYPED(prime_bytes,
	    sizeof (BIG_CHUNK_TYPE), size_t);
#ifdef _KERNEL
	s = kmem_alloc(s_size);
#else
	s = malloc(s_size);
#endif
	if (s == NULL) {
...

/* lines 380-381 */
	kmem_free(s, s_size);

One other minor nit dealt with in the changes suggested above:
Putting a left curly brace inside #if ... #else blocks messes with
paren matching in some editors.  Preferable to avoid that if it's
easy to do.  (as it is in this case)

All of the above is advisory, BTW.  I won't argue if you decline.

Thanks for your work on this.
Gordon