Skip to contentSkip to conversation

netlink: no need to crash if table does not exist.

PATCH In review Evgeniy Polyakov 3 participants 5 messages Mar 21, 2007 – Mar 28, 2007

Subsystems (2)
Networking Network drivers
lore
Share & tools
lore

Evgeniy Polyakov

author
#0 lore raw

PATCH 1/1 netlink: no need to crash if table does not exist.

We would already do that on init.
Some things become very confused, when nl_table is not used to store netlink sockets.

Signed-off-by Evgeniy Polyakov <[email protected]>

23ebdcf1f439cde050a63f33897d5b099fe08c95

net/netlink/af_netlink.c +0 -2
index 9b69d9b..071e4d7 100644
@@ -1330,8 +1330,6 @@ netlink_kernel_create(int unit, unsigned int groups,
 	struct netlink_sock *nlk;
 	unsigned long *listeners = NULL;
 
-	BUG_ON(!nl_table);
-
 	if (unit<0 || unit>=MAX_LINKS)
 		return NULL;
Signature · 1 line
	Evgeniy Polyakov
#1 lore raw
Quoted 3 lines · Evgeniy Polyakov

We would already do that on init.
Some things become very confused, when nl_table is not used to store netlink sockets.

Its unnecessary, but I don't understand what the problem is. Why would it be NULL and what gets confused?

author
#2 lore raw
Quoted 8 lines · Patrick McHardy
Quoted 3 lines · Evgeniy Polyakov

We would already do that on init.
Some things become very confused, when nl_table is not used to store netlink sockets.

Its unnecessary, but I don't understand what the problem is. Why would it be NULL and what gets confused?

There is no problem as-is, but I implement unified cache for different sockets (currently tcp/udp/raw and netlink are supported), which does not use that table, so I currently wrap all access code into special ifdefs, this one can be wrapped too, but since it is not needed, it saves couple of lines of code.

Signature · 1 line
	Evgeniy Polyakov
maintainer
#3 lore raw

From: Evgeniy Polyakov <[email protected]>
Date: Wed, 21 Mar 2007 13:58:47 +0300

Quoted 7 lines · Patrick McHardy
Quoted 6 lines
Quoted 3 lines · Evgeniy Polyakov

We would already do that on init.
Some things become very confused, when nl_table is not used to store netlink sockets.

Quoted 8 lines
Quoted 2 lines

Its unnecessary, but I don't understand what the problem is. Why would it be NULL and what gets confused?

There is no problem as-is, but I implement unified cache for different sockets (currently tcp/udp/raw and netlink are supported), which does not use that table, so I currently wrap all access code into special ifdefs, this one can be wrapped too, but since it is not needed, it saves couple of lines of code.

It is needed. It is there to make sure that a kernel netlink socket is not created before the af_netlink init code runs.

We've had sequencing bugs like that in the initcall call chain in the past, that's why the check is there.

author
#4 lore raw
Quoted 11 lines · David Miller
Quoted 5 lines

There is no problem as-is, but I implement unified cache for different sockets (currently tcp/udp/raw and netlink are supported), which does not use that table, so I currently wrap all access code into special ifdefs, this one can be wrapped too, but since it is not needed, it saves couple of lines of code.

It is needed. It is there to make sure that a kernel netlink socket is not created before the af_netlink init code runs.

We've had sequencing bugs like that in the initcall call chain in the past, that's why the check is there.

Argh, I see.
I fail to find exact commit (at least it was not in 2.4 and was created before 2.6.12), but it is ineed neeed.

Thanks for explaination.

Signature · 1 line
	Evgeniy Polyakov