Linux内核链表与排序问题：进程链表初始化及list_sort调用报错排查

Great question—let's work through your issues step by step, fix the bugs, and clarify the best practices here:

Your current static initialization fix (struct processInfo listHead = { .list = LIST_HEAD_INIT(listHead.list) }) is totally valid and reliable. Here are the other standard, production-grade options:

Option 1: Initialize in Module Entry (Dynamic Initialization) #

The module's __init function runs exactly once when the module loads, making it the perfect place to set up global structures without concurrency risks (your original code tried to initialize in a syscall, which could be called by multiple processes at once leading to race conditions):

static int __init my_syscall_module_init(void) {
    INIT_LIST_HEAD(&listHead.list);
    // Add your syscall registration logic here
    return 0;
}
module_init(my_syscall_module_init);

This is more flexible than static initialization if you need to handle dynamic configuration later.

Option 2: Static Initialization (Your Current Fix) #

Using LIST_HEAD_INIT sets the list head's prev and next pointers to point to itself at compile time. This is zero-overhead, thread-safe, and ideal for global list heads—there's nothing wrong with using this as your primary solution.

Why Your Original Initialization Failed #

Checking list_empty(&listHead.list) before initializing is unsafe because an uninitialized list_head has random garbage values. The list_empty check can incorrectly report the list as non-empty, leaving your list uninitialized and causing memory corruption when you try to add nodes.

The page fault is caused by a critical mistake: you're directly copying the entire struct task_struct, which is strictly forbidden in the kernel.

struct task_struct is a huge, dynamic structure filled with pointers to kernel-managed resources (memory maps, file lists, etc.). When you do newProcess->task = *task;, you're copying stale or invalid pointer values. Later, when your compare function tries to access p1->task.pid, it's actually dereferencing an invalid pointer buried in the copied task_struct, triggering the page fault.

Fix Steps: #

1. Rewrite your processInfo struct to store only needed data:

struct processInfo {
    pid_t pid;  // Only store the PID, not the entire task_struct
    int len_files;
    struct list_head file_list_head;  // Renamed for clarity
    struct list_head list;
};

2. Update sys_init_process_list to save just the PID:

// Replace newProcess->task = *task; with this
newProcess->pid = task->pid;

3. Fix the compare function to use the stored PID:

int compare(void* priv, struct list_head *a, struct list_head *b) {
    struct processInfo *p1 = container_of(a, struct processInfo, list);
    struct processInfo *p2 = container_of(b, struct processInfo, list);
    
    if (p1->pid > p2->pid) return 1;    // 1 = a should come after b (descending)
    else if (p1->pid < p2->pid) return -1; // -1 = a should come before b (ascending)
    return 0; // Equal PIDs (shouldn't happen, but handle it)
}

This eliminates all invalid memory accesses from copying task_struct.

Memory Leak Prevention #

Your sys_clear_process_list only frees processInfo nodes but leaves the fileDescriptor lists untouched. Add this cleanup logic to avoid memory leaks:

asmlinkage long sys_clear_process_list(void) {
    struct processInfo *aProcess, *tmp_proc;
    struct fileDescriptor *fd_node, *tmp_fd;

    if(list_empty(&listHead.list)) {
        printk(KERN_INFO "empty list\n");
        return 1;
    }
    printk(KERN_INFO "deleting the list\n");
    
    list_for_each_entry_safe(aProcess, tmp_proc, &listHead.list, list) {
        printk(KERN_INFO "freeing process %d\n", aProcess->pid);
        
        // Clean up file descriptors first
        list_for_each_entry_safe(fd_node, tmp_fd, &aProcess->file_list_head, list) {
            list_del(&fd_node->list);
            kfree(fd_node);
        }
        
        // Clean up the process node
        list_del(&aProcess->list);
        kfree(aProcess);
    }
    return 0;
}

Avoid Stack Overflow from Recursive Child Process Traversal #

Your recursive call to sys_init_process_list for child processes will cause a kernel stack overflow if the process tree is deep (e.g., hundreds of nested child processes). Use a queue-based approach instead:

asmlinkage long sys_init_process_list(pid_t p) {
    struct pid* pid;
    struct task_struct *task;
    struct files_struct *processFiles;
    struct fdtable *filesTable;
    struct processInfo *newProcess;
    struct list_head task_queue;  // Queue to hold processes to process
    struct task_struct *current_task;
    struct list_head *pos, *tmp;

    INIT_LIST_HEAD(&task_queue);

    // Start with the target PID (fall back to init if invalid)
    pid = find_get_pid(p);
    if (!pid) {
        printk(KERN_WARNING "Invalid PID %d, using init (PID 1)\n", p);
        pid = find_get_pid(1);
        if (!pid) return 1;
    }
    task = pid_task(pid, PIDTYPE_PID);
    if (!task) {
        put_pid(pid);
        return 1;
    }
    get_task_struct(task);
    list_add_tail(&task->tasks, &task_queue);
    put_pid(pid);

    while (!list_empty(&task_queue)) {
        // Dequeue a process
        pos = task_queue.next;
        list_del(pos);
        current_task = list_entry(pos, struct task_struct, tasks);

        // Skip if we already added this process to our list
        int exists = 0;
        struct processInfo *tmp_proc;
        list_for_each_entry(tmp_proc, &listHead.list, list) {
            if (tmp_proc->pid == current_task->pid) {
                exists = 1;
                break;
            }
        }
        if (exists) {
            put_task_struct(current_task);
            continue;
        }

        // Allocate and populate process info
        newProcess = kmalloc(sizeof(*newProcess), GFP_KERNEL);
        if (!newProcess) {
            printk(KERN_ERR "Failed to allocate memory for process info\n");
            put_task_struct(current_task);
            continue;
        }
        newProcess->pid = current_task->pid;
        INIT_LIST_HEAD(&newProcess->file_list_head);
        newProcess->len_files = 0;

        // Collect file descriptors
        processFiles = current_task->files;
        if (processFiles) {
            filesTable = files_fdtable(processFiles);
            for (int i = 0; i < filesTable->max_fds; i++) {
                if (filesTable->fd[i]) {
                    struct fileDescriptor *newfd = kmalloc(sizeof(*newfd), GFP_KERNEL);
                    if (newfd) {
                        newfd->fd = i;
                        INIT_LIST_HEAD(&newfd->list);
                        list_add_tail(&newfd->list, &newProcess->file_list_head);
                        newProcess->len_files++;
                    }
                }
            }
        }

        // Add to our process list
        INIT_LIST_HEAD(&newProcess->list);
        list_add_tail(&newProcess->list, &listHead.list);

        // Enqueue child processes
        list_for_each_safe(pos, tmp, &current_task->children) {
            struct task_struct *child = list_entry(pos, struct task_struct, sibling);
            get_task_struct(child);
            list_add_tail(&child->tasks, &task_queue);
        }

        put_task_struct(current_task);
    }

    return 0;
}

• Stick with static initialization or module-entry initialization for your list head—avoid initializing in syscalls.
• Never copy struct task_struct directly; only store the minimal data you need (like PID, UID, etc.).
• Recursion in kernel code is risky due to limited stack space—use iterative approaches for tree traversal.
• Always clean up all dynamically allocated memory to avoid leaks.

内容的提问来源于stack exchange，提问作者Sajad