Re: [RFC]Something wrong with my module
On Thu, Apr 12, 2012 at 10:33 PM, Jonathan Neuschäfer wrote: Hi Jonathan > On Thu, Apr 12, 2012 at 09:52:02PM +0800, harryxiyou wrote: >> On Thu, Apr 12, 2012 at 9:03 PM, Jonathan Neuschäfer >> wrote: >> >> Hi Jonathan, >> [...] >> >> 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? > > This has to do with the way accessing struct fields works in C: > For each struct each field name is translated by the compiler into an > offset which is used to compute the address of a field given the struct's > address. When you access the pid field of a struct task_struct the offset > will be at least around 20 * sizeof(int), which is an invalid offset to > your struct pcb, where the offsets are (most of the time): > pid: 0 > state: sizeof(int) > flag: 2 * sizeof(int) > comm: 3 * sizeof(int) > tasks: 3 * sizeof(int) + sizeof(char *) > (You get (an approximation of) the offset of a field by adding the size > of the previous field (the compiler also adds some padding - see > Documentation/unaligned-memory-access.txt in the kernel tree and > http://en.wikipedia.org/wiki/Data_padding#Data_structure_padding)) > It sounds well. I will test it, which delare a structure named 'pcb' but including all the fileds as task_struct structure. Thanks Harry Wei ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: [RFC]Something wrong with my module
On 2012-04-12 22:08:08 (+0800), harryxiyou wrote: > On Thu, Apr 12, 2012 at 10:04 PM, harryxiyou wrote: > > On Thu, Apr 12, 2012 at 9:59 PM, Frank Ch. Eigler wrote: > >> > >> kristof wrote: > >> > >>> [...] > >>> You're iterating over the tasks list without locking it. > >>> [...] > >>> Try to take the tasklist_lock. > >> > >> Unfortunately, tasklist_lock is not SYMBOL_EXPORT*'d to modules. > > > > What about other ones? Do you have any other suggestions? > > What about task_lock() ? > task_lock() locks a single task, not the list of tasks. I'd be included to take the fact that tasklist_lock is not exported as a hint that you're not supposed to be messing with the list of tasks from kernel modules. Regards, Kristof ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: [RFC]Something wrong with my module
On Thu, Apr 12, 2012 at 09:52:02PM +0800, harryxiyou wrote:
> On Thu, Apr 12, 2012 at 9:03 PM, Jonathan Neuschäfer
> wrote:
>
> Hi Jonathan,
>
> > On Thu, Apr 12, 2012 at 06:16:56PM +0800, harryxiyou wrote:
> >> Hi greg,
> >>
> ...
> >>
> >> hw2.c
> >>
> >> #include
> >> #include
> >> #include
> >> #include
> >> #include
> >> #include
> >>
> >> struct pcb {
> >> int pid;
> >> int state;
> >> int flag;
> >> char *comm;
> >> struct list_head tasks;
> >> };
[...]
(from print_pid:)
> >> struct task_struct *p = NULL;
[...]
> >> printk("pid: %d, state: %ld, comm: %s\n", p->pid, p->state,
> >> p->comm);
>
> 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.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?
This has to do with the way accessing struct fields works in C:
For each struct each field name is translated by the compiler into an
offset which is used to compute the address of a field given the struct's
address. When you access the pid field of a struct task_struct the offset
will be at least around 20 * sizeof(int), which is an invalid offset to
your struct pcb, where the offsets are (most of the time):
pid: 0
state: sizeof(int)
flag: 2 * sizeof(int)
comm: 3 * sizeof(int)
tasks: 3 * sizeof(int) + sizeof(char *)
(You get (an approximation of) the offset of a field by adding the size
of the previous field (the compiler also adds some padding - see
Documentation/unaligned-memory-access.txt in the kernel tree and
http://en.wikipedia.org/wiki/Data_padding#Data_structure_padding))
Thanks,
Jonathan Neuschäfer
___
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: [RFC]Something wrong with my module
On Thu, Apr 12, 2012 at 10:04 PM, harryxiyou wrote: > On Thu, Apr 12, 2012 at 9:59 PM, Frank Ch. Eigler wrote: >> >> kristof wrote: >> >>> [...] >>> You're iterating over the tasks list without locking it. >>> [...] >>> Try to take the tasklist_lock. >> >> Unfortunately, tasklist_lock is not SYMBOL_EXPORT*'d to modules. > > What about other ones? Do you have any other suggestions? What about task_lock() ? -- Thanks Harry Wei ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: [RFC]Something wrong with my module
On Thu, Apr 12, 2012 at 9:59 PM, Frank Ch. Eigler wrote: > > kristof wrote: > >> [...] >> You're iterating over the tasks list without locking it. >> [...] >> Try to take the tasklist_lock. > > Unfortunately, tasklist_lock is not SYMBOL_EXPORT*'d to modules. What about other ones? Do you have any other suggestions? -- Thanks Harry Wei ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: [RFC]Something wrong with my module
kristof wrote: > [...] > You're iterating over the tasks list without locking it. > [...] > Try to take the tasklist_lock. Unfortunately, tasklist_lock is not SYMBOL_EXPORT*'d to modules. - FChE ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: [RFC]Something wrong with my module
On Thu, Apr 12, 2012 at 9:03 PM, Jonathan Neuschäfer
wrote:
Hi Jonathan,
> On Thu, Apr 12, 2012 at 06:16:56PM +0800, harryxiyou wrote:
>> Hi greg,
>>
...
>>
>> hw2.c
>>
>> #include
>> #include
>> #include
>> #include
>> #include
>> #include
>>
>> 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 "".
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),
Re: [RFC]Something wrong with my module
On Thu, Apr 12, 2012 at 7:18 PM, Kristof Provost wrote:
Hi Kristof
> On 2012-04-12 18:16:56 (+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"
>>
>>
>> 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);
>> }
>>
> You're iterating over the tasks list without locking it.
>
> What do you think happens if someone else comes along and modifies it
> while you're reading it?
Hmmm, Yes, i should lock it.
>
> Try to take the tasklist_lock.
I will try, thanks.
--
Thanks
Harry Wei
___
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: [RFC]Something wrong with my module
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
> #include
> #include
> #include
> #include
> #include
>
> 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 "".
>
>
> 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
___
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: [RFC]Something wrong with my module
On 2012-04-12 18:16:56 (+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"
>
>
> 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);
> }
>
You're iterating over the tasks list without locking it.
What do you think happens if someone else comes along and modifies it
while you're reading it?
Try to take the tasklist_lock.
Regards,
Kristof
___
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
[RFC]Something wrong with my module
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
#include
#include
#include
#include
#include
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");
}
pcb1->state = 8;
pcb1->flag = 8;
pcb1->pid= 2;
pcb1->comm = "jiawei";
list_add(&pcb1->tasks, &p->tasks);
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) {
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);
printk("the number of process is %d\n", count);
count = 0;
printk("show all tasks>\n");
task = &init_task;
list_for_each(pos, &task->tasks) {
p = list_entry(pos, struct task_struct, tasks);
count++;
printk("pid: %d, state: %ld, comm: %s\n", p->pid, p->state,
p->comm);
}
printk("the number of process is %d\n", count);
return 0;
}
#endif
static int __init hello_init(void) {
printk("<1> Hello World\n");
print_pid();
return 0;
}
static void __exit hello_exit(void) {
printk("<1> Goodbey\n");
return ;
}
MODULE_LICENSE("GPL");
module_init(hello_init);
module_exit(hello_exit);
Dmesg logs:
[ 1174.738305] Search for insert task>
[ 1174.738308] pid: 1, state: 1, comm: init
[ 1174.738312] pid: 2, state: 1, comm: kthreadd
[ 1174.738316] pid: 3, state: 1, comm: ksoftirqd/0
[ 1174.738319] pid: 4, state: 1, comm: migration/0
[ 1174.738323] pid: 5, state: 1, comm: watchdog/0
[ 1174.738326] pid: 6, state: 1, comm: events/0
[ 1174.738329] pid: 7, state: 1, comm: cpuset
[ 1174.738333] pid: 8, state: 1, comm: khelper
[ 1174.738336] pid: 9, state: 1, comm: netns
[ 1174.738340] pid: 10, state: 1, comm: async/mgr
[ 1174.738343] pid: 11, state: 1, comm: pm
[ 1174.738346] pid: 12, state: 1, comm: sync_supers
[ 1174.738350] pid: 13, state: 1, comm: bdi-default
[ 1174.738353] pid: 14, state: 1, comm: kintegrityd/0
[ 1174.738357] pid: 15, state: 1, comm: kblockd/0
[ 1174.738360] pid: 16, state: 1, comm: ata_aux
[ 1174.738364] pid: 17, state: 1, comm: ata_sff/0
[ 1174.738367] pid: 18, state: 1, comm: khubd
[ 1174.738370] pid: 19, state: 1, comm: kseriod
[ 1174.738374] pid: 20, state: 1, comm: kmmcd
[ 1174.738377] pid: 23, state: 1, comm: kswapd0
[ 1174.738380] pid: 24, state: 1, comm: ksmd
[ 1174.738383] pid: 25, state: 1, comm: aio/0
[ 1174.738387] pid: 26, state: 1, comm: ecryptfs-kthrea
[ 1174.738390] pid: 27, state: 1, comm: crypto/0
[ 1174.738394] pid: 33, state: 1, comm: scsi_eh_0
[ 1174.738397] pid: 35, state: 1, comm: scsi_eh_1
[ 1174.738400] pid: 38, state: 1, comm: kstriped
[ 1174.738404] pid: 39, state: 1, comm: kmpathd/0
[ 1174.738407] pid: 40, state: 1, comm: kmpath_handlerd
[ 1174.738411] pid: 41, state: 1, comm: ksnapd
[ 1174.738414] pid: 42, state: 1, comm: kondemand/0
[ 1174.738417] pid: 43, state: 1, comm: kconservative/0
[ 1174.738421] pid: 147, state: 1, comm: scsi_eh_2
[ 1174.738424] pid: 148, state: 1, comm: scsi_eh_3
[ 1174.738427] pid: 160, state: 1, comm: jbd2/sda1-8
[ 1174.738431] pid: 161, state: 1, comm: ext4-dio-unwrit
[ 1174.738434] pid: 194, state: 1, comm: flush-8:0
[ 1174.738438] pid: 223, state: 1, comm: upstart-udev-br
[ 1174.738441] pid: 225, state: 1, comm: udevd
[ 1174.738445] pid: 284, state: 1, comm: udevd
[ 1174.738448] pid: 289, state: 1, comm: udevd
[ 1174.738451] pid: 323, state: 1, comm: kpsmoused
[ 1174.738455] pid: 477, state: 1, comm: hd-audio0
[ 1174.738458] pid: 502, state: 1, comm: sshd
[ 1174.738462] pid: 506, state: 1, comm: rsyslogd
[ 1174.738465] pid: 529, state: 1, comm: dbus-daemon
[ 1174.738469] pid: 561, state: 1, comm: gdm-binary
[ 1174.738472] pid: 566, state: 1, comm: NetworkManager
[ 1174.738476] pid: 577, state: 1, comm: avahi-daemon
[ 1174.738479] pid: 586, state: 1, comm: avahi-daemon
[ 1174.738483] pid: 587, state: 1, c
