[RFC]Something wrong with my module

[RFC]Something wrong with my module

[harryxiyou]

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

[RFC]Something wrong with my module

[Kristof Provost]

On 2012-04-12 18:16:56 (+0800), harryxiyou <harryxiyou@gmail.com> 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

[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

[RFC]Something wrong with my module

[harryxiyou]

On Thu, Apr 12, 2012 at 7:18 PM, Kristof Provost <kristof@sigsegv.be> wrote:

Hi Kristof

> On 2012-04-12 18:16:56 (+0800), harryxiyou <harryxiyou@gmail.com> 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

[RFC]Something wrong with my module

[harryxiyou]

On Thu, Apr 12, 2012 at 9:03 PM, Jonathan Neusch?fer
<j.neuschaefer@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

[RFC]Something wrong with my module

[Frank Ch. Eigler]

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

[RFC]Something wrong with my module

[harryxiyou]

On Thu, Apr 12, 2012 at 9:59 PM, Frank Ch. Eigler <fche@redhat.com> 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

[RFC]Something wrong with my module

[harryxiyou]

On Thu, Apr 12, 2012 at 10:04 PM, harryxiyou <harryxiyou@gmail.com> wrote:
> On Thu, Apr 12, 2012 at 9:59 PM, Frank Ch. Eigler <fche@redhat.com> 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

[RFC]Something wrong with my module

[Jonathan Neuschäfer]

On Thu, Apr 12, 2012 at 09:52:02PM +0800, harryxiyou wrote:
> On Thu, Apr 12, 2012 at 9:03 PM, Jonathan Neusch?fer
> <j.neuschaefer@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;
> >> };
[...]

(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

[RFC]Something wrong with my module

[Kristof Provost]

On 2012-04-12 22:08:08 (+0800), harryxiyou <harryxiyou@gmail.com> wrote:
> On Thu, Apr 12, 2012 at 10:04 PM, harryxiyou <harryxiyou@gmail.com> wrote:
> > On Thu, Apr 12, 2012 at 9:59 PM, Frank Ch. Eigler <fche@redhat.com> 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

[RFC]Something wrong with my module

[harryxiyou]

On Thu, Apr 12, 2012 at 10:33 PM, Jonathan Neusch?fer
<j.neuschaefer@gmx.net> 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
>> <j.neuschaefer@gmx.net> 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