Skip to content

Need to document callback best practices #1061

Open

Description

@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.

Activity

  1. jpoimboe commented on Nov 20, 2019

    @jpoimboe
    MemberAuthor

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

  2. jpoimboe commented on Nov 20, 2019

    @jpoimboe
    MemberAuthor

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

  3. joe-lawrence commented on Dec 10, 2019

    @joe-lawrence
    Contributor

    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);
    }
  4. joe-lawrence commented on Dec 10, 2019

    @joe-lawrence
    Contributor

    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);
    }
  5. github-actions commented on Aug 3, 2023

    @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.

  6. jpoimboe commented on Aug 3, 2023

    @jpoimboe
    MemberAuthor

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

  7. github-actions commented on Sep 3, 2023

    @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.

  8. github-actions commented on Sep 11, 2023

    @github-actions

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

  9. self-assigned this
    on Sep 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions