Re: [PATCH crypto -v4 2/2] AES-NI: Add support to Intel AES-NIinstructions for x86_64 platform

[Huang Ying]

On Fri, 2009-01-16 at 09:53 +0800, Herbert Xu wrote:
> On Fri, Jan 16, 2009 at 09:20:58AM +0800, Huang Ying wrote:
> > On Thu, 2009-01-15 at 17:47 +0800, roel kluin wrote:
> >
> > > > +       kernel_fpu_begin();
> > > > +       while ((nbytes = walk.nbytes)) {
> > > > +               aesni_ecb_enc(ctx, walk.dst.virt.addr, walk.src.virt.addr,
> > > > +                             nbytes & AES_BLOCK_MASK);
> > > > +               nbytes &= AES_BLOCK_SIZE - 1;
> > > > +               err = blkcipher_walk_done(desc, &walk, nbytes);
> > > > +       }
> > > > +       kernel_fpu_end();
> > > > +
> > > > +       return err;
> > > > +}
> > > 
> > > if blkcipher_walk_{virt,done} returns an error, don't we have to break
> > > out of the loop?
> > > i.e.
> > > 
> > > while (!err && (nbytes = walk.nbytes))
> > > 
> > > (if that's erroneous, it occurs in other places as well)
> 
> That's not necessary because blkcipher_walk_done will ensure
> that walk.nbytes == 0 if there is an error.

But after checking blkcipher_walk_done() in 2.6.28, If input argument
err != 0 and walk->flags & BLKCIPHER_WALK_SLOW != 0, when
blkcipher_walk_done() return non-zero, walk->nbytes != 0. So I think it
is a little fragile to use walk->nbytes == 0 indicate error.

int blkcipher_walk_done(struct blkcipher_desc *desc,
                        struct blkcipher_walk *walk, int err)
{
        struct crypto_blkcipher *tfm = desc->tfm;
        unsigned int nbytes = 0;

        if (likely(err >= 0)) {
                unsigned int n = walk->nbytes - err;

                if (likely(!(walk->flags & BLKCIPHER_WALK_SLOW)))
                        n = blkcipher_done_fast(walk, n);
                else if (WARN_ON(err)) {
                        err = -EINVAL;
                        goto err;
                } else
                        n = blkcipher_done_slow(tfm, walk, n);

                nbytes = walk->total - n;
                err = 0;
        }

        scatterwalk_done(&walk->in, 0, nbytes);
        scatterwalk_done(&walk->out, 1, nbytes);

        walk->total = nbytes;
        walk->nbytes = nbytes;

        if (nbytes) {
                crypto_yield(desc->flags);
                return blkcipher_walk_next(desc, walk);
        }

err:
        if (walk->iv != desc->info)
                memcpy(desc->info, walk->iv, crypto_blkcipher_ivsize(tfm));
        if (walk->buffer != walk->page)
                kfree(walk->buffer);
        if (walk->page)
                free_page((unsigned long)walk->page);

        return err;
}

Best Regards,
Huang Ying

Attachment: signature.asc
Description: This is a digitally signed message part