]> git.ipfire.org Git - thirdparty/kernel/linux.git/commitdiff
configfs-tsm-report: Fix NULL dereference of tsm_ops
authorDan Williams <dan.j.williams@intel.com>
Wed, 30 Apr 2025 20:33:31 +0000 (13:33 -0700)
committerDan Williams <dan.j.williams@intel.com>
Wed, 7 May 2025 23:48:37 +0000 (16:48 -0700)
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>
Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Reported-by: Cedric Xing <cedric.xing@intel.com>
Reviewed-by: Kai Huang <kai.huang@intel.com>
Link: https://patch.msgid.link/20250430203331.1177062-1-dan.j.williams@intel.com
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
drivers/virt/coco/tsm.c

index 9432d4e303f16b7f65fe8d21d39d0b27c2702674..8a638bc34d4a9ee52642f762142893da41fc6c06 100644 (file)
@@ -15,6 +15,7 @@
 static struct tsm_provider {
        const struct tsm_ops *ops;
        void *data;
+       atomic_t count;
 } provider;
 static DECLARE_RWSEM(tsm_rwsem);
 
@@ -92,6 +93,10 @@ static ssize_t tsm_report_privlevel_store(struct config_item *cfg,
        if (rc)
                return rc;
 
+       guard(rwsem_write)(&tsm_rwsem);
+       if (!provider.ops)
+               return -ENXIO;
+
        /*
         * The valid privilege levels that a TSM might accept, if it accepts a
         * privilege level setting at all, are a max of TSM_PRIVLEVEL_MAX (see
@@ -101,7 +106,6 @@ static ssize_t tsm_report_privlevel_store(struct config_item *cfg,
        if (provider.ops->privlevel_floor > val || val > TSM_PRIVLEVEL_MAX)
                return -EINVAL;
 
-       guard(rwsem_write)(&tsm_rwsem);
        rc = try_advance_write_generation(report);
        if (rc)
                return rc;
@@ -115,6 +119,10 @@ static ssize_t tsm_report_privlevel_floor_show(struct config_item *cfg,
                                               char *buf)
 {
        guard(rwsem_read)(&tsm_rwsem);
+
+       if (!provider.ops)
+               return -ENXIO;
+
        return sysfs_emit(buf, "%u\n", provider.ops->privlevel_floor);
 }
 CONFIGFS_ATTR_RO(tsm_report_, privlevel_floor);
@@ -217,6 +225,9 @@ CONFIGFS_ATTR_RO(tsm_report_, generation);
 static ssize_t tsm_report_provider_show(struct config_item *cfg, char *buf)
 {
        guard(rwsem_read)(&tsm_rwsem);
+       if (!provider.ops)
+               return -ENXIO;
+
        return sysfs_emit(buf, "%s\n", provider.ops->name);
 }
 CONFIGFS_ATTR_RO(tsm_report_, provider);
@@ -284,7 +295,7 @@ static ssize_t tsm_report_read(struct tsm_report *report, void *buf,
        guard(rwsem_write)(&tsm_rwsem);
        ops = provider.ops;
        if (!ops)
-               return -ENOTTY;
+               return -ENXIO;
        if (!report->desc.inblob_len)
                return -EINVAL;
 
@@ -421,12 +432,20 @@ static struct config_item *tsm_report_make_item(struct config_group *group,
        if (!state)
                return ERR_PTR(-ENOMEM);
 
+       atomic_inc(&provider.count);
        config_item_init_type_name(&state->cfg, name, &tsm_report_type);
        return &state->cfg;
 }
 
+static void tsm_report_drop_item(struct config_group *group, struct config_item *item)
+{
+       config_item_put(item);
+       atomic_dec(&provider.count);
+}
+
 static struct configfs_group_operations tsm_report_group_ops = {
        .make_item = tsm_report_make_item,
+       .drop_item = tsm_report_drop_item,
 };
 
 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/report not empty\n");
+               return -EBUSY;
+       }
+
        provider.ops = ops;
        provider.data = priv;
        return 0;
@@ -470,6 +494,9 @@ int tsm_unregister(const struct tsm_ops *ops)
        guard(rwsem_write)(&tsm_rwsem);
        if (ops != provider.ops)
                return -EBUSY;
+       if (atomic_read(&provider.count))
+               pr_warn("\"%s\" unregistered with items present in configfs/tsm/report\n",
+                       provider.ops->name);
        provider.ops = NULL;
        provider.data = NULL;
        return 0;