Re: [PATCH/proposal] dm-crypt: add digest-based iv generation mode

[Matt Mackall <mpm@selenic.com>]

On Thu, Feb 26, 2004 at 08:35:46PM +0100, Christophe Saout wrote:
> Am Mi, den 25.02.2004 schrieb Matt Mackall um 22:43:
> 
> > Ok, here's my proposed API extension (currently untested). Christophe,
> > care to give it a spin?
> >
> > diff -puN crypto/api.c~crypto-copy crypto/api.c
> > --- tiny/crypto/api.c~crypto-copy	2004-02-25 15:12:43.000000000 -0600
> > +++ tiny-mpm/crypto/api.c	2004-02-25 15:37:39.000000000 -0600
> > @@ -161,6 +161,27 @@ void crypto_free_tfm(struct crypto_tfm *
> >  	kfree(tfm);
> >  }
> >  
> > +int crypto_copy_tfm(char *dst, const struct crypto_tfm *src, unsigned size)
> > +{
> > +	int s = crypto_tfm_size(src);
> > +
> > +	if (size < s)
> > +		return 0;
> 
> Why the extra check?

User is giving us the size of his buffer, not the size of the tfm
which we already know. We refuse to copy if buffer is not big enough,
otherwise return number of bytes copied. This may seem a little
redundant for the on-stack usage of the API, but may make sense in
other cases.
 
> > +void crypto_cleanup_copy_tfm(char *user_tfm)
> > +{
> > +	crypto_exit_ops((struct crypto_tfm *)user_tfm);
> 
> This looks dangerous. The algorithm might free a buffer. This is only
> safe if we introduce per-algorithm copy methods that also duplicate
> external buffers.

I'm currently working under the assumption that such external buffers
are unnecessary but I haven't done the audit. If and when such code
exist, such code should be added, yes. Hence the comment in the copy
function:

+       /* currently assumes shallow copy is sufficient */

> I'd like to avoid a kmalloc in crypto_copy_tfm.

Well you're in luck because there isn't one.

> This function also does the same as crypto_free_tfm except for the
> final kfree(tfm). So crypto_free_tfm could call this function. 

Yes.

> And it could have a better name.

Just about everything could have a better name if the poets among us
were to speak up.

-- 
Matt Mackall : http://www.selenic.com : Linux development and consulting