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.

Sign up for GitHub

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

fix(driver): avoid kmod crash when a CPU gets enabled at runtime #2252

Merged
merged 2 commits into from
Jan 28, 2025
+28 −127
Merged

fix(driver): avoid kmod crash when a CPU gets enabled at runtime #2252

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c09b3b6
fix(driver): avoid crashing when an offline CPU prior to agent start,…
FedeDP Jan 22, 2025
f121b29
chore(driver): always initialize enum ppm_overlay.
FedeDP Jan 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
144 changes: 22 additions & 122 deletions driver/main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -277,10 +277,6 @@ static bool verbose = 0;

static unsigned int max_consumers = 5;

#if(LINUX_VERSION_CODE >= KERNEL_VERSION(4, 10, 0))
static enum cpuhp_state hp_state = 0;
#endif

#define vpr_info(fmt, ...) \
do { \
if(verbose) \
Expand Down Expand Up @@ -446,6 +442,7 @@ static int ppm_open(struct inode *inode, struct file *filp) {
consumer->consumer_id = consumer_id;
consumer->buffer_bytes_dim = g_buffer_bytes_dim;
consumer->tracepoints_attached = 0; /* Start with no tracepoints */
consumer->hotplug_cpu = -1;

/*
* Initialize the ring buffers array
Expand Down Expand Up @@ -476,14 +473,6 @@ static int ppm_open(struct inode *inode, struct file *filp) {
ring->info = NULL;
}

/*
* If a cpu is offline when the consumer is first created, we
* will never get events for that cpu even if it later comes
* online via hotplug. We could allocate these rings on-demand
* later in this function if needed for hotplug, but that
* requires the consumer to know to call open again, and that is
* not supported.
*/
for_each_online_cpu(cpu) {
ring = per_cpu_ptr(consumer->ring_buffers, cpu);

Expand Down Expand Up @@ -1820,6 +1809,27 @@ static int record_event_consumer(struct ppm_consumer_t *consumer,
ASSERT(ring);

ring_info = ring->info;
if(!ring_info) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We won't send any event on the newly enabled CPU; ring_info will be NULL and we would panic.

// If we haven't got the ring info, it means
// the event was generated by a CPU that was not
// online when the ring buffers were initialized.
// Store info about hotplugged CPU here to later
// send hotplug events on cpu0.
consumer->hotplug_cpu = cpu;
put_cpu();
return res;
}

// Manage hotplug on cpu 0
if(consumer->hotplug_cpu != -1 && cpu == 0) {
event_type = PPME_CPU_HOTPLUG_E;
drop_flags = UF_NEVER_DROP;
tp_type = INTERNAL_EVENTS;
event_datap->category = PPMC_CONTEXT_SWITCH;
event_datap->event_info.context_data.sched_prev = (void *)(long)consumer->hotplug_cpu;
event_datap->event_info.context_data.sched_next = (void *)(long)0;
}

if(event_datap->category == PPMC_CONTEXT_SWITCH &&
event_datap->event_info.context_data.sched_prev != NULL) {
if(event_type != PPME_SCAPEVENT_E && event_type != PPME_CPU_HOTPLUG_E) {
Expand Down Expand Up @@ -2771,96 +2781,12 @@ static char *ppm_devnode(struct device *dev, mode_t *mode)
}
#endif /* LINUX_VERSION_CODE > KERNEL_VERSION(2, 6, 20) */

static int do_cpu_callback(unsigned long cpu, long sd_action) {
struct ppm_ring_buffer_context *ring;
struct ppm_consumer_t *consumer;
struct event_data_t event_data;

if(sd_action != 0) {
rcu_read_lock();

list_for_each_entry_rcu(consumer, &g_consumer_list, node) {
ring = per_cpu_ptr(consumer->ring_buffers, cpu);
if(sd_action == 1) {
/*
* If the cpu was offline when the consumer was created,
* this won't do anything because we never created a ring
* buffer. We can't safely create one here because we're
* in atomic context, and the consumer needs to call open
* on this device anyways, so do it in ppm_open.
*/
ring->cpu_online = true;
} else if(sd_action == 2) {
ring->cpu_online = false;
}
}

rcu_read_unlock();

event_data.category = PPMC_CONTEXT_SWITCH;
event_data.event_info.context_data.sched_prev = (void *)cpu;
event_data.event_info.context_data.sched_next = (void *)sd_action;
record_event_all_consumers(PPME_CPU_HOTPLUG_E, UF_NEVER_DROP, &event_data, INTERNAL_EVENTS);
}
return 0;
}

#if(LINUX_VERSION_CODE >= KERNEL_VERSION(4, 10, 0))
static int scap_cpu_online(unsigned int cpu) {
vpr_info("scap_cpu_online on cpu %d\n", cpu);
return do_cpu_callback(cpu, 1);
}

static int scap_cpu_offline(unsigned int cpu) {
vpr_info("scap_cpu_offline on cpu %d\n", cpu);
return do_cpu_callback(cpu, 2);
}
#else /* LINUX_VERSION_CODE < KERNEL_VERSION(4, 10, 0)) */
/*
* This gets called every time a CPU is added or removed
*/
static int cpu_callback(struct notifier_block *self, unsigned long action, void *hcpu) {
unsigned long cpu = (unsigned long)hcpu;
long sd_action = 0;

switch(action) {
case CPU_UP_PREPARE:
#if LINUX_VERSION_CODE > KERNEL_VERSION(2, 6, 20)
case CPU_UP_PREPARE_FROZEN:
#endif
sd_action = 1;
break;
case CPU_DOWN_PREPARE:
#if LINUX_VERSION_CODE > KERNEL_VERSION(2, 6, 20)
case CPU_DOWN_PREPARE_FROZEN:
#endif
sd_action = 2;
break;
default:
break;
}

if(do_cpu_callback(cpu, sd_action) < 0)
return NOTIFY_BAD;
else
return NOTIFY_OK;
}

static struct notifier_block cpu_notifier = {
.notifier_call = &cpu_callback,
.next = NULL,
};
#endif /* LINUX_VERSION_CODE >= KERNEL_VERSION(4, 10, 0) */

static int scap_init(void) {
dev_t dev;
unsigned int cpu;
unsigned int num_cpus;
int ret;
int acrret = 0;
#if(LINUX_VERSION_CODE >= KERNEL_VERSION(4, 10, 0))
int hp_ret;
#endif
int j;
int n_created_devices = 0;
#if LINUX_VERSION_CODE > KERNEL_VERSION(2, 6, 20)
Expand Down Expand Up @@ -2964,25 +2890,6 @@ static int scap_init(void) {
goto init_module_err;
}

/*
* Set up our callback in case we get a hotplug even while we are
* initializing the cpu structures
*/
#if(LINUX_VERSION_CODE >= KERNEL_VERSION(4, 10, 0))
hp_ret = cpuhp_setup_state_nocalls(CPUHP_AP_ONLINE_DYN,
DRIVER_NAME "/driver:online",
scap_cpu_online,
scap_cpu_offline);
if(hp_ret <= 0) {
pr_err("error registering cpu hotplug callback\n");
ret = hp_ret;
goto init_module_err;
}
hp_state = hp_ret;
#else
register_cpu_notifier(&cpu_notifier);
#endif

// Initialize globals
g_tracepoints_attached = 0;
for(j = 0; j < KMOD_PROG_ATTACHED_MAX; j++) {
Expand Down Expand Up @@ -3041,13 +2948,6 @@ static void scap_exit(void) {
#if LINUX_VERSION_CODE > KERNEL_VERSION(2, 6, 20)
tracepoint_synchronize_unregister();
#endif

#if(LINUX_VERSION_CODE >= KERNEL_VERSION(4, 10, 0))
if(hp_state > 0)
cpuhp_remove_state_nocalls(hp_state);
#else
unregister_cpu_notifier(&cpu_notifier);
#endif
}

module_init(scap_init);
Expand Down
1 change: 1 addition & 0 deletions driver/ppm_consumer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ or GPL2.txt for full copies of the license.
struct ppm_consumer_t {
unsigned int id; // numeric id for the consumer (ie: registration index)
struct task_struct *consumer_id;
int16_t hotplug_cpu;
#ifdef __percpu
struct ppm_ring_buffer_context __percpu *ring_buffers;
#else
Expand Down
10 changes: 5 additions & 5 deletions driver/ppm_fillers.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ int f_sys_open_x(struct event_filler_arguments *args) {
uint64_t ino = 0;
int res;
int64_t retval;
enum ppm_overlay ol;
enum ppm_overlay ol = PPM_NOT_OVERLAY_FS;

/*
* fd
Expand Down Expand Up @@ -3313,7 +3313,7 @@ int f_sys_creat_x(struct event_filler_arguments *args) {
uint64_t ino = 0;
int res;
int64_t retval;
enum ppm_overlay ol;
enum ppm_overlay ol = PPM_NOT_OVERLAY_FS;
int16_t creat_flags = 0;

/*
Expand Down Expand Up @@ -3877,7 +3877,7 @@ int f_sys_openat_x(struct event_filler_arguments *args) {
int res;
int32_t fd;
int64_t retval;
enum ppm_overlay ol;
enum ppm_overlay ol = PPM_NOT_OVERLAY_FS;

retval = (int64_t)syscall_get_return_value(current, args->regs);
res = val_to_ring(args, retval, 0, false, 0);
Expand Down Expand Up @@ -5403,7 +5403,7 @@ int f_sys_openat2_x(struct event_filler_arguments *args) {
int res;
int32_t fd;
int64_t retval;
enum ppm_overlay ol;
enum ppm_overlay ol = PPM_NOT_OVERLAY_FS;
#ifdef __NR_openat2
struct open_how how;
#endif
Expand Down Expand Up @@ -5561,7 +5561,7 @@ int f_sys_open_by_handle_at_x(struct event_filler_arguments *args) {
long retval = 0;
char *pathname = NULL;
int32_t mountfd = 0;
enum ppm_overlay ol;
enum ppm_overlay ol = PPM_NOT_OVERLAY_FS;

/* Parameter 1: ret (type: PT_FD) */
retval = syscall_get_return_value(current, args->regs);
Expand Down
Loading