diff mbox series

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

Message ID 20200429144205.5291-4-t-kristo@ti.com (mailing list archive)
State New, archived
Headers show
Series [1/6] crypto: omap-aes: avoid spamming console with self tests | expand

Commit Message

Tero Kristo April 29, 2020, 2:42 p.m. UTC
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 <t-kristo@ti.com>
---
 drivers/crypto/omap-crypto.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

Comments

Herbert Xu May 8, 2020, 5:08 a.m. UTC | #1
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,
Tero Kristo May 8, 2020, 5:47 a.m. UTC | #2
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
diff mbox series

Patch

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;