| 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: <b429613e-f543-41f4-bb7f-bbd22de7f0f0@kernel.org>
Date: Wed, 7 Jan 2026 21:01:39 -0600
From: "Mario Limonciello (AMD) (kernel.org)" <superm1@...nel.org>
To: Aristo Chen <jj251510319013@...il.com>
Cc: linux-kernel@...r.kernel.org, jens.wiklander@...aro.org,
sumit.garg@...nel.org, op-tee@...ts.trustedfirmware.org,
harshal.dev@....qualcomm.com, Rijo-john.Thomas@....com,
amirreza.zarrabi@....qualcomm.com, Aristo Chen <aristo.chen@...onical.com>
Subject: Re: [PATCH v5 1/2] tee: add revision sysfs attribute
On 1/7/2026 8:55 PM, Aristo Chen wrote:
> Hi Mario,
>
> Mario Limonciello <superm1@...nel.org> 於 2026年1月7日週三 下午11:28寫道：
>>
>> On 1/7/26 9:26 AM, Aristo Chen wrote:
>>> Add a generic TEE revision sysfs attribute backed by a new
>>> optional get_tee_revision() callback. The revision string is
>>> diagnostic-only and must not be used to infer feature support.
>>>
>>> Signed-off-by: Aristo Chen <aristo.chen@...onical.com>
>>> ---
>>> Documentation/ABI/testing/sysfs-class-tee | 10 +++++
>>> drivers/tee/tee_core.c | 51 ++++++++++++++++++++++-
>>> include/linux/tee_core.h | 9 ++++
>>> 3 files changed, 69 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/ABI/testing/sysfs-class-tee b/Documentation/ABI/testing/sysfs-class-tee
>>> index c9144d16003e..6e783210104e 100644
>>> --- a/Documentation/ABI/testing/sysfs-class-tee
>>> +++ b/Documentation/ABI/testing/sysfs-class-tee
>>> @@ -13,3 +13,13 @@ Description:
>>> space if the variable is absent. The primary purpose
>>> of this variable is to let systemd know whether
>>> tee-supplicant is needed in the early boot with initramfs.
>>> +
>>> +What: /sys/class/tee/tee{,priv}X/revision
>>> +Date: Dec 2025
>>> +KernelVersion: 6.18
>>
>> This needs to be bumped up and dates pushed out.
>
> I will fix this in the v6 patch, thanks!
>
>>
>>> +Contact: op-tee@...ts.trustedfirmware.org
>>> +Description:
>>> + Read-only revision string reported by the TEE driver. This is
>>> + for diagnostics only and must not be used to infer feature
>>> + support. Use TEE_IOC_VERSION for capability and compatibility
>>> + checks.
>>> diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c
>>> index d65d47cc154e..0a00499811c1 100644
>>> --- a/drivers/tee/tee_core.c
>>> +++ b/drivers/tee/tee_core.c
>>> @@ -1146,7 +1146,56 @@ static struct attribute *tee_dev_attrs[] = {
>>> NULL
>>> };
>>>
>>> -ATTRIBUTE_GROUPS(tee_dev);
>>> +static const struct attribute_group tee_dev_group = {
>>> + .attrs = tee_dev_attrs,
>>> +};
>>> +
>>> +static ssize_t revision_show(struct device *dev,
>>> + struct device_attribute *attr, char *buf)
>>> +{
>>> + struct tee_device *teedev = container_of(dev, struct tee_device, dev);
>>> + char version[TEE_REVISION_STR_SIZE];
>>> + int ret;
>>> +
>>> + if (!teedev->desc->ops->get_tee_revision)
>>> + return -ENODEV;
>>> +
>>> + ret = teedev->desc->ops->get_tee_revision(teedev, version,
>>> + sizeof(version));
>>> + if (ret)
>>> + return ret;
>>> +
>>> + return sysfs_emit(buf, "%s\n", version);
>>> +}
>>> +static DEVICE_ATTR_RO(revision);
>>> +
>>> +static struct attribute *tee_revision_attrs[] = {
>>> + &dev_attr_revision.attr,
>>> + NULL
>>> +};
>>> +
>>> +static umode_t tee_revision_attr_is_visible(struct kobject *kobj,
>>> + struct attribute *attr, int n)
>>> +{
>>> + struct device *dev = kobj_to_dev(kobj);
>>> + struct tee_device *teedev = container_of(dev, struct tee_device, dev);
>>> +
>>> + if (teedev->desc->ops->get_tee_revision)
>>> + return attr->mode;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static const struct attribute_group tee_revision_group = {
>>> + .attrs = tee_revision_attrs,
>>> + .is_visible = tee_revision_attr_is_visible,
>>> +};
>>> +
>>> +static const struct attribute_group *tee_dev_groups[] = {
>>> + &tee_dev_group,
>>> + &tee_revision_group,
>>> + NULL
>>> +};
>>>
>>> static const struct class tee_class = {
>>> .name = "tee",
>>> diff --git a/include/linux/tee_core.h b/include/linux/tee_core.h
>>> index 1f3e5dad6d0d..ee5f0bd41f43 100644
>>> --- a/include/linux/tee_core.h
>>> +++ b/include/linux/tee_core.h
>>> @@ -76,6 +76,9 @@ struct tee_device {
>>> /**
>>> * struct tee_driver_ops - driver operations vtable
>>> * @get_version: returns version of driver
>>> + * @get_tee_revision: returns revision string (diagnostic only);
>>
>> Why is this comment here about it being for diagnostics only? I feel
>> it's up to the implementation how it would be used.
>
> According to the previous discussion, we would like to prevent user
> thinking about optee os version x.y means z feature, and we should
> always use TEE_IOC_VERSION for capability and compatibility
> check.
>
> Is there any other specific use case that makes you think removing
> the wording is required?
Ah I didn't realize there was previous discussion that lead to this, I
saw some earlier versions in my holiday mailbox glut but ignored them
when I saw the new one.
Leave it as is then.
>
>>
>>> + * do not infer feature support from this, use
>>> + * TEE_IOC_VERSION instead
>>> * @open: called for a context when the device file is opened
>>> * @close_context: called when the device file is closed
>>> * @release: called to release the context
>>> @@ -95,9 +98,12 @@ struct tee_device {
>>> * client closes the device file, even if there are existing references to the
>>> * context. The TEE driver can use @close_context to start cleaning up.
>>> */
>>> +
>>> struct tee_driver_ops {
>>> void (*get_version)(struct tee_device *teedev,
>>> struct tee_ioctl_version_data *vers);
>>> + int (*get_tee_revision)(struct tee_device *teedev,
>>> + char *buf, size_t len);
>>> int (*open)(struct tee_context *ctx);
>>> void (*close_context)(struct tee_context *ctx);
>>> void (*release)(struct tee_context *ctx);
>>> @@ -123,6 +129,9 @@ struct tee_driver_ops {
>>> int (*shm_unregister)(struct tee_context *ctx, struct tee_shm *shm);
>>> };
>>>
>>> +/* Size for TEE revision string buffer used by get_tee_revision(). */
>>> +#define TEE_REVISION_STR_SIZE 128
>>> +
>>> /**
>>> * struct tee_desc - Describes the TEE driver to the subsystem
>>> * @name: name of driver
>>
>
> Best regards,
> Aristo
Powered by blists - more mailing lists