* IV generation in cryptographic driver in AEAD
From: Denis B @ 2016-05-18 12:06 UTC (permalink / raw)
To: linux-crypto
Hello,
In AEAD mode (or in any case, in IPSec ESP IPv4 – esp4.c), in kernel
versions prior to 4.2 the cryptographic driver is expected to generate
an IV.
What if my driver is unable to generate an IV?
Thanks,
Dennis.
^ permalink raw reply
* Re: [patch] crypto: omap-sham - potential Oops on error in probe
From: Peter Ujfalusi @ 2016-05-18 11:50 UTC (permalink / raw)
To: Dan Carpenter; +Cc: Herbert Xu, David S. Miller, linux-crypto, kernel-janitors
In-Reply-To: <20160518114443.GG11011@mwanda>
On 05/18/16 14:44, Dan Carpenter wrote:
> On Wed, May 18, 2016 at 01:42:36PM +0300, Peter Ujfalusi wrote:
>> On 05/18/16 13:39, Dan Carpenter wrote:
>>> This if statement is reversed so we end up either leaking or Oopsing on
>>> error.
>>
>> Oops, sorry for that.
>> Probably the other omap-* crypto drivers have the same issue? Can you send a
>> patch for them or should I do it?
>
> I didn't see those static checker warnings so probably it means I can't
> compile the drivers. Could you do it?
I have taken a look at omap-aes and omap-des. They are OK.
--
Péter
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [patch] crypto: omap-sham - potential Oops on error in probe
From: Dan Carpenter @ 2016-05-18 11:44 UTC (permalink / raw)
To: Peter Ujfalusi; +Cc: Herbert Xu, David S. Miller, linux-crypto, kernel-janitors
In-Reply-To: <21712178-666f-e2e2-24f3-fb0c513acc83@ti.com>
On Wed, May 18, 2016 at 01:42:36PM +0300, Peter Ujfalusi wrote:
> On 05/18/16 13:39, Dan Carpenter wrote:
> > This if statement is reversed so we end up either leaking or Oopsing on
> > error.
>
> Oops, sorry for that.
> Probably the other omap-* crypto drivers have the same issue? Can you send a
> patch for them or should I do it?
I didn't see those static checker warnings so probably it means I can't
compile the drivers. Could you do it?
regards,
dan carpenter
^ permalink raw reply
* Re: Decrypting data in RX path
From: Stephan Mueller @ 2016-05-18 10:57 UTC (permalink / raw)
To: Catalin Vasile; +Cc: Gadre Nayan, linux-crypto@vger.kernel.org
In-Reply-To: <DB5PR04MB13021E5E7CAF57D1188D65AEEE490@DB5PR04MB1302.eurprd04.prod.outlook.com>
Am Mittwoch, 18. Mai 2016, 07:01:18 schrieb Catalin Vasile:
Hi Catalin,
> Inline Comments
> ________________________________________
> From: linux-crypto-owner@vger.kernel.org
> <linux-crypto-owner@vger.kernel.org> on behalf of Gadre Nayan
> <gadrenayan@gmail.com> Sent: Monday, May 16, 2016 5:40 PM
> To: Stephan Mueller; linux-crypto@vger.kernel.org
> Subject: Re: Decrypting data in RX path
>
> Hi,
>
> 1. The context of the question "best place to decrypt in
> kernel(module/driver)" is I want to encrypt network packets sent from
> my system and decrypt them back to work with crypto apis. So the
> encryption part I have done in a Kernel thread, decryption part could
> be either in driver or a pre-routing hook. Which is appropriate.
>
> 2. I went through the esp_input function for rx.
>
> As I understand, It allocates a decrypt request and and calls
> crypto_aead_decrypt(req).
>
> A. Since this request is asynchronous, it would be handled through
> condition variables, Am i right on this?
I am not sure I understand your term "condition variables". The async handling
is implemented by invoking the callback esp_input_done once the cipher
operation completes.
So, esp_input invokes the cipher operation. The invocation will return
immediately. Once the operation completes, the callback is triggered where the
callback must perform the appropriate post-operation handling.
See the kernel crypto API documentation for async handling.
> B. Also the IPSEC routines like input and output would run in softirq
> context ?
I am not sure about whether it runs in softirq, but at least it is not in
process context.
> C. esp_input_done() is a callback for decrypt, so as soon as
> crypto_aead_decrypt(req) is called and the encryption does not happen
> immediately, it will return the error _EINPROGRESS. Now this will
> cause the esp_input function to return immediately. So then when is
> the deferred decryption checked. I see esp_input_done2 as well. How is
> the flow and call of these callbacks happening.
Please read the kernel crypto API documentation at [1]
[1] http://www.chronox.de/crypto-API/index.html
>
> [Catalin Vasile]
> "aead_request_set_callback(req, 0, esp_output_done_esn, skb);"
> This piece of code sets into the request structure a callback.
> After the job is queued, you will receive an -EINPROGRESS and your current
> context will just return till it ends the current context (having nothing
> other to do). Later, the device will trigger an interrupt in the Kernel to
> announce it that a job has been done. The interrupt usually starts a
> kthread or a softirq which will process what jobs have ended. One of the
> processing part thingies is triggering the callback you have set. Although
> the context that sent the job no longer exist, the hardware driver uses its
> own contexts to run some custom you have sent (that code is represented by
> your callback).
>
> Apologize for being so verbose.
>
> Thanks.
>
> On Mon, May 16, 2016 at 6:02 PM, Stephan Mueller <smueller@chronox.de>
wrote:
> > Am Montag, 16. Mai 2016, 17:24:12 schrieb Gadre Nayan:
> >
> > Hi Gadre,
> >
> >> Hi,
> >>
> >> I am able to encrypt data using the asynchronous kernel crypto API's.
> >> I can observe the encrypted data on the protocol analyzer.
> >>
> >> I wanted to decry-pt the data now on the receiver side, So I have
> >> following questions.
> >>
> >> 1. What is the best place to decrypt the data, in kernel space (module
> >> (pre-routing hook) or driver) OR user space using (maybe using raw
> >> sockets or after socket recv).
> >
> > This is a very broad question and cannot be answered without knowning the
> > context.
> >
> >> What precautions should be taken in terms of locking while using
> >> crypto api's in kernel space in RX path (Softirq context) --> Can
> >> someone point to existing sample in kernel where decryption is done in
> >> RX path.
> >
> > net/ipv4/esp4.c:esp_input for rx and esp_output for tx.
> >
> >> 2. If I encrypt data in kernel space can I decrypt it in User-space
> >> using same encryption methods and Keys.
> >
> > Sure, if you have the keys and all information about the used crypto.
> >
> >> Thanks.
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe linux-crypto"
> >> in
> >> the body of a message to majordomo@vger.kernel.org
> >> More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
> > Ciao
> > Stephan
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Ciao
Stephan
^ permalink raw reply
* Re: [patch] crypto: omap-sham - potential Oops on error in probe
From: Peter Ujfalusi @ 2016-05-18 10:42 UTC (permalink / raw)
To: Dan Carpenter, Herbert Xu; +Cc: David S. Miller, linux-crypto, kernel-janitors
In-Reply-To: <20160518103905.GA10470@mwanda>
On 05/18/16 13:39, Dan Carpenter wrote:
> This if statement is reversed so we end up either leaking or Oopsing on
> error.
Oops, sorry for that.
Probably the other omap-* crypto drivers have the same issue? Can you send a
patch for them or should I do it?
Acked-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> Fixes: dbe246209bc1 ('crypto: omap-sham - Use dma_request_chan() for requesting DMA channel')
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> diff --git a/drivers/crypto/omap-sham.c b/drivers/crypto/omap-sham.c
> index 6eefaa2..63464e8 100644
> --- a/drivers/crypto/omap-sham.c
> +++ b/drivers/crypto/omap-sham.c
> @@ -1986,7 +1986,7 @@ err_algs:
> &dd->pdata->algs_info[i].algs_list[j]);
> err_pm:
> pm_runtime_disable(dev);
> - if (dd->polling_mode)
> + if (!dd->polling_mode)
> dma_release_channel(dd->dma_lch);
> data_err:
> dev_err(dev, "initialization failed.\n");
>
--
Péter
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [patch] crypto: omap-sham - potential Oops on error in probe
From: Dan Carpenter @ 2016-05-18 10:39 UTC (permalink / raw)
To: Herbert Xu, Peter Ujfalusi; +Cc: David S. Miller, linux-crypto, kernel-janitors
This if statement is reversed so we end up either leaking or Oopsing on
error.
Fixes: dbe246209bc1 ('crypto: omap-sham - Use dma_request_chan() for requesting DMA channel')
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/drivers/crypto/omap-sham.c b/drivers/crypto/omap-sham.c
index 6eefaa2..63464e8 100644
--- a/drivers/crypto/omap-sham.c
+++ b/drivers/crypto/omap-sham.c
@@ -1986,7 +1986,7 @@ err_algs:
&dd->pdata->algs_info[i].algs_list[j]);
err_pm:
pm_runtime_disable(dev);
- if (dd->polling_mode)
+ if (!dd->polling_mode)
dma_release_channel(dd->dma_lch);
data_err:
dev_err(dev, "initialization failed.\n");
^ permalink raw reply related
* [PATCH 0/2] crypto: engine: permit to enqueue ashash_request
From: LABBE Corentin @ 2016-05-18 9:21 UTC (permalink / raw)
To: baolin.wang, herbert, davem; +Cc: linux-crypto, linux-kernel, LABBE Corentin
Hello
I wanted to use the crypto engine for my Allwinner crypto driver but something
prevented me to use it: it cannot enqueue hash requests.
The first patch convert crypto engine to permit enqueuing of ahash_requests.
The second patch convert the only driver using crypto engine.
The second patch was only compile tested but the crypto engine with
hash support was tested on two different offtree driver (sun4i-ss and sun8i-ce)
Regards
LABBE Corentin (2):
crypto: engine: permit to enqueue ashash_request
crypto: omap: convert to the new cryptoengine API
crypto/crypto_engine.c | 17 +++++++----------
drivers/crypto/omap-aes.c | 10 ++++++----
include/crypto/algapi.h | 14 +++++++-------
3 files changed, 20 insertions(+), 21 deletions(-)
--
2.7.3
^ permalink raw reply
* [PATCH 2/2] crypto: omap: convert to the new cryptoengine API
From: LABBE Corentin @ 2016-05-18 9:21 UTC (permalink / raw)
To: baolin.wang, herbert, davem; +Cc: linux-crypto, linux-kernel, LABBE Corentin
In-Reply-To: <1463563285-11742-1-git-send-email-clabbe.montjoie@gmail.com>
Since the crypto engine has been converted to use crypto_async_request
instead of ablkcipher_request, minor changes are needed to use it.
Signed-off-by: LABBE Corentin <clabbe.montjoie@gmail.com>
---
drivers/crypto/omap-aes.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/crypto/omap-aes.c b/drivers/crypto/omap-aes.c
index d420ec7..1368ab1 100644
--- a/drivers/crypto/omap-aes.c
+++ b/drivers/crypto/omap-aes.c
@@ -530,7 +530,7 @@ static void omap_aes_finish_req(struct omap_aes_dev *dd, int err)
pr_debug("err: %d\n", err);
- crypto_finalize_request(dd->engine, req, err);
+ crypto_finalize_request(dd->engine, &req->base, err);
}
static int omap_aes_crypt_dma_stop(struct omap_aes_dev *dd)
@@ -603,14 +603,15 @@ static int omap_aes_handle_queue(struct omap_aes_dev *dd,
struct ablkcipher_request *req)
{
if (req)
- return crypto_transfer_request_to_engine(dd->engine, req);
+ return crypto_transfer_request_to_engine(dd->engine, &req->base);
return 0;
}
static int omap_aes_prepare_req(struct crypto_engine *engine,
- struct ablkcipher_request *req)
+ struct crypto_async_request *areq)
{
+ struct ablkcipher_request *req = ablkcipher_request_cast(areq);
struct omap_aes_ctx *ctx = crypto_ablkcipher_ctx(
crypto_ablkcipher_reqtfm(req));
struct omap_aes_dev *dd = omap_aes_find_dev(ctx);
@@ -653,8 +654,9 @@ static int omap_aes_prepare_req(struct crypto_engine *engine,
}
static int omap_aes_crypt_req(struct crypto_engine *engine,
- struct ablkcipher_request *req)
+ struct crypto_async_request *areq)
{
+ struct ablkcipher_request *req = ablkcipher_request_cast(areq);
struct omap_aes_ctx *ctx = crypto_ablkcipher_ctx(
crypto_ablkcipher_reqtfm(req));
struct omap_aes_dev *dd = omap_aes_find_dev(ctx);
--
2.7.3
^ permalink raw reply related
* [PATCH 1/2] crypto: engine: permit to enqueue ashash_request
From: LABBE Corentin @ 2016-05-18 9:21 UTC (permalink / raw)
To: baolin.wang, herbert, davem; +Cc: linux-crypto, linux-kernel, LABBE Corentin
In-Reply-To: <1463563285-11742-1-git-send-email-clabbe.montjoie@gmail.com>
The current crypto engine allow only ablkcipher_request to be enqueued.
Thus denying any use of it for hardware that also handle hash algo.
This patch convert all ablkcipher_request references to the
more general crypto_async_request.
Signed-off-by: LABBE Corentin <clabbe.montjoie@gmail.com>
---
crypto/crypto_engine.c | 17 +++++++----------
include/crypto/algapi.h | 14 +++++++-------
2 files changed, 14 insertions(+), 17 deletions(-)
diff --git a/crypto/crypto_engine.c b/crypto/crypto_engine.c
index a55c82d..b658cb8 100644
--- a/crypto/crypto_engine.c
+++ b/crypto/crypto_engine.c
@@ -19,7 +19,7 @@
#define CRYPTO_ENGINE_MAX_QLEN 10
void crypto_finalize_request(struct crypto_engine *engine,
- struct ablkcipher_request *req, int err);
+ struct crypto_async_request *req, int err);
/**
* crypto_pump_requests - dequeue one request from engine queue to process
@@ -34,7 +34,6 @@ static void crypto_pump_requests(struct crypto_engine *engine,
bool in_kthread)
{
struct crypto_async_request *async_req, *backlog;
- struct ablkcipher_request *req;
unsigned long flags;
bool was_busy = false;
int ret;
@@ -82,9 +81,7 @@ static void crypto_pump_requests(struct crypto_engine *engine,
if (!async_req)
goto out;
- req = ablkcipher_request_cast(async_req);
-
- engine->cur_req = req;
+ engine->cur_req = async_req;
if (backlog)
backlog->complete(backlog, -EINPROGRESS);
@@ -142,7 +139,7 @@ static void crypto_pump_work(struct kthread_work *work)
* @req: the request need to be listed into the engine queue
*/
int crypto_transfer_request(struct crypto_engine *engine,
- struct ablkcipher_request *req, bool need_pump)
+ struct crypto_async_request *req, bool need_pump)
{
unsigned long flags;
int ret;
@@ -154,7 +151,7 @@ int crypto_transfer_request(struct crypto_engine *engine,
return -ESHUTDOWN;
}
- ret = ablkcipher_enqueue_request(&engine->queue, req);
+ ret = crypto_enqueue_request(&engine->queue, req);
if (!engine->busy && need_pump)
queue_kthread_work(&engine->kworker, &engine->pump_requests);
@@ -171,7 +168,7 @@ EXPORT_SYMBOL_GPL(crypto_transfer_request);
* @req: the request need to be listed into the engine queue
*/
int crypto_transfer_request_to_engine(struct crypto_engine *engine,
- struct ablkcipher_request *req)
+ struct crypto_async_request *req)
{
return crypto_transfer_request(engine, req, true);
}
@@ -184,7 +181,7 @@ EXPORT_SYMBOL_GPL(crypto_transfer_request_to_engine);
* @err: error number
*/
void crypto_finalize_request(struct crypto_engine *engine,
- struct ablkcipher_request *req, int err)
+ struct crypto_async_request *req, int err)
{
unsigned long flags;
bool finalize_cur_req = false;
@@ -208,7 +205,7 @@ void crypto_finalize_request(struct crypto_engine *engine,
spin_unlock_irqrestore(&engine->queue_lock, flags);
}
- req->base.complete(&req->base, err);
+ req->complete(req, err);
queue_kthread_work(&engine->kworker, &engine->pump_requests);
}
diff --git a/include/crypto/algapi.h b/include/crypto/algapi.h
index eeafd21..d720a2a 100644
--- a/include/crypto/algapi.h
+++ b/include/crypto/algapi.h
@@ -173,26 +173,26 @@ struct crypto_engine {
int (*unprepare_crypt_hardware)(struct crypto_engine *engine);
int (*prepare_request)(struct crypto_engine *engine,
- struct ablkcipher_request *req);
+ struct crypto_async_request *req);
int (*unprepare_request)(struct crypto_engine *engine,
- struct ablkcipher_request *req);
+ struct crypto_async_request *req);
int (*crypt_one_request)(struct crypto_engine *engine,
- struct ablkcipher_request *req);
+ struct crypto_async_request *req);
struct kthread_worker kworker;
struct task_struct *kworker_task;
struct kthread_work pump_requests;
void *priv_data;
- struct ablkcipher_request *cur_req;
+ struct crypto_async_request *cur_req;
};
int crypto_transfer_request(struct crypto_engine *engine,
- struct ablkcipher_request *req, bool need_pump);
+ struct crypto_async_request *req, bool need_pump);
int crypto_transfer_request_to_engine(struct crypto_engine *engine,
- struct ablkcipher_request *req);
+ struct crypto_async_request *req);
void crypto_finalize_request(struct crypto_engine *engine,
- struct ablkcipher_request *req, int err);
+ struct crypto_async_request *req, int err);
int crypto_engine_start(struct crypto_engine *engine);
int crypto_engine_stop(struct crypto_engine *engine);
struct crypto_engine *crypto_engine_alloc_init(struct device *dev, bool rt);
--
2.7.3
^ permalink raw reply related
* Re: Decrypting data in RX path
From: Catalin Vasile @ 2016-05-18 7:01 UTC (permalink / raw)
To: Gadre Nayan, Stephan Mueller, linux-crypto@vger.kernel.org
In-Reply-To: <CAKJ7aR5gJ=ebBgYPq6=eUQoVLi8xXz9EsRnRqWvh2_EG6s7p1g@mail.gmail.com>
Inline Comments
________________________________________
From: linux-crypto-owner@vger.kernel.org <linux-crypto-owner@vger.kernel.org> on behalf of Gadre Nayan <gadrenayan@gmail.com>
Sent: Monday, May 16, 2016 5:40 PM
To: Stephan Mueller; linux-crypto@vger.kernel.org
Subject: Re: Decrypting data in RX path
Hi,
1. The context of the question "best place to decrypt in
kernel(module/driver)" is I want to encrypt network packets sent from
my system and decrypt them back to work with crypto apis. So the
encryption part I have done in a Kernel thread, decryption part could
be either in driver or a pre-routing hook. Which is appropriate.
2. I went through the esp_input function for rx.
As I understand, It allocates a decrypt request and and calls
crypto_aead_decrypt(req).
A. Since this request is asynchronous, it would be handled through
condition variables, Am i right on this?
B. Also the IPSEC routines like input and output would run in softirq context ?
C. esp_input_done() is a callback for decrypt, so as soon as
crypto_aead_decrypt(req) is called and the encryption does not happen
immediately, it will return the error _EINPROGRESS. Now this will
cause the esp_input function to return immediately. So then when is
the deferred decryption checked. I see esp_input_done2 as well. How is
the flow and call of these callbacks happening.
[Catalin Vasile]
"aead_request_set_callback(req, 0, esp_output_done_esn, skb);"
This piece of code sets into the request structure a callback.
After the job is queued, you will receive an -EINPROGRESS and your current
context will just return till it ends the current context (having nothing other to do).
Later, the device will trigger an interrupt in the Kernel to announce it
that a job has been done. The interrupt usually starts a kthread or a softirq
which will process what jobs have ended. One of the processing part thingies
is triggering the callback you have set. Although the context that sent the job
no longer exist, the hardware driver uses its own contexts to run some custom
you have sent (that code is represented by your callback).
Apologize for being so verbose.
Thanks.
On Mon, May 16, 2016 at 6:02 PM, Stephan Mueller <smueller@chronox.de> wrote:
> Am Montag, 16. Mai 2016, 17:24:12 schrieb Gadre Nayan:
>
> Hi Gadre,
>
>> Hi,
>>
>> I am able to encrypt data using the asynchronous kernel crypto API's.
>> I can observe the encrypted data on the protocol analyzer.
>>
>> I wanted to decry-pt the data now on the receiver side, So I have
>> following questions.
>>
>> 1. What is the best place to decrypt the data, in kernel space (module
>> (pre-routing hook) or driver) OR user space using (maybe using raw
>> sockets or after socket recv).
>
> This is a very broad question and cannot be answered without knowning the
> context.
>>
>> What precautions should be taken in terms of locking while using
>> crypto api's in kernel space in RX path (Softirq context) --> Can
>> someone point to existing sample in kernel where decryption is done in
>> RX path.
>
> net/ipv4/esp4.c:esp_input for rx and esp_output for tx.
>>
>>
>> 2. If I encrypt data in kernel space can I decrypt it in User-space
>> using same encryption methods and Keys.
>
> Sure, if you have the keys and all information about the used crypto.
>>
>> Thanks.
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
> Ciao
> Stephan
^ permalink raw reply
* Re: linux-4.6/drivers/crypto/ux500/hash/hash_core.c: 2 * possible bad size ?
From: Herbert Xu @ 2016-05-18 7:46 UTC (permalink / raw)
To: David Binderman
Cc: davem, linux-crypto, linux-kernel, dcb314, Andreas Westin,
Linus Walleij
In-Reply-To: <CAMzoamabHQ0SaMmkTfwDGGZBpOxSE6Qci4TwEUAEMbtWO3X-Ew@mail.gmail.com>
On Mon, May 16, 2016 at 07:13:12PM +0100, David Binderman wrote:
> Hello there,
>
> 1.
>
> linux-4.6/drivers/crypto/ux500/hash/hash_core.c:784]: (warning) Division by
> result of sizeof(). memmove() expects a size in bytes, did you intend to
> multiply instead?
>
> Source code is
>
> memmove(req_ctx->state.buffer,
> device_data->state.buffer,
> HASH_BLOCK_SIZE / sizeof(u32));
>
> Maybe better code
>
> memmove(req_ctx->state.buffer,
> device_data->state.buffer,
> HASH_BLOCK_SIZE);
>
> 2.
>
> linux-4.6/drivers/crypto/ux500/hash/hash_core.c:835]: (warning) Division by
> result of sizeof(). memmove() expects a size in bytes, did you intend to
> multiply instead?
>
> Duplicate.
Thanks for noticing these bugs. This driver hasn't been maintained
since 2012, so unless someone steps up I'm going to just delete it.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: Typos and RSA
From: Stephan Mueller @ 2016-05-17 22:56 UTC (permalink / raw)
To: Gary R Hook; +Cc: Tadeusz Struk, linux-crypto
In-Reply-To: <573B9F54.7050006@amd.com>
Am Dienstag, 17. Mai 2016, 17:46:44 schrieb Gary R Hook:
Hi Gary,
> Thanks so much.
>
> There are exactly 3 references to that symbol (in my freshly pulled copy
> of cryptodev-2.6).
> testmgr.c precipitates my questions, and public_key.c doesn't actually
> provide any content
> in the source input buffer, neither modulus nor plaintext. Thus, it
> doesn't clarify things
> either.
>
> But I truly appreciate your attention.
>
> So I'll go ahead and ask, because I did look at the two areas mentioned
> by Stephan, but
> neither seem to clarify (to my admittedly ignorant eyes... I'm a real
> newbie on crypto here)
> usage.
Here is an example from a current code of mine (all kccavs symbols are private
to my code):
tfm = crypto_alloc_akcipher(kccavs_test->name, 0, 0);
if (IS_ERR(tfm)) {
pr_info("could not allocate akcipher handle for %s %ld\n",
kccavs_test->name, PTR_ERR(tfm));
return PTR_ERR(tfm);
}
req = akcipher_request_alloc(tfm, GFP_KERNEL);
if (!req)
goto err;
if (crypto_akcipher_maxsize(tfm) > MAXDATALEN) {
err = -ENOMEM;
pr_info("output datasize is too big\n");
goto err;
}
err = crypto_akcipher_set_pub_key(tfm, key->data, key->len);
if (err)
goto err;
err = kccavs_akcipher_hashdata();
if (err)
goto err;
rsa.tfm = tfm;
rsa.req = req;
init_completion(&rsa.result.completion);
sg_init_one(&src, rsa_sig->data, rsa_sig->len);
akcipher_request_set_crypt(req, &src, &src, rsa_sig->len,
crypto_akcipher_maxsize(tfm));
err = crypto_akcipher_verify(req);
kccavs_akcipher_wait(&rsa, err);
dbg("RSA signature verification result %d\n", err);
rsa_sig->len = min_t(u64, rsa_sig->len, data->len);
if (!err) {
/* sig ver passed */
if (data->len == rsa_sig->len &&
!memcmp(rsa_sig->data, data->data, rsa_sig->len)) {
dbg("sig ver op passed, good data");
memcpy(kccavs_test->data.data, "\xbe\xef\xbe\xef", 4);
} else {
dbg("sig ver op passed, wrong data");
memcpy(kccavs_test->data.data, "\xde\xad\xbe\xef", 4);
}
} else {
/*
* Signature verify failed
* Note, the CAVS vectors may sometimes even provide wrong
* input data that do not comply with some requirements. In
* this case, the kernel returns errors other than EBADMSG.
*/
memcpy(kccavs_test->data.data, "\xde\xad\xbe\xef", 4);
}
>
> Here's my question:
>
> The source input (as set via akcipher_request_set_crypt()) to akcipher
> is supposed to be
> the modulus + plaintext. testmgr hands this in via a scatterlist with 2
> elements, wherein
> the first is the modulus, the second the payload. The source length
> however, appears to
> be the aggregate length of these two elements. Okay, good enough.
>
> Since the API provides no information about the modulus length, one
crypto_akcipher_maxsize(tfm)?
> might guess that the
> only way to ascertain the separate lengths of the two parts is to read
> the values from
> the scatterlist structures. I get that the key (exponent) length is what
> really matters,
> but the source input fields are clearly of arbitrary and unequal length.
Depending on what type of RSA operation you use (raw or pkcs1) or the
operation type (siggen/ver), the input is not just the modulus.
So, to help you, you need to give a bit more information about what type of
RSA operation you want to perform.
>
> So please forgive my ignorance, but I'm not seeing it: how do we
> decompose the two parts
> of src and get their lengths? What can we expect on the receiving end of
> the encrypt
> function with respect to the source data structure? A two-element
> scatterlist? Or...?
> What can we conclude from the calls made in do_test_rsa() in testmgr.c,
> vs the call in
> public_key_verify_signature() in public_key.c re: invocation and
> provided data.
>
> Any clarification is greatly appreciated.
>
> Gary
>
> On 05/17/2016 05:18 PM, Tadeusz Struk wrote:
> > On 05/17/2016 03:16 PM, Stephan Mueller wrote:
> >>> I am working on hooking up RSA functionality to the akcipher API. It
> >>> appears>>>
> >>>> that no other code, to date, uses this API. Can anyone confirm or deny
> >>>> that
> >>>> conclusion?
> >>
> >> This is not correct. The asymmetric key API uses that code. So does the
> >> module signing code.
> >
> > Also you can have a look at testmgr.c for examples.
> > "git grep crypto_alloc_akcipher" is also useful.
> >
> > Thanks,
Ciao
Stephan
^ permalink raw reply
* Re: Typos and RSA
From: Gary R Hook @ 2016-05-17 22:46 UTC (permalink / raw)
To: Tadeusz Struk; +Cc: Stephan Mueller, linux-crypto
In-Reply-To: <ae33e745-feca-c588-015d-bf2b4ca461f0@intel.com>
Thanks so much.
There are exactly 3 references to that symbol (in my freshly pulled copy
of cryptodev-2.6).
testmgr.c precipitates my questions, and public_key.c doesn't actually
provide any content
in the source input buffer, neither modulus nor plaintext. Thus, it
doesn't clarify things
either.
But I truly appreciate your attention.
So I'll go ahead and ask, because I did look at the two areas mentioned
by Stephan, but
neither seem to clarify (to my admittedly ignorant eyes... I'm a real
newbie on crypto here)
usage.
Here's my question:
The source input (as set via akcipher_request_set_crypt()) to akcipher
is supposed to be
the modulus + plaintext. testmgr hands this in via a scatterlist with 2
elements, wherein
the first is the modulus, the second the payload. The source length
however, appears to
be the aggregate length of these two elements. Okay, good enough.
Since the API provides no information about the modulus length, one
might guess that the
only way to ascertain the separate lengths of the two parts is to read
the values from
the scatterlist structures. I get that the key (exponent) length is what
really matters,
but the source input fields are clearly of arbitrary and unequal length.
So please forgive my ignorance, but I'm not seeing it: how do we
decompose the two parts
of src and get their lengths? What can we expect on the receiving end of
the encrypt
function with respect to the source data structure? A two-element
scatterlist? Or...?
What can we conclude from the calls made in do_test_rsa() in testmgr.c,
vs the call in
public_key_verify_signature() in public_key.c re: invocation and
provided data.
Any clarification is greatly appreciated.
Gary
On 05/17/2016 05:18 PM, Tadeusz Struk wrote:
> On 05/17/2016 03:16 PM, Stephan Mueller wrote:
>>> I am working on hooking up RSA functionality to the akcipher API. It appears
>>>> that no other code, to date, uses this API. Can anyone confirm or deny that
>>>> conclusion?
>> This is not correct. The asymmetric key API uses that code. So does the module
>> signing code.
> Also you can have a look at testmgr.c for examples.
> "git grep crypto_alloc_akcipher" is also useful.
>
> Thanks,
^ permalink raw reply
* Re: Typos and RSA
From: Tadeusz Struk @ 2016-05-17 22:18 UTC (permalink / raw)
To: Gary R Hook; +Cc: Stephan Mueller, linux-crypto
In-Reply-To: <1468644.THAbu50UCt@positron.chronox.de>
On 05/17/2016 03:16 PM, Stephan Mueller wrote:
>> I am working on hooking up RSA functionality to the akcipher API. It appears
>> > that no other code, to date, uses this API. Can anyone confirm or deny that
>> > conclusion?
> This is not correct. The asymmetric key API uses that code. So does the module
> signing code.
Also you can have a look at testmgr.c for examples.
"git grep crypto_alloc_akcipher" is also useful.
Thanks,
--
TS
^ permalink raw reply
* Re: Typos and RSA
From: Stephan Mueller @ 2016-05-17 22:16 UTC (permalink / raw)
To: Gary R Hook; +Cc: linux-crypto
In-Reply-To: <573B8BA3.8020902@amd.com>
Am Dienstag, 17. Mai 2016, 16:22:43 schrieb Gary R Hook:
Hi Gary,
> I am working on hooking up RSA functionality to the akcipher API. It appears
> that no other code, to date, uses this API. Can anyone confirm or deny that
> conclusion?
This is not correct. The asymmetric key API uses that code. So does the module
signing code.
>
> I have questions about invoking akcipher transform functions, and can
> find no
> information about specifics that vex me. If there is other code to look
> at, I'm
> happy to. Otherwise, I can embarrass myself here with a display of
> ignorance.
>
> Gary
> --
> To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Ciao
Stephan
^ permalink raw reply
* Typos and RSA
From: Gary R Hook @ 2016-05-17 21:22 UTC (permalink / raw)
To: linux-crypto
I am working on hooking up RSA functionality to the akcipher API. It appears
that no other code, to date, uses this API. Can anyone confirm or deny that
conclusion?
I have questions about invoking akcipher transform functions, and can
find no
information about specifics that vex me. If there is other code to look
at, I'm
happy to. Otherwise, I can embarrass myself here with a display of
ignorance.
Gary
^ permalink raw reply
* [PATCH] crypto: qat - fix typos sizeof for ctx
From: Tadeusz Struk @ 2016-05-17 17:53 UTC (permalink / raw)
To: herbert; +Cc: linux-crypto, tadeusz.struk
The sizeof(*ctx->dec_cd) and sizeof(*ctx->enc_cd) are equal,
but we should use the correct one for freeing memory anyway.
Signed-off-by: Tadeusz Struk <tadeusz.struk@intel.com>
---
drivers/crypto/qat/qat_common/qat_algs.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/crypto/qat/qat_common/qat_algs.c b/drivers/crypto/qat/qat_common/qat_algs.c
index 1e8852a8..769148d 100644
--- a/drivers/crypto/qat/qat_common/qat_algs.c
+++ b/drivers/crypto/qat/qat_common/qat_algs.c
@@ -947,13 +947,13 @@ static int qat_alg_ablkcipher_setkey(struct crypto_ablkcipher *tfm,
return 0;
out_free_all:
- memset(ctx->dec_cd, 0, sizeof(*ctx->enc_cd));
- dma_free_coherent(dev, sizeof(*ctx->enc_cd),
+ memset(ctx->dec_cd, 0, sizeof(*ctx->dec_cd));
+ dma_free_coherent(dev, sizeof(*ctx->dec_cd),
ctx->dec_cd, ctx->dec_cd_paddr);
ctx->dec_cd = NULL;
out_free_enc:
- memset(ctx->enc_cd, 0, sizeof(*ctx->dec_cd));
- dma_free_coherent(dev, sizeof(*ctx->dec_cd),
+ memset(ctx->enc_cd, 0, sizeof(*ctx->enc_cd));
+ dma_free_coherent(dev, sizeof(*ctx->enc_cd),
ctx->enc_cd, ctx->enc_cd_paddr);
ctx->enc_cd = NULL;
return -ENOMEM;
--
2.1.4
^ permalink raw reply related
* Re: [PATCH] crypto/sha1-mb: make sha1_x8_avx2() conform to C function ABI
From: Herbert Xu @ 2016-05-17 6:31 UTC (permalink / raw)
To: Megha Dey; +Cc: Josh Poimboeuf, Ingo Molnar, linux-crypto, linux-kernel
In-Reply-To: <1463440073.2547.1.camel@megha-Z97X-UD7-TH>
On Mon, May 16, 2016 at 04:07:53PM -0700, Megha Dey wrote:
>
> ok. I tried at my end too using some printk's and nothing seems to be
> broken. Also, if you intend to use the same commit message, I spell my
> name as Megha Dey and not Day :)
Patch applied with the name corrected.
Thanks,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH v2 0/8] crypto: caam - add support for LS1043A SoC
From: Herbert Xu @ 2016-05-17 0:08 UTC (permalink / raw)
To: Horia Ioan Geanta Neag
Cc: Arnd Bergmann, Catalin Marinas, Will Deacon,
Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Rob Herring, Olof Johansson, linux-crypto@vger.kernel.org,
linux-kernel@vger.kernel.org, David S. Miller, Cristian Stoica,
Scott Wood, Alexandru Porosanu, Tudor-Dan Ambarus
In-Reply-To: <AM2PR04MB067320109DD38D223BEB6C4598770@AM2PR04MB0673.eurprd04.prod.outlook.com>
On Mon, May 16, 2016 at 03:49:27PM +0000, Horia Ioan Geanta Neag wrote:
>
> I assume it's too late for 4.7, however applying the patches would solve
> dependencies b/w on-going caam development.
I will be merging this after the merge window is closed.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* RE:
From: Weichert, Brian @ 2016-05-16 23:02 UTC (permalink / raw)
To: Weichert, Brian
In-Reply-To: <E5ACCB586875944EB0AE0E3EFA32B4F526FAD24C@exchange0.winona.edu>
________________________________
Do you need money to start up your own business and also to assist the needy around you ? if yes, please email (john_robin01@outlook.com) for immediate financial assistance.
Thank you.
^ permalink raw reply
* Re: [PATCH] crypto/sha1-mb: make sha1_x8_avx2() conform to C function ABI
From: Megha Dey @ 2016-05-16 23:07 UTC (permalink / raw)
To: Josh Poimboeuf; +Cc: Ingo Molnar, Herbert Xu, linux-crypto, linux-kernel
In-Reply-To: <20160516214631.5cvnfgvppw74acah@treble>
On Mon, 2016-05-16 at 16:46 -0500, Josh Poimboeuf wrote:
> On Mon, May 16, 2016 at 02:39:06PM -0700, Megha Dey wrote:
> > On Mon, 2016-05-16 at 15:16 -0500, Josh Poimboeuf wrote:
> > > On Mon, May 16, 2016 at 11:31:12AM -0700, Megha Dey wrote:
> > > > On Mon, 2016-05-16 at 09:44 -0500, Josh Poimboeuf wrote:
> > > > > On Fri, May 13, 2016 at 10:32:26AM -0700, Megha Dey wrote:
> > > > > > On Fri, 2016-05-13 at 07:51 +0200, Ingo Molnar wrote:
> > > > > > > * Herbert Xu <herbert@gondor.apana.org.au> wrote:
> > > > > > >
> > > > > > > > On Thu, May 12, 2016 at 04:31:06PM -0700, Megha Dey wrote:
> > > > > > > > > Hi,
> > > > > > > > >
> > > > > > > > > When booting latest kernel with the CONFIG_CRYPTO_SHA1_MB enabled, I
> > > > > > > > > observe a panic.
> > > > > > > > >
> > > > > > > > > After having a quick look, on reverting the following patches, I am able
> > > > > > > > > to complete the booting process.
> > > > > > > > > aec4d0e301f17bb143341c82cc44685b8af0b945
> > > > > > > > > 8691ccd764f9ecc69a6812dfe76214c86ac9ba06
> > > > > > > > > 68874ac3304ade7ed5ebb12af00d6b9bbbca0a16
> > > > > > > > >
> > > > > > > > > Of the 3 patches, aec4d0e301f17bb143341c82cc44685b8af0b945 seems wrong.
> > > > > > > > > The r10 to r15 registers are used in sha1_x8_avx2.S, which is called
> > > > > > > > > from sha1_mb_mgr_flush_avx2.S.
> > > > > > > > >
> > > > > > > > > I do not think the functionality of the SHA1-MB crypto algorithm has
> > > > > > > > > been tested after applying these changes. (I am not sure if any of the
> > > > > > > > > other crypto algorithms have been affected by these changes).
> > > > > > > >
> > > > > > > > Josh, Ingo:
> > > > > > > >
> > > > > > > > Any ideas on this? Should we revert?
> > > > > > >
> > > > > > > Yeah, I think so - although another option would be to standardize sha1_x8_avx2()
> > > > > > > - the problem is that it is a function that clobbers registers without
> > > > > > > saving/restoring them, breaking the C function ABI. I realize it's written in
> > > > > > > assembly, but unless there are strong performance reasons to deviate from the
> > > > > > > regular calling convention it might make sense to fix that.
> > > > > > >
> > > > > > > Do any warnings get generated after the revert, if you enable
> > > > > > > CONFIG_STACK_VALIDATION=y?
> > > > > >
> > > > > > After the revert and enabling CONFIG_STACK_VALIDATION:
> > > > > > arch/x86/crypto/sha1-mb/sha1_mb_mgr_flush_avx2.o: warning: objtool:
> > > > > > sha1_mb_mgr_flush_avx2()+0x20d: call without frame pointer save/setup
> > > > > >
> > > > > > arch/x86/crypto/sha1-mb/sha1_mb_mgr_submit_avx2.o: warning: objtool:
> > > > > > sha1_mb_mgr_submit_avx2()+0x115: call without frame pointer save/setup
> > > > >
> > > > > Megha,
> > > > >
> > > > > Sorry for breaking it. I completely missed the fact that the function
> > > > > calls sha1_x8_avx2() which clobbers registers.
> > > > >
> > > > > If the performance penalty isn't too bad, I'll submit a patch to
> > > > > standardize sha1_x8_avx2() to follow the C ABI.
> > > > >
> > > > > Do you have any tips for testing this code? I've tried using the tcrypt
> > > > > module, but no luck.
> > > > >
> > > > Josh,
> > > > Build the kernel with the following configs:
> > > > CONFIG_CRYPTO_SHA1_MB=y
> > > > CONFIG_CRYPTO_TEST=m
> > > > CONFIG_CRYPTO_MANAGER_DISABLE_TESTS=n
> > > > There was a kernel panic while booting.
> > > > So if after applying your new patch, we are able to get complete the
> > > > boot, then we are good.
> > > >
> > > > Could you please send a copy of the patch, I could test it on my end
> > > > too.
> > >
> > > Thanks. I was able to run the tests, though I didn't see a panic. Can
> > > you test with this patch?
> > >
> > > ----
> > >
> > > From: Josh Poimboeuf <jpoimboe@redhat.com>
> > > Subject: [PATCH] crypto/sha1-mb: make sha1_x8_avx2() conform to C function ABI
> > >
> > > Megha Day reported a kernel panic in crypto code. The problem is that
> > > sha1_x8_avx2() clobbers registers r12-r15 without saving and restoring
> > > them.
> > >
> > > Before commit aec4d0e301f1 ("x86/asm/crypto: Simplify stack usage in
> > > sha-mb functions"), those registers were saved and restored by the
> > > callers of the function. I removed them with that commit because I
> > > didn't realize sha1_x8_avx2() clobbered them.
> > >
> > > Fix the potential undefined behavior associated with clobbering the
> > > registers and make the behavior less surprising by changing the
> > > registers to be callee saved/restored to conform with the C function
> > > call ABI.
> > >
> > > Also, rdx (aka RSP_SAVE) doesn't need to be saved: I verified that none
> > > of the callers rely on it being saved, and it's not a callee-saved
> > > register in the C ABI.
> > >
> > > Fixes: aec4d0e301f1 ("x86/asm/crypto: Simplify stack usage in sha-mb functions")
> > > Cc: stable@vger.kernel.org # 4.6
> > > Reported-by: Megha Dey <megha.dey@linux.intel.com>
> > > Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
> > > ---
> > > arch/x86/crypto/sha-mb/sha1_x8_avx2.S | 13 +++++++++++--
> > > 1 file changed, 11 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/arch/x86/crypto/sha-mb/sha1_x8_avx2.S b/arch/x86/crypto/sha-mb/sha1_x8_avx2.S
> > > index 8e1b477..c9dae1c 100644
> > > --- a/arch/x86/crypto/sha-mb/sha1_x8_avx2.S
> > > +++ b/arch/x86/crypto/sha-mb/sha1_x8_avx2.S
> > > @@ -296,7 +296,11 @@ W14 = TMP_
> > > #
> > > ENTRY(sha1_x8_avx2)
> > >
> > > - push RSP_SAVE
> > > + # save callee-saved clobbered registers to comply with C function ABI
> > > + push %r12
> > > + push %r13
> > > + push %r14
> > > + push %r15
> > >
> > > #save rsp
> > > mov %rsp, RSP_SAVE
> > > @@ -446,7 +450,12 @@ lloop:
> > > ## Postamble
> > >
> > > mov RSP_SAVE, %rsp
> > > - pop RSP_SAVE
> > > +
> > > + # restore callee-saved clobbered registers
> > > + pop %r15
> > > + pop %r14
> > > + pop %r13
> > > + pop %r12
> > >
> > > ret
> > > ENDPROC(sha1_x8_avx2)
> >
> > Hi Josh,
> > I don't see the panic and am able to boot. However, I am not able to see
> > the tests running. You said you were able to insert the tcrypt module
> > and test this right?
>
> I didn't insert tcrypt manually, but I set the config flags you
> suggested, plus I added a few printks, and was at least able to verify
> that this code ran during boot without any crypto test errors or other
> warnings being reported.
>
ok. I tried at my end too using some printk's and nothing seems to be
broken. Also, if you intend to use the same commit message, I spell my
name as Megha Dey and not Day :)
^ permalink raw reply
* Re: [PATCH] crypto/sha1-mb: make sha1_x8_avx2() conform to C function ABI
From: Josh Poimboeuf @ 2016-05-16 21:46 UTC (permalink / raw)
To: Megha Dey; +Cc: Ingo Molnar, Herbert Xu, linux-crypto, linux-kernel
In-Reply-To: <1463434746.2538.1.camel@megha-Z97X-UD7-TH>
On Mon, May 16, 2016 at 02:39:06PM -0700, Megha Dey wrote:
> On Mon, 2016-05-16 at 15:16 -0500, Josh Poimboeuf wrote:
> > On Mon, May 16, 2016 at 11:31:12AM -0700, Megha Dey wrote:
> > > On Mon, 2016-05-16 at 09:44 -0500, Josh Poimboeuf wrote:
> > > > On Fri, May 13, 2016 at 10:32:26AM -0700, Megha Dey wrote:
> > > > > On Fri, 2016-05-13 at 07:51 +0200, Ingo Molnar wrote:
> > > > > > * Herbert Xu <herbert@gondor.apana.org.au> wrote:
> > > > > >
> > > > > > > On Thu, May 12, 2016 at 04:31:06PM -0700, Megha Dey wrote:
> > > > > > > > Hi,
> > > > > > > >
> > > > > > > > When booting latest kernel with the CONFIG_CRYPTO_SHA1_MB enabled, I
> > > > > > > > observe a panic.
> > > > > > > >
> > > > > > > > After having a quick look, on reverting the following patches, I am able
> > > > > > > > to complete the booting process.
> > > > > > > > aec4d0e301f17bb143341c82cc44685b8af0b945
> > > > > > > > 8691ccd764f9ecc69a6812dfe76214c86ac9ba06
> > > > > > > > 68874ac3304ade7ed5ebb12af00d6b9bbbca0a16
> > > > > > > >
> > > > > > > > Of the 3 patches, aec4d0e301f17bb143341c82cc44685b8af0b945 seems wrong.
> > > > > > > > The r10 to r15 registers are used in sha1_x8_avx2.S, which is called
> > > > > > > > from sha1_mb_mgr_flush_avx2.S.
> > > > > > > >
> > > > > > > > I do not think the functionality of the SHA1-MB crypto algorithm has
> > > > > > > > been tested after applying these changes. (I am not sure if any of the
> > > > > > > > other crypto algorithms have been affected by these changes).
> > > > > > >
> > > > > > > Josh, Ingo:
> > > > > > >
> > > > > > > Any ideas on this? Should we revert?
> > > > > >
> > > > > > Yeah, I think so - although another option would be to standardize sha1_x8_avx2()
> > > > > > - the problem is that it is a function that clobbers registers without
> > > > > > saving/restoring them, breaking the C function ABI. I realize it's written in
> > > > > > assembly, but unless there are strong performance reasons to deviate from the
> > > > > > regular calling convention it might make sense to fix that.
> > > > > >
> > > > > > Do any warnings get generated after the revert, if you enable
> > > > > > CONFIG_STACK_VALIDATION=y?
> > > > >
> > > > > After the revert and enabling CONFIG_STACK_VALIDATION:
> > > > > arch/x86/crypto/sha1-mb/sha1_mb_mgr_flush_avx2.o: warning: objtool:
> > > > > sha1_mb_mgr_flush_avx2()+0x20d: call without frame pointer save/setup
> > > > >
> > > > > arch/x86/crypto/sha1-mb/sha1_mb_mgr_submit_avx2.o: warning: objtool:
> > > > > sha1_mb_mgr_submit_avx2()+0x115: call without frame pointer save/setup
> > > >
> > > > Megha,
> > > >
> > > > Sorry for breaking it. I completely missed the fact that the function
> > > > calls sha1_x8_avx2() which clobbers registers.
> > > >
> > > > If the performance penalty isn't too bad, I'll submit a patch to
> > > > standardize sha1_x8_avx2() to follow the C ABI.
> > > >
> > > > Do you have any tips for testing this code? I've tried using the tcrypt
> > > > module, but no luck.
> > > >
> > > Josh,
> > > Build the kernel with the following configs:
> > > CONFIG_CRYPTO_SHA1_MB=y
> > > CONFIG_CRYPTO_TEST=m
> > > CONFIG_CRYPTO_MANAGER_DISABLE_TESTS=n
> > > There was a kernel panic while booting.
> > > So if after applying your new patch, we are able to get complete the
> > > boot, then we are good.
> > >
> > > Could you please send a copy of the patch, I could test it on my end
> > > too.
> >
> > Thanks. I was able to run the tests, though I didn't see a panic. Can
> > you test with this patch?
> >
> > ----
> >
> > From: Josh Poimboeuf <jpoimboe@redhat.com>
> > Subject: [PATCH] crypto/sha1-mb: make sha1_x8_avx2() conform to C function ABI
> >
> > Megha Day reported a kernel panic in crypto code. The problem is that
> > sha1_x8_avx2() clobbers registers r12-r15 without saving and restoring
> > them.
> >
> > Before commit aec4d0e301f1 ("x86/asm/crypto: Simplify stack usage in
> > sha-mb functions"), those registers were saved and restored by the
> > callers of the function. I removed them with that commit because I
> > didn't realize sha1_x8_avx2() clobbered them.
> >
> > Fix the potential undefined behavior associated with clobbering the
> > registers and make the behavior less surprising by changing the
> > registers to be callee saved/restored to conform with the C function
> > call ABI.
> >
> > Also, rdx (aka RSP_SAVE) doesn't need to be saved: I verified that none
> > of the callers rely on it being saved, and it's not a callee-saved
> > register in the C ABI.
> >
> > Fixes: aec4d0e301f1 ("x86/asm/crypto: Simplify stack usage in sha-mb functions")
> > Cc: stable@vger.kernel.org # 4.6
> > Reported-by: Megha Dey <megha.dey@linux.intel.com>
> > Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
> > ---
> > arch/x86/crypto/sha-mb/sha1_x8_avx2.S | 13 +++++++++++--
> > 1 file changed, 11 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/x86/crypto/sha-mb/sha1_x8_avx2.S b/arch/x86/crypto/sha-mb/sha1_x8_avx2.S
> > index 8e1b477..c9dae1c 100644
> > --- a/arch/x86/crypto/sha-mb/sha1_x8_avx2.S
> > +++ b/arch/x86/crypto/sha-mb/sha1_x8_avx2.S
> > @@ -296,7 +296,11 @@ W14 = TMP_
> > #
> > ENTRY(sha1_x8_avx2)
> >
> > - push RSP_SAVE
> > + # save callee-saved clobbered registers to comply with C function ABI
> > + push %r12
> > + push %r13
> > + push %r14
> > + push %r15
> >
> > #save rsp
> > mov %rsp, RSP_SAVE
> > @@ -446,7 +450,12 @@ lloop:
> > ## Postamble
> >
> > mov RSP_SAVE, %rsp
> > - pop RSP_SAVE
> > +
> > + # restore callee-saved clobbered registers
> > + pop %r15
> > + pop %r14
> > + pop %r13
> > + pop %r12
> >
> > ret
> > ENDPROC(sha1_x8_avx2)
>
> Hi Josh,
> I don't see the panic and am able to boot. However, I am not able to see
> the tests running. You said you were able to insert the tcrypt module
> and test this right?
I didn't insert tcrypt manually, but I set the config flags you
suggested, plus I added a few printks, and was at least able to verify
that this code ran during boot without any crypto test errors or other
warnings being reported.
--
Josh
^ permalink raw reply
* Re: [PATCH] crypto/sha1-mb: make sha1_x8_avx2() conform to C function ABI
From: Megha Dey @ 2016-05-16 21:39 UTC (permalink / raw)
To: Josh Poimboeuf; +Cc: Ingo Molnar, Herbert Xu, linux-crypto, linux-kernel
In-Reply-To: <20160516201618.u4zethkab3ohgtfb@treble>
On Mon, 2016-05-16 at 15:16 -0500, Josh Poimboeuf wrote:
> On Mon, May 16, 2016 at 11:31:12AM -0700, Megha Dey wrote:
> > On Mon, 2016-05-16 at 09:44 -0500, Josh Poimboeuf wrote:
> > > On Fri, May 13, 2016 at 10:32:26AM -0700, Megha Dey wrote:
> > > > On Fri, 2016-05-13 at 07:51 +0200, Ingo Molnar wrote:
> > > > > * Herbert Xu <herbert@gondor.apana.org.au> wrote:
> > > > >
> > > > > > On Thu, May 12, 2016 at 04:31:06PM -0700, Megha Dey wrote:
> > > > > > > Hi,
> > > > > > >
> > > > > > > When booting latest kernel with the CONFIG_CRYPTO_SHA1_MB enabled, I
> > > > > > > observe a panic.
> > > > > > >
> > > > > > > After having a quick look, on reverting the following patches, I am able
> > > > > > > to complete the booting process.
> > > > > > > aec4d0e301f17bb143341c82cc44685b8af0b945
> > > > > > > 8691ccd764f9ecc69a6812dfe76214c86ac9ba06
> > > > > > > 68874ac3304ade7ed5ebb12af00d6b9bbbca0a16
> > > > > > >
> > > > > > > Of the 3 patches, aec4d0e301f17bb143341c82cc44685b8af0b945 seems wrong.
> > > > > > > The r10 to r15 registers are used in sha1_x8_avx2.S, which is called
> > > > > > > from sha1_mb_mgr_flush_avx2.S.
> > > > > > >
> > > > > > > I do not think the functionality of the SHA1-MB crypto algorithm has
> > > > > > > been tested after applying these changes. (I am not sure if any of the
> > > > > > > other crypto algorithms have been affected by these changes).
> > > > > >
> > > > > > Josh, Ingo:
> > > > > >
> > > > > > Any ideas on this? Should we revert?
> > > > >
> > > > > Yeah, I think so - although another option would be to standardize sha1_x8_avx2()
> > > > > - the problem is that it is a function that clobbers registers without
> > > > > saving/restoring them, breaking the C function ABI. I realize it's written in
> > > > > assembly, but unless there are strong performance reasons to deviate from the
> > > > > regular calling convention it might make sense to fix that.
> > > > >
> > > > > Do any warnings get generated after the revert, if you enable
> > > > > CONFIG_STACK_VALIDATION=y?
> > > >
> > > > After the revert and enabling CONFIG_STACK_VALIDATION:
> > > > arch/x86/crypto/sha1-mb/sha1_mb_mgr_flush_avx2.o: warning: objtool:
> > > > sha1_mb_mgr_flush_avx2()+0x20d: call without frame pointer save/setup
> > > >
> > > > arch/x86/crypto/sha1-mb/sha1_mb_mgr_submit_avx2.o: warning: objtool:
> > > > sha1_mb_mgr_submit_avx2()+0x115: call without frame pointer save/setup
> > >
> > > Megha,
> > >
> > > Sorry for breaking it. I completely missed the fact that the function
> > > calls sha1_x8_avx2() which clobbers registers.
> > >
> > > If the performance penalty isn't too bad, I'll submit a patch to
> > > standardize sha1_x8_avx2() to follow the C ABI.
> > >
> > > Do you have any tips for testing this code? I've tried using the tcrypt
> > > module, but no luck.
> > >
> > Josh,
> > Build the kernel with the following configs:
> > CONFIG_CRYPTO_SHA1_MB=y
> > CONFIG_CRYPTO_TEST=m
> > CONFIG_CRYPTO_MANAGER_DISABLE_TESTS=n
> > There was a kernel panic while booting.
> > So if after applying your new patch, we are able to get complete the
> > boot, then we are good.
> >
> > Could you please send a copy of the patch, I could test it on my end
> > too.
>
> Thanks. I was able to run the tests, though I didn't see a panic. Can
> you test with this patch?
>
> ----
>
> From: Josh Poimboeuf <jpoimboe@redhat.com>
> Subject: [PATCH] crypto/sha1-mb: make sha1_x8_avx2() conform to C function ABI
>
> Megha Day reported a kernel panic in crypto code. The problem is that
> sha1_x8_avx2() clobbers registers r12-r15 without saving and restoring
> them.
>
> Before commit aec4d0e301f1 ("x86/asm/crypto: Simplify stack usage in
> sha-mb functions"), those registers were saved and restored by the
> callers of the function. I removed them with that commit because I
> didn't realize sha1_x8_avx2() clobbered them.
>
> Fix the potential undefined behavior associated with clobbering the
> registers and make the behavior less surprising by changing the
> registers to be callee saved/restored to conform with the C function
> call ABI.
>
> Also, rdx (aka RSP_SAVE) doesn't need to be saved: I verified that none
> of the callers rely on it being saved, and it's not a callee-saved
> register in the C ABI.
>
> Fixes: aec4d0e301f1 ("x86/asm/crypto: Simplify stack usage in sha-mb functions")
> Cc: stable@vger.kernel.org # 4.6
> Reported-by: Megha Dey <megha.dey@linux.intel.com>
> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
> ---
> arch/x86/crypto/sha-mb/sha1_x8_avx2.S | 13 +++++++++++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/crypto/sha-mb/sha1_x8_avx2.S b/arch/x86/crypto/sha-mb/sha1_x8_avx2.S
> index 8e1b477..c9dae1c 100644
> --- a/arch/x86/crypto/sha-mb/sha1_x8_avx2.S
> +++ b/arch/x86/crypto/sha-mb/sha1_x8_avx2.S
> @@ -296,7 +296,11 @@ W14 = TMP_
> #
> ENTRY(sha1_x8_avx2)
>
> - push RSP_SAVE
> + # save callee-saved clobbered registers to comply with C function ABI
> + push %r12
> + push %r13
> + push %r14
> + push %r15
>
> #save rsp
> mov %rsp, RSP_SAVE
> @@ -446,7 +450,12 @@ lloop:
> ## Postamble
>
> mov RSP_SAVE, %rsp
> - pop RSP_SAVE
> +
> + # restore callee-saved clobbered registers
> + pop %r15
> + pop %r14
> + pop %r13
> + pop %r12
>
> ret
> ENDPROC(sha1_x8_avx2)
Hi Josh,
I don't see the panic and am able to boot. However, I am not able to see
the tests running. You said you were able to insert the tcrypt module
and test this right?
^ permalink raw reply
* Re: [PATCH v6 0/6] crypto: algif - add akcipher
From: Tadeusz Struk @ 2016-05-16 20:46 UTC (permalink / raw)
To: dhowells
Cc: herbert, smueller, linux-api, marcel, linux-kernel, keyrings,
linux-crypto, dwmw2, davem
In-Reply-To: <20160515041645.15888.94903.stgit@tstruk-mobl1>
On 05/14/2016 09:16 PM, Tadeusz Struk wrote:
> First four patches are a resend of the v3 algif_akcipher from
> Stephan Mueller, with minor changes after rebase on top of 4.6-rc1.
>
> The next three patches add support for keys stored in system
> keyring subsystem.
>
> First patch adds algif_akcipher nokey hadlers.
>
> Second patch adds generic sign, verify, encrypt, decrypt accessors
> functions to the asymmetric key type. These will be defined by
> asymmetric subtypes, similarly to how public_key currently defines
> the verify_signature function.
>
> Third patch adds support for ALG_SET_KEY_ID and ALG_SET_PUBKEY_ID
> commands to AF_ALG and setkeyid operation to the af_alg_type struct.
> If the keyid is used then the afalg layer acquires the key for the
> keyring subsystem and uses the new asymmetric accessor functions
> instead of akcipher api. The asymmetric subtypes can use akcipher
> api internally.
>
> This is the same v5 version as before rebased on top of
> http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=keys-asym-keyctl
>
> v6 changes:
> - update to reflect changes in kernel_pkey_params struct
>
> v5 changes:
> - drop public key changes and use new version provided by David
>
> v4 changes:
> - don't use internal public_key struct in af_alg.
> - add generic accessor functions to asymmetric key type, which take
> the generic struct key type and resolve the specific subtype internally
>
> v3 changes:
> - include Stephan's patches (rebased on 4.6-rc1)
> - add algif_akcipher nokey hadlers
> - add public_key info struct to public_key and helper query functions
> - add a check if a key is a software accessible key on af_alg, and
> return -ENOKEY if it isn't
>
> v2 changes:
> - pass the original skcipher request in ablkcipher.base.data instead of
> casting it back from the ablkcipher request.
> - rename _req to base_req
> - dropped 3/3
>
> ---
>
> Stephan Mueller (4):
> crypto: AF_ALG -- add sign/verify API
> crypto: AF_ALG -- add setpubkey setsockopt call
> crypto: AF_ALG -- add asymmetric cipher interface
> crypto: algif_akcipher - enable compilation
>
> Tadeusz Struk (2):
> crypto: algif_akcipher - add ops_nokey
> crypto: AF_ALG - add support for key_id
>
> crypto/Kconfig | 9
> crypto/Makefile | 1
> crypto/af_alg.c | 28 +
> crypto/algif_akcipher.c | 884 +++++++++++++++++++++++++++++++++++++++++++
> include/crypto/if_alg.h | 2
> include/uapi/linux/if_alg.h | 5
> 6 files changed, 924 insertions(+), 5 deletions(-)
> create mode 100644 crypto/algif_akcipher.c
Hi David,
Are you ok to take it into 4.7 via your linux-fs?
Thanks,
--
TS
^ permalink raw reply
* [PATCH] crypto/sha1-mb: make sha1_x8_avx2() conform to C function ABI
From: Josh Poimboeuf @ 2016-05-16 20:16 UTC (permalink / raw)
To: Megha Dey; +Cc: Ingo Molnar, Herbert Xu, linux-crypto, linux-kernel
In-Reply-To: <1463423472.5329.6.camel@megha-Z97X-UD7-TH>
On Mon, May 16, 2016 at 11:31:12AM -0700, Megha Dey wrote:
> On Mon, 2016-05-16 at 09:44 -0500, Josh Poimboeuf wrote:
> > On Fri, May 13, 2016 at 10:32:26AM -0700, Megha Dey wrote:
> > > On Fri, 2016-05-13 at 07:51 +0200, Ingo Molnar wrote:
> > > > * Herbert Xu <herbert@gondor.apana.org.au> wrote:
> > > >
> > > > > On Thu, May 12, 2016 at 04:31:06PM -0700, Megha Dey wrote:
> > > > > > Hi,
> > > > > >
> > > > > > When booting latest kernel with the CONFIG_CRYPTO_SHA1_MB enabled, I
> > > > > > observe a panic.
> > > > > >
> > > > > > After having a quick look, on reverting the following patches, I am able
> > > > > > to complete the booting process.
> > > > > > aec4d0e301f17bb143341c82cc44685b8af0b945
> > > > > > 8691ccd764f9ecc69a6812dfe76214c86ac9ba06
> > > > > > 68874ac3304ade7ed5ebb12af00d6b9bbbca0a16
> > > > > >
> > > > > > Of the 3 patches, aec4d0e301f17bb143341c82cc44685b8af0b945 seems wrong.
> > > > > > The r10 to r15 registers are used in sha1_x8_avx2.S, which is called
> > > > > > from sha1_mb_mgr_flush_avx2.S.
> > > > > >
> > > > > > I do not think the functionality of the SHA1-MB crypto algorithm has
> > > > > > been tested after applying these changes. (I am not sure if any of the
> > > > > > other crypto algorithms have been affected by these changes).
> > > > >
> > > > > Josh, Ingo:
> > > > >
> > > > > Any ideas on this? Should we revert?
> > > >
> > > > Yeah, I think so - although another option would be to standardize sha1_x8_avx2()
> > > > - the problem is that it is a function that clobbers registers without
> > > > saving/restoring them, breaking the C function ABI. I realize it's written in
> > > > assembly, but unless there are strong performance reasons to deviate from the
> > > > regular calling convention it might make sense to fix that.
> > > >
> > > > Do any warnings get generated after the revert, if you enable
> > > > CONFIG_STACK_VALIDATION=y?
> > >
> > > After the revert and enabling CONFIG_STACK_VALIDATION:
> > > arch/x86/crypto/sha1-mb/sha1_mb_mgr_flush_avx2.o: warning: objtool:
> > > sha1_mb_mgr_flush_avx2()+0x20d: call without frame pointer save/setup
> > >
> > > arch/x86/crypto/sha1-mb/sha1_mb_mgr_submit_avx2.o: warning: objtool:
> > > sha1_mb_mgr_submit_avx2()+0x115: call without frame pointer save/setup
> >
> > Megha,
> >
> > Sorry for breaking it. I completely missed the fact that the function
> > calls sha1_x8_avx2() which clobbers registers.
> >
> > If the performance penalty isn't too bad, I'll submit a patch to
> > standardize sha1_x8_avx2() to follow the C ABI.
> >
> > Do you have any tips for testing this code? I've tried using the tcrypt
> > module, but no luck.
> >
> Josh,
> Build the kernel with the following configs:
> CONFIG_CRYPTO_SHA1_MB=y
> CONFIG_CRYPTO_TEST=m
> CONFIG_CRYPTO_MANAGER_DISABLE_TESTS=n
> There was a kernel panic while booting.
> So if after applying your new patch, we are able to get complete the
> boot, then we are good.
>
> Could you please send a copy of the patch, I could test it on my end
> too.
Thanks. I was able to run the tests, though I didn't see a panic. Can
you test with this patch?
----
From: Josh Poimboeuf <jpoimboe@redhat.com>
Subject: [PATCH] crypto/sha1-mb: make sha1_x8_avx2() conform to C function ABI
Megha Day reported a kernel panic in crypto code. The problem is that
sha1_x8_avx2() clobbers registers r12-r15 without saving and restoring
them.
Before commit aec4d0e301f1 ("x86/asm/crypto: Simplify stack usage in
sha-mb functions"), those registers were saved and restored by the
callers of the function. I removed them with that commit because I
didn't realize sha1_x8_avx2() clobbered them.
Fix the potential undefined behavior associated with clobbering the
registers and make the behavior less surprising by changing the
registers to be callee saved/restored to conform with the C function
call ABI.
Also, rdx (aka RSP_SAVE) doesn't need to be saved: I verified that none
of the callers rely on it being saved, and it's not a callee-saved
register in the C ABI.
Fixes: aec4d0e301f1 ("x86/asm/crypto: Simplify stack usage in sha-mb functions")
Cc: stable@vger.kernel.org # 4.6
Reported-by: Megha Dey <megha.dey@linux.intel.com>
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
arch/x86/crypto/sha-mb/sha1_x8_avx2.S | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/arch/x86/crypto/sha-mb/sha1_x8_avx2.S b/arch/x86/crypto/sha-mb/sha1_x8_avx2.S
index 8e1b477..c9dae1c 100644
--- a/arch/x86/crypto/sha-mb/sha1_x8_avx2.S
+++ b/arch/x86/crypto/sha-mb/sha1_x8_avx2.S
@@ -296,7 +296,11 @@ W14 = TMP_
#
ENTRY(sha1_x8_avx2)
- push RSP_SAVE
+ # save callee-saved clobbered registers to comply with C function ABI
+ push %r12
+ push %r13
+ push %r14
+ push %r15
#save rsp
mov %rsp, RSP_SAVE
@@ -446,7 +450,12 @@ lloop:
## Postamble
mov RSP_SAVE, %rsp
- pop RSP_SAVE
+
+ # restore callee-saved clobbered registers
+ pop %r15
+ pop %r14
+ pop %r13
+ pop %r12
ret
ENDPROC(sha1_x8_avx2)
--
2.4.11
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox