[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 <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");
	}
	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, comm: modem-manager
[ 1174.738486] pid: 597, state: 1, comm: console-kit-dae
[ 1174.738490] pid: 675, state: 1, comm: gdm-simple-slav
[ 1174.738493] pid: 679, state: 1, comm: cupsd
[ 1174.738497] pid: 684, state: 1, comm: wpa_supplicant
[ 1174.738501] pid: 688, state: 1, comm: dhclient
[ 1174.738504] pid: 707, state: 1, comm: Xorg
[ 1174.738507] pid: 742, state: 1, comm: getty
[ 1174.738511] pid: 748, state: 1, comm: getty
[ 1174.738514] pid: 758, state: 1, comm: getty
[ 1174.738517] pid: 760, state: 1, comm: getty
[ 1174.738520] pid: 764, state: 1, comm: getty
[ 1174.738524] pid: 766, state: 1, comm: acpid
[ 1174.738527] pid: 771, state: 1, comm: cron
[ 1174.738530] pid: 772, state: 1, comm: atd
[ 1174.738534] pid: 909, state: 1, comm: dbus-launch
[ 1174.738537] pid: 978, state: 1, comm: gdm-session-wor
[ 1174.738541] pid: 986, state: 1, comm: upowerd
[ 1174.738544] pid: 990, state: 1, comm: rtkit-daemon
[ 1174.738548] pid: 1000, state: 1, comm: polkitd
[ 1174.738551] pid: 1071, state: 1, comm: getty
[ 1174.738555] pid: 1082, state: 1, comm: gnome-keyring-d
[ 1174.738559] pid: 1101, state: 1, comm: gnome-session
[ 1174.738562] pid: 1128, state: 1, comm: ibus-daemon
[ 1174.738566] pid: 1132, state: 1, comm: ssh-agent
[ 1174.738569] pid: 1135, state: 1, comm: dbus-launch
[ 1174.738573] pid: 1136, state: 1, comm: dbus-daemon
[ 1174.738576] pid: 1142, state: 1, comm: gconfd-2
[ 1174.738580] pid: 1146, state: 1, comm: gnome-power-man
[ 1174.738583] pid: 1152, state: 1, comm: gnome-settings-
[ 1174.738587] pid: 1157, state: 1, comm: gvfsd
[ 1174.738590] pid: 1162, state: 1, comm: gvfs-fuse-daemo
[ 1174.738593] pid: 1170, state: 1, comm: polkit-gnome-au
[ 1174.738597] pid: 1172, state: 1, comm: pulseaudio
[ 1174.738600] pid: 1173, state: 1, comm: bluetooth-apple
[ 1174.738604] pid: 1176, state: 1, comm: evolution-alarm
[ 1174.738608] pid: 1177, state: 1, comm: nm-applet
[ 1174.738611] pid: 1178, state: 1, comm: gnome-panel
[ 1174.738615] pid: 1180, state: 1, comm: metacity
[ 1174.738618] pid: 1182, state: 1, comm: nautilus
[ 1174.738621] pid: 1189, state: 1, comm: gconf-helper
[ 1174.738625] pid: 1202, state: 1, comm: ibus-gconf
[ 1174.738629] pid: 1204, state: 1, comm: python
[ 1174.738632] pid: 1206, state: 1, comm: ibus-x11
[ 1174.738635] pid: 1214, state: 1, comm: ibus-engine-pin
[ 1174.738639] pid: 1226, state: 1, comm: gvfs-gdu-volume
[ 1174.738642] pid: 1249, state: 1, comm: udisks-daemon
[ 1174.738646] pid: 1271, state: 1, comm: udisks-daemon
[ 1174.738649] pid: 1279, state: 1, comm: gvfs-afc-volume
[ 1174.738653] pid: 1281, state: 1, comm: gvfsd-trash
[ 1174.738656] pid: 1284, state: 1, comm: gvfs-gphoto2-vo
[ 1174.738660] pid: 1289, state: 1, comm: gnome-screensav
[ 1174.738663] pid: 1291, state: 1, comm: bonobo-activati
[ 1174.738667] pid: 1299, state: 1, comm: wnck-applet
[ 1174.738670] pid: 1301, state: 1, comm: trashapplet
[ 1174.738674] pid: 1313, state: 1, comm: indicator-apple
[ 1174.738677] pid: 1317, state: 1, comm: clock-applet
[ 1174.738681] pid: 1318, state: 1, comm: indicator-apple
[ 1174.738684] pid: 1319, state: 1, comm: notification-ar
[ 1174.738688] pid: 1329, state: 1, comm: indicator-me-se
[ 1174.738691] pid: 1333, state: 1, comm: indicator-sessi
[ 1174.738695] pid: 1337, state: 1, comm: gvfsd-burn
[ 1174.738698] pid: 1343, state: 1, comm: indicator-messa
[ 1174.738702] pid: 1347, state: 1, comm: indicator-appli
[ 1174.738705] pid: 1349, state: 1, comm: indicator-sound
[ 1174.738709] pid: 1350, state: 1, comm: gdu-notificatio
[ 1174.738712] pid: 1353, state: 1, comm: gnome-terminal
[ 1174.738716] pid: 1356, state: 1, comm: gnome-pty-helpe
[ 1174.738719] pid: 1357, state: 1, comm: bash
[ 1174.738722] pid: 1381, state: 1, comm: firefox
[ 1174.738726] pid: 1385, state: 1, comm: run-mozilla.sh
[ 1174.738729] pid: 1389, state: 1, comm: firefox-bin
[ 1174.738733] pid: 1419, state: 1, comm: applet.py
[ 1174.738736] pid: 1427, state: 1, comm: sshd
[ 1174.738740] pid: 1519, state: 1, comm: sshd
[ 1174.738743] pid: 1520, state: 1, comm: bash
[ 1174.738746] pid: 1557, state: 1, comm: update-notifier
[ 1174.738750] pid: 1561, state: 1, comm: sshd
[ 1174.738753] pid: 1657, state: 1, comm: sshd
[ 1174.738756] pid: 1658, state: 1, comm: bash
[ 1174.738760] pid: 1671, state: 1, comm: sshd
[ 1174.738763] pid: 1784, state: 1, comm: sshd
[ 1174.738766] pid: 1785, state: 1, comm: bash
[ 1174.738769] pid: 1812, state: 1, comm: sshd
[ 1174.738773] pid: 1911, state: 1, comm: sshd
[ 1174.738776] pid: 1912, state: 1, comm: bash
[ 1174.738779] pid: 1925, state: 1, comm: sshd
[ 1174.738783] pid: 2038, state: 1, comm: sshd
[ 1174.738786] pid: 2039, state: 1, comm: bash
[ 1174.738789] pid: 2170, state: 1, comm: su
[ 1174.738792] pid: 2177, state: 1, comm: bash
[ 1174.738796] pid: 2285, state: 1, comm: vim
[ 1174.738799] pid: 2319, state: 1, comm: vim
[ 1174.738802] pid: 2320, state: 1, comm: sshd
[ 1174.738806] pid: 2413, state: 1, comm: bash
[ 1174.738809] pid: 2456, state: 1, comm: man
[ 1174.738812] pid: 2465, state: 1, comm: pager
[ 1174.738816] pid: 2474, state: 1, comm: su
[ 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
[ 1174.738845] IP: [<f072a08d>] hello_init+0x8d/0x1c8 [hw2]
[ 1174.738853] *pde = 00000000
[ 1174.738857] Oops: 0000 [#3] SMP
[ 1174.738860] last sysfs file:
/sys/devices/pci0000:00/0000:00:03.3/usb1/1-0:1.0/uevent
[ 1174.738865] Modules linked in: hw2(+) hw1(+) binfmt_misc parport_pc
ppdev snd_hda_codec_realtek snd_hda_intel snd_hda_codec snd_hwdep
snd_pcm snd_seq_midi snd_rawmidi snd_seq_midi_event snd_seq snd_timer
snd_seq_device snd sis_agp psmouse serio_raw soundcore snd_page_alloc
shpchp agpgart lp parport 8139too 8139cp sata_sis mii [last unloaded:
hw1]
[ 1174.738895]
[ 1174.738900] Pid: 4340, comm: insmod Tainted: G      D
2.6.35-22-generic #33-Ubuntu /
[ 1174.738904] EIP: 0060:[<f072a08d>] EFLAGS: 00010292 CPU: 0
[ 1174.738908] EIP is at hello_init+0x8d/0x1c8 [hw2]
[ 1174.738911] EAX: 0000002b EBX: 00100100 ECX: c07cdd3c EDX: 00000000
[ 1174.738914] ESI: d85650e0 EDI: 00000097 EBP: d8781f5c ESP: d8781f34
[ 1174.738917]  DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068
[ 1174.738921] Process insmod (pid: 4340, ti=d8780000 task=ef098000
task.ti=d8780000)
[ 1174.738923] Stack:
[ 1174.738925]  f07210d8 00000000 00000001 d85653d0 00000001 c07d0f50
00000001 096bb018
[ 1174.738933] <0> f0721120 00000000 d8781f88 c0101132 f0721120
c07d0f60 096bb018 f0721120
[ 1174.738940] <0> 00004000 f072a000 096bb018 f0721120 00004000
d8781fac c0180c2b 00000003
[ 1174.738948] Call Trace:
[ 1174.738958]  [<c0101132>] ? do_one_initcall+0x32/0x1a0
[ 1174.738963]  [<f072a000>] ? hello_init+0x0/0x1c8 [hw2]
[ 1174.738969]  [<c0180c2b>] ? sys_init_module+0x9b/0x1e0
[ 1174.738975]  [<c02169c5>] ? sys_close+0x75/0xc0
[ 1174.738982]  [<c05c90a4>] ? syscall_call+0x7/0xb
[ 1174.738984] Code: 00 00 00 02 20 00 8b 16 8d 8e f0 02 00 00 83 c7
01 89 4c 24 0c 89 44 24 04 c7 04 24 d8 10 72 f0 89 54 24 08 e8 b0 c3
e9 cf 8b 1b <8b> 03 0f 18 00 90 81 fb 90 78 7c c0 75 92 ba d0 00 00 00
b8 68
[ 1174.739024] EIP: [<f072a08d>] hello_init+0x8d/0x1c8 [hw2] SS:ESP
0068:d8781f34
[ 1174.739030] CR2: 0000000000100100
[ 1174.739035] ---[ end trace babb4642fde28352 ]---
[ 1620.965086] ------------[ cut here ]------------
[ 1620.965105] WARNING: at
/build/buildd/linux-2.6.35/net/sched/sch_generic.c:258
dev_watchdog+0x1fd/0x210()
[ 1620.965109] NETDEV WATCHDOG: eth1 (8139too): transmit queue 0 timed out
[ 1620.965112] Modules linked in: hw2(+) hw1(+) binfmt_misc parport_pc
ppdev snd_hda_codec_realtek snd_hda_intel snd_hda_codec snd_hwdep
snd_pcm snd_seq_midi snd_rawmidi snd_seq_midi_event snd_seq snd_timer
snd_seq_device snd sis_agp psmouse serio_raw soundcore snd_page_alloc
shpchp agpgart lp parport 8139too 8139cp sata_sis mii [last unloaded:
hw1]
[ 1620.965147] Pid: 0, comm: swapper Tainted: G      D
2.6.35-22-generic #33-Ubuntu
[ 1620.965150] Call Trace:
[ 1620.965160]  [<c014ac52>] warn_slowpath_common+0x72/0xa0
[ 1620.965166]  [<c050e62d>] ? dev_watchdog+0x1fd/0x210
[ 1620.965170]  [<c050e62d>] ? dev_watchdog+0x1fd/0x210
[ 1620.965175]  [<c014ad23>] warn_slowpath_fmt+0x33/0x40
[ 1620.965179]  [<c050e62d>] dev_watchdog+0x1fd/0x210
[ 1620.965185]  [<c0108a28>] ? sched_clock+0x8/0x10
[ 1620.965191]  [<c016bf14>] ? sched_clock_local+0xa4/0x180
[ 1620.965195]  [<c050e430>] ? dev_watchdog+0x0/0x210
[ 1620.965200]  [<c0157e1f>] call_timer_fn+0x2f/0xf0
[ 1620.965204]  [<c0169a5a>] ? hrtimer_run_pending+0x4a/0xe0
[ 1620.965209]  [<c0159064>] run_timer_softirq+0x104/0x210
[ 1620.965213]  [<c016c121>] ? sched_clock_cpu+0x131/0x190
[ 1620.965217]  [<c050e430>] ? dev_watchdog+0x0/0x210
[ 1620.965222]  [<c015127c>] __do_softirq+0x9c/0x1b0
[ 1620.965226]  [<c0158af3>] ? update_process_times+0x53/0x70
[ 1620.965230]  [<c01513d5>] do_softirq+0x45/0x50
[ 1620.965234]  [<c0151545>] irq_exit+0x65/0x70
[ 1620.965239]  [<c05cf72b>] smp_apic_timer_interrupt+0x5b/0x8a
[ 1620.965243]  [<c016c1ce>] ? sched_clock_idle_sleep_event+0xe/0x10
[ 1620.965248]  [<c05c9535>] apic_timer_interrupt+0x31/0x38
[ 1620.965254]  [<c010a766>] ? mwait_idle+0x56/0xb0
[ 1620.965257]  [<c0101fcc>] cpu_idle+0x8c/0xd0
[ 1620.965262]  [<c05b2431>] rest_init+0x71/0x80
[ 1620.965269]  [<c081981a>] start_kernel+0x36e/0x374
[ 1620.965273]  [<c08199dd>] ? pass_all_bootoptions+0x0/0xa
[ 1620.965277]  [<c08190d7>] i386_start_kernel+0xd7/0xdf
[ 1620.965280] ---[ end trace babb4642fde28353 ]---

Cloud you please give me some help?

-- 
Thanks
Harry Wei

On 2012-04-12 18:16:56 (+0800), harryxiyou [off-list ref] 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

On Thu, Apr 12, 2012 at 7:18 PM, Kristof Provost [off-list ref] wrote:

Hi Kristof

On 2012-04-12 18:16:56 (+0800), harryxiyou [off-list ref] wrote:

quoted

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

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

On Thu, Apr 12, 2012 at 9:59 PM, Frank Ch. Eigler [off-list ref] wrote:

kristof wrote:

quoted

[...]
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

On Thu, Apr 12, 2012 at 10:04 PM, harryxiyou [off-list ref] wrote:

On Thu, Apr 12, 2012 at 9:59 PM, Frank Ch. Eigler [off-list ref] wrote:

quoted

kristof wrote:

quoted

[...]
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

On 2012-04-12 22:08:08 (+0800), harryxiyou [off-list ref] wrote:

On Thu, Apr 12, 2012 at 10:04 PM, harryxiyou [off-list ref] wrote:

quoted

On Thu, Apr 12, 2012 at 9:59 PM, Frank Ch. Eigler [off-list ref] wrote:

quoted

kristof wrote:

quoted

[...]
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

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

On Thu, Apr 12, 2012 at 9:03 PM, Jonathan Neusch?fer
[off-list ref] wrote:

Hi Jonathan,

On Thu, Apr 12, 2012 at 06:16:56PM +0800, harryxiyou wrote:

quoted

Hi greg,

...

quoted

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.

quoted

? ? ? }
? ? ? 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?

quoted

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

quoted

? ? ? 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?

quoted

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

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

Yup, that would be fine.

quoted

Dmesg logs:

[ 1174.738305] Search for insert task-------->

[...]

quoted

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

quoted

Cloud you please give me some help?

Hope This Helps,
? ? ? ?Jonathan Neusch?fer

It do helps me, thanks very much ;-)



-- 
Thanks
Harry Wei

On Thu, Apr 12, 2012 at 09:52:02PM +0800, harryxiyou wrote:

On Thu, Apr 12, 2012 at 9:03 PM, Jonathan Neusch?fer
[off-list ref] wrote:

Hi Jonathan,

quoted

On Thu, Apr 12, 2012 at 06:16:56PM +0800, harryxiyou wrote:

quoted

Hi greg,

...

quoted

quoted

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;
};

[...]

(from print_pid:)

quoted

quoted

? ? ? struct task_struct *p = NULL;

[...]

quoted

quoted

? ? ? ? ? ? ? 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

On Thu, Apr 12, 2012 at 10:33 PM, Jonathan Neusch?fer
[off-list ref] wrote:

Hi Jonathan

On Thu, Apr 12, 2012 at 09:52:02PM +0800, harryxiyou wrote:

quoted

On Thu, Apr 12, 2012 at 9:03 PM, Jonathan Neusch?fer
[off-list ref] wrote:

Hi Jonathan,

[...]

quoted

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