LINUX.ORG.RU

Проблемма с выделением памяти в модуле.


0

0

Уже задавал подобный вопрос но ответа так и не получил, веренее была 
пара советов, но они не оказали эффекта.

Так что повторюсь. Имеется модуль, в нём реализована функция write ну и
конечно же создаётся девайс, который юзается через внешнюю прогу.

Вот код проги:

/*****************************************************************/
#include <unistd.h>
#include <stdio.h>
#include <stdlib.h>
#include <fcntl.h>

char *num;	//num of parametrs

//convert int to char
void int_to_char(int n)
{
}

int main(int argc, char *argv[])
{
	int fd;
	char *end = "$";

	//this is dev for netfw control
	fd = open("/dev/netfw", O_RDWR);
	if(fd == -1){
		printf("open failed\n");
		return -1;
	}
	printf("Device netfw open\nTry to write\n");

	//calc num of parametrs like: from, to, fport, tport
	int ap = argc - 1 - 3;
	printf("ap = %d\n",ap);	//check it
	int i;
	//write control element like: add/del, pass/drop and name of table
	printf("argc = %d\n",argc);	//check it
	if(!argv[1])
	{
		printf("write 1$\n");
		write(fd,end,strlen(end) + 1);
		return 0;
	}
	else write(fd, argv[1], strlen(argv[1]) + 1);
	if(!argv[2])
	{
		printf("wrte 2$\n");
		write(fd,end,strlen(end) + 1);
		return 0;
	}
	else write(fd, argv[2], strlen(argv[2]) + 1);
	if(!argv[3])
	{
		printf("write 3$\n");
		write(fd,end,strlen(end) + 1);
		return 0;
	}
	else write(fd, argv[3], strlen(argv[3]) + 1);
	if(ap > 0)	
	{
		//write num of parametrs
		int_to_char(ap/2);
		write(fd, num, strlen(num) + 1);
	
		//write parametrs
		for(i = argc - ap; i < argc; i++)
		{
			printf(argv[i]);
			printf("\n");
			write(fd, argv[i], strlen(argv[i]) + 1);
		}
	}
	//write symbol to finish transacton, in this case it's "$"
	printf("write 4$\n");
	write(fd,end,strlen(end) + 1);
		
	//close file
	close(fd);
	printf("Device netfw close\n");

	free(num);

	return EXIT_SUCCESS;
}
/*****************************************************************/
★★★★★

А вот та часть кода модуля которая глючит.

/*****************************************************************/
int netfw_add_rule(char *table_name, int id, int it, char **par, char **arg, int num)
{
	int i;
	for(i = 0; i < num; i++){
		if(par[i]) kfree(par[i]);
		if(arg[i]) kfree(arg[i]);
	}

	printk( KERN_DEBUG "NetFW: Rule was add successfull\n");

	netfw_num_rules++;
	return 1;
}

ssize_t netfw_write (struct file *f, const char *ch, size_t size, loff_t *loff)
{
	char *parg;
	if(num > 0)if(n < num*2){
		if(par_accept){
			par[n_par] = kmalloc((sizeof(char) * strlen(ch)) + 1,GFP_KERNEL);
			parg = kmalloc((sizeof(char) * strlen(ch)) + 1,GFP_KERNEL);
			if(!par[n_par]) return 0;
////			par[n_par] = strcpy(par[n_par],ch);
////			get_user(par[n_par], ch);
			if(copy_from_user(parg,ch,size)){
				printk("Error on copy par!!!\n");
				return 0;
			}
			par[n_par] = strcpy(par[n_par],parg);
			kfree(parg);
			if(par[n_par] == 0){
				printk("Error no par!!!\n");
				return 0;
			}
			printk("par[%d] = %s, ch = %s, parg = %s\n",n_par,par[n_par],ch,parg);
			par_accept = 0;
			n_par++;
		}
		else{
			arg[n_arg] = kmalloc((sizeof(char) * strlen(ch)) + 1,GFP_KERNEL);
			parg = kmalloc((sizeof(char) * strlen(ch)) + 1,GFP_KERNEL);
			if(!arg[n_arg]) return 0;
////			arg[n_arg] = strcpy(arg[n_arg],ch);
////			get_user(arg[n_arg], ch);
			if(copy_from_user(parg,ch,size)){
				printk("Error on copy arg!!!\n");
				return 0;
			}
			arg[n_arg] = strcpy(arg[n_arg],parg);
			kfree(parg);
			if(arg[n_arg] == 0){
				printk("Error no arg!!!\n");
				return 0;
			}
			printk("arg[%d] = %s, ch = %s, parg = %s\n",n_arg,arg[n_arg],ch,parg);
			par_accept = 1;
			n_arg++;
		}
		n++;
	}
	if(!strcmp(ch,"$")){
		step = -1;
		n = n_par = n_arg = 0;
		if(act > 0)	{
			struct netfw_table *tab;
			switch(act){
				case NETFW_ADD_RULE:
					/*if(*/netfw_add_rule(to_table,0,it,par,arg,num)/*)*/;
					       break;
				case NETFW_SET_POL:
					tab = netfw_get_table_by_name(to_table);
					if(tab)/*if(*/tab->set_rule(NETFW_SET_POL,0,0,0,it,0,0,0)/*)*/;
					else printk("NetFW: ERROR at set new policy for table - %s!!!\n",to_table);
					break;
			}
		}
		if(par) kfree(par);
		if(arg) kfree(arg);
		if(to_table) kfree(to_table);
		if(out) kfree(out);
		to_table = 0;
		num = 0;
	}
	step++;
	switch(step){
		//Action
		case 1:
			if(!strcmp(ch,"add")){
				act = NETFW_ADD_RULE;
			}
			if(!strcmp(ch,"del")){
				act = NETFW_DEL_RULE;
			}
			if(!strcmp(ch,"policy")){
				act = NETFW_SET_POL;
			}
			if(!strcmp(ch,"show")){
				act = NETFW_SHOW_RULE;
			}
			break;
		//Table name
		case 2:
			to_table = kmalloc(strlen(ch) + 1,GFP_ATOMIC);
			if(!to_table) return 0;
			to_table = strcpy(to_table,ch);
			break;
		//What to do ( aka "it" )
		case 3:
			if(!strcmp(ch,"accept")){
				it = NETFW_ACCEPT;
			}
			if(!strcmp(ch,"drop")){
				it = NETFW_DROP;
			}
			break;
		//Num of elements
		case 4:
			num = netfw_char_to_int((char*)ch);
			if(par) kfree(par);
			if(arg) kfree(arg);
			par = kmalloc((sizeof(char *) * num),GFP_ATOMIC);
			if(!par) return 0;
			arg = kmalloc((sizeof(char *) * num),GFP_ATOMIC);
			if(!arg) return 0;
			par_accept = 1;
			break;
	}

	return 0;
}
/*****************************************************************/

cyclon ★★★★★
() автор топика
Ответ на: комментарий от cyclon

Вообщем ситуация обстоит так. После третьего (а иногда и второго) 
вызова проги, т.е. после того как функция записи вызвана
происходит сегфолт. Вот вывод лога

Aug 19 12:20:24 dell kernel: par[0] = from, ch = from, parg = from
Aug 19 12:20:24 dell kernel: arg[0] = 127.0.0.1, ch = 127.0.0.1, parg = 127.0.0.1
Aug 19 12:20:28 dell kernel: par[0] = from, ch = from, parg = from
Aug 19 12:20:28 dell kernel: c8e954aa
Aug 19 12:20:28 dell kernel: Modules linked in: netfw ipv6 rfcomm hidp l2cap speedstep_lib freq_table thermal processor fan button battery ac hci_usb bluetooth snd_pcm_oss snd_mixer_oss snd_seq_midi snd_seq_midi_event snd_seq evdev joydev sg st sr_mod edd snd_es1968 gameport snd_ac97_codec snd_pcm snd_timer snd_page_alloc snd_mpu401_uart snd_rawmidi snd_seq_device intel_agp snd agpgart soundcore uhci_hcd usbcore yenta_socket rsrc_nonstatic i2c_piix4 pcmcia_core i2c_core parport_pc lp video1394 ohci1394 parport raw1394 ieee1394 capability subfs sd_mod scsi_mod dm_mod reiserfs ide_cd cdrom ide_disk piix ide_core
Aug 19 12:20:28 dell kernel: CPU:    0
Aug 19 12:20:28 dell kernel: EIP:    0060:[<c8e954aa>]    Tainted: P     U VLI
Aug 19 12:20:28 dell kernel: EFLAGS: 00010246   (2.6.11.4-20a-default)
Aug 19 12:20:28 dell kernel: EIP is at netfw_write+0x16a/0x5c0 [netfw]
Aug 19 12:20:28 dell kernel: eax: 2e373231   ebx: ffffffff   ecx: 00000000   edx: 00000000
Aug 19 12:20:28 dell kernel: esi: c60ec4e1   edi: 2e373231   ebp: c60ec4e0   esp: c4cfff6c
Aug 19 12:20:28 dell kernel: ds: 007b   es: 007b   ss: 0068
Aug 19 12:20:28 dell kernel: Process netfw (pid: 5854, threadinfo=c4cfe000 task=c7211a80)
Aug 19 12:20:28 dell kernel: Stack: c60ec4e0 2e373231 0000000a bffff615 c8e95340 0000000a c60dd880 bffff615
Aug 19 12:20:28 dell kernel:        c0151313 c4cfffac c60dd880 fffffff7 08048fa0 c4cfe000 c015143c c4cfffac
Aug 19 12:20:28 dell kernel:        00000000 00000000 00000000 00000003 40016cc0 c0102c49 00000003 bffff615
Aug 19 12:20:28 dell kernel: Call Trace:
Aug 19 12:20:28 dell kernel:  [<c8e95340>] netfw_write+0x0/0x5c0 [netfw]
Aug 19 12:20:28 dell kernel:  [<c0151313>] vfs_write+0x93/0x110
Aug 19 12:20:28 dell kernel:  [<c015143c>] sys_write+0x3c/0x70
Aug 19 12:20:28 dell kernel:  [<c0102c49>] sysenter_past_esp+0x52/0x79
Aug 19 12:20:28 dell kernel: Code: e8 3c f0 33 f7 85 c0 74 51 68 0f 6f e9 c8 eb db 8b 3d ac 86 e9 c8 8b 15 9c 86 e9 c8 89 3c 24 89 ee 8b 04 97 89 44 24 04 89 c7 ac <aa> 84 c0 75 fa 8b 04 24 8b 4c 24 04 89 0c 90 89 e8 e8 e0 7e 2a

Пробовал сделать par[] и arg[] в виде списков, не помогло.

Кстати заметил что сегфолты бывают не только в netfw_write 

Aug 19 19:58:46 dell kernel: par[0] = from, ch = from, parg = from
Aug 19 19:58:46 dell kernel: arg[0] = 127.0.0.1, ch = 127.0.0.1, parg = 127.0.0.1
Aug 19 19:58:46 dell kernel: NetFW: Rule was add successfull
Aug 19 19:58:47 dell kernel: par[0] = from, ch = from, parg = from
Aug 19 19:58:47 dell kernel: arg[0] = 127.0.0.1, ch = 127.0.0.1, parg = 127.0.0.1
Aug 19 19:58:47 dell kernel: Unable to handle kernel paging request at virtual address ffff9370
Aug 19 19:58:47 dell kernel:  printing eip:
Aug 19 19:58:47 dell kernel: c013d3c2
Aug 19 19:58:47 dell kernel: *pde = 00002067
Aug 19 19:58:47 dell kernel: Oops: 0000 [#1]
Aug 19 19:58:47 dell kernel: Modules linked in: netfw ppp_deflate zlib_deflate bsd_comp ppp_async crc_ccitt ppp_generic slhc ipv6 rfcomm hidp l2cap speedstep_lib freq_table thermal processor fan button battery ac hci_usb bluetooth snd_pcm_oss snd_mixer_oss snd_seq_midi snd_seq_midi_event snd_seq evdev joydev sg st sr_mod edd video1394 ohci1394 snd_es1968 gameport snd_ac97_codec snd_pcm snd_timer snd_page_alloc snd_mpu401_uart intel_agp snd_rawmidi agpgart raw1394 snd_seq_device ieee1394 snd i2c_piix4 soundcore i2c_core yenta_socket uhci_hcd rsrc_nonstatic pcmcia_core usbcore capability parport_pc lp parport subfs sd_mod scsi_mod dm_mod reiserfs ide_cd cdrom ide_disk piix ide_core
Aug 19 19:58:47 dell kernel: CPU:    0
Aug 19 19:58:47 dell kernel: EIP:    0060:[<c013d3c2>]    Tainted: P     U VLI
Aug 19 19:58:47 dell kernel: EFLAGS: 00010006   (2.6.11.4-20a-default)
Aug 19 19:58:47 dell kernel: EIP is at kfree+0x22/0x50
Aug 19 19:58:47 dell kernel: eax: ffff9370   ebx: 00000000   ecx: 000000c9   edx: c1000000
Aug 19 19:58:47 dell kernel: esi: 2e373231   edi: 00000202   ebp: c5e562c0   esp: c23d5f3c
Aug 19 19:58:47 dell kernel: ds: 007b   es: 007b   ss: 0068
Aug 19 19:58:47 dell kernel: Process netfw (pid: 8256, threadinfo=c23d4000 task=c6ac9020)
Aug 19 19:58:47 dell kernel: Stack: 00000000 00000001 c76e2980 c8e95031 c8e95340 08049077 c8e96f49 c5e562c0
Aug 19 19:58:47 dell kernel:        c8e958be c5e562c0 c76e2980 00000001 c7be600c 00000009 00000002 08049075
Aug 19 19:58:47 dell kernel:        c8e95340 00000002 c16cc4c0 08049075 c0151313 c23d5fac c16cc4c0 fffffff7
Aug 19 19:58:47 dell kernel: Call Trace:
Aug 19 19:58:47 dell kernel:  [<c8e95031>] netfw_add_rule+0x31/0x70 [netfw]
Aug 19 19:58:47 dell kernel:  [<c8e95340>] netfw_write+0x0/0x5c0 [netfw]
Aug 19 19:58:47 dell kernel:  [<c8e958be>] netfw_write+0x57e/0x5c0 [netfw]
Aug 19 19:58:47 dell kernel:  [<c8e95340>] netfw_write+0x0/0x5c0 [netfw]
Aug 19 19:58:47 dell kernel:  [<c0151313>] vfs_write+0x93/0x110
Aug 19 19:58:47 dell kernel:  [<c015143c>] sys_write+0x3c/0x70
Aug 19 19:58:47 dell kernel:  [<c0102c49>] sysenter_past_esp+0x52/0x79
Aug 19 19:58:47 dell kernel: Code: d0 5b 5e 5f c3 8d 74 26 00 57 85 c0 56 89 c6 53 74 2d 9c 5f fa 8d 80 00 00 00 40 c1 e8 0c 8b 15 70 3b 3e c0 c1 e0 05 8b 44 02 18 <8b> 18 8b 13 3b 53 04 73 0f 89 74 93 10 8b 03 40 89 03 57 9d 5b

Заранее спасибо!!!

cyclon ★★★★★
() автор топика

ok, давай тогда я прокомментирую

	/*****************************************************************/
	#include <unistd.h>
	#include <stdio.h>
	#include <stdlib.h>
	#include <fcntl.h>

	char *num;	//num of parametrs

	//convert int to char
	void int_to_char(int n)

Прекрасно. У нас тут функция, принимающая int, ничего не возвращающая
и при этом "переводящая int в char". Модификация глобальной переменной
как побочный эффект. В страшном сне такое не приснится.

Кроме того, char - обозначение типа данных для чисел размером 1 байт, то есть
в диапазоне от -128 до 127. То, что ты здесь имеешь в виду, называется строкой,
то есть 'string'

[Тело функции вызывает отвращение и я его пропускаю]

1) Смотри как это сделал бы рядовой homo sapiens (надеюсь я из таких):

char* unsigned_to_string (unsigned n)
{
	char *str;
	unsigned tmp, i, m, len;

	tmp = n;
	m = 1;
	len = 0;
	do {
		tmp /= 10;
		if (len != 0)
			m *= 10;
		len ++;
	} while (tmp != 0);

	str = malloc (len + 1);

	for (i = 0; i < len; i++) {
		tmp = n / m * m;
		n -= tmp;
		str [i] = tmp / m + '0';
		m /= 10;
	}

	str [len] = 0;
	return str;
}

2) man snprintf и больше никакой самодеятельности:
   в linux-kernel эта функция выполнена так же, как и в glibc

	int main(int argc, char *argv[])
	{
		int fd;
		char *end = "$";

		//this is dev for netfw control
		fd = open("/dev/netfw", O_RDWR);
		if(fd == -1){
			printf("open failed\n");

Рекомендую писать perror ("open") - эта функция добавит
расшифрованное содержимое переменной errno и получится,
например, красивое "open: device or resource is busy"

			return -1;
		}
		printf("Device netfw open\nTry to write\n");

		//calc num of parametrs like: from, to, fport, tport
		int ap = argc - 1 - 3;
		printf("ap = %d\n",ap);	//check it
		int i;
		//write control element like: add/del, pass/drop and name of table
		printf("argc = %d\n",argc);	//check it
		if(!argv[1])

Проверка !argv[N] лишена смысла: если параметр - пустая строка,
то параметра нет (т.е. пришли к противоречию).
А вообще это segfault, если параметров меньше N

		{
			printf("write 1$\n");
			write(fd,end,strlen(end) + 1);
			return 0;
		}
		else write(fd, argv[1], strlen(argv[1]) + 1);
		if(!argv[2])
		{
			printf("wrte 2$\n");
			write(fd,end,strlen(end) + 1);
			return 0;
		}
		else write(fd, argv[2], strlen(argv[2]) + 1);
		if(!argv[3])
		{
			printf("write 3$\n");
			write(fd,end,strlen(end) + 1);
			return 0;
		}
		else write(fd, argv[3], strlen(argv[3]) + 1);

Сделали три write'а. ok..

		if(ap > 0)
		{
			//write num of parametrs
			int_to_char(ap/2);
			write(fd, num, strlen(num) + 1);

Число перевели в строку и отправили в ядро. Странное действие, ну да ладно..
	
			//write parametrs
			for(i = argc - ap; i < argc; i++)

Ты имел в виду for (i = 4; ..

			{
				printf(argv[i]);
				printf("\n");
				write(fd, argv[i], strlen(argv[i]) + 1);
			}
		}
		//write symbol to finish transacton, in this case it's "$"
		printf("write 4$\n");
		write(fd,end,strlen(end) + 1);
		
		//close file
		close(fd);
		printf("Device netfw close\n");

		free(num);

		return EXIT_SUCCESS;

лучше return 0;
	}
	/*****************************************************************/

Вывод: и что? если с вводить строго задуманное количество параметров,
то летального исхода не будет (нормально всё)

erDiZz
()

Едем дальше.

	А вот та часть кода модуля которая глючит.

Программы не глючат. "Глючит" оборудование из-за помех. А в программах просто есть ошибки.

	/*****************************************************************/
	int netfw_add_rule(char *table_name, int id, int it, char **par, char **arg, int num)
	{
		int i;
		for(i = 0; i < num; i++){
			if(par[i]) kfree(par[i]);
			if(arg[i]) kfree(arg[i]);
		}

Второй приведённый сегфолт произойти мог ТОЛЬКО после одного из kfree,
потому что в netfw_add_rule вход был произведён, а printk сработать ещё не успел.
Вывод: ты ошибся при работе с par[] или arg[]. Это как минимум (ошибок-то может быть больше).
В принципе, с такой подробной информацией о месте ошибки в форумы не обращаются.
Это вопрос об уважении к себе и людям.

		printk( KERN_DEBUG "NetFW: Rule was add successfull\n");

		netfw_num_rules++;
		return 1;
	}

	ssize_t netfw_write (struct file *f, const char *ch, size_t size, loff_t *loff)
	{
		char *parg;
		if(num > 0)if(n < num*2){
			if(par_accept){
				par[n_par] = kmalloc((sizeof(char) * strlen(ch)) + 1,GFP_KERNEL);
				parg = kmalloc((sizeof(char) * strlen(ch)) + 1,GFP_KERNEL);

Далее ты делаешь compy_from_user (.., .., size), поэтому тут потенциальная ошибка,
хотя она и не должна проявляться при работе твоей клиентской программы.
Правильно будте parg = kmalloc (size, ..)

				if(!par[n_par]) return 0;
	////			par[n_par] = strcpy(par[n_par],ch);
	////			get_user(par[n_par], ch);
				if(copy_from_user(parg,ch,size)){
					printk("Error on copy par!!!\n");
					return 0;

Лучше return -EFAULT, потому что правильный клиент при получении нуля от write
повторит попытку записи. (man 2 write: zero indicates that nothing was written, не более)

				}
				par[n_par] = strcpy(par[n_par],parg);
				kfree(parg);

Просто par [n_par] = parg, и не надо делать kfree

				if(par[n_par] == 0){
					printk("Error no par!!!\n");
					return 0;
				}
				printk("par[%d] = %s, ch = %s, parg = %s\n",n_par,par[n_par],ch,parg);
				par_accept = 0;
				n_par++;
			}
			else{
				arg[n_arg] = kmalloc((sizeof(char) * strlen(ch)) + 1,GFP_KERNEL);
				parg = kmalloc((sizeof(char) * strlen(ch)) + 1,GFP_KERNEL);
				if(!arg[n_arg]) return 0;
	////			arg[n_arg] = strcpy(arg[n_arg],ch);
	////			get_user(arg[n_arg], ch);
				if(copy_from_user(parg,ch,size)){
					printk("Error on copy arg!!!\n");
					return 0;
				}
				arg[n_arg] = strcpy(arg[n_arg],parg);
				kfree(parg);
				if(arg[n_arg] == 0){
					printk("Error no arg!!!\n");
					return 0;
				}
				printk("arg[%d] = %s, ch = %s, parg = %s\n",n_arg,arg[n_arg],ch,parg);
				par_accept = 1;
				n_arg++;

Вижу, что всё то же самое, только arg вместо par

			}
			n++;
		}
		if(!strcmp(ch,"$")){
			step = -1;
			n = n_par = n_arg = 0;
			if(act > 0)	{
				struct netfw_table *tab;
				switch(act){
					case NETFW_ADD_RULE:
						/*if(*/netfw_add_rule(to_table,0,it,par,arg,num)/*)*/;

Что это за num (последний параметр)? Оно всегда равно 0?

						       break;
					case NETFW_SET_POL:
						tab = netfw_get_table_by_name(to_table);
						if(tab)/*if(*/tab->set_rule(NETFW_SET_POL,0,0,0,it,0,0,0)/*)*/;
						else printk("NetFW: ERROR at set new policy for table - %s!!!\n",to_table);
						break;
				}
			}
			if(par) kfree(par);
			if(arg) kfree(arg);

Ага, значит если я первой строкой запишу "$", то получу oops. Очень хорошо.
Если ты считаешь, что внимательно следишь за указателями,
то вынужден огорчить: тебе это только кажется :(

			if(to_table) kfree(to_table);
			if(out) kfree(out);
			to_table = 0;
			num = 0;

ЗАПОМНИМ. num = 0. Запишем это на бумажке

		}
		step++;
		switch(step){
			//Action
			case 1:
				if(!strcmp(ch,"add")){
					act = NETFW_ADD_RULE;
				}
				if(!strcmp(ch,"del")){
					act = NETFW_DEL_RULE;
				}
				if(!strcmp(ch,"policy")){
					act = NETFW_SET_POL;
				}
				if(!strcmp(ch,"show")){
					act = NETFW_SHOW_RULE;
				}

А если ничего не совпало, нужно сделать NETFW_BAD_RULE например

				break;
			//Table name
			case 2:
					to_table = kmalloc(strlen(ch) + 1,GFP_ATOMIC);
					if(!to_table) return 0;
					to_table = strcpy(to_table,ch);

Тут ты попался. Между прочим, это я предлагал тебе в прошлый раз проверить,
делаешь ли ты copy_from_user везде где надо. Ты тут передал указатель из userspace
в strcpy. При этом если нужная страница окажется в подкачке на диске, произойдёт
pagefault в ядре и oops. Но это не связано с массивами par и arg и, значит,
не связано со второй приведённой ошибкой (то есть ясно, что ошибок как минимум две).

					break;
			//What to do ( aka "it" )
			case 3:
				if(!strcmp(ch,"accept")){
					it = NETFW_ACCEPT;
				}
				if(!strcmp(ch,"drop")){
					it = NETFW_DROP;
				}
				break;

Опять же, если не 'accept' и не 'drop', то что?

			//Num of elements
			case 4:
				num = netfw_char_to_int((char*)ch);

Если char_to_int сделано так же, как и int_to_char, то я уже ни в чём не уверен

				if(par) kfree(par);
				if(arg) kfree(arg);
				par = kmalloc((sizeof(char *) * num),GFP_ATOMIC);

Тэкс. а чему там у нас равно num? (смотрим в бумажку) ой, num = 0 :(
Молодой человек, если вы ещё не запутались,
то с таким подходом это недолго продлится

				if(!par) return 0;
				arg = kmalloc((sizeof(char *) * num),GFP_ATOMIC);
				if(!arg) return 0;
				par_accept = 1;
				break;
			}

		return 0;
	}
	/*****************************************************************/

Оргвыводы: передавать правила настроек firewall'а через write вообще неправильно.
Это надо делать через ioctl, передавая в ядро уже готовую структуру-правило с бинарными полями,
а не устраивать поток строк в ядро и обратно. Так что код лучше переписать заново,
потому что как в старой русской пословице говорится, поспешишь - людей насмешишь.

erDiZz
()
Ответ на: комментарий от erDiZz

	ssize_t netfw_write (struct file *f, const char *ch, size_t size, loff_t *loff)
	{
		char *parg;
		if(num > 0)if(n < num*2){
			if(par_accept){
				par[n_par] = kmalloc((sizeof(char) * strlen(ch)) + 1,GFP_KERNEL);
				parg = kmalloc((sizeof(char) * strlen(ch)) + 1,GFP_KERNEL);

Да, и вот это твоё strlen (ch) это тоже криминал. Перечитай конкретно главу про read/write. Ты пока в ядро данные не скопировал, с ними работать не имеешь права.

erDiZz
()
Ответ на: комментарий от erDiZz

Хотелось бы сразу сказать спасибо за подробные камменты, они как всегда рулят. Недолго откладывая хотелось бы уточнить как через ioctl можно передавать данные, просто когда пытался выбрать способ записи в модуль, то хотел использовать его но не понял как (может недосмотрел что).

Вооббщем по порядку.

/* void int_to_char(int n)

Прекрасно. У нас тут функция, принимающая int, ничего не возвращающая и при этом "переводящая int в char". Модификация глобальной переменной как побочный эффект. В страшном сне такое не приснится. */

так вот, поскольку реализация этой функции страшно как сон, то я её просто обозначил не описывая, забыл поставить многоточие.

/* Рекомендую писать perror ("open") - эта функция добавит расшифрованное содержимое переменной errno и получится, например, красивое "open: device or resource is busy" */

просто это прога всего лишь тестовая морда :)

/* Проверка !argv[N] лишена смысла: если параметр - пустая строка, то параметра нет (т.е. пришли к противоречию). А вообще это segfault, если параметров меньше N */

на сей камент как и на подобные, с твоего позволения, отвечу так - я стараюсь не ошибаться на вводе, а после будет добавлено как надо.

/* Программы не глючат. "Глючит" оборудование из-за помех. А в программах просто есть ошибки. */

ну да ладно :))) можно сказать, что оговорился

/* Второй приведённый сегфолт произойти мог ТОЛЬКО после одного из kfree, потому что в netfw_add_rule вход был произведён, а printk сработать ещё не успел. Вывод: ты ошибся при работе с par[] или arg[]. Это как минимум (ошибок-то может быть больше). В принципе, с такой подробной информацией о месте ошибки в форумы не обращаются. Это вопрос об уважении к себе и людям. */

ну если бы ошибка была только там, то ладно, а у мя она то есть то нет

/* Далее ты делаешь compy_from_user (.., .., size), поэтому тут потенциальная ошибка, хотя она и не должна проявляться при работе твоей клиентской программы. Правильно будте parg = kmalloc (size, ..) */

да вроде тут не должно быть ошибки...

/* Лучше return -EFAULT, потому что правильный клиент при получении нуля от write повторит попытку записи. (man 2 write: zero indicates that nothing was written, не более) */

клиент не правильный, ты его видел :)

/* Что это за num (последний параметр)? Оно всегда равно 0? */

нет

case 4: num = netfw_char_to_int((char*)ch); if(par) kfree(par); if(arg) kfree(arg); par = kmalloc((sizeof(char *) * num),GFP_ATOMIC);

/* Ага, значит если я первой строкой запишу "$", то получу oops. Очень хорошо. Если ты считаешь, что внимательно следишь за указателями, то вынужден огорчить: тебе это только кажется :( */

этому не бывать!!! %)

/* Тут ты попался. Между прочим, это я предлагал тебе в прошлый раз проверить, делаешь ли ты copy_from_user везде где надо. Ты тут передал указатель из userspace в strcpy. При этом если нужная страница окажется в подкачке на диске, произойдёт pagefault в ядре и oops. Но это не связано с массивами par и arg и, значит, не связано со второй приведённой ошибкой (то есть ясно, что ошибок как минимум две). */

тут согласен, недоглядел...

/* Оргвыводы: передавать правила настроек firewall'а через write вообще неправильно. Это надо делать через ioctl, передавая в ядро уже готовую структуру-правило с бинарными полями, а не устраивать поток строк в ядро и обратно. Так что код лучше переписать заново, потому что как в старой русской пословице говорится, поспешишь - людей насмешишь. */

Собсно а можно какуюнить ссылочку или ещё что о том как это реализовать. Т.е. как передать.

cyclon ★★★★★
() автор топика
Ответ на: комментарий от cyclon

С функцией int_to_char мне, похоже, повезло, и я её видел в полном варианте. Больше никогда не усну :)

Через ioctl на самом деле работать гораздо проще, чем через write.

По сути ioctl - это такая функция с номером, принимающая один параметр
(int, но на самом деле тип любой - лишь бы был размером 4 байта), так что у каждого устройства может быть несколько ioctl, все с
разными номерами. Делается так (подробнее см. в книжке):

1) Пишем .h-файл с описанием номера ioctl. Макрос _IO и "магическое число" нужны для того, чтобы номера ioctl меньше совпадали, и если ты например по ошибке откроешь другое устройство, то не произошло ничего страшного. Для примера будем передавать в ядро структуру MyRule

----- mymodule.h ----

/* Сразу замечу: чтобы передать в ядро строку, нужно либо вместе
 * со строкой указывать её длину, либо ограничить эту длину
 * небольшим значением 
 */
typedef struct {
    long number;
    unsigned long ipv4_address;
    unsigned shore tcp_port;
    char *string;             /* Строка произвольной длины... */
    unsigned long string_len; /* ...и тут же указываем длину,
                               * иначе никак. В этом примере
                               * string_len - длина, считая '\0'
                               * в конце строки */
    char username [64];       /* Строка фиксированной длины - так проще всего */
} MyRule;

#define MYMODULE_IOC_MAGIC 0xcf
#define MYMODULE_IOCTL_FOO _IO (MYMODULE_IOC_MAGIC, 0)
#define MYMODULE_IOCTL_BAR _IO (MYMODULE_IOC_MAGIC, 1)

-----------------

2) в модуле ядра:

static int my_ioctl (struct inode *inode, struct file *filp,
                     unsigned int cmd, unsigned long arg)
{
    switch (cmd) {
    case MYMODULE_IOCTL_FOO: {
        /* Параметром этой ioctl будет указатель на структуру MyRule.
         * Этот указатель относится к виртуальному адресному
         * пространству процесса, поэтому сначала нужно перенести
         * структуру "в ядро", то есть в физическую память.
         * Размер структуры небольшой (хотя ~80 байт - это уже
         * многовато), поэтому можем позволить себе разместить её
         * в стеке ядра (помним, что его размер - всеко 4 Кб). Если
         * понадобится передавать бОльшую структуру, то придётся
         * поработать kmalloc'ом, иначе рискуем переполнить стек
         * и обрушить систему */
        MyRule rule;
        char *user_string;

        if (copy_from_user (&rule, (void*) arg, sizeof rule)
            return -EFAULT;

        /* Ещё не всё, потому что строку переменной длины string
         * тоже надо скопировать.
         * ((MyRule*) arg)->string писать нельзя! */
        user_string = rule.string;
        /* strlen (rule.string) делать тоже нельзя! */
        rule.string = kmalloc (rule.string_len, GPF_KERNEL);
        if (rule.string == NULL)
            return -ENOMEM;

        if (copy_from_user (rule.string, user_string, rule.string_len)) {
            kfree (rule.string);
            return -EFAULT;
        }

        /* ВСЁ. Теперь можно делать с rule всё, что угодно*/

        /* Убираем за собой */
        kfree (rule.string);

        return 0;
    }
    case MYMODULE_IOC_BAR:
        /* Пустой ioctl для примера, который ничего не делает */
        return 0;
    }

    /* Вызвана ioctl с неправильным номером */
    return -EINVAL;
}

/* Описание символьного устройства */
struct file_operations my_fops =
{
    ...
    ioctl: my_ioctl;
};

Также надо помнить, что если с устройством могут одновременно
работать несколько процессов (например, программу-клиент
настройки firewall запустили одновременно два админа на разных
консолях - вполне жизненный вариант), то нужно следить за
одновременной работой ioctl (как обычно, в общем. locking)

-----------------

3) в клиентской программе:

#include "mymodule/mymodule.h"

int fd;
int r;
MyRule rule;

/* Открываем файл, заполняем rule и т п */

...

/* Вызываем ioctl */
r = ioctl (fd, MYMODULE_IOCTL_FOO, &rule);
if (r < 0) {
    perror ("ioctl (FOO)");
    /* Обрабатываем ошибку */
}

Конечно я мог и ошибиться в чём-нибудь выше.
В любом случае, надеюсь, стало понятнее.

erDiZz
()
Ответ на: комментарий от erDiZz

Да, ещё нужно помнить, что если модуль и клиентская программа
собираются раздельно на разных системах,
то могут возникнуть проблемы с выравниванием полей в структурах
(например, из-за разных версий gcc или если пользователь
вручную задаёт выравнивание в CFLAGS), поэтому передаваемую
структуру лучше от греха подальше объявить с атрибутом packed,
тогда она выравниванию подвергаться не будет, то есть получится

typedef struct {
    long number;
    unsigned long ipv4_address;
    unsigned shore tcp_port;
    char *string;             /* Строка произвольной длины... */
    unsigned long string_len; /* ...и тут же указываем длину,
                               * иначе никак. В этом примере
                               * string_len - длина, считая '\0'
                               * в конце строки */
    char username [64];       /* Строка фиксированной длины - так проще всего */
} __attribute__ ((packed)) MyRule;

erDiZz
()
Ответ на: комментарий от erDiZz

erDiZz, моя благодарнасть просто не знает границ. Огромное спасибо за помощь. Пойду реализовывать :)

cyclon ★★★★★
() автор топика
Ответ на: комментарий от cyclon

> С функцией int_to_char мне, похоже, повезло, и я её видел в полном варианте. Больше никогда не усну :)

Ты даже не представляешь сколько я тогда выпил и выкурил что бы такое написать :)

Пы.Сы. просто ща отдыхаю за городом, а тут сам понимаешь :))))

cyclon ★★★★★
() автор топика
Ответ на: комментарий от cyclon

Кстати а где можно почитать о таких вещах как размер стека ядра, поля, их смещение и применение различных атрибутов и пр. ???

Желательно ссылочку на такую книжку ну или название.

cyclon ★★★★★
() автор топика

Все понятно. У тебя тут явное переполнение буфера. Перешиши все на схеме или лиспе, ибо cхема или лисп это язык на котором можно все, не в пример твоему дурацкому перлу. Давай давай.

anonymous
()
Ответ на: комментарий от cyclon

Да книжки всё те же:

Understanding the Linux Kernel,

Linux Device Drivers,

Linux Kernel Development.

Главгая "книжка" - исходники, это такой бесконечный пример, потому что в ядре всегда нужно смотреть, как делают другие (у кого за плечами опыта поболее)

Про __attribute__ ((packed)) вычитал в info gcc, раздел "C Extensions". Просто когда структуры куда-то передаются, в ядро или по сети куда-нибудь, всегда о выравнивании полей нужно думать, а packed - это подходящий способ. Не знаю, правда, как поступают с другими компиляторами (не gcc)

erDiZz
()
Ответ на: комментарий от erDiZz

erDiZz  ну ты сила! интересный топик.

anonymous
()

Хорошее обсуждение. Такое редкость на ЛОР-е в последние годы...

anonymous
()
Вы не можете добавлять комментарии в эту тему. Тема перемещена в архив.