Good morning to all.
Yesterday, talking with Jan Engelhardt about an issue that happened to
me some time ago,
I would like to submit the question to your attention, to go deeper
into the problem.
Context details:
1. kernel module registered with netfilter hooks that takes the socket buffer
reads it and sends a verdict after consulting a ruleset.
2. the ruleset is a linked list of rules which is given to the kernel
via netlink
socket.
- struct command contains the rule to pass to kernel space:
struct command
{
short cmd; /* command type */
union{
ipfire_rule rule;
struct firesizes fwsizes;
}content;
int anumber; /* a number reserved for some cmd values */
/* .... other fields */
}
this command struct is about 800 bytes large and ipfire_rule structure
(which you
can see inside content union) contains integers and arrays of
integers, for instance
struct ipfire_rule
{
int a, b, c;
__u32 srcaddress[50];
__u32 dstaddress[50];
......
}
this programming pattern was causing serious problems:
int f()
{
struct command cmd;
/* modify cmd ... */
return g(&cmd);
}
int g(struct command *com)
{
com->a = 2;
/* .... and so on modify com */
return h(com);
}
int h(struct command *com)
{
com->a = 3; /* for example */
/* here in particular I copy com into a socket buffer and send
* the skb via netlink socket.
*/
return 0; /* returns to f() */
}
This pattern caused reproducible kernel panics, with many different messages
each time (NULL Pointer dereference, scheduling while atomic, or
nothing.. just hang)
The following pattern solved:
int f()
{
int ret;
struct command *cmd;
cmd = kmalloc(sizeof(struct command), GFP_ATOMIC);
ret = g(cmd);
kfree(cmd);
return ret;
}
The question is: has the first programming pattern something wrong in principle?
I know it hasn't in userspace, but in kernel, with preemption enabled
and inside the
context described?
Another detail: the first pattern used to work for a long time, until
the rule structure,
and so the command structure, grew in size, after the addition of
__u32 src_address[50]; /* and dst_address */
which before simply was
__u32 src_address.
Or is the pattern perfectly allowed, and I have to look for some other bug?
Thanks in advance.
Giacomo.
--
Giacomo S.
http://www.giacomos.it
- - - - - - - - - - - - - - - - - - - - - -
* Aprile 2008: iqfire-wall, un progetto
open source che implementa un
filtro di pacchetti di rete per Linux,
e` disponibile per il download qui:
http://sourceforge.net/projects/ipfire-wall
* Informazioni e pagina web ufficiale:
http://www.giacomos.it/iqfire/index.html
- - - - - - - - - - - - - - - - - - - - - -
. '' `.
: :' :
`. ` '
`- Debian GNU/Linux -- The power of freedom
http://www.debian.org
--
To unsubscribe from this list: send the line "unsubscribe linux-net" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
