[RFC]Something wrong with my module

[Jonathan Neuschäfer]

On Thu, Apr 12, 2012 at 06:16:56PM +0800, harryxiyou wrote:
> Hi greg,
> 
>     I write a module for inserting a PCB or delete a PCB to kernel's
> PCB tree, but when i run it something wrong happens to me like following.
> My environment is "Linux 10 2.6.35-22-generic #33-Ubuntu SMP Sun Sep
> 19 20:34:50 UTC 2010 i686 GNU/Linux"
> 
> hw2.c
> 
> #include <linux/module.h>
> #include <linux/kernel.h>
> #include <linux/init.h>
> #include <linux/sched.h>
> #include <linux/list.h>
> #include <linux/slab.h>
> 
> struct pcb {
> 	int pid;
> 	int state;
> 	int flag;
> 	char *comm;
> 	struct list_head tasks;
> };
> 
> static int insert_task(struct task_struct *p) {
> 	struct pcb *pcb1 = NULL;
> 	pcb1 = (struct pcb *)kmalloc(sizeof(struct pcb), GFP_KERNEL);
> 	if (NULL == pcb1) {
> 		printk("<0> kmalloc failed!\n");

If you don't return, you'll do an invalid memory access the next line.

> 	}
> 	pcb1->state = 8;
> 	pcb1->flag = 8;
> 	pcb1->pid= 2;
> 	pcb1->comm = "jiawei";
> 	list_add(&pcb1->tasks, &p->tasks);

You add your pcb structure to a list of struct task_structs, this looks
somewhat bogus.

> 	return 0;
> }
> 
> static int rm_task(struct task_struct *p){
> 	struct task_struct *del = p;
> 	list_del(&p->tasks);
> //	kfree(del);
> 	return 0;
> }
> #if 1
> static int print_pid(void) {

You do possibly destructive operations here, "print" doesn't quite imply
that.

> 	struct task_struct *task = NULL;
> 	struct task_struct *p = NULL;
> 	struct list_head *pos = NULL;
> 	int count = 0;
> 	
> 	printk("Search for insert task-------->\n");
> 	task = &init_task;
> 	list_for_each(pos, &task->tasks) {
> 		p = list_entry(pos, struct task_struct, tasks);
> 		count++;
> 		if (0 == p->pid) {
> 			rm_task(p);
> 		}
> 		printk("pid: %d, state: %ld, comm: %s\n", p->pid, p->state, p->comm);
> 	}
> 	insert_task(p);

Why do you want to insert your bogus struct after the last task?

> 	printk("<1> Hello World\n");

The KERN_* constants are a good replacement for a manual "<n>".

> 
> 
> Dmesg logs:
> 
> [ 1174.738305] Search for insert task-------->
[...]
> [ 1174.738819] pid: 2481, state: 1, comm: bash
> [ 1174.738822] pid: 0, state: 1, comm:
> [ 1174.738840] BUG: unable to handle kernel paging request at 00100100

This is probably in insert_task.
list_del sets tasks->next to LIST_POISON1 (which is 0x00100100), list_add
tries to access it and segfaults.

> 
> Cloud you please give me some help?

Hope This Helps,
	Jonathan Neuschäfer