On 4/23/25 8:07 PM, Dan Williams wrote:
Sathyanarayanan Kuppuswamy wrote:
On 4/23/25 2:01 PM, Dan Williams wrote:
Unlike sysfs, the lifetime of configfs objects is controlled by userspace. There is no mechanism for the kernel to find and delete all created config-items. Instead, the configfs-tsm-report mechanism has an expectation that tsm_unregister() can happen at any time and cause established config-item access to start failing.
That expectation is not fully satisfied. While tsm_report_read(), tsm_report_{is,is_bin}_visible(), and tsm_report_make_item() safely fail if tsm_ops have been unregistered, tsm_report_privlevel_store() tsm_report_provider_show() fail to check for ops registration. Add the missing checks for tsm_ops having been removed.
Now, in supporting the ability for tsm_unregister() to always succeed, it leaves the problem of what to do with lingering config-items. The expectation is that the admin that arranges for the ->remove() (unbind) of the ${tsm_arch}-guest driver is also responsible for deletion of all open config-items. Until that deletion happens, ->probe() (reload / bind) of the ${tsm_arch}-guest driver fails.
This allows for emergency shutdown / revocation of attestation interfaces, and requires coordinated restart.
Fixes: 70e6f7e2b985 ("configfs-tsm: Introduce a shared ABI for attestation reports") Cc: stable@vger.kernel.org Cc: Suzuki K Poulose suzuki.poulose@arm.com Cc: Steven Price steven.price@arm.com Cc: Sami Mujawar sami.mujawar@arm.com Cc: Borislav Petkov (AMD) bp@alien8.de Cc: Tom Lendacky thomas.lendacky@amd.com Cc: Kuppuswamy Sathyanarayanan sathyanarayanan.kuppuswamy@linux.intel.com Reported-by: Cedric Xing cedric.xing@intel.com Signed-off-by: Dan Williams dan.j.williams@intel.com
Looks good to me
Reviewed-by: Kuppuswamy Sathyanarayanan sathyanarayanan.kuppuswamy@linux.intel.com
Thanks!
[..]
static const struct config_item_type tsm_reports_type = { @@ -459,6 +478,11 @@ int tsm_register(const struct tsm_ops *ops, void *priv) return -EBUSY; }
- if (atomic_read(&provider.count)) {
pr_err("configfs/tsm not empty\n");
Nit: I think adding the provider ops name will make the debug log clear.
Recall though that the ->name field is a tsm_ops property. At this point tsm_ops is already unregistered. Even if we kept the name around by strdup() at register time the name does not help solving the conflict, only rmdir of the created configs-item unblocks the next registration.
Makes sense.