Dan,
Thanks for your review comments. My responses are inline.
Dan Anderson wrote:
> FILE usr/src/uts/common/crypto/api/kcf_random.c
>
> 580 #define RND_CPU_PAD (RND_CPU_PAD_SIZE - \
> 581 (sizeof (kmutex_t) + 3*sizeof (uint8_t *) + sizeof
> (HMAC_KEYSCHED) + \
> 582 sizeof (uint64_t) + 3*sizeof (uint32_t) + sizeof (rnd_stats_t)))
> + \
> 583 15*sizeof (uint32_t)
>
> DEA-1. This won't work in computing the correct padding size. Field
> rnd_stats_t has 64-bit integers and are aligned on 0mod8 byte boundaries on
> SPARC or x86/64-bit and, on x86/32-bit, 0mod4 byte boundaries.
>
> But from reading the comments, it seems like the padding size doesn't have to
> be exact. It appears that if the padding is greater than that necessary, the
> purpose is accomplished here (that is, exceed the cache-line size).
>
> Otherwise, a way to avoid padding is to order the 64-bit structure elements
> first, followed by 32-bit elements, and so forth.
>
Agreed. Took a general solution that we change the code to use two
structures
suggested by Ferenc.
#define RND_CPU_CACHE_SIZE 64
#define RND_CPU_PAD_SIZE RND_CPU_CACHE_SIZE*6
#define RND_CPU_PAD (RND_CPU_PAD_SIZE - \
sizeof(rndmag_t))
typedef struct rndmag_s
{
kmutex_t rm_lock;
uint8_t *rm_buffer; /* Start of buffer */
uint8_t *rm_eptr; /* End of buffer */
uint8_t *rm_rptr; /* Current read pointer */
HMAC_KEYSCHED rm_ks; /* seed */
uint64_t rm_counter; /* rotating counter for extracting */
uint32_t rm_oblocks; /* time to rekey? */
uint32_t rm_ofuzz; /* Rekey backoff state */
uint32_t rm_olimit; /* Hard rekey limit */
rnd_stats_t rm_stats; /* Per-CPU Statistics */
uint32_t rm_key[SHA1WORDS]; /* FIP XKEY */
uint32_t rm_seed[SHA1WORDS]; /* seed for rekey */
uint32_t rm_previous[SHA1WORDS]; /* previous random bytes */
} rndmag_t;
typedef struct rndmag_pad_s
{
rndmag_t rm_mag;
uint8_t rm_pad[RND_CPU_PAD];
} rndmag_pad_t;
and changed all the usages.
> 702 bcopy((uint8_t *)tempout, (uint8_t *)rmp->rm_previous,
> 703 HASHSIZE);
> 833 bcopy((uint8_t *)rmp->rm_seed, (uint8_t
> *)rmp->rm_previous,
> 834 HMAC_KEYSIZE);
> DEA-2. Casting uint8_t* is not needed for bcopy() as the bcopy prototype uses
> void*.
> The casting may be needed for other functions, but not bcopy().
> void* has no alignment or type restriction.
>
>
Ok. removed the casting.
> FILE usr/src/uts/common/crypto/io/swrand.c
> 258 bcopy((uint8_t *)swrand_XKEY, (uint8_t *)previous_bytes,
> HASHSIZE);
> 387 bcopy(digest, (uint8_t *)digest32, HASHSIZE);
> 409 bcopy((uint8_t *)tempout, (uint8_t *)previous_bytes,
> HASHSIZE);
> 410
> 414 bcopy((uint8_t *)tempout + bytes,
> 415 leftover, leftover_bytes);
>
> See comment above, DEA-2.
>
Ok. removed the casting.
I'll post the updated webrev after I return from conference.
Thanks,
Hai-May
