* [PATCH] tee: check shm references are consistent in offset/size
@ 2018-04-29 12:22 Etienne Carriere
2018-05-07 8:24 ` Jens Wiklander
0 siblings, 1 reply; 4+ messages in thread
From: Etienne Carriere @ 2018-04-29 12:22 UTC (permalink / raw)
To: alexandre.jutras, linux-kernel, jens.wiklander; +Cc: Etienne Carriere
This change prevents userland from referencing TEE shared memory
outside the area initially allocated by its owner. Prior this change an
application could not reference or access memory it did not own but
it could reference memory not explicitly allocated by owner but still
allocated to the owner due to the memory allocation granule.
Reported-by: Alexandre Jutras <alexandre.jutras@nxp.com>
Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
---
drivers/tee/tee_core.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c
index 0124a91..dd46b75 100644
--- a/drivers/tee/tee_core.c
+++ b/drivers/tee/tee_core.c
@@ -238,6 +238,17 @@ static int params_from_user(struct tee_context *ctx, struct tee_param *params,
if (IS_ERR(shm))
return PTR_ERR(shm);
+ /*
+ * Ensure offset + size does not overflow offset
+ * and does not overflow the size of the referred
+ * shared memory object.
+ */
+ if ((ip.a + ip.b) < ip.a ||
+ (ip.a + ip.b) > shm->size) {
+ tee_shm_put(shm);
+ return -EINVAL;
+ }
+
params[n].u.memref.shm_offs = ip.a;
params[n].u.memref.size = ip.b;
params[n].u.memref.shm = shm;
--
1.9.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] tee: check shm references are consistent in offset/size
2018-04-29 12:22 [PATCH] tee: check shm references are consistent in offset/size Etienne Carriere
@ 2018-05-07 8:24 ` Jens Wiklander
0 siblings, 0 replies; 4+ messages in thread
From: Jens Wiklander @ 2018-05-07 8:24 UTC (permalink / raw)
To: Etienne Carriere; +Cc: alexandre.jutras, Linux Kernel Mailing List
On Sun, Apr 29, 2018 at 2:22 PM, Etienne Carriere
<etienne.carriere@linaro.org> wrote:
> This change prevents userland from referencing TEE shared memory
> outside the area initially allocated by its owner. Prior this change an
> application could not reference or access memory it did not own but
> it could reference memory not explicitly allocated by owner but still
> allocated to the owner due to the memory allocation granule.
>
> Reported-by: Alexandre Jutras <alexandre.jutras@nxp.com>
> Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
> ---
> drivers/tee/tee_core.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c
> index 0124a91..dd46b75 100644
> --- a/drivers/tee/tee_core.c
> +++ b/drivers/tee/tee_core.c
> @@ -238,6 +238,17 @@ static int params_from_user(struct tee_context *ctx, struct tee_param *params,
> if (IS_ERR(shm))
> return PTR_ERR(shm);
>
> + /*
> + * Ensure offset + size does not overflow offset
> + * and does not overflow the size of the referred
> + * shared memory object.
> + */
> + if ((ip.a + ip.b) < ip.a ||
> + (ip.a + ip.b) > shm->size) {
> + tee_shm_put(shm);
> + return -EINVAL;
> + }
> +
> params[n].u.memref.shm_offs = ip.a;
> params[n].u.memref.size = ip.b;
> params[n].u.memref.shm = shm;
> --
> 1.9.1
>
Looks good to me, I'll pick this up.
Thanks,
Jens
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] tee: check shm references are consistent in offset/size
@ 2018-04-27 13:53 Etienne Carriere
2018-04-27 15:58 ` Randy Dunlap
0 siblings, 1 reply; 4+ messages in thread
From: Etienne Carriere @ 2018-04-27 13:53 UTC (permalink / raw)
To: linux-kernel, Jens Wiklander, Alexandre Jutras
This change prevents userland from referencing TEE shared memory
outside the area initially allocated by its owner. Prior this change an
application could not reference or access memory it did not own but
it could reference memory not explicitly allocated by owner.
Reported-by: Alexandre Jutras <alexandre.jutras@nxp.com>
Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
---
drivers/tee/tee_core.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c
index 0124a91..dd46b75 100644
--- a/drivers/tee/tee_core.c
+++ b/drivers/tee/tee_core.c
@@ -238,6 +238,17 @@ static int params_from_user(struct tee_context
*ctx, struct tee_param *params,
if (IS_ERR(shm))
return PTR_ERR(shm);
+ /*
+ * Ensure offset + size does not overflow offset
+ * and does not overflow the size of the referred
+ * shared memory object.
+ */
+ if ((ip.a + ip.b) < ip.a ||
+ (ip.a + ip.b) > shm->size) {
+ tee_shm_put(shm);
+ return -EINVAL;
+ }
+
params[n].u.memref.shm_offs = ip.a;
params[n].u.memref.size = ip.b;
params[n].u.memref.shm = shm;
--
1.9.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] tee: check shm references are consistent in offset/size
2018-04-27 13:53 Etienne Carriere
@ 2018-04-27 15:58 ` Randy Dunlap
0 siblings, 0 replies; 4+ messages in thread
From: Randy Dunlap @ 2018-04-27 15:58 UTC (permalink / raw)
To: Etienne Carriere, linux-kernel, Jens Wiklander, Alexandre Jutras
On 04/27/2018 06:53 AM, Etienne Carriere wrote:
> This change prevents userland from referencing TEE shared memory
> outside the area initially allocated by its owner. Prior this change an
> application could not reference or access memory it did not own but
> it could reference memory not explicitly allocated by owner.
>
> Reported-by: Alexandre Jutras <alexandre.jutras@nxp.com>
> Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
>
> ---
> drivers/tee/tee_core.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c
> index 0124a91..dd46b75 100644
> --- a/drivers/tee/tee_core.c
> +++ b/drivers/tee/tee_core.c
> @@ -238,6 +238,17 @@ static int params_from_user(struct tee_context
> *ctx, struct tee_param *params,
Those 2 lines above should be only one line.
> if (IS_ERR(shm))
> return PTR_ERR(shm);
>
> + /*
> + * Ensure offset + size does not overflow offset
> + * and does not overflow the size of the referred
> + * shared memory object.
> + */
> + if ((ip.a + ip.b) < ip.a ||
> + (ip.a + ip.b) > shm->size) {
> + tee_shm_put(shm);
> + return -EINVAL;
> + }
> +
Hi,
The patch makes sense but the whitespace and indentation are all mucked up.
Could be your email client.
Please check/fix and test by emailing it to yourself.
> params[n].u.memref.shm_offs = ip.a;
> params[n].u.memref.size = ip.b;
> params[n].u.memref.shm = shm;
>
thanks,
--
~Randy
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-05-07 8:24 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-29 12:22 [PATCH] tee: check shm references are consistent in offset/size Etienne Carriere
2018-05-07 8:24 ` Jens Wiklander
-- strict thread matches above, loose matches on Subject: below --
2018-04-27 13:53 Etienne Carriere
2018-04-27 15:58 ` Randy Dunlap
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.