[RFC]Something wrong with my module

[harryxiyou]

On Thu, Apr 12, 2012 at 9:03 PM, Jonathan Neuschäfer
<j.neuschaefer at gmx.net> wrote:

Hi Jonathan,

> On Thu, Apr 12, 2012 at 06:16:56PM +0800, harryxiyou wrote:
>> Hi greg,
>>
...
>>
>> 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.

Yup, you are right. I will correct it.
>
>>       }
>>       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.

Hmmm.., i just want to give a simplest task_struct, which is my pcb structure.
Of course, it is bogus but it is now wrong for inserting. It can not
print my fields
correctly. (I run this module after i take away the rm_task function)

Some wrong logs like this:

[ 1515.054547] Search for insert task-------->
[ 1515.054550] pid: 1, state: 1, comm: init
[ 1515.054554] pid: 2, state: 1, comm: kthreadd
[ 1515.054558] pid: 3, state: 1, comm: ksoftirqd/0
[ 1515.054561] pid: 4, state: 1, comm: migration/0
[ 1515.054564] pid: 5, state: 1, comm: watchdog/0
[ 1515.054568] pid: 6, state: 1, comm: events/0
[ 1515.054571] pid: 7, state: 1, comm: cpuset
[ 1515.054575] pid: 8, state: 1, comm: khelper
...
[ 1515.055011] pid: 2117, state: 1, comm: bash
[ 1515.055014] pid: 2234, state: 1, comm: vim
[ 1515.055017] pid: 2236, state: 1, comm: flush-8:0
[ 1515.055020] pid: 2370, state: 1, comm: su
[ 1515.055023] pid: 2377, state: 1, comm: bash
[ 1515.055027] pid: 2701, state: 0, comm: insmod
[ 1515.055030] the number of process is 144
[ 1515.055032] show all tasks-------->
[ 1515.055035] pid: 1, state: 1, comm: init
[ 1515.055038] pid: 2, state: 1, comm: kthreadd
[ 1515.055041] pid: 3, state: 1, comm: ksoftirqd/0
[ 1515.055044] pid: 4, state: 1, comm: migration/0
[ 1515.055047] pid: 5, state: 1, comm: watchdog/0
[ 1515.055051] pid: 6, state: 1, comm: events/0
[ 1515.055054] pid: 7, state: 1, comm: cpuset
[ 1515.055057] pid: 8, state: 1, comm: khelper
[ 1515.055060] pid: 9, state: 1, comm: netns
[ 1515.055063] pid: 10, state: 1, comm: async/mgr
[ 1515.055066] pid: 11, state: 1, comm: pm
[ 1515.055069] pid: 12, state: 1, comm: sync_supers
[ 1515.055072] pid: 13, state: 1, comm: bdi-default
[ 1515.055075] pid: 14, state: 1, comm: kintegrityd/0
[ 1515.055078] pid: 15, state: 1, comm: kblockd/0
[ 1515.055081] pid: 16, state: 1, comm: ata_aux
[ 1515.055084] pid: 17, state: 1, comm: ata_sff/0
[ 1515.055087] pid: 18, state: 1, comm: khubd
[ 1515.055090] pid: 19, state: 1, comm: kseriod
[ 1515.055093] pid: 20, state: 1, comm: kmmcd
[ 1515.055096] pid: 22, state: 1, comm: khungtaskd
...
[ 1515.055466] pid: 2234, state: 1, comm: vim
[ 1515.055468] pid: 2236, state: 1, comm: flush-8:0
[ 1515.055472] pid: 2370, state: 1, comm: su
[ 1515.055474] pid: 2377, state: 1, comm: bash
[ 1515.055477] pid: 2701, state: 0, comm: insmod
[ 1515.055481] pid: 0, state: 1, comm:
[ 1515.055483] the number of process is 145

I give the pid 8, state 8, and comm "jiawei" in my module. But it can
not print correctly. Maybe kernel can tell my bogus one,right?

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

Yup, that would be fine.
>
>>
>>
>> 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.

Hmm, it sounds well for me.
>
>>
>> Cloud you please give me some help?
>
> Hope This Helps,
>        Jonathan Neuschäfer

It do helps me, thanks very much ;-)



-- 
Thanks
Harry Wei