Уже задавал подобный вопрос но ответа так и не получил, веренее была
пара советов, но они не оказали эффекта.
Так что повторюсь. Имеется модуль, в нём реализована функция 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;
}
/*****************************************************************/
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;
}
/*****************************************************************/
Вывод: и что? если с вводить строго задуманное количество параметров,
то летального исхода не будет (нормально всё)
Едем дальше.
А вот та часть кода модуля которая глючит.
Программы не глючат. "Глючит" оборудование из-за помех. А в программах просто есть ошибки.
/*****************************************************************/
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, передавая в ядро уже готовую структуру-правило с бинарными полями,
а не устраивать поток строк в ядро и обратно. Так что код лучше переписать заново,
потому что как в старой русской пословице говорится, поспешишь - людей насмешишь.
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. Ты пока в ядро данные не скопировал, с ними работать не имеешь права.
Хотелось бы сразу сказать спасибо за подробные камменты, они как всегда рулят. Недолго откладывая хотелось бы уточнить как через 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, передавая в ядро уже готовую структуру-правило с бинарными полями,
а не устраивать поток строк в ядро и обратно. Так что код лучше переписать заново,
потому что как в старой русской пословице говорится, поспешишь - людей насмешишь.
*/
Собсно а можно какуюнить ссылочку или ещё что о том как это реализовать. Т.е. как передать.
С функцией 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)");
/* Обрабатываем ошибку */
}
Конечно я мог и ошибиться в чём-нибудь выше.
В любом случае, надеюсь, стало понятнее.
Да, ещё нужно помнить, что если модуль и клиентская программа
собираются раздельно на разных системах,
то могут возникнуть проблемы с выравниванием полей в структурах
(например, из-за разных версий 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;
Все понятно. У тебя тут явное переполнение буфера. Перешиши все на схеме или лиспе, ибо cхема или лисп это язык на котором можно все, не в пример твоему дурацкому перлу. Давай давай.
Главгая "книжка" - исходники, это такой бесконечный пример, потому что в ядре всегда нужно смотреть, как делают другие (у кого за плечами опыта поболее)
Про __attribute__ ((packed)) вычитал в info gcc, раздел "C Extensions". Просто когда структуры куда-то передаются, в ядро или по сети куда-нибудь, всегда о выравнивании полей нужно думать, а packed - это подходящий способ. Не знаю, правда, как поступают с другими компиляторами (не gcc)