Re: bug in entering/exiting promiscuous read mode
A number of people have mentioned that they get a weird situation where when
they *start* a program that does promiscuous network reads (with, say,
?tcpdump ?i eth0?). They then get a kernel message ?left promiscuous mode?
when the program starts and the message ?entered promiscuous mode? when it
exits ? the exact opposite of what should happen.? I?ve tracked this problem
down and fixed it in the 2.4.20 kernel sources.? I have also verified that
the problem still exists in the latest dev sources (2.5.73).? The patch file
is at the end of this email.
?
The problem comes when the interface is ?downed? the dev->promiscuity count
gets incorrectly decremented by 2.? This happens because the
dev_change_flags routine calls dev_mc_upload which decrements the count and
the count gets decremented a 2nd time when dev_change_flags calls dev_close
which calls
notifier_call_chain->packet_notifier->packet_dev_mclist->packet_dev_mc.? The
logic in dev_set_promiscuity is to test if dev->promiscuity == 0.? What
happens is the count is now set to -1.? When a new caller increments the
count by 1 to enter promisc mode but that means that the count is now set to
0 which means exit promisc; when the caller exits the count is decremented
and is once again set to -1 which means re-enter promisc mode (since the
count is not == 0).
?
There is a similar problem when the interface is enabled when there is
already a promisc reader.? The count is incremented twice.?
?
I?ve fixed the problem and verified the fix by running tcpdump and then
doing various combinations of ifconfig..up/down.? I also added one new
diagnostic message and changed the code so that it will not do the inverse
logic thing if the count should ever be wrong again in the future.
?
?
My solution involves minor changes to net/core/dev.c and
net/packet/af_packet.c.? The patch file is attached to this email.? The
changes are:
?
1.? Don?t increment/decrement the promisc count if the interface is not up
in packet_dev_mc routine.? Note: the request to be in promisc mode is still
stored on the multicast (mc) list so when the interface is enabled the
interface will be put back in promisc mode by dev_mc_upload as it should be.
?
2.? In packet_notifier call packet_dev_mclist to remove multicast/promisc
mode when the interface is GOING_DOWN not DOWN.? Since if it is ?DOWN? the
request would now be ignored.
?
3.? Change dev_set_promiscuity routine to check for a count of <= 0 and then
set the count to 0 and unset promisc mode.? What this means is that if the
count is ever wrong the interface will go out of promisc mode ?too early?.?
But a new program could still be started and it would enter promisc mode
again.? It will never invert the logic.
?
4.? Write a kernel info message whenever the use count is
incremented/decremented instead of *just* when promisc is turned on/off.? If
a 2nd caller uses setsockopt to put the interface in promisc mode, a message
would be generated:
?????????????? Jun 29 16:57:08 tabby kernel: device eth1 still in
promiscuous mode (use count now 2)
?
and when one of the existing callers closes the socket connection then the
message would be:
?????????????? Jun 29 16:57:20 tabby kernel: device eth1 still in
promiscuous mode (use count now 1)
?
?
For those who don?t want to patch the kernel, I?d say stay away from
ifconfig up/down if you have a promisc read operation going.
================= diff -u patch follows ===================
diff -ru linux-2.4.20-18.8/net/core/dev.c
linux-2.4.20-18.8-tablus/net/core/dev.c
--- linux-2.4.20-18.8/net/core/dev.c 2003-05-29 03:46:18.000000000 -0700
+++ linux-2.4.20-18.8-tablus/net/core/dev.c 2003-06-30
18:16:59.000000000 -0700
@@ -1949,8 +1949,10 @@
unsigned short old_flags = dev->flags;
dev->flags |= IFF_PROMISC;
- if ((dev->promiscuity += inc) == 0)
+ if ((dev->promiscuity += inc) <= 0) {
+ dev->promiscuity = 0;
dev->flags &= ~IFF_PROMISC;
+ }
if (dev->flags^old_flags) {
#ifdef CONFIG_NET_FASTROUTE
if (dev->flags&IFF_PROMISC) {
@@ -1963,6 +1965,12 @@
printk(KERN_INFO "device %s %s promiscuous mode\n",
dev->name, (dev->flags&IFF_PROMISC) ? "entered" :
"left");
}
+ else {
+ if (dev->flags&IFF_PROMISC)
+ printk(KERN_INFO "device %s still in promiscuous
mode"
+ " (use count now %d)\n",
+ dev->name, dev->promiscuity);
+ }
}
/**
diff -ru linux-2.4.20-18.8/net/packet/af_packet.c
linux-2.4.20-18.8-tablus/net/packet/af_packet.c
--- linux-2.4.20-18.8/net/packet/af_packet.c 2003-05-29
03:47:00.000000000 -0700
+++ linux-2.4.20-18.8-tablus/net/packet/af_packet.c 2003-06-30
18:19:47.000000000 -0700
@@ -1146,6 +1146,9 @@
#ifdef CONFIG_PACKET_MULTICAST
static void packet_dev_mc(struct net_device *dev, struct packet_mclist *i,
int what)
{
+ if (!(dev->flags & IFF_UP))
+ return;
+
switch (i->type) {
case PACKET_MR_MULTICAST:
if (what > 0)
@@ -1396,6 +1399,8 @@
}
spin_unlock(&po->bind_lock);
}
+ break;
+ case NETDEV_GOING_DOWN:
#ifdef CONFIG_PACKET_MULTICAST
if (po->mclist)
packet_dev_mclist(dev, po->mclist, -1);
-
: send the line "unsubscribe linux-net" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
