RE: [PATCH 2/2] crypto: kpp - Add DH software implementation

["Benedetto, Salvatore" <salvatore.benedetto@intel.com>]

> -----Original Message-----
> From: linux-crypto-owner@vger.kernel.org [mailto:linux-crypto-
> owner@vger.kernel.org] On Behalf Of Stephan Mueller
> Sent: Tuesday, April 12, 2016 2:21 PM
> To: Benedetto, Salvatore <salvatore.benedetto@intel.com>
> Cc: herbert@gondor.apana.org.au; linux-crypto@vger.kernel.org
> Subject: Re: [PATCH 2/2] crypto: kpp - Add DH software implementation
> 
> > >
> > > >  include/crypto/dh.h |  23 ++++++
> > > >  6 files changed, 631 insertions(+)  create mode 100644
> > > > crypto/dh.c  create mode 100644 include/crypto/dh.h
> > > >
> > > > +
> > > > +static int dh_check_params_length(unsigned int p_len) {
> > > > +	switch (p_len) {
> > > > +	case 1536:
> > > > +	case 2048:
> > > > +	case 3072:
> > > > +	case 4096:
> > > > +	case 6144:
> > > > +	case 8192:
> > > > +		return 0;
> > >
> > > Does the math require this check?
> > >
> > > Wouldn't it be better to implement limits to the low side (i.e.
> > > p_len <
> > > 1536) and then add a real limit due to the implementation (e.g. it
> > > must be multiple of full bytes)?
> >
> > The math itself does not require any check that I'm aware of.
> > As for the real limit, I think we have to add that as an hardware that
> > is only capable of handling up to 4096 bytes, should fall back to the
> > software implementation if a bigger param is used.
> 
> Then why not leave that check to the respective HW implementation and
> provide support for all parameters in software? I.e. simply replace this check
> with a lower boundary check only?
> 

I don't see any particular benefit in replacing this check with a lower boundary
check only. Values other than those listed are very unlikely.
Anyway, if you feel so strong about it and other people think the same I'm
OK with either check :-) Either case shouldn't harm.

Thanks,
Salvatore