xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
To: "pdurrant@amazon.com" <pdurrant@amazon.com>,
	"xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>
Cc: Julien Grall <julien@xen.org>,
	"op-tee@lists.trustedfirmware.org"
	<op-tee@lists.trustedfirmware.org>,
	Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>,
	Stefano Stabellini <sstabellini@kernel.org>
Subject: [PATCH v2 2/2] optee: allow plain TMEM buffers with NULL address
Date: Fri, 19 Jun 2020 22:34:01 +0000	[thread overview]
Message-ID: <20200619223332.438344-3-volodymyr_babchuk@epam.com> (raw)
In-Reply-To: <20200619223332.438344-1-volodymyr_babchuk@epam.com>

Trusted Applications use popular approach to determine required size
of buffer: client provides a memory reference with the NULL pointer to
a buffer. This is so called "Null memory reference". TA updates the
reference with the required size and returns it back to the
client. Then client allocates buffer of needed size and repeats the
operation.

This behavior is described in TEE Client API Specification, paragraph
3.2.5. Memory References.

OP-TEE represents this null memory reference as a TMEM parameter with
buf_ptr = 0x0. This is the only case when we should allow TMEM
buffer without the OPTEE_MSG_ATTR_NONCONTIG flag. This also the
special case for a buffer with OPTEE_MSG_ATTR_NONCONTIG flag.

This could lead to a potential issue, because IPA 0x0 is a valid
address, but OP-TEE will treat it as a special case. So, care should
be taken when construction OP-TEE enabled guest to make sure that such
guest have no memory at IPA 0x0 and none of its memory is mapped at PA
0x0.

Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
---

Changes from v1:
 - Added comment with TODO about possible PA/IPA 0x0 issue
 - The same is described in the commit message
 - Added check in translate_noncontig() for the NULL ptr buffer

---
 xen/arch/arm/tee/optee.c | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/xen/arch/arm/tee/optee.c b/xen/arch/arm/tee/optee.c
index 6963238056..70bfef7e5f 100644
--- a/xen/arch/arm/tee/optee.c
+++ b/xen/arch/arm/tee/optee.c
@@ -215,6 +215,15 @@ static bool optee_probe(void)
     return true;
 }
 
+/*
+ * TODO: There is a potential issue with guests that either have RAM
+ * at IPA of 0x0 or some of theirs memory is mapped at PA 0x0. This is
+ * because PA of 0x0 is considered as NULL pointer by OP-TEE. It will
+ * not be able to map buffer with such pointer to TA address space, or
+ * use such buffer for communication with the guest. We either need to
+ * check that guest have no such mappings or ensure that OP-TEE
+ * enabled guest will not be created with such mappings.
+ */
 static int optee_domain_init(struct domain *d)
 {
     struct arm_smccc_res resp;
@@ -725,6 +734,15 @@ static int translate_noncontig(struct optee_domain *ctx,
         uint64_t next_page_data;
     } *guest_data, *xen_data;
 
+    /*
+     * Special case: buffer with buf_ptr == 0x0 is considered as NULL
+     * pointer by OP-TEE. No translation is needed. This can lead to
+     * an issue as IPA 0x0 is a valid address for Xen. See the comment
+     * near optee_domain_init()
+     */
+    if ( !param->u.tmem.buf_ptr )
+        return 0;
+
     /* Offset of user buffer withing OPTEE_MSG_NONCONTIG_PAGE_SIZE-sized page */
     offset = param->u.tmem.buf_ptr & (OPTEE_MSG_NONCONTIG_PAGE_SIZE - 1);
 
@@ -865,9 +883,12 @@ static int translate_params(struct optee_domain *ctx,
             }
             else
             {
-                gdprintk(XENLOG_WARNING, "Guest tries to use old tmem arg\n");
-                ret = -EINVAL;
-                goto out;
+                if ( call->xen_arg->params[i].u.tmem.buf_ptr )
+                {
+                    gdprintk(XENLOG_WARNING, "Guest tries to use old tmem arg\n");
+                    ret = -EINVAL;
+                    goto out;
+                }
             }
             break;
         case OPTEE_MSG_ATTR_TYPE_NONE:
-- 
2.26.2


  parent reply	other threads:[~2020-06-19 22:34 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-19 22:33 [PATCH v2 0/2] optee: SHM handling fixes Volodymyr Babchuk
2020-06-19 22:33 ` [PATCH v2 1/2] optee: immediately free buffers that are released by OP-TEE Volodymyr Babchuk
2020-06-23  1:19   ` Stefano Stabellini
2020-06-19 22:34 ` Volodymyr Babchuk [this message]
2020-06-23  1:19   ` [PATCH v2 2/2] optee: allow plain TMEM buffers with NULL address Stefano Stabellini
2020-06-23  2:49     ` Volodymyr Babchuk
2020-06-23 13:31       ` Julien Grall
2020-06-23 21:09         ` Stefano Stabellini
2020-06-26 17:54           ` Julien Grall
2020-06-29  7:42             ` Paul Durrant
2020-07-01 10:03               ` Julien Grall

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200619223332.438344-3-volodymyr_babchuk@epam.com \
    --to=volodymyr_babchuk@epam.com \
    --cc=julien@xen.org \
    --cc=op-tee@lists.trustedfirmware.org \
    --cc=pdurrant@amazon.com \
    --cc=sstabellini@kernel.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).