| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAJwFiG+GKKa-vYwWzY8Dyw7GsaTq90kAyHWz1NJ28Bni43-nTQ@mail.gmail.com>
Date: Fri, 9 Jan 2026 23:07:40 +0800
From: Aristo Chen <jj251510319013@...il.com>
To: Sumit Garg <sumit.garg@...nel.org>
Cc: linux-kernel@...r.kernel.org, jens.wiklander@...aro.org,
op-tee@...ts.trustedfirmware.org, harshal.dev@....qualcomm.com,
mario.limonciello@....com, Rijo-john.Thomas@....com,
amirreza.zarrabi@....qualcomm.com, Aristo Chen <aristo.chen@...onical.com>
Subject: Re: [PATCH v6 2/2] tee: optee: store OS revision for TEE core
Hi Sumit
Sumit Garg <sumit.garg@...nel.org> 於 2026年1月9日週五 下午7:50寫道：
>
> On Thu, Jan 08, 2026 at 02:45:09PM +0800, Aristo Chen wrote:
> > Collect OP-TEE OS revision from secure world for both SMC and FF-A
> > ABIs, store it in the OP-TEE driver, and expose it through the
> > generic get_tee_revision() callback.
> >
> > Signed-off-by: Aristo Chen <aristo.chen@...onical.com>
> > ---
> > drivers/tee/optee/core.c | 23 +++++++++++++
> > drivers/tee/optee/ffa_abi.c | 57 ++++++++++++++++++++++++-------
> > drivers/tee/optee/optee_private.h | 19 +++++++++++
> > drivers/tee/optee/smc_abi.c | 15 ++++++--
> > 4 files changed, 99 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c
> > index 5b62139714ce..2d807bc748bc 100644
> > --- a/drivers/tee/optee/core.c
> > +++ b/drivers/tee/optee/core.c
> > @@ -63,6 +63,29 @@ int optee_set_dma_mask(struct optee *optee, u_int pa_width)
> > return dma_coerce_mask_and_coherent(&optee->teedev->dev, mask);
> > }
> >
> > +int optee_get_revision(struct tee_device *teedev, char *buf, size_t len)
> > +{
> > + struct optee *optee = tee_get_drvdata(teedev);
> > + u64 build_id;
> > +
> > + if (!optee)
> > + return -ENODEV;
> > + if (!buf || !len)
> > + return -EINVAL;
> > +
> > + build_id = optee->revision.os_build_id;
> > + if (build_id)
> > + scnprintf(buf, len, "%u.%u (%016llx)",
> > + optee->revision.os_major,
> > + optee->revision.os_minor,
> > + (unsigned long long)build_id);
> > + else
> > + scnprintf(buf, len, "%u.%u", optee->revision.os_major,
> > + optee->revision.os_minor);
> > +
> > + return 0;
> > +}
> > +
> > static void optee_bus_scan(struct work_struct *work)
> > {
> > WARN_ON(optee_enumerate_devices(PTA_CMD_GET_DEVICES_SUPP));
> > diff --git a/drivers/tee/optee/ffa_abi.c b/drivers/tee/optee/ffa_abi.c
> > index bf8390789ecf..82dbed1c87e5 100644
> > --- a/drivers/tee/optee/ffa_abi.c
> > +++ b/drivers/tee/optee/ffa_abi.c
> > @@ -775,6 +775,42 @@ static int optee_ffa_reclaim_protmem(struct optee *optee,
> > * with a matching configuration.
> > */
> >
> > +static bool optee_ffa_get_os_revision(struct ffa_device *ffa_dev,
> > + const struct ffa_ops *ops,
> > + struct optee_revision *revision,
> > + bool log)
> > +{
> > + const struct ffa_msg_ops *msg_ops = ops->msg_ops;
> > + struct ffa_send_direct_data data = {
> > + .data0 = OPTEE_FFA_GET_OS_VERSION,
> > + };
> > + int rc;
> > +
> > + msg_ops->mode_32bit_set(ffa_dev);
> > +
> > + rc = msg_ops->sync_send_receive(ffa_dev, &data);
> > + if (rc) {
> > + pr_err("Unexpected error %d\n", rc);
> > + return false;
> > + }
> > +
> > + if (revision) {
> > + revision->os_major = data.data0;
> > + revision->os_minor = data.data1;
> > + revision->os_build_id = data.data2;
> > + }
> > +
> > + if (log) {
> > + if (data.data2)
> > + pr_info("revision %lu.%lu (%08lx)",
> > + data.data0, data.data1, data.data2);
> > + else
> > + pr_info("revision %lu.%lu", data.data0, data.data1);
> > + }
> > +
> > + return true;
> > +}
> > +
> > static bool optee_ffa_api_is_compatible(struct ffa_device *ffa_dev,
> > const struct ffa_ops *ops)
> > {
> > @@ -798,19 +834,8 @@ static bool optee_ffa_api_is_compatible(struct ffa_device *ffa_dev,
> > return false;
> > }
> >
> > - data = (struct ffa_send_direct_data){
> > - .data0 = OPTEE_FFA_GET_OS_VERSION,
> > - };
> > - rc = msg_ops->sync_send_receive(ffa_dev, &data);
> > - if (rc) {
> > - pr_err("Unexpected error %d\n", rc);
> > + if (!optee_ffa_get_os_revision(ffa_dev, ops, NULL, true))
>
> Why do you need to invoke optee_ffa_get_os_revision() here?
I’m calling optee_ffa_get_os_revision() here to avoid duplicating the
GET_OS_VERSION call/printing logic. The original code in
optee_ffa_api_is_compatible() already did the OS version query and
printed it; after factoring it out, the helper keeps that behavior in one
place. If we don’t call it there, we’d either lose the existing dmesg log
or re‑implement the same OS‑version query/print block again, which I’m
trying to avoid.
If you’d rather avoid the extra call at this stage, maybe we can always
print the log in the optee_ffa_get_os_revision() and remove the call
from optee_ffa_api_is_compatible() entirely.
>
> -Sumit
>
> > return false;
> > - }
> > - if (data.data2)
> > - pr_info("revision %lu.%lu (%08lx)",
> > - data.data0, data.data1, data.data2);
> > - else
> > - pr_info("revision %lu.%lu", data.data0, data.data1);
> >
> > return true;
> > }
> > @@ -900,6 +925,7 @@ static int optee_ffa_open(struct tee_context *ctx)
> >
> > static const struct tee_driver_ops optee_ffa_clnt_ops = {
> > .get_version = optee_ffa_get_version,
> > + .get_tee_revision = optee_get_revision,
> > .open = optee_ffa_open,
> > .release = optee_release,
> > .open_session = optee_open_session,
> > @@ -918,6 +944,7 @@ static const struct tee_desc optee_ffa_clnt_desc = {
> >
> > static const struct tee_driver_ops optee_ffa_supp_ops = {
> > .get_version = optee_ffa_get_version,
> > + .get_tee_revision = optee_get_revision,
> > .open = optee_ffa_open,
> > .release = optee_release_supp,
> > .supp_recv = optee_supp_recv,
> > @@ -1060,6 +1087,12 @@ static int optee_ffa_probe(struct ffa_device *ffa_dev)
> > if (!optee)
> > return -ENOMEM;
> >
> > + if (!optee_ffa_get_os_revision(ffa_dev, ffa_ops, &optee->revision,
> > + false)) {
> > + rc = -EINVAL;
> > + goto err_free_optee;
> > + }
> > +
> > pool = optee_ffa_shm_pool_alloc_pages();
> > if (IS_ERR(pool)) {
> > rc = PTR_ERR(pool);
> > diff --git a/drivers/tee/optee/optee_private.h b/drivers/tee/optee/optee_private.h
> > index db9ea673fbca..acd3051c4879 100644
> > --- a/drivers/tee/optee/optee_private.h
> > +++ b/drivers/tee/optee/optee_private.h
> > @@ -171,6 +171,24 @@ struct optee_ffa {
> >
> > struct optee;
> >
> > +/**
> > + * struct optee_revision - OP-TEE OS revision reported by secure world
> > + * @os_major: OP-TEE OS major version
> > + * @os_minor: OP-TEE OS minor version
> > + * @os_build_id: OP-TEE OS build identifier (0 if unspecified)
> > + *
> > + * Values come from OPTEE_SMC_CALL_GET_OS_REVISION (SMC ABI) or
> > + * OPTEE_FFA_GET_OS_VERSION (FF-A ABI); this is the trusted OS revision, not an
> > + * FF-A ABI version.
> > + */
> > +struct optee_revision {
> > + u32 os_major;
> > + u32 os_minor;
> > + u64 os_build_id;
> > +};
> > +
> > +int optee_get_revision(struct tee_device *teedev, char *buf, size_t len);
> > +
> > /**
> > * struct optee_ops - OP-TEE driver internal operations
> > * @do_call_with_arg: enters OP-TEE in secure world
> > @@ -249,6 +267,7 @@ struct optee {
> > bool in_kernel_rpmb_routing;
> > struct work_struct scan_bus_work;
> > struct work_struct rpmb_scan_bus_work;
> > + struct optee_revision revision;
> > };
> >
> > struct optee_session {
> > diff --git a/drivers/tee/optee/smc_abi.c b/drivers/tee/optee/smc_abi.c
> > index 0be663fcd52b..51fae1ab8ef8 100644
> > --- a/drivers/tee/optee/smc_abi.c
> > +++ b/drivers/tee/optee/smc_abi.c
> > @@ -1242,6 +1242,7 @@ static int optee_smc_open(struct tee_context *ctx)
> >
> > static const struct tee_driver_ops optee_clnt_ops = {
> > .get_version = optee_get_version,
> > + .get_tee_revision = optee_get_revision,
> > .open = optee_smc_open,
> > .release = optee_release,
> > .open_session = optee_open_session,
> > @@ -1261,6 +1262,7 @@ static const struct tee_desc optee_clnt_desc = {
> >
> > static const struct tee_driver_ops optee_supp_ops = {
> > .get_version = optee_get_version,
> > + .get_tee_revision = optee_get_revision,
> > .open = optee_smc_open,
> > .release = optee_release_supp,
> > .supp_recv = optee_supp_recv,
> > @@ -1323,7 +1325,8 @@ static bool optee_msg_api_uid_is_optee_image_load(optee_invoke_fn *invoke_fn)
> > }
> > #endif
> >
> > -static void optee_msg_get_os_revision(optee_invoke_fn *invoke_fn)
> > +static void optee_msg_get_os_revision(optee_invoke_fn *invoke_fn,
> > + struct optee_revision *revision)
> > {
> > union {
> > struct arm_smccc_res smccc;
> > @@ -1337,6 +1340,12 @@ static void optee_msg_get_os_revision(optee_invoke_fn *invoke_fn)
> > invoke_fn(OPTEE_SMC_CALL_GET_OS_REVISION, 0, 0, 0, 0, 0, 0, 0,
> > &res.smccc);
> >
> > + if (revision) {
> > + revision->os_major = res.result.major;
> > + revision->os_minor = res.result.minor;
> > + revision->os_build_id = res.result.build_id;
> > + }
> > +
> > if (res.result.build_id)
> > pr_info("revision %lu.%lu (%0*lx)", res.result.major,
> > res.result.minor, (int)sizeof(res.result.build_id) * 2,
> > @@ -1745,8 +1754,6 @@ static int optee_probe(struct platform_device *pdev)
> > return -EINVAL;
> > }
> >
> > - optee_msg_get_os_revision(invoke_fn);
> > -
> > if (!optee_msg_api_revision_is_compatible(invoke_fn)) {
> > pr_warn("api revision mismatch\n");
> > return -EINVAL;
> > @@ -1815,6 +1822,8 @@ static int optee_probe(struct platform_device *pdev)
> > goto err_free_shm_pool;
> > }
> >
> > + optee_msg_get_os_revision(invoke_fn, &optee->revision);
> > +
> > optee->ops = &optee_ops;
> > optee->smc.invoke_fn = invoke_fn;
> > optee->smc.sec_caps = sec_caps;
> > --
> > 2.43.0
> >
Best regards,
Aristo
Powered by blists - more mailing lists