| Message ID | 20190821143253.30209-9-ard.biesheuvel@linaro.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | crypto: arm/aes - XTS ciphertext stealing and other updates | expand |
On Wed, Aug 21, 2019 at 05:32:44PM +0300, Ard Biesheuvel wrote: > After starting a skcipher walk, the only way to ensure that all > resources it has tied up are released is to complete it. In some > cases, it will be useful to be able to abort a walk cleanly after > it has started, so add this ability to the skcipher walk API. > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > crypto/skcipher.c | 3 +++ > include/crypto/internal/skcipher.h | 5 +++++ > 2 files changed, 8 insertions(+) > > diff --git a/crypto/skcipher.c b/crypto/skcipher.c > index 5d836fc3df3e..973ab1c7dcca 100644 > --- a/crypto/skcipher.c > +++ b/crypto/skcipher.c > @@ -140,6 +140,9 @@ int skcipher_walk_done(struct skcipher_walk *walk, int err) > goto already_advanced; > } > > + if (unlikely(!n)) > + goto finish; > + > scatterwalk_advance(&walk->in, n); > scatterwalk_advance(&walk->out, n); > already_advanced: > diff --git a/include/crypto/internal/skcipher.h b/include/crypto/internal/skcipher.h > index d68faa5759ad..bc488173531f 100644 > --- a/include/crypto/internal/skcipher.h > +++ b/include/crypto/internal/skcipher.h > @@ -148,6 +148,11 @@ int skcipher_walk_aead_decrypt(struct skcipher_walk *walk, > struct aead_request *req, bool atomic); > void skcipher_walk_complete(struct skcipher_walk *walk, int err); > > +static inline void skcipher_walk_abort(struct skcipher_walk *walk) > +{ > + skcipher_walk_done(walk, walk->nbytes); > +} Couldn't you just abort it by supplying an error in place of walk->bytes? IOW I'm fine with this helper but you don't need to touch skcipher_walk_done as just giving it an negative err value should do the trick. Thanks,
On Fri, 30 Aug 2019 at 11:03, Herbert Xu <herbert@gondor.apana.org.au> wrote: > > On Wed, Aug 21, 2019 at 05:32:44PM +0300, Ard Biesheuvel wrote: > > After starting a skcipher walk, the only way to ensure that all > > resources it has tied up are released is to complete it. In some > > cases, it will be useful to be able to abort a walk cleanly after > > it has started, so add this ability to the skcipher walk API. > > > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > --- > > crypto/skcipher.c | 3 +++ > > include/crypto/internal/skcipher.h | 5 +++++ > > 2 files changed, 8 insertions(+) > > > > diff --git a/crypto/skcipher.c b/crypto/skcipher.c > > index 5d836fc3df3e..973ab1c7dcca 100644 > > --- a/crypto/skcipher.c > > +++ b/crypto/skcipher.c > > @@ -140,6 +140,9 @@ int skcipher_walk_done(struct skcipher_walk *walk, int err) > > goto already_advanced; > > } > > > > + if (unlikely(!n)) > > + goto finish; > > + > > scatterwalk_advance(&walk->in, n); > > scatterwalk_advance(&walk->out, n); > > already_advanced: > > diff --git a/include/crypto/internal/skcipher.h b/include/crypto/internal/skcipher.h > > index d68faa5759ad..bc488173531f 100644 > > --- a/include/crypto/internal/skcipher.h > > +++ b/include/crypto/internal/skcipher.h > > @@ -148,6 +148,11 @@ int skcipher_walk_aead_decrypt(struct skcipher_walk *walk, > > struct aead_request *req, bool atomic); > > void skcipher_walk_complete(struct skcipher_walk *walk, int err); > > > > +static inline void skcipher_walk_abort(struct skcipher_walk *walk) > > +{ > > + skcipher_walk_done(walk, walk->nbytes); > > +} > > Couldn't you just abort it by supplying an error in place of > walk->bytes? IOW I'm fine with this helper but you don't need > to touch skcipher_walk_done as just giving it an negative err > value should do the trick. > This might be a problem with the implementation of skcipher_walk_done() in general rather than a limitation in this particular case, but when calling skcipher_walk_done() with a negative err value, we never kunmap the src and dst pages. So should I propose a fix for that instead? Or are the internal callers dealing with this correctly? (and is it forbidden for external callers to pass negative values?)
diff --git a/crypto/skcipher.c b/crypto/skcipher.c index 5d836fc3df3e..973ab1c7dcca 100644 --- a/crypto/skcipher.c +++ b/crypto/skcipher.c @@ -140,6 +140,9 @@ int skcipher_walk_done(struct skcipher_walk *walk, int err) goto already_advanced; } + if (unlikely(!n)) + goto finish; + scatterwalk_advance(&walk->in, n); scatterwalk_advance(&walk->out, n); already_advanced: diff --git a/include/crypto/internal/skcipher.h b/include/crypto/internal/skcipher.h index d68faa5759ad..bc488173531f 100644 --- a/include/crypto/internal/skcipher.h +++ b/include/crypto/internal/skcipher.h @@ -148,6 +148,11 @@ int skcipher_walk_aead_decrypt(struct skcipher_walk *walk, struct aead_request *req, bool atomic); void skcipher_walk_complete(struct skcipher_walk *walk, int err); +static inline void skcipher_walk_abort(struct skcipher_walk *walk) +{ + skcipher_walk_done(walk, walk->nbytes); +} + static inline void ablkcipher_request_complete(struct ablkcipher_request *req, int err) {
After starting a skcipher walk, the only way to ensure that all resources it has tied up are released is to complete it. In some cases, it will be useful to be able to abort a walk cleanly after it has started, so add this ability to the skcipher walk API. Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> --- crypto/skcipher.c | 3 +++ include/crypto/internal/skcipher.h | 5 +++++ 2 files changed, 8 insertions(+)