Need to document callback best practices

[jpoimboe]

Need to document the callback best practices which we defined while working on the IFU and TAA patches. Not sure whether it belongs in the author guide or in the kernel callback docs. Maybe both.

[jpoimboe]

Also may need to mention other things discussed like how globals are harmful though I did open another issue for that.

[jpoimboe]

Can maybe also document how to add/remove sysfs files to existing dirs?

[joe-lawrence]

Can maybe also document how to add/remove sysfs files to existing dirs?

For the recent TAA mitigation, we added something like this to arch/x86/kernel/cpu/bugs.c, is it generic enough?

#include <linux/cpu.h>

static ssize_t kpatch_tsx_async_abort_show_state(struct device *dev,
                                                struct device_attribute *attr,
                                                char *buf)
{
	/* ... typical sysfs code goes here ... */
	return sprintf(buf, "Vulnerable\n");
}

static DEVICE_ATTR(tsx_async_abort, 0444, kpatch_tsx_async_abort_show_state, NULL);

static struct attribute *kpatch_sysfs_taa_attrs[] = {
       &dev_attr_tsx_async_abort.attr,
       NULL
};

static const struct attribute_group kpatch_sysfs_group = {
       .name  = "vulnerabilities",
       .attrs = kpatch_sysfs_taa_attrs,
};

/* 
 * kpatch note: called from a KPATCH_PRE_PATCH_CALLBACK
 */
static int kpatch_init_taa_sysfs(void)
{
       int ret;

       ret = sysfs_merge_group(&cpu_subsys.dev_root->kobj,
                               &kpatch_sysfs_group);

       if (ret)
               pr_err("Unable to register TAA vulnerabilities sysfs entry\n");

       return ret;
}

/*
 * kpatch note: called from a KPATCH_POST_UNPATCH_CALLBACK and
 * KPATCH_PRE_PATCH_CALLBACK error path
 */
static void kpatch_exit_taa_sysfs(void)
{
       sysfs_unmerge_group(&cpu_subsys.dev_root->kobj, &kpatch_sysfs_group);
}

[joe-lawrence]

For the IFU mitigation, we used a simple shadow-variable refcount mechanism to run pre-patch callbacks on the first kpatch load and the post-unpatch callback on the last kpatch unload:

/*
 * kpatch note: called from a KPATCH_PRE_PATCH_CALLBACK
 */
static int kpatch_ifu_pre_patch(void)
{
	int *refcount;
	int ret;

	/* patch upgrade ref counting - only do init the first time */
	refcount = klp_shadow_get(NULL, KLP_SHADOW_CPU_BUGS_COUNT);
	if (refcount) {
		(*refcount)++;
		return 0;
	}
	refcount = klp_shadow_alloc(NULL, KLP_SHADOW_CPU_BUGS_COUNT,
				    sizeof(*refcount), GFP_KERNEL,
				    NULL, NULL);
	if (WARN_ON(!refcount))
		return -ENOMEM;

	*refcount = 1;

	/*
	 *  ... typical pre-patch init code here, only runs once,
         *  assuming there isn't a problem that triggers the error
         *  path below ...
         */

	return 0;

	/* ... typical pre-patch error code here ... */

err:
	klp_shadow_free(NULL, KLP_SHADOW_CPU_BUGS_COUNT, NULL);
	return ret;
}

/*
 * kpatch note: called from a KPATCH_POST_UNPATCH_CALLBACK
 */
static void kpatch_ifu_post_unpatch(void)
{
	int *refcount;

	/* patch upgrade ref counting - only do exit the last time */
	refcount = klp_shadow_get(NULL, KLP_SHADOW_CPU_BUGS_COUNT);
	if (WARN_ON(!refcount))
		return;
	(*refcount)--;
	if (*refcount > 0)
		return;

	/*
	 * ... typical post-unpatch error code here, only runs once
	 * the refcount is 0 (ie, the last kpatch is unloading) ...
	 */

	klp_shadow_free(NULL, KLP_SHADOW_CPU_BUGS_COUNT, NULL);
}

[github-actions]

This issue has been open for 30 days with no activity and no assignee. It will be closed in 7 days unless a comment is added.

[jpoimboe]

bump, we should probably do this if we haven't already :-)

[github-actions]

This issue has been open for 30 days with no activity and no assignee. It will be closed in 7 days unless a comment is added.

[github-actions]

This issue was closed because it was inactive for 7 days after being marked stale.