Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

smith_pagefault_disable/enable may not work as expected on some kernel versions #500

Open
Percivalll opened this issue May 24, 2023 · 1 comment
Assignees

Comments

@Percivalll
Copy link
Contributor

Percivalll commented May 24, 2023

from 3.10.0-1160.el7.x86_64's uaccess.h:

static inline void pagefault_disable(void)
{
	pagefault_disabled_inc();
	/*
	 * make sure to have issued the store before a pagefault
	 * can hit.
	 */
	barrier();
}

static inline void pagefault_enable(void)
{
	/*
	 * make sure to issue those last loads/stores before enabling
	 * the pagefault handler again.
	 */
	barrier();
	pagefault_disabled_dec();
	/*
	 * make sure we do..
	 */
	barrier();
	preempt_check_resched();
}

which matches:

static inline void smith_pagefault_disable(void)
{
#if LINUX_VERSION_CODE >= KERNEL_VERSION(4, 2, 0)
    pagefault_disable();
#else
    preempt_disable();
#endif
#if 0
    preempt_count_inc();
    /*
     * make sure to have issued the store before a pagefault
     * can hit.
     */
    barrier();
#endif
}

/* from kernel 3.14.0, preempt_enable_no_resched() only defined for kernel */
static inline void smith_pagefault_enable(void)
{
#if LINUX_VERSION_CODE >= KERNEL_VERSION(4, 2, 0)
    pagefault_enable();
#elif defined(CONFIG_PREEMPT)
#ifdef preempt_enable_no_resched
    preempt_enable_no_resched();
#else
    /*
     * make sure to issue those last loads/stores before enabling
     * the pagefault handler again.
     */
    barrier();
    preempt_count_dec();
#endif
#else /* < 4.2.0 && !CONFIG_PREEMPT */
    preempt_enable();
#endif
}

if !CONFIG_PREEMPT on this kernel version, smith_pagefault_enable/disable will be no-op.
if CONFIG_PREEMPT, preempt_disable/enable also don't action as expected, as the comment metions Please NEVER use preempt_disable() to disable the fault handler..
should we check faulthandler_disabled or pagefault_disabled instead of LINUX_VERSION_CODE?

@shenping-bd
Copy link
Collaborator

Right, if CONFIG_PREEMPT is unset, preempt_disable will do nothing, and most of server platforms are having preemption disabled.

The purpose of disabling pagefault is to avoid the involvement of page fault handler when accessing invalid pointers (or page is absent/swapped out) given by user since kprobe callbacks runs in atomic context.

But it's ok to permit page fault handler involved, which might crash the user program . That's actually an issue of user program of giving invalid parameters.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants