| Message ID | 20241203-crypto-qce-refactor-v1-2-c5901d2dd45c@linaro.org (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | crypto: qce - refactor the driver | expand |
On 03/12/2024 10:19, Bartosz Golaszewski wrote: > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > If we encounter an error when registering alorithms with the crypto > framework, we just bail out and don't unregister the ones we > successfully registered in prior iterations of the loop. > > Add code that goes back over the algos and unregisters them before > returning an error from qce_register_algs(). > > Cc: stable@vger.kernel.org > Fixes: ec8f5d8f6f76 ("crypto: qce - Qualcomm crypto engine driver") > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > --- > drivers/crypto/qce/core.c | 11 +++++++---- > 1 file changed, 7 insertions(+), 4 deletions(-) > > diff --git a/drivers/crypto/qce/core.c b/drivers/crypto/qce/core.c > index 58ea93220f015..848e5e802b92b 100644 > --- a/drivers/crypto/qce/core.c > +++ b/drivers/crypto/qce/core.c > @@ -51,16 +51,19 @@ static void qce_unregister_algs(struct qce_device *qce) > static int qce_register_algs(struct qce_device *qce) > { > const struct qce_algo_ops *ops; > - int i, ret = -ENODEV; > + int i, j, ret = -ENODEV; > > for (i = 0; i < ARRAY_SIZE(qce_ops); i++) { > ops = qce_ops[i]; > ret = ops->register_algs(qce); > - if (ret) > - break; > + if (ret) { > + for (j = i - 1; j >= 0; j--) > + ops->unregister_algs(qce); > + return ret; > + } > } > > - return ret; > + return 0; > } > > static int qce_handle_request(struct crypto_async_request *async_req) > Perhaps you can also use the devm trick here aswell ? Neil
On Tue, Dec 3, 2024 at 2:55 PM <neil.armstrong@linaro.org> wrote: > > On 03/12/2024 10:19, Bartosz Golaszewski wrote: > > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > > > If we encounter an error when registering alorithms with the crypto > > framework, we just bail out and don't unregister the ones we > > successfully registered in prior iterations of the loop. > > > > Add code that goes back over the algos and unregisters them before > > returning an error from qce_register_algs(). > > > > Cc: stable@vger.kernel.org > > Fixes: ec8f5d8f6f76 ("crypto: qce - Qualcomm crypto engine driver") > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > --- > > drivers/crypto/qce/core.c | 11 +++++++---- > > 1 file changed, 7 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/crypto/qce/core.c b/drivers/crypto/qce/core.c > > index 58ea93220f015..848e5e802b92b 100644 > > --- a/drivers/crypto/qce/core.c > > +++ b/drivers/crypto/qce/core.c > > @@ -51,16 +51,19 @@ static void qce_unregister_algs(struct qce_device *qce) > > static int qce_register_algs(struct qce_device *qce) > > { > > const struct qce_algo_ops *ops; > > - int i, ret = -ENODEV; > > + int i, j, ret = -ENODEV; > > > > for (i = 0; i < ARRAY_SIZE(qce_ops); i++) { > > ops = qce_ops[i]; > > ret = ops->register_algs(qce); > > - if (ret) > > - break; > > + if (ret) { > > + for (j = i - 1; j >= 0; j--) > > + ops->unregister_algs(qce); > > + return ret; > > + } > > } > > > > - return ret; > > + return 0; > > } > > > > static int qce_handle_request(struct crypto_async_request *async_req) > > > > Perhaps you can also use the devm trick here aswell ? > I wanted to keep this one brief for backporting but I also think that scheduling a separate action here for every algo would be a bit overkill. This is quite concise so I'd keep it this way. Bart
On 03/12/2024 16:05, Bartosz Golaszewski wrote: > On Tue, Dec 3, 2024 at 2:55 PM <neil.armstrong@linaro.org> wrote: >> >> On 03/12/2024 10:19, Bartosz Golaszewski wrote: >>> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> >>> >>> If we encounter an error when registering alorithms with the crypto >>> framework, we just bail out and don't unregister the ones we >>> successfully registered in prior iterations of the loop. >>> >>> Add code that goes back over the algos and unregisters them before >>> returning an error from qce_register_algs(). >>> >>> Cc: stable@vger.kernel.org >>> Fixes: ec8f5d8f6f76 ("crypto: qce - Qualcomm crypto engine driver") >>> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> >>> --- >>> drivers/crypto/qce/core.c | 11 +++++++---- >>> 1 file changed, 7 insertions(+), 4 deletions(-) >>> >>> diff --git a/drivers/crypto/qce/core.c b/drivers/crypto/qce/core.c >>> index 58ea93220f015..848e5e802b92b 100644 >>> --- a/drivers/crypto/qce/core.c >>> +++ b/drivers/crypto/qce/core.c >>> @@ -51,16 +51,19 @@ static void qce_unregister_algs(struct qce_device *qce) >>> static int qce_register_algs(struct qce_device *qce) >>> { >>> const struct qce_algo_ops *ops; >>> - int i, ret = -ENODEV; >>> + int i, j, ret = -ENODEV; >>> >>> for (i = 0; i < ARRAY_SIZE(qce_ops); i++) { >>> ops = qce_ops[i]; >>> ret = ops->register_algs(qce); >>> - if (ret) >>> - break; >>> + if (ret) { >>> + for (j = i - 1; j >= 0; j--) >>> + ops->unregister_algs(qce); >>> + return ret; >>> + } >>> } >>> >>> - return ret; >>> + return 0; >>> } >>> >>> static int qce_handle_request(struct crypto_async_request *async_req) >>> >> >> Perhaps you can also use the devm trick here aswell ? >> > > I wanted to keep this one brief for backporting but I also think that > scheduling a separate action here for every algo would be a bit > overkill. This is quite concise so I'd keep it this way. Sure understandable! Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org> > > Bart
diff --git a/drivers/crypto/qce/core.c b/drivers/crypto/qce/core.c index 58ea93220f015..848e5e802b92b 100644 --- a/drivers/crypto/qce/core.c +++ b/drivers/crypto/qce/core.c @@ -51,16 +51,19 @@ static void qce_unregister_algs(struct qce_device *qce) static int qce_register_algs(struct qce_device *qce) { const struct qce_algo_ops *ops; - int i, ret = -ENODEV; + int i, j, ret = -ENODEV; for (i = 0; i < ARRAY_SIZE(qce_ops); i++) { ops = qce_ops[i]; ret = ops->register_algs(qce); - if (ret) - break; + if (ret) { + for (j = i - 1; j >= 0; j--) + ops->unregister_algs(qce); + return ret; + } } - return ret; + return 0; } static int qce_handle_request(struct crypto_async_request *async_req)