From: Colin Ian King <colin.king(a)canonical.com>
Currently the memory allocation failure checks on drv_data and
amdtee are using IS_ERR rather than checking for a null pointer.
Fix these checks to use the conventional null pointer check.
Addresses-Coverity: ("Dereference null return")
Fixes: 757cc3e9ff1d ("tee: add AMD-TEE driver")
Signed-off-by: Colin Ian King <colin.king(a)canonical.com>
---
drivers/tee/amdtee/core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/tee/amdtee/core.c b/drivers/tee/amdtee/core.c
index 9d0cee1c837f..5fda810c79dc 100644
--- a/drivers/tee/amdtee/core.c
+++ b/drivers/tee/amdtee/core.c
@@ -444,11 +444,11 @@ static int __init amdtee_driver_init(void)
goto err_fail;
drv_data = kzalloc(sizeof(*drv_data), GFP_KERNEL);
- if (IS_ERR(drv_data))
+ if (!drv_data)
return -ENOMEM;
amdtee = kzalloc(sizeof(*amdtee), GFP_KERNEL);
- if (IS_ERR(amdtee)) {
+ if (!amdtee) {
rc = -ENOMEM;
goto err_kfree_drv_data;
}
--
2.24.0
From: Colin Ian King <colin.king(a)canonical.com>
Currently the memory allocation failure checks on drv_data and
amdtee are using IS_ERR rather than checking for a null pointer.
Fix these checks to use the conventional null pointer check.
Addresses-Coverity: ("Dereference null return")
Fixes: 757cc3e9ff1d ("tee: add AMD-TEE driver")
Signed-off-by: Colin Ian King <colin.king(a)canonical.com>
---
drivers/tee/amdtee/core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/tee/amdtee/core.c b/drivers/tee/amdtee/core.c
index 9d0cee1c837f..5fda810c79dc 100644
--- a/drivers/tee/amdtee/core.c
+++ b/drivers/tee/amdtee/core.c
@@ -444,11 +444,11 @@ static int __init amdtee_driver_init(void)
goto err_fail;
drv_data = kzalloc(sizeof(*drv_data), GFP_KERNEL);
- if (IS_ERR(drv_data))
+ if (!drv_data)
return -ENOMEM;
amdtee = kzalloc(sizeof(*amdtee), GFP_KERNEL);
- if (IS_ERR(amdtee)) {
+ if (!amdtee) {
rc = -ENOMEM;
goto err_kfree_drv_data;
}
--
2.24.0
This patch series addresses the bug report submitted by Dan Carpenter.
Link: https://lists.linaro.org/pipermail/tee-dev/2020-January/001417.html
Since, these patches are based on cryptodev-2.6 tree, I have included
linux-crypto list as well.
This patch series does not fix the static checker warning reported due
to incorrect use of IS_ERR. Colin Ian King has submitted a fix for this
issue. Link: https://lkml.org/lkml/2020/1/8/88
Rijo Thomas (5):
tee: amdtee: remove unused variable initialization
tee: amdtee: print error message if tee not present
tee: amdtee: skip tee_device_unregister if tee_device_alloc fails
tee: amdtee: rename err label to err_device_unregister
tee: amdtee: remove redundant NULL check for pool
drivers/tee/amdtee/call.c | 14 +++++++-------
drivers/tee/amdtee/core.c | 32 +++++++++++++++++---------------
2 files changed, 24 insertions(+), 22 deletions(-)
--
2.17.1
Hello Rijo Thomas,
The patch 757cc3e9ff1d: "tee: add AMD-TEE driver" from Dec 27, 2019,
leads to the following static checker warning:
drivers/tee/amdtee/core.c:447 amdtee_driver_init()
warn: 'drv_data' isn't an ERR_PTR
drivers/tee/amdtee/core.c
435 static int __init amdtee_driver_init(void)
436 {
437 struct amdtee *amdtee = NULL;
^^^^^^^^^^^^^
438 struct tee_device *teedev;
439 struct tee_shm_pool *pool = ERR_PTR(-EINVAL);
^^^^^^^^^^^^^^^^^^^^^^^^
These look like they are left overs from one error label style error
handling. We're trying to enable the warning about unused assignments
so it would be good to get rid of these.
440 int rc;
441
442 rc = psp_check_tee_status();
443 if (rc)
444 goto err_fail;
^^^^^^^^^^^^^
Ideally psp_check_tee_status() would print its own error warning and we
could return directly here.
445
446 drv_data = kzalloc(sizeof(*drv_data), GFP_KERNEL);
447 if (IS_ERR(drv_data))
^^^^^^^^^^^^^^^^
This should check for NULL instead of error pointer.
448 return -ENOMEM;
449
450 amdtee = kzalloc(sizeof(*amdtee), GFP_KERNEL);
451 if (IS_ERR(amdtee)) {
^^^^^^^^^^^^^^
Same.
452 rc = -ENOMEM;
453 goto err_kfree_drv_data;
454 }
455
456 pool = amdtee_config_shm();
457 if (IS_ERR(pool)) {
458 pr_err("shared pool configuration error\n");
459 rc = PTR_ERR(pool);
460 goto err_kfree_amdtee;
461 }
462
463 teedev = tee_device_alloc(&amdtee_desc, NULL, pool, amdtee);
464 if (IS_ERR(teedev)) {
465 rc = PTR_ERR(teedev);
466 goto err;
Better to create a "err_free_pool" label. The tee_device_unregister()
is a no-op on this path so that's fine, but it's easier to read if we
have the err_free_pool label.
467 }
468 amdtee->teedev = teedev;
469
470 rc = tee_device_register(amdtee->teedev);
471 if (rc)
472 goto err;
"goto err;" is very vague. We are unregistering a device which was
never registered which feels wrong but actually works fine...
473
474 amdtee->pool = pool;
475
476 drv_data->amdtee = amdtee;
477
478 pr_info("amd-tee driver initialization successful\n");
479 return 0;
480
481 err:
482 tee_device_unregister(amdtee->teedev);
483 if (pool)
^^^^^^^^^
Always true.
484 tee_shm_pool_free(pool);
485
486 err_kfree_amdtee:
487 kfree(amdtee);
488
489 err_kfree_drv_data:
490 kfree(drv_data);
491 drv_data = NULL;
492
493 err_fail:
494 pr_err("amd-tee driver initialization failed\n");
495 return rc;
496 }
regards,
dan carpenter
This patch series introduces Trusted Execution Environment (TEE) driver
for AMD APU enabled systems. The TEE is a secure area of a processor which
ensures that sensitive data is stored, processed and protected in an
isolated and trusted environment. The AMD Secure Processor is a dedicated
processor which provides TEE to enable HW platform security. It offers
protection against software attacks generated in Rich Operating
System (Rich OS) such as Linux running on x86. The AMD-TEE Trusted OS
running on AMD Secure Processor allows loading and execution of security
sensitive applications called Trusted Applications (TAs). An example of
a TA would be a DRM (Digital Rights Management) TA written to enforce
content protection.
Linux already provides a tee subsystem, which is described in [1]. The tee
subsystem provides a generic TEE ioctl interface which can be used by user
space to talk to a TEE driver. AMD-TEE driver registers with tee subsystem
and implements tee function callbacks in an AMD platform specific manner.
The following TEE commands are recognized by AMD-TEE Trusted OS:
1. TEE_CMD_ID_LOAD_TA : Load Trusted Application (TA) binary into TEE
environment
2. TEE_CMD_ID_UNLOAD_TA : Unload TA binary from TEE environment
3. TEE_CMD_ID_OPEN_SESSION : Open session with loaded TA
4. TEE_CMD_ID_CLOSE_SESSION : Close session with loaded TA
5. TEE_CMD_ID_INVOKE_CMD : Invoke a command with loaded TA
6. TEE_CMD_ID_MAP_SHARED_MEM : Map shared memory
7. TEE_CMD_ID_UNMAP_SHARED_MEM : Unmap shared memory
Each command has its own payload format. The AMD-TEE driver creates a
command buffer payload for submission to AMD-TEE Trusted OS. The driver
uses the services of AMD Secure Processor driver to submit commands
to the Trusted OS. Further details can be found in [1].
This patch series is based on cryptodev-2.6 tree with base commit
c6d633a92749 (crypto: algapi - make unregistration functions return void).
[1] https://www.kernel.org/doc/Documentation/tee.txt
Rijo Thomas (4):
tee: allow compilation of tee subsystem for AMD CPUs
tee: add AMD-TEE driver
tee: amdtee: check TEE status during driver initialization
Documentation: tee: add AMD-TEE driver details
Documentation/tee.txt | 81 ++++++
drivers/crypto/ccp/tee-dev.c | 11 +
drivers/tee/Kconfig | 4 +-
drivers/tee/Makefile | 1 +
drivers/tee/amdtee/Kconfig | 8 +
drivers/tee/amdtee/Makefile | 5 +
drivers/tee/amdtee/amdtee_if.h | 183 +++++++++++++
drivers/tee/amdtee/amdtee_private.h | 159 +++++++++++
drivers/tee/amdtee/call.c | 373 ++++++++++++++++++++++++++
drivers/tee/amdtee/core.c | 516 ++++++++++++++++++++++++++++++++++++
drivers/tee/amdtee/shm_pool.c | 93 +++++++
include/linux/psp-tee.h | 18 ++
include/uapi/linux/tee.h | 1 +
13 files changed, 1451 insertions(+), 2 deletions(-)
create mode 100644 drivers/tee/amdtee/Kconfig
create mode 100644 drivers/tee/amdtee/Makefile
create mode 100644 drivers/tee/amdtee/amdtee_if.h
create mode 100644 drivers/tee/amdtee/amdtee_private.h
create mode 100644 drivers/tee/amdtee/call.c
create mode 100644 drivers/tee/amdtee/core.c
create mode 100644 drivers/tee/amdtee/shm_pool.c
--
1.9.1
Hello arm-soc maintainers,
Please pull this OP-TEE driver fix for kernel allocated dynamic shared
memory.
Thanks,
Jens
The following changes since commit d1eef1c619749b2a57e514a3fa67d9a516ffa919:
Linux 5.5-rc2 (2019-12-15 15:16:08 -0800)
are available in the Git repository at:
git://git.linaro.org:/people/jens.wiklander/linux-tee.git tags/tee-optee-fix-for-5.5
for you to fetch changes up to 5a769f6ff439cedc547395a6dc78faa26108f741:
optee: Fix multi page dynamic shm pool alloc (2020-01-03 11:21:12 +0100)
----------------------------------------------------------------
Fix OP-TEE multi page dynamic shm pool alloc
----------------------------------------------------------------
Sumit Garg (1):
optee: Fix multi page dynamic shm pool alloc
drivers/tee/optee/shm_pool.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
Hello arm-soc maintainers,
Please pull this OP-TEE driver update where the driver is modeled as a
platform driver instead.
Thanks,
Jens
The following changes since commit d1eef1c619749b2a57e514a3fa67d9a516ffa919:
Linux 5.5-rc2 (2019-12-15 15:16:08 -0800)
are available in the Git repository at:
git://git.linaro.org:/people/jens.wiklander/linux-tee.git tags/tee-optee-pldrv-for-5.6
for you to fetch changes up to f349710e413ad29132373e170c87dd35f2b62069:
optee: model OP-TEE as a platform device/driver (2020-01-03 09:26:40 +0100)
----------------------------------------------------------------
Model OP-TEE as a platform device/driver
----------------------------------------------------------------
Ard Biesheuvel (1):
optee: model OP-TEE as a platform device/driver
drivers/tee/optee/core.c | 153 ++++++++++++++++++++---------------------------
1 file changed, 64 insertions(+), 89 deletions(-)
Hi,
I've been hunting down some hackbench regression between 5.4-rc8 and 5.5-rc1
on my Juno r0, one of the offenders seems to be:
246880958ac9 ("firmware: broadcom: add OP-TEE based BNXT f/w manager")
This is tested on a kernel built with defconfig (TEE_BNXT_FW gets selected)
and with:
echo performance > /sys/devices/system/cpu/cpu0/cpufreq/scaling_governor
echo performance > /sys/devices/system/cpu/cpu1/cpufreq/scaling_governor
./perf stat --null --sync --repeat 200 ./hackbench
The regression is easily reproducible on my end, this is 3 runs of the above
comparing the patch and its parent:
-PATCH:
0.71062 +- 0.00150 seconds time elapsed ( +- 0.21% )
0.71121 +- 0.00181 seconds time elapsed ( +- 0.25% )
0.71277 +- 0.00181 seconds time elapsed ( +- 0.25% )
+PATCH:
0.72556 +- 0.00174 seconds time elapsed ( +- 0.24% )
0.72695 +- 0.00192 seconds time elapsed ( +- 0.26% )
0.72559 +- 0.00178 seconds time elapsed ( +- 0.25% )
AIUI Vincent found something different while hunting down a similar
regression:
df323337e507 ("apparmor: Use a memory pool instead per-CPU caches")
but it seems this one is another cause. Seeing as this involves security
stuff the overhead may be acceptable, nevertheless now that I have some
reproducer I figured I'd send this out.
Cheers,
Valentin