2020-04-29 14:43:04

by Tero Kristo

Subject: [PATCH 3/6] crypto: omap-crypto: fix userspace copied buffer access

In case buffers are copied from userspace, directly accessing the page
will most likely fail because it hasn't been mapped into the kernel
memory space. Fix the issue by forcing a kmap / kunmap within the
cleanup functionality.

Signed-off-by: Tero Kristo <[email protected]>
---
drivers/crypto/omap-crypto.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/crypto/omap-crypto.c b/drivers/crypto/omap-crypto.c
index cc88b7362bc2..cbc5a4151c3c 100644
--- a/drivers/crypto/omap-crypto.c
+++ b/drivers/crypto/omap-crypto.c
@@ -178,11 +178,14 @@ static void omap_crypto_copy_data(struct scatterlist *src,
amt = min(src->length - srco, dst->length - dsto);
amt = min(len, amt);

- srcb = sg_virt(src) + srco;
- dstb = sg_virt(dst) + dsto;
+ srcb = kmap_atomic(sg_page(src)) + srco + src->offset;
+ dstb = kmap_atomic(sg_page(dst)) + dsto + dst->offset;

memcpy(dstb, srcb, amt);

+ kunmap_atomic(srcb);
+ kunmap_atomic(dstb);
+
srco += amt;
dsto += amt;
len -= amt;
--
2.17.1

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

2020-05-08 05:10:01

by Herbert Xu

Subject: Re: [PATCH 3/6] crypto: omap-crypto: fix userspace copied buffer access

On Wed, Apr 29, 2020 at 05:42:02PM +0300, Tero Kristo wrote:
>
> diff --git a/drivers/crypto/omap-crypto.c b/drivers/crypto/omap-crypto.c
> index cc88b7362bc2..cbc5a4151c3c 100644
> --- a/drivers/crypto/omap-crypto.c
> +++ b/drivers/crypto/omap-crypto.c
> @@ -178,11 +178,14 @@ static void omap_crypto_copy_data(struct scatterlist *src,
> amt = min(src->length - srco, dst->length - dsto);
> amt = min(len, amt);
>
> - srcb = sg_virt(src) + srco;
> - dstb = sg_virt(dst) + dsto;
> + srcb = kmap_atomic(sg_page(src)) + srco + src->offset;
> + dstb = kmap_atomic(sg_page(dst)) + dsto + dst->offset;
>
> memcpy(dstb, srcb, amt);
>
> + kunmap_atomic(srcb);
> + kunmap_atomic(dstb);

With dst you also need to flush the cache. Please refer to the
flush dcache call in include/crypto/scatterwalk.h.

Thanks,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2020-05-08 05:50:28

by Tero Kristo

Subject: Re: [PATCH 3/6] crypto: omap-crypto: fix userspace copied buffer access

On 08/05/2020 08:08, Herbert Xu wrote:
> On Wed, Apr 29, 2020 at 05:42:02PM +0300, Tero Kristo wrote:
>>
>> diff --git a/drivers/crypto/omap-crypto.c b/drivers/crypto/omap-crypto.c
>> index cc88b7362bc2..cbc5a4151c3c 100644
>> --- a/drivers/crypto/omap-crypto.c
>> +++ b/drivers/crypto/omap-crypto.c
>> @@ -178,11 +178,14 @@ static void omap_crypto_copy_data(struct scatterlist *src,
>> amt = min(src->length - srco, dst->length - dsto);
>> amt = min(len, amt);
>>
>> - srcb = sg_virt(src) + srco;
>> - dstb = sg_virt(dst) + dsto;
>> + srcb = kmap_atomic(sg_page(src)) + srco + src->offset;
>> + dstb = kmap_atomic(sg_page(dst)) + dsto + dst->offset;
>>
>> memcpy(dstb, srcb, amt);
>>
>> + kunmap_atomic(srcb);
>> + kunmap_atomic(dstb);
>
> With dst you also need to flush the cache. Please refer to the
> flush dcache call in include/crypto/scatterwalk.h.

Ok, let me try that out, flushing a single page should be fine (meaning
not catastrophic to performance.)

-Tero
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki