Re: [PATCH v8 03/15] virt: coco: tsm: Complete PCI registration before publishing the device

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] virt: coco: tsm: stale sysfs update and commit reference in commit log
--

commit 5fae6df00051f68d8e86275946ecd6ac727fb344
Author: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>

virt: coco: tsm: Complete PCI registration before publishing the device

This commit moves PCI TSM registration ahead of device_add() in tsm_register()
so that a TSM device is not discoverable through class lookup while PCI
registration can still fail, and rolls back PCI registration if device_add()
fails.

>     virt: coco: tsm: Complete PCI registration before publishing the device
>     
>     tsm_register() calls device_add() before initializing and validating the
>     PCI operations. This makes the new TSM discoverable through the class
>     lookup while PCI registration can still fail. This ordering was introduced
>     by commit 3225f52cde56 ("PCI/TSM: Establish Secure Sessions and Link
>     Encryption").
>     
>     An existing link TSM can keep the PCI connect attribute visible during
>     registration of another TSM. A concurrent write naming the new TSM can
>     therefore reach it through:
>     
>       connect_store()
>         find_tsm_dev()
>           class_find_device()
>         is_link_tsm()
>         pci_tsm_connect()
>     
>     If the new backend supplies both link and device security operations,
>     pci_tsm_register() rejects it before taking pci_tsm_rwsem. The registration
>     thread then follows:
>     
>       tsm_register()
>         tsm_register_pci()
>           pci_tsm_register() -> -EINVAL
>           tsm_dev->pci_ops = NULL
>         device_del()
>         put_tsm_dev() [cleanup guard]
>     
>     If connect has already attached a PCI context, its temporary device
>     reference does not protect the stored tsm_dev pointer after the call
>     returns. Registration cleanup can then free a TSM still referenced by
>     that context. The original tsm_register_pci_or_reset() error path also
>     had this problem: device_unregister() removed the class device and dropped
>     its reference without tearing down an attached PCI context.
>     
>     Commit 37a73b0448ad ("tsm: Remove the device from lookup before PCI
>     teardown") subsequently added the unlocked pci_ops reset shown above.
>     That reset can also race with the connect path's accesses to pci_ops.
>     
>     Complete PCI registration before device_add() publishes the class device.
>     On PCI registration failure, the TSM remains undiscoverable. If
>     device_add() fails instead, unregister the PCI operations to roll back
>     the counters and PCI sysfs visibility before releasing the device.
>     
>     With pci_ops initialized before device_add(), the class PCI attributes
>     are created with the correct visibility, so remove the subsequent sysfs
>     group update. Keep the PCI change notification after successful device
>     publication.
>     
>     Fixes: 3225f52cde56 ("PCI/TSM: Establish Secure Sessions and Link Encryption")
>     Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>

[Severity: Medium]
Does the commit message match the actual changes in
drivers/virt/coco/tsm-core.c?

The final paragraph states that class PCI attributes are created with the
correct visibility and that a subsequent sysfs group update is removed.
However, in drivers/virt/coco/tsm-core.c, tsm_class only defines .name and
.dev_release without any class PCI attributes or attribute groups:

static const struct class tsm_class = {
	.name		= "tsm",
	.dev_release	= tsm_release
};

Neither tsm_register_pci() nor tsm_register() contained or removed a
sysfs_update_group() call; the patch only reorders tsm_register_pci() before
device_add(), adds pci_tsm_unregister() on the device_add() error path, and
moves kobject_uevent(&dev->kobj, KOBJ_CHANGE) from tsm_register_pci() into
tsm_register() after device_add().

Also, the commit message references commit 37a73b0448ad ("tsm: Remove the
device from lookup before PCI teardown"), which does not exist in the commit
history because the preceding commit in the series is 2ae926219f58 ("virt:
coco: tsm: Remove the device from lookup before PCI teardown"). Could the
commit message be updated to remove the stale sysfs group description and
reference the right commit?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104161632.3547423-1-aneesh.kumar@kernel.org?part=3