* [PATCH 3/4] netfilter: xtables: fix reentrancy
From: kaber @ 2011-03-20 15:32 UTC (permalink / raw)
To: davem; +Cc: netfilter-devel, netdev
In-Reply-To: <1300635148-22648-1-git-send-email-kaber@trash.net>
From: Eric Dumazet <eric.dumazet@gmail.com>
commit f3c5c1bfd4308 (make ip_tables reentrant) introduced a race in
handling the stackptr restore, at the end of ipt_do_table()
We should do it before the call to xt_info_rdunlock_bh(), or we allow
cpu preemption and another cpu overwrites stackptr of original one.
A second fix is to change the underflow test to check the origptr value
instead of 0 to detect underflow, or else we allow a jump from different
hooks.
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Jan Engelhardt <jengelh@medozas.de>
Signed-off-by: Patrick McHardy <kaber@trash.net>
---
net/ipv4/netfilter/ip_tables.c | 4 ++--
net/ipv6/netfilter/ip6_tables.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c
index b09ed0d..ffcea0d 100644
--- a/net/ipv4/netfilter/ip_tables.c
+++ b/net/ipv4/netfilter/ip_tables.c
@@ -387,7 +387,7 @@ ipt_do_table(struct sk_buff *skb,
verdict = (unsigned)(-v) - 1;
break;
}
- if (*stackptr == 0) {
+ if (*stackptr <= origptr) {
e = get_entry(table_base,
private->underflow[hook]);
pr_debug("Underflow (this is normal) "
@@ -427,10 +427,10 @@ ipt_do_table(struct sk_buff *skb,
/* Verdict */
break;
} while (!acpar.hotdrop);
- xt_info_rdunlock_bh();
pr_debug("Exiting %s; resetting sp from %u to %u\n",
__func__, *stackptr, origptr);
*stackptr = origptr;
+ xt_info_rdunlock_bh();
#ifdef DEBUG_ALLOW_ALL
return NF_ACCEPT;
#else
diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c
index c9598a9..0b2af9b 100644
--- a/net/ipv6/netfilter/ip6_tables.c
+++ b/net/ipv6/netfilter/ip6_tables.c
@@ -410,7 +410,7 @@ ip6t_do_table(struct sk_buff *skb,
verdict = (unsigned)(-v) - 1;
break;
}
- if (*stackptr == 0)
+ if (*stackptr <= origptr)
e = get_entry(table_base,
private->underflow[hook]);
else
@@ -441,8 +441,8 @@ ip6t_do_table(struct sk_buff *skb,
break;
} while (!acpar.hotdrop);
- xt_info_rdunlock_bh();
*stackptr = origptr;
+ xt_info_rdunlock_bh();
#ifdef DEBUG_ALLOW_ALL
return NF_ACCEPT;
--
1.7.4
^ permalink raw reply related
* [PATCH 4/4] netfilter: ipt_CLUSTERIP: fix buffer overflow
From: kaber @ 2011-03-20 15:32 UTC (permalink / raw)
To: davem; +Cc: netfilter-devel, netdev
In-Reply-To: <1300635148-22648-1-git-send-email-kaber@trash.net>
From: Vasiliy Kulikov <segoon@openwall.com>
'buffer' string is copied from userspace. It is not checked whether it is
zero terminated. This may lead to overflow inside of simple_strtoul().
Changli Gao suggested to copy not more than user supplied 'size' bytes.
It was introduced before the git epoch. Files "ipt_CLUSTERIP/*" are
root writable only by default, however, on some setups permissions might be
relaxed to e.g. network admin user.
Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>
Acked-by: Changli Gao <xiaosuo@gmail.com>
Signed-off-by: Patrick McHardy <kaber@trash.net>
---
net/ipv4/netfilter/ipt_CLUSTERIP.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/net/ipv4/netfilter/ipt_CLUSTERIP.c b/net/ipv4/netfilter/ipt_CLUSTERIP.c
index 403ca57..d609ac3 100644
--- a/net/ipv4/netfilter/ipt_CLUSTERIP.c
+++ b/net/ipv4/netfilter/ipt_CLUSTERIP.c
@@ -664,8 +664,11 @@ static ssize_t clusterip_proc_write(struct file *file, const char __user *input,
char buffer[PROC_WRITELEN+1];
unsigned long nodenum;
- if (copy_from_user(buffer, input, PROC_WRITELEN))
+ if (size > PROC_WRITELEN)
+ return -EIO;
+ if (copy_from_user(buffer, input, size))
return -EFAULT;
+ buffer[size] = 0;
if (*buffer == '+') {
nodenum = simple_strtoul(buffer+1, NULL, 10);
--
1.7.4
^ permalink raw reply related
* [PATCH 0/4] netfilter: netfilter fixes
From: kaber @ 2011-03-20 15:32 UTC (permalink / raw)
To: davem; +Cc: netfilter-devel, netdev
Hi Dave,
the following patches fix a couple of netfilter bugs:
- a bug in ipset which caused the hash:port set types to silently
ignore address ranges when non-TCP/UDP entries were added, from
Jozsef
- a missing check for set type revision in ipset, from Jozsef
- a fix for iptables reentrancy, from Eric
- a fix for a buffer overflow in the ipt_CLUSTERIP proc handler,
from Vasiliy Kulikov
Please apply or pull from:
git://git.kernel.org/pub/scm/linux/kernel/git/kaber/nf-2.6.git master
Thanks!
^ permalink raw reply
* Re: [RFC v3 5/6] j1939: add documentation and MAINTAINERS
From: Oliver Hartkopp @ 2011-03-20 15:56 UTC (permalink / raw)
To: Kurt Van Dijck
Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20110314135917.GF333-MxZ6Iy/zr/UdbCeoMzGj59i2O/JbrIOy@public.gmane.org>
On 14.03.2011 14:59, Kurt Van Dijck wrote:
> This patch adds the documentation and MAINTAINERS.
Hello Kurt,
even after our F2F-discussion on the Embedded World i'm still not convinced,
why it should be a good idea to handle all the address claiming process inside
the kernel.
Besides the fact, that other j1939 implementation are *completely* implemented
in userspace (and can cope with the time restrictions), i do not see why you
put the address claiming into the kernel and not only the transport layer stuff.
The address claiming can be compared to something like DHCP or DNS in the
internet protocol world, that are both handled and implemented in userspace
apps or userspace libraries.
E.g. these bits from the documentation look like you are starting some kind of
'addressing service' daemon:
> +4.1 rtnetlink interface
> +
> + Per default j1939 is not active. Specifying can_ifindex != 0 in bind(2)
> + or connect(2) needs an active j1939 on that interface. You must have done
> + $ ip link set canX j1939 on
> + on that interface.
> +
> + $ ip link set canX j1939 down
> + disables j1939 on canX.
You are activating the 'addressing service' on specific CAN interfaces.
Then you suggest to attach static and/or dynamic addresses to the interface.
> + Assigning addresses is done via
> + $ ip addr add dev canX j1939 0xXX
> + statically or
> + $ ip addr add dev canX j1939 name 0xXX
> + dynamically. In the latter case, address claiming must take place
> + before other traffic can leave.
like you would have using DHCP/DNS (adapted for j1939) ...
> + Removing addresses is done similarly via
> + $ ip addr del dev canX j1939 0xXX
> + $ ip addr del dev canX j1939 name 0xXX
> +
> + A static address cannot be assigned together with a 64bit name.
Ah. You provide two kernel interfaces instead of handling the address claiming
in userspace and provide only one simple (static) interface to the kernel.
This artifact brings this fact out again:
> + can_addr.j1939.name contains the 64-bit J1939 NAME.
> +
> + can_addr.j1939.addr contains the source address.
> +
> + When sending data, the source address is applied as follows: If
> + can_addr.j1939.name != 0 the NAME is looked up by the kernel and the
> + corresponding Source Address is used. If can_addr.j1939.name == 0,
> + can_addr.j1939.addr is used.
Yes. You are providing two programming interfaces to the kernel that can be
used exclusive-OR only.
As other j1939 implementations implement the address claiming in userspace
too, there's no necessity to implement this inside the kernel. DHCP and DNS
can also lead to address changes and some 'new j1939 addressing daemon' could
be implemented in a way that allows to tell registered j1939 apps address
changes in a fast way (trigger/signal/select/whatever). I don't know how much
of the 4838 lines of code from your RFC can be removed - but i assume it would
vastly reduce the complexity of your posted j1939 implementation.
If you would implement only the j1939 transport layer stuff (using static
addressing) together with some userspace 'address claiming' daemon and/or
library (compared to DNS) this would make more sense to me. Especially it
makes the kernel API clear and simple(!). As a side-effect this would remove
the need for a separate system-wide configuration which would need to be added
with your suggested netlink extensions in af_can.c, iproute2 and the other
adaptions to the current code to extend the sockaddr_can in size.
I know you're a fan of j1939 address claiming inside the kernel but IMO this
does not fit other networking addressing concepts inside the kernel and messes
up the kernel API for j1939, sigh.
If it makes sense to you, i would offer my help to implement the address
claiming daemon based on CAN_RAW and CAN_BCM ... 8-)
Anyway: Thanks for you contribution!
Oliver
^ permalink raw reply
* [net-next 2/5] bnx2x: allow WoL for every function in MF modes
From: Dmitry Kravkov @ 2011-03-20 16:16 UTC (permalink / raw)
To: davem, netdev; +Cc: Eilon Greenstein
Signed-off-by: Dmitry Kravkov <dmitry@broadcom.com>
Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
---
drivers/net/bnx2x/bnx2x_main.c | 10 +++-------
1 files changed, 3 insertions(+), 7 deletions(-)
diff --git a/drivers/net/bnx2x/bnx2x_main.c b/drivers/net/bnx2x/bnx2x_main.c
index bc847b1..259d1d9 100644
--- a/drivers/net/bnx2x/bnx2x_main.c
+++ b/drivers/net/bnx2x/bnx2x_main.c
@@ -8063,13 +8063,9 @@ static void __devinit bnx2x_get_common_hwinfo(struct bnx2x *bp)
(val >= REQ_BC_VER_4_VRFY_SPECIFIC_PHY_OPT_MDL) ?
FEATURE_CONFIG_BC_SUPPORTS_DUAL_PHY_OPT_MDL_VRFY : 0;
- if (BP_E1HVN(bp) == 0) {
- pci_read_config_word(bp->pdev, bp->pm_cap + PCI_PM_PMC, &pmc);
- bp->flags |= (pmc & PCI_PM_CAP_PME_D3cold) ? 0 : NO_WOL_FLAG;
- } else {
- /* no WOL capability for E1HVN != 0 */
- bp->flags |= NO_WOL_FLAG;
- }
+ pci_read_config_word(bp->pdev, bp->pm_cap + PCI_PM_PMC, &pmc);
+ bp->flags |= (pmc & PCI_PM_CAP_PME_D3cold) ? 0 : NO_WOL_FLAG;
+
BNX2X_DEV_INFO("%sWoL capable\n",
(bp->flags & NO_WOL_FLAG) ? "not " : "");
--
1.7.2.2
^ permalink raw reply related
* [net-next 3/5] bnx2x: Do storage mac address validation for SF mode.
From: Dmitry Kravkov @ 2011-03-20 16:17 UTC (permalink / raw)
To: davem, netdev; +Cc: Eilon Greenstein
Signed-off-by: Dmitry Kravkov <dmitry@broadcom.com>
Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
---
drivers/net/bnx2x/bnx2x_main.c | 32 ++++++++++++++++----------------
1 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/drivers/net/bnx2x/bnx2x_main.c b/drivers/net/bnx2x/bnx2x_main.c
index 259d1d9..32f9f7f 100644
--- a/drivers/net/bnx2x/bnx2x_main.c
+++ b/drivers/net/bnx2x/bnx2x_main.c
@@ -8571,15 +8571,6 @@ static void __devinit bnx2x_get_mac_hwinfo(struct bnx2x *bp)
BNX2X_DEV_INFO("Read iSCSI MAC: "
"0x%x:0x%04x\n", val2, val);
bnx2x_set_mac_buf(iscsi_mac, val, val2);
-
- /* Disable iSCSI OOO if MAC configuration is
- * invalid.
- */
- if (!is_valid_ether_addr(iscsi_mac)) {
- bp->flags |= NO_ISCSI_OOO_FLAG |
- NO_ISCSI_FLAG;
- memset(iscsi_mac, 0, ETH_ALEN);
- }
} else
bp->flags |= NO_ISCSI_OOO_FLAG | NO_ISCSI_FLAG;
@@ -8592,13 +8583,6 @@ static void __devinit bnx2x_get_mac_hwinfo(struct bnx2x *bp)
"0x%x:0x%04x\n", val2, val);
bnx2x_set_mac_buf(fip_mac, val, val2);
- /* Disable FCoE if MAC configuration is
- * invalid.
- */
- if (!is_valid_ether_addr(fip_mac)) {
- bp->flags |= NO_FCOE_FLAG;
- memset(bp->fip_mac, 0, ETH_ALEN);
- }
} else
bp->flags |= NO_FCOE_FLAG;
}
@@ -8629,6 +8613,22 @@ static void __devinit bnx2x_get_mac_hwinfo(struct bnx2x *bp)
else if (!IS_MF(bp))
memcpy(fip_mac, iscsi_mac, ETH_ALEN);
}
+
+ /* Disable iSCSI if MAC configuration is
+ * invalid.
+ */
+ if (!is_valid_ether_addr(iscsi_mac)) {
+ bp->flags |= NO_ISCSI_FLAG;
+ memset(iscsi_mac, 0, ETH_ALEN);
+ }
+
+ /* Disable FCoE if MAC configuration is
+ * invalid.
+ */
+ if (!is_valid_ether_addr(fip_mac)) {
+ bp->flags |= NO_FCOE_FLAG;
+ memset(bp->fip_mac, 0, ETH_ALEN);
+ }
#endif
}
--
1.7.2.2
^ permalink raw reply related
* [net-next 5/5] bnx2x: update year to 2011 and version to 1.62.12-0
From: Dmitry Kravkov @ 2011-03-20 16:27 UTC (permalink / raw)
To: davem, netdev; +Cc: Eilon Greenstein
Signed-off-by: Dmitry Kravkov <dmitry@broadcom.com>
Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
---
drivers/net/bnx2x/bnx2x.h | 6 +++---
drivers/net/bnx2x/bnx2x_cmn.c | 2 +-
drivers/net/bnx2x/bnx2x_cmn.h | 2 +-
drivers/net/bnx2x/bnx2x_dcb.c | 2 +-
drivers/net/bnx2x/bnx2x_dcb.h | 2 +-
drivers/net/bnx2x/bnx2x_ethtool.c | 2 +-
drivers/net/bnx2x/bnx2x_fw_defs.h | 2 +-
drivers/net/bnx2x/bnx2x_fw_file_hdr.h | 2 +-
drivers/net/bnx2x/bnx2x_hsi.h | 2 +-
drivers/net/bnx2x/bnx2x_init.h | 2 +-
drivers/net/bnx2x/bnx2x_init_ops.h | 2 +-
drivers/net/bnx2x/bnx2x_main.c | 2 +-
drivers/net/bnx2x/bnx2x_reg.h | 2 +-
drivers/net/bnx2x/bnx2x_stats.c | 2 +-
drivers/net/bnx2x/bnx2x_stats.h | 2 +-
15 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/drivers/net/bnx2x/bnx2x.h b/drivers/net/bnx2x/bnx2x.h
index 86e23c7..22a2045 100644
--- a/drivers/net/bnx2x/bnx2x.h
+++ b/drivers/net/bnx2x/bnx2x.h
@@ -1,6 +1,6 @@
/* bnx2x.h: Broadcom Everest network driver.
*
- * Copyright (c) 2007-2010 Broadcom Corporation
+ * Copyright (c) 2007-2011 Broadcom Corporation
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
@@ -22,8 +22,8 @@
* (you will need to reboot afterwards) */
/* #define BNX2X_STOP_ON_ERROR */
-#define DRV_MODULE_VERSION "1.62.11-0"
-#define DRV_MODULE_RELDATE "2011/01/31"
+#define DRV_MODULE_VERSION "1.62.12-0"
+#define DRV_MODULE_RELDATE "2011/03/20"
#define BNX2X_BC_VER 0x040200
#define BNX2X_MULTI_QUEUE
diff --git a/drivers/net/bnx2x/bnx2x_cmn.c b/drivers/net/bnx2x/bnx2x_cmn.c
index 9632f85..8c63cb7 100644
--- a/drivers/net/bnx2x/bnx2x_cmn.c
+++ b/drivers/net/bnx2x/bnx2x_cmn.c
@@ -1,6 +1,6 @@
/* bnx2x_cmn.c: Broadcom Everest network driver.
*
- * Copyright (c) 2007-2010 Broadcom Corporation
+ * Copyright (c) 2007-2011 Broadcom Corporation
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
diff --git a/drivers/net/bnx2x/bnx2x_cmn.h b/drivers/net/bnx2x/bnx2x_cmn.h
index b50570c..cdaf64e 100644
--- a/drivers/net/bnx2x/bnx2x_cmn.h
+++ b/drivers/net/bnx2x/bnx2x_cmn.h
@@ -1,6 +1,6 @@
/* bnx2x_cmn.h: Broadcom Everest network driver.
*
- * Copyright (c) 2007-2010 Broadcom Corporation
+ * Copyright (c) 2007-2011 Broadcom Corporation
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
diff --git a/drivers/net/bnx2x/bnx2x_dcb.c b/drivers/net/bnx2x/bnx2x_dcb.c
index 9a24d79..919d6fa 100644
--- a/drivers/net/bnx2x/bnx2x_dcb.c
+++ b/drivers/net/bnx2x/bnx2x_dcb.c
@@ -1,6 +1,6 @@
/* bnx2x_dcb.c: Broadcom Everest network driver.
*
- * Copyright 2009-2010 Broadcom Corporation
+ * Copyright 2009-2011 Broadcom Corporation
*
* Unless you and Broadcom execute a separate written software license
* agreement governing use of this software, this software is licensed to you
diff --git a/drivers/net/bnx2x/bnx2x_dcb.h b/drivers/net/bnx2x/bnx2x_dcb.h
index 71b8eda..369d592 100644
--- a/drivers/net/bnx2x/bnx2x_dcb.h
+++ b/drivers/net/bnx2x/bnx2x_dcb.h
@@ -1,6 +1,6 @@
/* bnx2x_dcb.h: Broadcom Everest network driver.
*
- * Copyright 2009-2010 Broadcom Corporation
+ * Copyright 2009-2011 Broadcom Corporation
*
* Unless you and Broadcom execute a separate written software license
* agreement governing use of this software, this software is licensed to you
diff --git a/drivers/net/bnx2x/bnx2x_ethtool.c b/drivers/net/bnx2x/bnx2x_ethtool.c
index 14ed2aa..611a21e 100644
--- a/drivers/net/bnx2x/bnx2x_ethtool.c
+++ b/drivers/net/bnx2x/bnx2x_ethtool.c
@@ -1,6 +1,6 @@
/* bnx2x_ethtool.c: Broadcom Everest network driver.
*
- * Copyright (c) 2007-2010 Broadcom Corporation
+ * Copyright (c) 2007-2011 Broadcom Corporation
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
diff --git a/drivers/net/bnx2x/bnx2x_fw_defs.h b/drivers/net/bnx2x/bnx2x_fw_defs.h
index f4e5b1c..9fe3678 100644
--- a/drivers/net/bnx2x/bnx2x_fw_defs.h
+++ b/drivers/net/bnx2x/bnx2x_fw_defs.h
@@ -1,6 +1,6 @@
/* bnx2x_fw_defs.h: Broadcom Everest network driver.
*
- * Copyright (c) 2007-2010 Broadcom Corporation
+ * Copyright (c) 2007-2011 Broadcom Corporation
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
diff --git a/drivers/net/bnx2x/bnx2x_fw_file_hdr.h b/drivers/net/bnx2x/bnx2x_fw_file_hdr.h
index f807262..f4a07fb 100644
--- a/drivers/net/bnx2x/bnx2x_fw_file_hdr.h
+++ b/drivers/net/bnx2x/bnx2x_fw_file_hdr.h
@@ -1,6 +1,6 @@
/* bnx2x_fw_file_hdr.h: FW binary file header structure.
*
- * Copyright (c) 2007-2009 Broadcom Corporation
+ * Copyright (c) 2007-2011 Broadcom Corporation
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
diff --git a/drivers/net/bnx2x/bnx2x_hsi.h b/drivers/net/bnx2x/bnx2x_hsi.h
index be503cc..6381a54 100644
--- a/drivers/net/bnx2x/bnx2x_hsi.h
+++ b/drivers/net/bnx2x/bnx2x_hsi.h
@@ -1,6 +1,6 @@
/* bnx2x_hsi.h: Broadcom Everest network driver.
*
- * Copyright (c) 2007-2010 Broadcom Corporation
+ * Copyright (c) 2007-2011 Broadcom Corporation
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
diff --git a/drivers/net/bnx2x/bnx2x_init.h b/drivers/net/bnx2x/bnx2x_init.h
index fa6dbe3..d539920 100644
--- a/drivers/net/bnx2x/bnx2x_init.h
+++ b/drivers/net/bnx2x/bnx2x_init.h
@@ -1,7 +1,7 @@
/* bnx2x_init.h: Broadcom Everest network driver.
* Structures and macroes needed during the initialization.
*
- * Copyright (c) 2007-2009 Broadcom Corporation
+ * Copyright (c) 2007-2011 Broadcom Corporation
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
diff --git a/drivers/net/bnx2x/bnx2x_init_ops.h b/drivers/net/bnx2x/bnx2x_init_ops.h
index 66df29f..aafd023 100644
--- a/drivers/net/bnx2x/bnx2x_init_ops.h
+++ b/drivers/net/bnx2x/bnx2x_init_ops.h
@@ -2,7 +2,7 @@
* Static functions needed during the initialization.
* This file is "included" in bnx2x_main.c.
*
- * Copyright (c) 2007-2010 Broadcom Corporation
+ * Copyright (c) 2007-2011 Broadcom Corporation
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
diff --git a/drivers/net/bnx2x/bnx2x_main.c b/drivers/net/bnx2x/bnx2x_main.c
index 3ddc436..c8e5ff0 100644
--- a/drivers/net/bnx2x/bnx2x_main.c
+++ b/drivers/net/bnx2x/bnx2x_main.c
@@ -1,6 +1,6 @@
/* bnx2x_main.c: Broadcom Everest network driver.
*
- * Copyright (c) 2007-2010 Broadcom Corporation
+ * Copyright (c) 2007-2011 Broadcom Corporation
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
diff --git a/drivers/net/bnx2x/bnx2x_reg.h b/drivers/net/bnx2x/bnx2x_reg.h
index 1c89f19..eb959be 100644
--- a/drivers/net/bnx2x/bnx2x_reg.h
+++ b/drivers/net/bnx2x/bnx2x_reg.h
@@ -1,6 +1,6 @@
/* bnx2x_reg.h: Broadcom Everest network driver.
*
- * Copyright (c) 2007-2010 Broadcom Corporation
+ * Copyright (c) 2007-2011 Broadcom Corporation
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
diff --git a/drivers/net/bnx2x/bnx2x_stats.c b/drivers/net/bnx2x/bnx2x_stats.c
index 3445ded..e535bfa 100644
--- a/drivers/net/bnx2x/bnx2x_stats.c
+++ b/drivers/net/bnx2x/bnx2x_stats.c
@@ -1,6 +1,6 @@
/* bnx2x_stats.c: Broadcom Everest network driver.
*
- * Copyright (c) 2007-2010 Broadcom Corporation
+ * Copyright (c) 2007-2011 Broadcom Corporation
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
diff --git a/drivers/net/bnx2x/bnx2x_stats.h b/drivers/net/bnx2x/bnx2x_stats.h
index 596798c..45d14d8 100644
--- a/drivers/net/bnx2x/bnx2x_stats.h
+++ b/drivers/net/bnx2x/bnx2x_stats.h
@@ -1,6 +1,6 @@
/* bnx2x_stats.h: Broadcom Everest network driver.
*
- * Copyright (c) 2007-2010 Broadcom Corporation
+ * Copyright (c) 2007-2011 Broadcom Corporation
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
--
1.7.2.2
^ permalink raw reply related
* [net-next 1/5] bnx2x: link report improvements
From: Dmitry Kravkov @ 2011-03-20 16:28 UTC (permalink / raw)
To: davem, netdev; +Cc: Eilon Greenstein, Vladislav Zolotarov
To avoid link notification duplication, remember last reported state.
From: Vladislav Zolotarov <vladz@broadcom.com>
Signed-off-by: Dmitry Kravkov <dmitry@broadcom.com>
Signed-off-by: Vladislav Zolotarov <vladz@broadcom.com>
Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
---
drivers/net/bnx2x/bnx2x.h | 21 ++++++++
drivers/net/bnx2x/bnx2x_cmn.c | 99 +++++++++++++++++++++++++++++++--------
drivers/net/bnx2x/bnx2x_cmn.h | 2 -
drivers/net/bnx2x/bnx2x_main.c | 21 ++++-----
4 files changed, 108 insertions(+), 35 deletions(-)
diff --git a/drivers/net/bnx2x/bnx2x.h b/drivers/net/bnx2x/bnx2x.h
index b7ff87b..6f3d48f 100644
--- a/drivers/net/bnx2x/bnx2x.h
+++ b/drivers/net/bnx2x/bnx2x.h
@@ -893,6 +893,22 @@ typedef enum {
(&bp->def_status_blk->sp_sb.\
index_values[HC_SP_INDEX_EQ_CONS])
+/* This is a data that will be used to create a link report message.
+ * We will keep the data used for the last link report in order
+ * to prevent reporting the same link parameters twice.
+ */
+struct bnx2x_link_report_data {
+ u16 line_speed; /* Effective line speed */
+ unsigned long link_report_flags;/* BNX2X_LINK_REPORT_XXX flags */
+};
+
+enum {
+ BNX2X_LINK_REPORT_FD, /* Full DUPLEX */
+ BNX2X_LINK_REPORT_LINK_DOWN,
+ BNX2X_LINK_REPORT_RX_FC_ON,
+ BNX2X_LINK_REPORT_TX_FC_ON,
+};
+
struct bnx2x {
/* Fields used in the tx and intr/napi performance paths
* are grouped together in the beginning of the structure
@@ -1026,6 +1042,9 @@ struct bnx2x {
struct link_params link_params;
struct link_vars link_vars;
+ u32 link_cnt;
+ struct bnx2x_link_report_data last_reported_link;
+
struct mdio_if_info mdio;
struct bnx2x_common common;
@@ -1442,6 +1461,8 @@ struct bnx2x_func_init_params {
#define WAIT_RAMROD_POLL 0x01
#define WAIT_RAMROD_COMMON 0x02
+void bnx2x_read_mf_cfg(struct bnx2x *bp);
+
/* dmae */
void bnx2x_read_dmae(struct bnx2x *bp, u32 src_addr, u32 len32);
void bnx2x_write_dmae(struct bnx2x *bp, dma_addr_t dma_addr, u32 dst_addr,
diff --git a/drivers/net/bnx2x/bnx2x_cmn.c b/drivers/net/bnx2x/bnx2x_cmn.c
index e83ac6d..267177b 100644
--- a/drivers/net/bnx2x/bnx2x_cmn.c
+++ b/drivers/net/bnx2x/bnx2x_cmn.c
@@ -758,35 +758,91 @@ u16 bnx2x_get_mf_speed(struct bnx2x *bp)
return line_speed;
}
+/* Fill Link report data according to the current link
+ * configuration.
+ *
+ * @param bp
+ * @param data
+ */
+static inline void bnx2x_fill_report_data(struct bnx2x *bp,
+ struct bnx2x_link_report_data *data)
+{
+ u16 line_speed = bnx2x_get_mf_speed(bp);
+
+ memset(data, 0, sizeof(*data));
+
+ /* Fill the report data: efective line speed */
+ data->line_speed = line_speed;
+
+ /* Link is down */
+ if (!bp->link_vars.link_up || (bp->flags & MF_FUNC_DIS))
+ set_bit(BNX2X_LINK_REPORT_LINK_DOWN, &data->link_report_flags);
+
+ /* Full DUPLEX */
+ if (bp->link_vars.duplex == DUPLEX_FULL)
+ set_bit(BNX2X_LINK_REPORT_FD, &data->link_report_flags);
+
+ /* Rx Flow Control is ON */
+ if (bp->link_vars.flow_ctrl & BNX2X_FLOW_CTRL_RX)
+ set_bit(BNX2X_LINK_REPORT_RX_FC_ON, &data->link_report_flags);
+
+ /* Tx Flow Control is ON */
+ if (bp->link_vars.flow_ctrl & BNX2X_FLOW_CTRL_TX)
+ set_bit(BNX2X_LINK_REPORT_TX_FC_ON, &data->link_report_flags);
+}
+
void bnx2x_link_report(struct bnx2x *bp)
{
- if (bp->flags & MF_FUNC_DIS) {
- netif_carrier_off(bp->dev);
- netdev_err(bp->dev, "NIC Link is Down\n");
- return;
- }
+ struct bnx2x_link_report_data cur_data;
- if (bp->link_vars.link_up) {
- u16 line_speed;
+ /* reread mf_cfg */
+ if (!CHIP_IS_E1(bp))
+ bnx2x_read_mf_cfg(bp);
- if (bp->state == BNX2X_STATE_OPEN)
- netif_carrier_on(bp->dev);
- netdev_info(bp->dev, "NIC Link is Up, ");
+ /* Read the current link report info */
+ bnx2x_fill_report_data(bp, &cur_data);
+
+ /* Don't report link down or exactly the same link status twice */
+ if (!memcmp(&cur_data, &bp->last_reported_link, sizeof(cur_data)) ||
+ (test_bit(BNX2X_LINK_REPORT_LINK_DOWN,
+ &bp->last_reported_link.link_report_flags) &&
+ test_bit(BNX2X_LINK_REPORT_LINK_DOWN,
+ &cur_data.link_report_flags)))
+ return;
- line_speed = bnx2x_get_mf_speed(bp);
+ bp->link_cnt++;
- pr_cont("%d Mbps ", line_speed);
+ /* We are going to report a new link parameters now -
+ * remember the current data for the next time.
+ */
+ memcpy(&bp->last_reported_link, &cur_data, sizeof(cur_data));
- if (bp->link_vars.duplex == DUPLEX_FULL)
+ if (test_bit(BNX2X_LINK_REPORT_LINK_DOWN,
+ &cur_data.link_report_flags)) {
+ netif_carrier_off(bp->dev);
+ netdev_err(bp->dev, "NIC Link is Down\n");
+ return;
+ } else {
+ netif_carrier_on(bp->dev);
+ netdev_info(bp->dev, "NIC Link is Up, ");
+ pr_cont("%d Mbps ", cur_data.line_speed);
+
+ if (test_and_clear_bit(BNX2X_LINK_REPORT_FD,
+ &cur_data.link_report_flags))
pr_cont("full duplex");
else
pr_cont("half duplex");
- if (bp->link_vars.flow_ctrl != BNX2X_FLOW_CTRL_NONE) {
- if (bp->link_vars.flow_ctrl & BNX2X_FLOW_CTRL_RX) {
+ /* Handle the FC at the end so that only these flags would be
+ * possibly set. This way we may easily check if there is no FC
+ * enabled.
+ */
+ if (cur_data.link_report_flags) {
+ if (test_bit(BNX2X_LINK_REPORT_RX_FC_ON,
+ &cur_data.link_report_flags)) {
pr_cont(", receive ");
- if (bp->link_vars.flow_ctrl &
- BNX2X_FLOW_CTRL_TX)
+ if (test_bit(BNX2X_LINK_REPORT_TX_FC_ON,
+ &cur_data.link_report_flags))
pr_cont("& transmit ");
} else {
pr_cont(", transmit ");
@@ -794,10 +850,6 @@ void bnx2x_link_report(struct bnx2x *bp)
pr_cont("flow control ON");
}
pr_cont("\n");
-
- } else { /* link_down */
- netif_carrier_off(bp->dev);
- netdev_err(bp->dev, "NIC Link is Down\n");
}
}
@@ -1345,6 +1397,11 @@ int bnx2x_nic_load(struct bnx2x *bp, int load_mode)
bp->state = BNX2X_STATE_OPENING_WAIT4_LOAD;
+ /* Set the initial link reported state to link down */
+ memset(&bp->last_reported_link, 0, sizeof(bp->last_reported_link));
+ set_bit(BNX2X_LINK_REPORT_LINK_DOWN,
+ &bp->last_reported_link.link_report_flags);
+
/* must be called before memory allocation and HW init */
bnx2x_ilt_set_info(bp);
diff --git a/drivers/net/bnx2x/bnx2x_cmn.h b/drivers/net/bnx2x/bnx2x_cmn.h
index ef37b98..10cf462 100644
--- a/drivers/net/bnx2x/bnx2x_cmn.h
+++ b/drivers/net/bnx2x/bnx2x_cmn.h
@@ -67,8 +67,6 @@ void bnx2x__link_status_update(struct bnx2x *bp);
* Report link status to upper layer
*
* @param bp
- *
- * @return int
*/
void bnx2x_link_report(struct bnx2x *bp);
diff --git a/drivers/net/bnx2x/bnx2x_main.c b/drivers/net/bnx2x/bnx2x_main.c
index 9d48659..bc847b1 100644
--- a/drivers/net/bnx2x/bnx2x_main.c
+++ b/drivers/net/bnx2x/bnx2x_main.c
@@ -2043,7 +2043,7 @@ static int bnx2x_get_cmng_fns_mode(struct bnx2x *bp)
return CMNG_FNS_NONE;
}
-static void bnx2x_read_mf_cfg(struct bnx2x *bp)
+void bnx2x_read_mf_cfg(struct bnx2x *bp)
{
int vn, n = (CHIP_MODE_IS_4_PORT(bp) ? 2 : 1);
@@ -2130,7 +2130,6 @@ static inline void bnx2x_link_sync_notify(struct bnx2x *bp)
/* This function is called upon link interrupt */
static void bnx2x_link_attn(struct bnx2x *bp)
{
- u32 prev_link_status = bp->link_vars.link_status;
/* Make sure that we are synced with the current statistics */
bnx2x_stats_handle(bp, STATS_EVENT_STOP);
@@ -2175,17 +2174,15 @@ static void bnx2x_link_attn(struct bnx2x *bp)
"single function mode without fairness\n");
}
+ bnx2x_link_report(bp);
+
if (IS_MF(bp))
bnx2x_link_sync_notify(bp);
-
- /* indicate link status only if link status actually changed */
- if (prev_link_status != bp->link_vars.link_status)
- bnx2x_link_report(bp);
}
void bnx2x__link_status_update(struct bnx2x *bp)
{
- if ((bp->state != BNX2X_STATE_OPEN) || (bp->flags & MF_FUNC_DIS))
+ if (bp->state != BNX2X_STATE_OPEN)
return;
bnx2x_link_status_update(&bp->link_params, &bp->link_vars);
@@ -2195,10 +2192,6 @@ void bnx2x__link_status_update(struct bnx2x *bp)
else
bnx2x_stats_handle(bp, STATS_EVENT_STOP);
- /* the link status update could be the result of a DCC event
- hence re-read the shmem mf configuration */
- bnx2x_read_mf_cfg(bp);
-
/* indicate link status */
bnx2x_link_report(bp);
}
@@ -3127,10 +3120,14 @@ static inline void bnx2x_attn_int_deasserted3(struct bnx2x *bp, u32 attn)
if (val & DRV_STATUS_SET_MF_BW)
bnx2x_set_mf_bw(bp);
- bnx2x__link_status_update(bp);
if ((bp->port.pmf == 0) && (val & DRV_STATUS_PMF))
bnx2x_pmf_update(bp);
+ /* Always call it here: bnx2x_link_report() will
+ * prevent the link indication duplication.
+ */
+ bnx2x__link_status_update(bp);
+
if (bp->port.pmf &&
(val & DRV_STATUS_DCBX_NEGOTIATION_RESULTS) &&
bp->dcbx_enabled > 0)
--
1.7.2.2
^ permalink raw reply related
* [net-next 0/5] bnx2x enhancements and date update
From: Dmitry Kravkov @ 2011-03-20 16:36 UTC (permalink / raw)
To: davem, netdev; +Cc: Eilon Greenstein, Vladislav Zolotarov
Hello Dave,
Please consider applying following patch series to the "net-next".
Thank you
Dmitry
^ permalink raw reply
* Re: [PATCH v2] ROSE: prevent heap corruption with bad facilities
From: Ben Hutchings @ 2011-03-20 16:48 UTC (permalink / raw)
To: Dan Rosenberg; +Cc: ralf, davem, netdev, security
In-Reply-To: <1300603423.1869.18.camel@dan>
On Sun, 2011-03-20 at 02:43 -0400, Dan Rosenberg wrote:
> When parsing the FAC_NATIONAL_DIGIS facilities field, it's possible for
> a remote host to provide more digipeaters than expected, resulting in
> heap corruption. Check against ROSE_MAX_DIGIS to prevent overflows, and
> abort facilities parsing on failure.
>
> Additionally, when parsing the FAC_CCITT_DEST_NSAP and
> FAC_CCITT_SRC_NSAP facilities fields, a remote host can provide a length
> of less than 10, resulting in an underflow in a memcpy size, causing a
> kernel panic due to massive heap corruption. A length of greater than
> 20 results in a stack overflow of the callsign array. Abort facilities
> parsing on these invalid length values.
[...]
Look further and you'll see that the *entire* parsing code makes not the
slightest attempt to validate lengths.
This applies on top of your patch, and hopefully fixes the remaining
bugs. It's compile-tested only.
Ben.
---
Subject: [PATCH] rose: Add length checks to CALL_REQUEST parsing
Define some constant offsets for CALL_REQUEST based on the description
at <http://www.techfest.com/networking/wan/x25plp.htm> and the
definition of ROSE as using 10-digit (5-byte) addresses. Use them
consistently. Validate all implicit and explicit facilities lengths.
Validate the address length byte rather than either trusting or
assuming its value.
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
include/net/rose.h | 8 ++++-
net/rose/af_rose.c | 8 ++--
net/rose/rose_loopback.c | 13 ++++++-
net/rose/rose_route.c | 20 +++++++----
net/rose/rose_subr.c | 91 +++++++++++++++++++++++++++++-----------------
5 files changed, 93 insertions(+), 47 deletions(-)
diff --git a/include/net/rose.h b/include/net/rose.h
index 5ba9f02..555dd19 100644
--- a/include/net/rose.h
+++ b/include/net/rose.h
@@ -14,6 +14,12 @@
#define ROSE_MIN_LEN 3
+#define ROSE_CALL_REQ_ADDR_LEN_OFF 3
+#define ROSE_CALL_REQ_ADDR_LEN_VAL 0xAA /* each address is 10 digits */
+#define ROSE_CALL_REQ_DEST_ADDR_OFF 4
+#define ROSE_CALL_REQ_SRC_ADDR_OFF 9
+#define ROSE_CALL_REQ_FACILITIES_OFF 14
+
#define ROSE_GFI 0x10
#define ROSE_Q_BIT 0x80
#define ROSE_D_BIT 0x40
@@ -214,7 +220,7 @@ extern void rose_requeue_frames(struct sock *);
extern int rose_validate_nr(struct sock *, unsigned short);
extern void rose_write_internal(struct sock *, int);
extern int rose_decode(struct sk_buff *, int *, int *, int *, int *, int *);
-extern int rose_parse_facilities(unsigned char *, struct rose_facilities_struct *);
+extern int rose_parse_facilities(unsigned char *, unsigned int, struct rose_facilities_struct *);
extern void rose_disconnect(struct sock *, int, int, int);
/* rose_timer.c */
diff --git a/net/rose/af_rose.c b/net/rose/af_rose.c
index 5ee0c62..a80aef6 100644
--- a/net/rose/af_rose.c
+++ b/net/rose/af_rose.c
@@ -978,7 +978,7 @@ int rose_rx_call_request(struct sk_buff *skb, struct net_device *dev, struct ros
struct sock *make;
struct rose_sock *make_rose;
struct rose_facilities_struct facilities;
- int n, len;
+ int n;
skb->sk = NULL; /* Initially we don't know who it's for */
@@ -987,9 +987,9 @@ int rose_rx_call_request(struct sk_buff *skb, struct net_device *dev, struct ros
*/
memset(&facilities, 0x00, sizeof(struct rose_facilities_struct));
- len = (((skb->data[3] >> 4) & 0x0F) + 1) >> 1;
- len += (((skb->data[3] >> 0) & 0x0F) + 1) >> 1;
- if (!rose_parse_facilities(skb->data + len + 4, &facilities)) {
+ if (!rose_parse_facilities(skb->data + ROSE_CALL_REQ_FACILITIES_OFF,
+ skb->len - ROSE_CALL_REQ_FACILITIES_OFF,
+ &facilities)) {
rose_transmit_clear_request(neigh, lci, ROSE_INVALID_FACILITY, 76);
return 0;
}
diff --git a/net/rose/rose_loopback.c b/net/rose/rose_loopback.c
index ae4a9d9..3444562 100644
--- a/net/rose/rose_loopback.c
+++ b/net/rose/rose_loopback.c
@@ -73,9 +73,20 @@ static void rose_loopback_timer(unsigned long param)
unsigned int lci_i, lci_o;
while ((skb = skb_dequeue(&loopback_queue)) != NULL) {
+ if (skb->len < ROSE_MIN_LEN) {
+ kfree_skb(skb);
+ continue;
+ }
lci_i = ((skb->data[0] << 8) & 0xF00) + ((skb->data[1] << 0) & 0x0FF);
frametype = skb->data[2];
- dest = (rose_address *)(skb->data + 4);
+ if (frametype == ROSE_CALL_REQUEST &&
+ (skb->len <= ROSE_CALL_REQ_FACILITIES_OFF ||
+ skb->data[ROSE_CALL_REQ_ADDR_LEN_OFF] !=
+ ROSE_CALL_REQ_ADDR_LEN_VAL)) {
+ kfree_skb(skb);
+ continue;
+ }
+ dest = (rose_address *)(skb->data + ROSE_CALL_REQ_DEST_ADDR_OFF);
lci_o = ROSE_DEFAULT_MAXVC + 1 - lci_i;
skb_reset_transport_header(skb);
diff --git a/net/rose/rose_route.c b/net/rose/rose_route.c
index 88a77e9..08dcd2f 100644
--- a/net/rose/rose_route.c
+++ b/net/rose/rose_route.c
@@ -861,7 +861,7 @@ int rose_route_frame(struct sk_buff *skb, ax25_cb *ax25)
unsigned int lci, new_lci;
unsigned char cause, diagnostic;
struct net_device *dev;
- int len, res = 0;
+ int res = 0;
char buf[11];
#if 0
@@ -869,10 +869,17 @@ int rose_route_frame(struct sk_buff *skb, ax25_cb *ax25)
return res;
#endif
+ if (skb->len < ROSE_MIN_LEN)
+ return res;
frametype = skb->data[2];
lci = ((skb->data[0] << 8) & 0xF00) + ((skb->data[1] << 0) & 0x0FF);
- src_addr = (rose_address *)(skb->data + 9);
- dest_addr = (rose_address *)(skb->data + 4);
+ if (frametype == ROSE_CALL_REQUEST &&
+ (skb->len <= ROSE_CALL_REQ_FACILITIES_OFF ||
+ skb->data[ROSE_CALL_REQ_ADDR_LEN_OFF] !=
+ ROSE_CALL_REQ_ADDR_LEN_VAL))
+ return res;
+ src_addr = (rose_address *)(skb->data + ROSE_CALL_REQ_SRC_ADDR_OFF);
+ dest_addr = (rose_address *)(skb->data + ROSE_CALL_REQ_DEST_ADDR_OFF);
spin_lock_bh(&rose_neigh_list_lock);
spin_lock_bh(&rose_route_list_lock);
@@ -1010,12 +1017,11 @@ int rose_route_frame(struct sk_buff *skb, ax25_cb *ax25)
goto out;
}
- len = (((skb->data[3] >> 4) & 0x0F) + 1) >> 1;
- len += (((skb->data[3] >> 0) & 0x0F) + 1) >> 1;
-
memset(&facilities, 0x00, sizeof(struct rose_facilities_struct));
- if (!rose_parse_facilities(skb->data + len + 4, &facilities)) {
+ if (!rose_parse_facilities(skb->data + ROSE_CALL_REQ_FACILITIES_OFF,
+ skb->len - ROSE_CALL_REQ_FACILITIES_OFF,
+ &facilities)) {
rose_transmit_clear_request(rose_neigh, lci, ROSE_INVALID_FACILITY, 76);
goto out;
}
diff --git a/net/rose/rose_subr.c b/net/rose/rose_subr.c
index 174d51c..f6c71ca 100644
--- a/net/rose/rose_subr.c
+++ b/net/rose/rose_subr.c
@@ -142,7 +142,7 @@ void rose_write_internal(struct sock *sk, int frametype)
*dptr++ = ROSE_GFI | lci1;
*dptr++ = lci2;
*dptr++ = frametype;
- *dptr++ = 0xAA;
+ *dptr++ = ROSE_CALL_REQ_ADDR_LEN_VAL;
memcpy(dptr, &rose->dest_addr, ROSE_ADDR_LEN);
dptr += ROSE_ADDR_LEN;
memcpy(dptr, &rose->source_addr, ROSE_ADDR_LEN);
@@ -246,12 +246,16 @@ static int rose_parse_national(unsigned char *p, struct rose_facilities_struct *
do {
switch (*p & 0xC0) {
case 0x00:
+ if (len < 2)
+ return -1;
p += 2;
n += 2;
len -= 2;
break;
case 0x40:
+ if (len < 3)
+ return -1;
if (*p == FAC_NATIONAL_RAND)
facilities->rand = ((p[1] << 8) & 0xFF00) + ((p[2] << 0) & 0x00FF);
p += 3;
@@ -260,32 +264,48 @@ static int rose_parse_national(unsigned char *p, struct rose_facilities_struct *
break;
case 0x80:
+ if (len < 4)
+ return -1;
p += 4;
n += 4;
len -= 4;
break;
case 0xC0:
+ if (len < 2)
+ return -1;
l = p[1];
+ if (len < 2 + l)
+ return -1;
if (*p == FAC_NATIONAL_DEST_DIGI) {
if (!fac_national_digis_received) {
+ if (l < AX25_ADDR_LEN)
+ return -1;
memcpy(&facilities->source_digis[0], p + 2, AX25_ADDR_LEN);
facilities->source_ndigis = 1;
}
}
else if (*p == FAC_NATIONAL_SRC_DIGI) {
if (!fac_national_digis_received) {
+ if (l < AX25_ADDR_LEN)
+ return -1;
memcpy(&facilities->dest_digis[0], p + 2, AX25_ADDR_LEN);
facilities->dest_ndigis = 1;
}
}
else if (*p == FAC_NATIONAL_FAIL_CALL) {
+ if (l < AX25_ADDR_LEN)
+ return -1;
memcpy(&facilities->fail_call, p + 2, AX25_ADDR_LEN);
}
else if (*p == FAC_NATIONAL_FAIL_ADD) {
+ if (l < 1 + ROSE_ADDR_LEN)
+ return -1;
memcpy(&facilities->fail_addr, p + 3, ROSE_ADDR_LEN);
}
else if (*p == FAC_NATIONAL_DIGIS) {
+ if (l % AX25_ADDR_LEN)
+ return -1;
fac_national_digis_received = 1;
facilities->source_ndigis = 0;
facilities->dest_ndigis = 0;
@@ -319,24 +339,32 @@ static int rose_parse_ccitt(unsigned char *p, struct rose_facilities_struct *fac
do {
switch (*p & 0xC0) {
case 0x00:
+ if (len < 2)
+ return -1;
p += 2;
n += 2;
len -= 2;
break;
case 0x40:
+ if (len < 3)
+ return -1;
p += 3;
n += 3;
len -= 3;
break;
case 0x80:
+ if (len < 4)
+ return -1;
p += 4;
n += 4;
len -= 4;
break;
case 0xC0:
+ if (len < 2)
+ return -1;
l = p[1];
/* Prevent overflows*/
@@ -365,49 +393,44 @@ static int rose_parse_ccitt(unsigned char *p, struct rose_facilities_struct *fac
return n;
}
-int rose_parse_facilities(unsigned char *p,
+int rose_parse_facilities(unsigned char *p, unsigned packet_len,
struct rose_facilities_struct *facilities)
{
int facilities_len, len;
facilities_len = *p++;
- if (facilities_len == 0)
+ if (facilities_len == 0 || (unsigned)facilities_len > packet_len)
return 0;
- while (facilities_len > 0) {
- if (*p == 0x00) {
- facilities_len--;
- p++;
-
- switch (*p) {
- case FAC_NATIONAL: /* National */
- len = rose_parse_national(p + 1, facilities, facilities_len - 1);
- if (len < 0)
- return 0;
- facilities_len -= len + 1;
- p += len + 1;
- break;
-
- case FAC_CCITT: /* CCITT */
- len = rose_parse_ccitt(p + 1, facilities, facilities_len - 1);
- if (len < 0)
- return 0;
- facilities_len -= len + 1;
- p += len + 1;
- break;
-
- default:
- printk(KERN_DEBUG "ROSE: rose_parse_facilities - unknown facilities family %02X\n", *p);
- facilities_len--;
- p++;
- break;
- }
- } else
- break; /* Error in facilities format */
+ while (facilities_len >= 3 && *p == 0x00) {
+ facilities_len--;
+ p++;
+
+ switch (*p) {
+ case FAC_NATIONAL: /* National */
+ len = rose_parse_national(p + 1, facilities, facilities_len - 1);
+ break;
+
+ case FAC_CCITT: /* CCITT */
+ len = rose_parse_ccitt(p + 1, facilities, facilities_len - 1);
+ break;
+
+ default:
+ printk(KERN_DEBUG "ROSE: rose_parse_facilities - unknown facilities family %02X\n", *p);
+ len = 1;
+ break;
+ }
+
+ if (len < 0)
+ return 0;
+ if (WARN_ON(len >= facilities_len))
+ return 0;
+ facilities_len -= len + 1;
+ p += len + 1;
}
- return 1;
+ return facilities_len == 0;
}
static int rose_create_facilities(unsigned char *buffer, struct rose_sock *rose)
--
1.7.4.1
--
Ben Hutchings
Once a job is fouled up, anything done to improve it makes it worse.
^ permalink raw reply related
* Re: [PATCH] net: drop NETIF_F_GSO from hw_features without NETIF_F_SG
From: Roger Luethi @ 2011-03-20 18:56 UTC (permalink / raw)
To: Micha? Miros?aw; +Cc: netdev
In-Reply-To: <AANLkTinCdaEs=G=AC45ePA-J99Vik9fYE+ZyF1miCiVy@mail.gmail.com>
On Sat, 19 Mar 2011 17:18:17 +0100, Micha? Miros?aw wrote:
> > However, I still think the current, unpatched behavior is confusing users.
> > ethtool_get_features() will happily tell the user that GSO is available for
> > a NIC even if it is not (because the NIC does not support SG). Then, if the
> > user tries to enable GSO, the operation fails silently: ethtool won't get
> > an error code, so it looks as if the operation succeeded. The user needs to
> > look into the kernel log or actually verify the offload settings to realize
> > that something went wrong.
>
> Do you want to prepare a patch for this? Please remember about
> compatibility with per-offload ethtool ops if you do - this needs to
> be there until all drivers are converted to hw_features.
Something like this?
diff --git a/net/core/dev.c b/net/core/dev.c
index 0b88eba..5861a59 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5442,6 +5442,9 @@ int register_netdevice(struct net_device *dev)
if (!(dev->wanted_features & NETIF_F_SG)) {
dev->wanted_features &= ~NETIF_F_GSO;
dev->features &= ~NETIF_F_GSO;
+ if (!(dev->hw_features & NETIF_F_SG) &&
+ (!dev->ethtool_ops || !dev->ethtool_ops->set_sg))
+ dev->hw_features &= ~NETIF_F_GSO;
}
/* Enable GRO and NETIF_F_HIGHDMA for vlans by default,
^ permalink raw reply related
* Re: [PATCH 2/2] netfilter: get rid of atomic ops in fast path
From: Eric Dumazet @ 2011-03-20 19:06 UTC (permalink / raw)
To: Patrick McHardy
Cc: David Miller, Netfilter Development Mailinglist, netdev,
Jan Engelhardt
In-Reply-To: <4D86121C.7050503@trash.net>
Le dimanche 20 mars 2011 à 15:41 +0100, Patrick McHardy a écrit :
> Am 18.03.2011 18:07, schrieb Eric Dumazet:
> > We currently use a percpu spinlock to 'protect' rule bytes/packets
> > counters, after various attempts to use RCU instead.
> >
> > Lately we added a seqlock so that get_counters() can run without
> > blocking BH or 'writers'. But we really use the seqcount in it.
> >
> > Spinlock itself is only locked by the current/owner cpu, so we can
> > remove it completely.
> >
> > This cleanups api, using correct 'writer' vs 'reader' semantic.
> >
> > At replace time, the get_counters() call makes sure all cpus are done
> > using the old table.
>
> I think this will have to wait until net-next opens up again since
> its not a bugfix.
Sure :)
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* oops / kernel panic in bonding.
From: Nicolas de Pesloüan @ 2011-03-20 20:17 UTC (permalink / raw)
To: Jiri Pirko; +Cc: Jay Vosburgh, Andy Gospodarek, netdev@vger.kernel.org
Hi Jiri,
I suspect we have a race condition somewhere in the new bond_handle_frame function:
The following commands produce one of the following errors:
modprobe bonding max_bonds=0
echo +bond0>/sys/class/net/bonding_masters
echo +bond1>/sys/class/net/bonding_masters
echo +eth1>/sys/class/net/bond1/bonding/slaves
This is mostly reproducible, under VirtualBox.
All tests done with 08351fc6a75731226e1112fc7254542bd3a2912e at the top commit (current net-next-2.6).
Nicolas.
First try:
[ 42.478455] BUG: unable to handle kernel NULL pointer dereference at 0000000000000280
[ 42.480035] IP: [<ffffffffa040c9b0>] bond_handle_frame+0x1f/0x138 [bonding]
[ 42.480035] PGD 0
[ 42.480035] Oops: 0000 [#1] SMP
[ 42.480035] last sysfs file: /sys/devices/virtual/net/bond1/bonding/slaves
[ 42.480035] CPU 0
[ 42.480035] Modules linked in: bonding loop snd_intel8x0 snd_ac97_codec ac97_bus snd_pcm
snd_timer tpm_tis tpm snd psmouse tpm_bios parport_pc processor evdev pcspkr parport battery
i2c_piix4 serio_raw ac i2c_core button thermal_sys soundcore snd_page_alloc ext3 jbd mbcache
ide_gd_mod ide_cd_mod cdrom ata_generic ata_piix libata scsi_mod ohci_hcd piix ide_core floppy
ehci_hcd usbcore e1000 nls_base [last unloaded: scsi_wait_scan]
[ 42.480035]
[ 42.480035] Pid: 206, comm: udevd Not tainted 2.6.38+ #5 innotek GmbH VirtualBox
[ 42.480035] RIP: 0010:[<ffffffffa040c9b0>] [<ffffffffa040c9b0>] bond_handle_frame+0x1f/0x138
[bonding]
[ 42.480035] RSP: 0018:ffff88003fc03c20 EFLAGS: 00010282
[ 42.480035] RAX: 0000000000000000 RBX: 0000000000000000 RCX: ffff88002ecb0608
[ 42.480035] RDX: ffff880023392600 RSI: ffff880023392600 RDI: ffff880023392600
[ 42.480035] RBP: ffffffffa040c991 R08: ffff880023392600 R09: 00000000ffffffff
[ 42.480035] R10: ffff88002322c740 R11: dead000000200200 R12: ffff88002ec0ea00
[ 42.480035] R13: ffff88003fc03c58 R14: 0000000000000000 R15: 0000000000000001
[ 42.480035] FS: 00007fbab75c57a0(0000) GS:ffff88003fc00000(0000) knlGS:0000000000000000
[ 42.480035] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
[ 42.480035] CR2: 0000000000000280 CR3: 000000003defd000 CR4: 00000000000006f0
[ 42.480035] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 42.480035] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
[ 42.480035] Process udevd (pid: 206, threadinfo ffff880030066000, task ffff8800231c5040)
[ 42.480035] Stack:
[ 42.480035] 00000000ffffffff 0000000000000000 ffffffffa040c991 ffff88002322c000
[ 42.480035] ffff88003fc03c68 ffffffff81267192 0000000000000000 ffff880023392600
[ 42.480035] 0000000000000080
[ 42.524186] bonding: bond1: enslaving eth1 as an active interface with an up link.
Another try:
[ 308.145200] BUG: unable to handle kernel NULL pointer dereference at 0000000000000280
[ 308.146140] IP: [<ffffffffa042c9b0>] bond_handle_frame+0x1f/0x138 [bonding]
[ 308.146993] PGD 0
[ 308.147249] Oops: 0000 [#1] SMP
[ 308.147669] last sysfs file: /sys/devices/virtual/net/bond0/uevent
[ 308.148024] CPU 0
[ 308.148024] Modules linked in: bonding loop snd_intel8x0 snd_ac97_codec ac97_bus snd_pcm
snd_timer psmouse tpm_tis snd tpm tpm_bios serio_raw parport_pc i2c_piix4 pcspkr soundcore processor
evdev snd_page_alloc i2c_core parport battery ac button thermal_sys ext3 jbd mbcache ide_cd_mod
ide_gd_mod cdrom ata_generic ata_piix libata scsi_mod ohci_hcd piix ide_core ehci_hcd usbcore floppy
e1000 nls_base [last unloaded: bonding]
[ 308.148024]
[ 308.148024] Pid: 1291, comm: udevd Not tainted 2.6.38+ #5 innotek GmbH VirtualBox
[ 308.165445] RIP: 0010:[<ffffffffa042c9b0>] [<ffffffffa042c9b0>] bond_handle_frame+0x1f/0x138
[bonding]
[ 308.165445] RSP: 0000:ffff88003fc03c20 EFLAGS: 00010282
[ 308.165445] RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000148
[ 308.165445] RDX: ffff880023246000 RSI: ffff880023246000 RDI: ffff880023246000
[ 308.165445] RBP: ffffffffa042c991 R08: ffff880023246000 R09: 0000000000000000
[ 308.165445] R10: ffff88002ee2e740 R11: ffffffff81051a61 R12: ffff880039e50800
[ 308.165445] R13: ffff88003fc03c58 R14: 0000000000000000 R15: 0000000000000001
[ 308.165445] FS: 00007f1f30e837a0(0000) GS:ffff88003fc00000(0000) knlGS:0000000000000000
[ 308.165445] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
[ 308.165445] CR2: 0000000000000280 CR3: 000000002e582000 CR4: 00000000000006f0
[ 308.165445] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 308.165445] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
[ 308.165445] Process udevd (pid: 1291, threadinfo ffff88002eea0000, task ffff88002f808d60)
[ 308.165445] Stack:
[ 308.165445] ffff88002f80eb00 0000000000000000 ffffffffa042c991 ffff88002ee2e000
[ 308.165445] ffff88003fc03c68 ffffffff81267192 ffffffff810345f4 ffff880023246000
[ 308.165445] 0000000000000000 ffffffff81679c20 ffff880023246000 ffff880023246000
[ 308.165445] Call Trace:
[ 308.165445] <IRQ>
[ 308.165445] [<ffffffffa042c991>] ? bond_handle_frame+0x0/0x138 [bonding]
[ 308.165445] [<ffffffff81267192>] ? __netif_receive_skb+0x2f9/0x4c5
[ 308.165445] [<ffffffff810345f4>] ? __wake_up_common+0x41/0x78
[ 308.165445] [<ffffffff81267656>] ? netif_receive_skb+0x67/0x6d
[ 308.165445] [<ffffffff81267b5d>] ? napi_gro_receive+0x1f/0x2d
[ 308.165445] [<ffffffff8126772b>] ? napi_skb_finish+0x1c/0x31
[ 308.165445] [<ffffffffa000b25c>] ? e1000_clean_rx_irq+0x2fd/0x3b0 [e1000]
[ 308.165445] [<ffffffffa000ab46>] ? e1000_clean+0x30f/0x490 [e1000]
[ 308.165445] [<ffffffff81020bf6>] ? ack_apic_level+0x6e/0x134
[ 308.165445] [<ffffffff81091a7f>] ? handle_fasteoi_irq+0x9c/0xb4
[ 308.165445] [<ffffffff8104aa01>] ? irq_exit+0x6e/0xa0
[ 308.165445] [<ffffffff81267c8a>] ? net_rx_action+0xa8/0x206
[ 308.165445] [<ffffffff8104abf3>] ? __do_softirq+0xc3/0x19e
[ 308.165445] [<ffffffff8108f6d0>] ? handle_irq_event_percpu+0x171/0x18f
[ 308.165445] [<ffffffff8104ac6b>] ? __do_softirq+0x13b/0x19e
[ 308.165445] [<ffffffff81323edc>] ? call_softirq+0x1c/0x30
[ 308.165445] [<ffffffff8100aa53>] ? do_softirq+0x3f/0x79
[ 308.165445] [<ffffffff8104a9d2>] ? irq_exit+0x3f/0xa0
[ 308.165445] [<ffffffff8100a39f>] ? do_IRQ+0x94/0xaa
[ 308.165445] [<ffffffff8131cbd3>] ? ret_from_intr+0x0/0x15
[ 308.165445] <EOI>
[ 308.165445] [<ffffffff810da903>] ? ptep_clear_flush+0x17/0x34
[ 308.165445] [<ffffffff810cb8ba>] ? copy_user_highpage+0x27/0x40
[ 308.165445] [<ffffffff810cf45c>] ? do_wp_page+0x5c1/0x689
[ 308.165445] [<ffffffff810d0155>] ? handle_pte_fault+0x8a5/0x8f2
[ 308.165445] [<ffffffff810d02c4>] ? handle_mm_fault+0x122/0x18b
[ 308.165445] [<ffffffff8131f9b7>] ? do_page_fault+0x32a/0x34c
[ 308.165445] [<ffffffff8131ce95>] ? page_fault+0x25/0x30
[ 308.165445] [<ffffffff81199b0d>] ? __put_user_4+0x1d/0x30
[ 308.165445] [<ffffffff8131ce95>] ? page_fault+0x25/0x30
[ 308.165445] Code: e8 ed fc e2 e0 5a 48 89 d8 5b 5d c3 41 55 49 89 fd 41 54 55 53 48 83 ec 08 48
8b 3f 48 8b 47 20 4c 8b a0 f0 02 00 00 49 8b 04 24
[ 308.165445] 8b a8 80 02 00 00 b8 03 00 00 00 48 85 ed 0f 84 fc 00 00 00
[ 308.165445] RIP [<ffffffffa042c9b0>] bond_handle_frame+0x1f/0x138 [bonding]
[ 308.165445] RSP <ffff88003fc03c20>
[ 308.165445] CR2: 0000000000000280
[ 308.246179] ---[ end trace 31773bac6ab820b4 ]---
[ 308.246897] Kernel panic - not syncing: Fatal exception in interrupt
[ 308.248076] Pid: 1291, comm: udevd Tainted: G D 2.6.38+ #5
[ 308.249089] Call Trace:
[ 308.249496] <IRQ> [<ffffffff8131abe8>] ? panic+0x92/0x197
[ 308.250396] [<ffffffff810457a5>] ? kmsg_dump+0x41/0xe3
[ 308.251211] [<ffffffff8131d9a6>] ? oops_end+0xa9/0xb6
[ 308.252077] [<ffffffff8102c9ff>] ? no_context+0x1f4/0x201
[ 308.252967] [<ffffffffa000a7ed>] ? e1000_xmit_frame+0xa5b/0xaa5 [e1000]
[ 308.254014] [<ffffffff8131f83c>] ? do_page_fault+0x1af/0x34c
[ 308.254915] [<ffffffff81264479>] ? dev_hard_start_xmit+0x3de/0x53c
[ 308.256937] [<ffffffffa042c991>] ? bond_handle_frame+0x0/0x138 [bonding]
[ 308.258683] [<ffffffff8131ce95>] ? page_fault+0x25/0x30
[ 308.260431] [<ffffffffa042c991>] ? bond_handle_frame+0x0/0x138 [bonding]
[ 308.262420] [<ffffffff81051a61>] ? __mod_timer+0x145/0x157
[ 308.264009] [<ffffffffa042c9b0>] ? bond_handle_frame+0x1f/0x138 [bonding]
[ 308.265944] [<ffffffffa042c991>] ? bond_handle_frame+0x0/0x138 [bonding]
[ 308.267404] bonding: bond1: enslaving eth1 as an active interface with an up link.
[ 308.269896] [<ffffffff81267192>] ? __netif_receive_skb+0x2f9/0x4c5
[ 308.271792] [<ffffffff810345f4>] ? __wake_up_common+0x41/0x78
[ 308.271795] [<ffffffff81267656>] ? netif_receive_skb+0x67/0x6d
[ 308.271797] [<ffffffff81267b5d>] ? napi_gro_receive+0x1f/0x2d
[ 308.271799] [<ffffffff8126772b>] ? napi_skb_finish+0x1c/0x31
[ 308.271811] [<ffffffffa000b25c>] ? e1000_clean_rx_irq+0x2fd/0x3b0 [e1000]
[ 308.271815] [<ffffffffa000ab46>] ? e1000_clean+0x30f/0x490 [e1000]
[ 308.271819] [<ffffffff81020bf6>] ? ack_apic_level+0x6e/0x134
[ 308.271822] [<ffffffff81091a7f>] ? handle_fasteoi_irq+0x9c/0xb4
^ permalink raw reply
* Re: [PATCH 0/4] netfilter: netfilter fixes
From: David Miller @ 2011-03-20 20:42 UTC (permalink / raw)
To: kaber; +Cc: netfilter-devel, netdev
In-Reply-To: <1300635148-22648-1-git-send-email-kaber@trash.net>
From: kaber@trash.net
Date: Sun, 20 Mar 2011 16:32:24 +0100
> the following patches fix a couple of netfilter bugs:
>
> - a bug in ipset which caused the hash:port set types to silently
> ignore address ranges when non-TCP/UDP entries were added, from
> Jozsef
>
> - a missing check for set type revision in ipset, from Jozsef
>
> - a fix for iptables reentrancy, from Eric
>
> - a fix for a buffer overflow in the ipt_CLUSTERIP proc handler,
> from Vasiliy Kulikov
>
> Please apply or pull from:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/kaber/nf-2.6.git master
Pulled, thanks a lot Patrick.
^ permalink raw reply
* Re: [net-next 0/5] bnx2x enhancements and date update
From: David Miller @ 2011-03-20 20:47 UTC (permalink / raw)
To: dmitry; +Cc: netdev, eilong, vladz
In-Reply-To: <1300638963.17303.29.camel@lb-tlvb-dmitry>
From: "Dmitry Kravkov" <dmitry@broadcom.com>
Date: Sun, 20 Mar 2011 18:36:03 +0200
> Please consider applying following patch series to the "net-next".
Please consider paying attention to announcements I make about
what is appropriate to submit at this point in time:
http://marc.info/?l=linux-netdev&m=130023005214910&w=2
By the time I open up the net-next-2.6 tree and am actually willing to
apply patches, something you submit now might not even apply cleanly.
That's why I don't want people submitting net-next-2.6 patches at this
time.
It also is distracting to submit such patches at this time. All
efforts should be placed on regression fixing and closing out the
merge window cleanly.
^ permalink raw reply
* Re: oops / kernel panic in bonding.
From: Nicolas de Pesloüan @ 2011-03-20 21:19 UTC (permalink / raw)
To: Jiri Pirko; +Cc: Jay Vosburgh, Andy Gospodarek, netdev@vger.kernel.org
In-Reply-To: <4D8660EC.6080102@gmail.com>
Le 20/03/2011 21:17, Nicolas de Pesloüan a écrit :
> Hi Jiri,
>
> I suspect we have a race condition somewhere in the new
> bond_handle_frame function:
>
> The following commands produce one of the following errors:
>
> modprobe bonding max_bonds=0
> echo +bond0>/sys/class/net/bonding_masters
> echo +bond1>/sys/class/net/bonding_masters
> echo +eth1>/sys/class/net/bond1/bonding/slaves
>
> This is mostly reproducible, under VirtualBox.
>
> All tests done with 08351fc6a75731226e1112fc7254542bd3a2912e at the top
> commit (current net-next-2.6).
I suspect netdev_rx_handler_register is called too early in bond_enslave.
I think it should be the last thing we do in bond_enslave, if we don't want to face the risk to have
bond_handle_frame being called before everything is properly setup.
Nicolas.
^ permalink raw reply
* RE: [net-next 0/5] bnx2x enhancements and date update
From: Dmitry Kravkov @ 2011-03-20 21:25 UTC (permalink / raw)
To: David Miller
Cc: netdev@vger.kernel.org, Eilon Greenstein, Vladislav Zolotarov
In-Reply-To: <20110320.134750.104058019.davem@davemloft.net>
> From: netdev-owner@vger.kernel.org [mailto:netdev-
> owner@vger.kernel.org] On Behalf Of David Miller
> Sent: Sunday, March 20, 2011 10:48 PM
>
> From: "Dmitry Kravkov" <dmitry@broadcom.com>
> Date: Sun, 20 Mar 2011 18:36:03 +0200
>
> > Please consider applying following patch series to the "net-next".
>
> Please consider paying attention to announcements I make about
> what is appropriate to submit at this point in time:
Oops, my bad! I will re-submit them when you re-open the window.
Please ignore them totally for now. Thanks.
^ permalink raw reply
* [PATCH 1/1] solos-pci: Use shared code for VCC management
From: Philip Prindeville @ 2011-03-20 22:30 UTC (permalink / raw)
To: Netdev; +Cc: linux-atm-general@lists.sourceforge.net
The solos-pci driver duplicates the net/atm/common.c function atm_dev_release_vccs() locally as the static function release_vccs(). Let's have the driver share code.
Signed-off-by: Philip A. Prindeville <philipp@redfish-solutions.com>
---
--- a/drivers/atm/solos-pci.c 2011-03-20 01:42:22.000000000 -0600
+++ b/drivers/atm/solos-pci.c 2011-03-20 14:23:26.000000000 -0600
@@ -165,7 +165,6 @@ static uint32_t fpga_tx(struct solos_car
static irqreturn_t solos_irq(int irq, void *dev_id);
static struct atm_vcc* find_vcc(struct atm_dev *dev, short vpi, int vci);
static int list_vccs(int vci);
-static void release_vccs(struct atm_dev *dev);
static int atm_init(struct solos_card *, struct device *);
static void atm_remove(struct solos_card *);
static int send_command(struct solos_card *card, int dev, const char *buf, size_t size);
@@ -384,7 +383,7 @@ static int process_status(struct solos_c
/* Anything but 'Showtime' is down */
if (strcmp(state_str, "Showtime")) {
atm_dev_signal_change(card->atmdev[port], ATM_PHY_SIG_LOST);
- release_vccs(card->atmdev[port]);
+ atm_dev_release_vccs(card->atmdev[port]);
dev_info(&card->dev->dev, "Port %d: %s\n", port, state_str);
return 0;
}
@@ -837,28 +836,6 @@ static int list_vccs(int vci)
return num_found;
}
-static void release_vccs(struct atm_dev *dev)
-{
- int i;
-
- write_lock_irq(&vcc_sklist_lock);
- for (i = 0; i< VCC_HTABLE_SIZE; i++) {
- struct hlist_head *head =&vcc_hash[i];
- struct hlist_node *node, *tmp;
- struct sock *s;
- struct atm_vcc *vcc;
-
- sk_for_each_safe(s, node, tmp, head) {
- vcc = atm_sk(s);
- if (vcc->dev == dev) {
- vcc_release_async(vcc, -EPIPE);
- sk_del_node_init(s);
- }
- }
- }
- write_unlock_irq(&vcc_sklist_lock);
-}
-
static int popen(struct atm_vcc *vcc)
{
--- a/net/atm/common.c 2011-01-04 17:50:19.000000000 -0700
+++ b/net/atm/common.c 2011-03-20 15:22:59.000000000 -0600
@@ -252,6 +252,7 @@ void atm_dev_release_vccs(struct atm_dev
}
write_unlock_irq(&vcc_sklist_lock);
}
+EXPORT_SYMBOL(atm_dev_release_vccs);
static int adjust_tp(struct atm_trafprm *tp, unsigned char aal)
{
--- a/include/linux/atmdev.h 2011-01-04 17:50:19.000000000 -0700
+++ b/include/linux/atmdev.h 2011-03-20 15:25:08.000000000 -0600
@@ -443,6 +443,7 @@ void atm_dev_signal_change(struct atm_de
void vcc_insert_socket(struct sock *sk);
+void atm_dev_release_vccs(struct atm_dev *dev);
/*
* This is approximately the algorithm used by alloc_skb.
^ permalink raw reply
* [PATCH 1/2] socket: increase default maximum listen queue length
From: Hagen Paul Pfeifer @ 2011-03-20 23:04 UTC (permalink / raw)
To: netdev; +Cc: Hagen Paul Pfeifer, Eric Dumazet
In-Reply-To: <20110320121414.GC3038@nuttenaction>
sysctl_somaxconn (SOMAXCONN: 128) specifies the maximum number of
sockets in state SYN_RECV per listen socket queue. At listen(2) time the
backlog is adjusted to this limit if bigger then that.
Afterwards in reqsk_queue_alloc() the backlog value is checked again
(nr_table_entries == backlog):
nr_table_entries = min_t(u32, nr_table_entries, sysctl_max_syn_backlog);
nr_table_entries = max_t(u32, nr_table_entries, 8);
nr_table_entries = roundup_pow_of_two(nr_table_entries + 1);
sysctl_max_syn_backlog on the other hand is dynamically adjusted,
depending on the memory characteristic of the system. Default is 256,
128 for small systems and up to 1024 for bigger systems.
For real server work the defacto sysctl_somaxconn limit seems inadequate:
Experiments with real servers show, that it is absolutely not enough
even at 100conn/sec. 256 cures most of problems.
Increase default sysctl_somaxconn from 128 to 256 to meet todays condition by
simultaneously limit nr_table_entries by sysctl_max_syn_backlog which is
based on memory condition (max(128, (tcp_hashinfo.ehash_mask + 1 / 256)).
Signed_off-by: Hagen Paul Pfeifer <hagen@jauu.net>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
---
include/linux/socket.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/include/linux/socket.h b/include/linux/socket.h
index edbb1d0..bf35ce2 100644
--- a/include/linux/socket.h
+++ b/include/linux/socket.h
@@ -237,7 +237,7 @@ struct ucred {
#define PF_MAX AF_MAX
/* Maximum queue length specifiable by listen. */
-#define SOMAXCONN 128
+#define SOMAXCONN 256
/* Flags we can use with send/ and recv.
Added those for 1003.1g not all are supported yet
--
1.7.4.1.57.g0466.dirty
^ permalink raw reply related
* [PATCH 2/2] socket: add minimum listen queue length sysctl
From: Hagen Paul Pfeifer @ 2011-03-20 23:04 UTC (permalink / raw)
To: netdev; +Cc: Hagen Paul Pfeifer
In-Reply-To: <1300662282-24741-1-git-send-email-hagen@jauu.net>
In the case that a server programmer misjudge network characteristic the
backlog parameter for listen(2) may not adequate to utilize hosts
capabilities and lead to unrequired SYN retransmission - a small backlog
value can form an artificial limitation. From Erics server setup, a
listen queue length of 8 is often a way to small):
ss -a | head
State Recv-Q Send-Q Local Address:Port Peer
Address:Port
LISTEN 0 8 *:imaps *:*
LISTEN 0 8 *:pop3s *:*
LISTEN 0 50 *:mysql *:*
LISTEN 0 8 *:pop3 *:*
LISTEN 0 8 *:imap2 *:*
LISTEN 0 511 *:www *:*
Until now it is not possible for the system (network) administrator to
increase this value. A bug report must be filled, the backlog increased,
a new version released or even worse: if using closed source software
you cannot make anything.
sysctl_min_syn_backlog provides the ability to increase the minimum
queue length. The default is 8.
Signed-off-by: Hagen Paul Pfeifer <hagen@jauu.net>
---
I will spin a second documentation patch if Davem accept this patch.
---
include/net/request_sock.h | 1 +
net/core/request_sock.c | 5 ++++-
net/ipv4/sysctl_net_ipv4.c | 7 +++++++
3 files changed, 12 insertions(+), 1 deletions(-)
diff --git a/include/net/request_sock.h b/include/net/request_sock.h
index 99e6e19..3e8865f 100644
--- a/include/net/request_sock.h
+++ b/include/net/request_sock.h
@@ -89,6 +89,7 @@ static inline void reqsk_free(struct request_sock *req)
}
extern int sysctl_max_syn_backlog;
+extern int sysctl_min_syn_backlog;
/** struct listen_sock - listen state
*
diff --git a/net/core/request_sock.c b/net/core/request_sock.c
index 182236b..e937e9c 100644
--- a/net/core/request_sock.c
+++ b/net/core/request_sock.c
@@ -35,6 +35,9 @@
int sysctl_max_syn_backlog = 256;
EXPORT_SYMBOL(sysctl_max_syn_backlog);
+int sysctl_min_syn_backlog = 8;
+EXPORT_SYMBOL(sysctl_min_syn_backlog);
+
int reqsk_queue_alloc(struct request_sock_queue *queue,
unsigned int nr_table_entries)
{
@@ -42,7 +45,7 @@ int reqsk_queue_alloc(struct request_sock_queue *queue,
struct listen_sock *lopt;
nr_table_entries = min_t(u32, nr_table_entries, sysctl_max_syn_backlog);
- nr_table_entries = max_t(u32, nr_table_entries, 8);
+ nr_table_entries = max_t(u32, nr_table_entries, sysctl_min_syn_backlog);
nr_table_entries = roundup_pow_of_two(nr_table_entries + 1);
lopt_size += nr_table_entries * sizeof(struct request_sock *);
if (lopt_size > PAGE_SIZE)
diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
index 1a45665..cc03c62 100644
--- a/net/ipv4/sysctl_net_ipv4.c
+++ b/net/ipv4/sysctl_net_ipv4.c
@@ -298,6 +298,13 @@ static struct ctl_table ipv4_table[] = {
.proc_handler = proc_dointvec
},
{
+ .procname = "tcp_min_syn_backlog",
+ .data = &sysctl_min_syn_backlog,
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = proc_dointvec
+ },
+ {
.procname = "ip_local_port_range",
.data = &sysctl_local_ports.range,
.maxlen = sizeof(sysctl_local_ports.range),
--
1.7.4.1.57.g0466.dirty
^ permalink raw reply related
* Re: [PATCH 1/2] socket: increase default maximum listen queue length
From: David Miller @ 2011-03-20 23:09 UTC (permalink / raw)
To: hagen; +Cc: netdev, eric.dumazet
In-Reply-To: <1300662282-24741-1-git-send-email-hagen@jauu.net>
From: Hagen Paul Pfeifer <hagen@jauu.net>
Date: Mon, 21 Mar 2011 00:04:41 +0100
> For real server work the defacto sysctl_somaxconn limit seems inadequate:
>
> Experiments with real servers show, that it is absolutely not enough
> even at 100conn/sec. 256 cures most of problems.
Absolutely no context is provided for this number.
What's the RTT? How fast are the cpus? etc.
You must tell the whole story in order to justify these changes
properly.
^ permalink raw reply
* [PATCH 1/1] solos-pci: Fix regression introduced by newest firmware
From: Philip Prindeville @ 2011-03-20 23:41 UTC (permalink / raw)
To: Netdev; +Cc: linux-atm-general@lists.sourceforge.net
The newest FPGA firmware on the Solos processors correctly signals carrier transitions, bitrate, etc.
The driver previously ignored these messages, and the physical state was always ATM_PHY_SIG_UNKNOWN.
Now that the board reports its state, we expose a bug whereby the transition from UNKNOWN to LOST causes us to release all VC's. It should not: only transitions from FOUND to LOST or UNKNOWN should do this (actually, it's bears examining if the VC's should be released at all).
So we add a check to the previous state on transitions for leaving FOUND state.
Signed-off-by: Philip A Prindeville <philipp@redfish-solutions.com>
---
--- a/drivers/atm/solos-pci.c 2011-03-20 15:27:40.000000000 -0600
+++ b/drivers/atm/solos-pci.c 2011-03-20 16:32:11.000000000 -0600
@@ -382,8 +382,11 @@ static int process_status(struct solos_c
/* Anything but 'Showtime' is down */
if (strcmp(state_str, "Showtime")) {
+ char old_signal = card->atmdev[port]->signal;
+
atm_dev_signal_change(card->atmdev[port], ATM_PHY_SIG_LOST);
- atm_dev_release_vccs(card->atmdev[port]);
+ if (old_signal == ATM_PHY_SIG_FOUND)
+ atm_dev_release_vccs(card->atmdev[port]);
dev_info(&card->dev->dev, "Port %d: %s\n", port, state_str);
return 0;
}
^ permalink raw reply
* Re: [PATCH 1/2] socket: increase default maximum listen queue length
From: Hagen Paul Pfeifer @ 2011-03-20 23:52 UTC (permalink / raw)
To: David Miller; +Cc: netdev, eric.dumazet
In-Reply-To: <20110320.160906.112595807.davem@davemloft.net>
* David Miller | 2011-03-20 16:09:06 [-0700]:
>Absolutely no context is provided for this number.
>
>What's the RTT? How fast are the cpus? etc.
>
>You must tell the whole story in order to justify these changes
>properly.
(you can skip the first paragraphs and read the last one ;)
The number is somewhat magically - like many other values. I greped
tglx/history.git but the comment (at that time tcp_ipv4.c) seems pre 2002 era.
Providing context is a little bit artificial: I can construct an scenario with
a RTT of 200ms and 1000 connection request per second and the table will
overflow. This can happen, sure. On the other hand there are scenarios with a
RTT of 20ms and 10 connection requests per second - no problem there.
Increasing the number _has_ one essential advantage: it is aligned on
sysctl_max_syn_backlog which in turn is determined by memory characteristics.
Without patch (sysctl not modified, BUT sysctl_max_syn_backlog depending on memory characteristic):
listen-queue-length = max(8, min(userspace_backlog, min(128, sysctl_max_syn_backlog))
Wit patch (sysctl not modified, BUT sysctl_max_syn_backlog depending on memory characteristic):
listen-queue-length = max(8, min(userspace_backlog, min(256, sysctl_max_syn_backlog))
The point is now: sysctl_max_syn_backlog is per default 256, 128 for small
systems and up to 1024 for larger systems. But sysctl_somaxconn (128) will
_always_ restrict the queue length to 128 and make therefore
sysctl_max_syn_backlog defacto unfeasible - it will always restrict the value
to 128. IMHO sysctl_somaxconn should be removed, the overhead of the
listen-queue size per listening socket is insignificant. Especially because
sysctl_max_syn_backlog already consider the memory characteristic. There are a
bunch more connected sockets as these <10 listening sockets, but performance
lack because of will always be noticeable:
netstat -s | grep overflowed
2621 times the listen queue of a socket overflowed
Hagen
^ permalink raw reply
* Re: [PATCH 1/2] socket: increase default maximum listen queue length
From: Hagen Paul Pfeifer @ 2011-03-20 23:57 UTC (permalink / raw)
To: David Miller; +Cc: netdev, eric.dumazet
In-Reply-To: <20110320.160906.112595807.davem@davemloft.net>
* David Miller | 2011-03-20 16:09:06 [-0700]:
>You must tell the whole story in order to justify these changes
>properly.
BTW: the second patch is independent and provides a new feature, mentioned by
Eric.
Hagen
^ permalink raw reply
* Re: [PATCH 1/2] socket: increase default maximum listen queue length
From: David Miller @ 2011-03-21 0:18 UTC (permalink / raw)
To: hagen; +Cc: netdev, eric.dumazet
In-Reply-To: <20110320235253.GD3038@nuttenaction>
From: Hagen Paul Pfeifer <hagen@jauu.net>
Date: Mon, 21 Mar 2011 00:52:53 +0100
> The number is somewhat magically - like many other values. I greped
> tglx/history.git but the comment (at that time tcp_ipv4.c) seems pre 2002 era.
Then don't use that number as part of the justification for the
change.
Describe what matters, and only what matters. Providing magic and
arbitrary numbers doesn't help people reading your commit message.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox