* [PATCH 3/3] Bluetooth: Fix kernel crash on L2CAP stress tests
From: Marcel Holtmann @ 2010-03-21 5:41 UTC (permalink / raw)
To: David Miller, Linus Torvalds; +Cc: netdev
In-Reply-To: <cover.1269149354.git.marcel@holtmann.org>
From: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
Added very simple check that req buffer has enough space to
fit configuration parameters. Shall be enough to reject packets
with configuration size more than req buffer.
Crash trace below
[ 6069.659393] Unable to handle kernel paging request at virtual address 02000205
[ 6069.673034] Internal error: Oops: 805 [#1] PREEMPT
...
[ 6069.727172] PC is at l2cap_add_conf_opt+0x70/0xf0 [l2cap]
[ 6069.732604] LR is at l2cap_recv_frame+0x1350/0x2e78 [l2cap]
...
[ 6070.030303] Backtrace:
[ 6070.032806] [<bf1c2880>] (l2cap_add_conf_opt+0x0/0xf0 [l2cap]) from
[<bf1c6624>] (l2cap_recv_frame+0x1350/0x2e78 [l2cap])
[ 6070.043823] r8:dc5d3100 r7:df2a91d6 r6:00000001 r5:df2a8000 r4:00000200
[ 6070.050659] [<bf1c52d4>] (l2cap_recv_frame+0x0/0x2e78 [l2cap]) from
[<bf1c8408>] (l2cap_recv_acldata+0x2bc/0x350 [l2cap])
[ 6070.061798] [<bf1c814c>] (l2cap_recv_acldata+0x0/0x350 [l2cap]) from
[<bf0037a4>] (hci_rx_task+0x244/0x478 [bluetooth])
[ 6070.072631] r6:dc647700 r5:00000001 r4:df2ab740
[ 6070.077362] [<bf003560>] (hci_rx_task+0x0/0x478 [bluetooth]) from
[<c006b9fc>] (tasklet_action+0x78/0xd8)
[ 6070.087005] [<c006b984>] (tasklet_action+0x0/0xd8) from [<c006c160>]
Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
Acked-by: Gustavo F. Padovan <gustavo@padovan.org>
Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
---
net/bluetooth/l2cap.c | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 43e17f7..7794a2e 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -2832,6 +2832,11 @@ static inline int l2cap_config_rsp(struct l2cap_conn *conn, struct l2cap_cmd_hdr
int len = cmd->len - sizeof(*rsp);
char req[64];
+ if (len > sizeof(req) - sizeof(struct l2cap_conf_req)) {
+ l2cap_send_disconn_req(conn, sk);
+ goto done;
+ }
+
/* throw out any old stored conf requests */
result = L2CAP_CONF_SUCCESS;
len = l2cap_parse_conf_rsp(sk, rsp->data,
--
1.6.6.1
^ permalink raw reply related
* [PATCH 2/3] Bluetooth: Convert debug files to actually use debugfs instead of sysfs
From: Marcel Holtmann @ 2010-03-21 5:41 UTC (permalink / raw)
To: David Miller, Linus Torvalds; +Cc: netdev
In-Reply-To: <cover.1269149354.git.marcel@holtmann.org>
Some of the debug files ended up wrongly in sysfs, because at that point
of time, debugfs didn't exist. Convert these files to use debugfs and
also seq_file. This patch converts all of these files at once and then
removes the exported symbol for the Bluetooth sysfs class.
Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
---
include/net/bluetooth/bluetooth.h | 2 +-
net/bluetooth/hci_sysfs.c | 3 +-
net/bluetooth/l2cap.c | 51 +++++++++++++++++++++---------------
net/bluetooth/rfcomm/core.c | 52 ++++++++++++++++++++----------------
net/bluetooth/rfcomm/sock.c | 47 +++++++++++++++++++--------------
net/bluetooth/sco.c | 47 ++++++++++++++++++---------------
6 files changed, 114 insertions(+), 88 deletions(-)
diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
index 04a6908..ff77e8f 100644
--- a/include/net/bluetooth/bluetooth.h
+++ b/include/net/bluetooth/bluetooth.h
@@ -176,6 +176,6 @@ extern void hci_sock_cleanup(void);
extern int bt_sysfs_init(void);
extern void bt_sysfs_cleanup(void);
-extern struct class *bt_class;
+extern struct dentry *bt_debugfs;
#endif /* __BLUETOOTH_H */
diff --git a/net/bluetooth/hci_sysfs.c b/net/bluetooth/hci_sysfs.c
index cafb55b..05fd125 100644
--- a/net/bluetooth/hci_sysfs.c
+++ b/net/bluetooth/hci_sysfs.c
@@ -8,8 +8,7 @@
#include <net/bluetooth/bluetooth.h>
#include <net/bluetooth/hci_core.h>
-struct class *bt_class = NULL;
-EXPORT_SYMBOL_GPL(bt_class);
+static struct class *bt_class;
struct dentry *bt_debugfs = NULL;
EXPORT_SYMBOL_GPL(bt_debugfs);
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 2755182..43e17f7 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -40,6 +40,8 @@
#include <linux/skbuff.h>
#include <linux/list.h>
#include <linux/device.h>
+#include <linux/debugfs.h>
+#include <linux/seq_file.h>
#include <linux/uaccess.h>
#include <linux/crc16.h>
#include <net/sock.h>
@@ -3937,39 +3939,42 @@ drop:
return 0;
}
-static ssize_t l2cap_sysfs_show(struct class *dev,
- struct class_attribute *attr,
- char *buf)
+static int l2cap_debugfs_show(struct seq_file *f, void *p)
{
struct sock *sk;
struct hlist_node *node;
- char *str = buf;
- int size = PAGE_SIZE;
read_lock_bh(&l2cap_sk_list.lock);
sk_for_each(sk, node, &l2cap_sk_list.head) {
struct l2cap_pinfo *pi = l2cap_pi(sk);
- int len;
-
- len = snprintf(str, size, "%s %s %d %d 0x%4.4x 0x%4.4x %d %d %d\n",
- batostr(&bt_sk(sk)->src), batostr(&bt_sk(sk)->dst),
- sk->sk_state, __le16_to_cpu(pi->psm), pi->scid,
- pi->dcid, pi->imtu, pi->omtu, pi->sec_level);
-
- size -= len;
- if (size <= 0)
- break;
- str += len;
+ seq_printf(f, "%s %s %d %d 0x%4.4x 0x%4.4x %d %d %d\n",
+ batostr(&bt_sk(sk)->src),
+ batostr(&bt_sk(sk)->dst),
+ sk->sk_state, __le16_to_cpu(pi->psm),
+ pi->scid, pi->dcid,
+ pi->imtu, pi->omtu, pi->sec_level);
}
read_unlock_bh(&l2cap_sk_list.lock);
- return str - buf;
+ return 0;
}
-static CLASS_ATTR(l2cap, S_IRUGO, l2cap_sysfs_show, NULL);
+static int l2cap_debugfs_open(struct inode *inode, struct file *file)
+{
+ return single_open(file, l2cap_debugfs_show, inode->i_private);
+}
+
+static const struct file_operations l2cap_debugfs_fops = {
+ .open = l2cap_debugfs_open,
+ .read = seq_read,
+ .llseek = seq_lseek,
+ .release = single_release,
+};
+
+static struct dentry *l2cap_debugfs;
static const struct proto_ops l2cap_sock_ops = {
.family = PF_BLUETOOTH,
@@ -4029,8 +4034,12 @@ static int __init l2cap_init(void)
goto error;
}
- if (class_create_file(bt_class, &class_attr_l2cap) < 0)
- BT_ERR("Failed to create L2CAP info file");
+ if (bt_debugfs) {
+ l2cap_debugfs = debugfs_create_file("l2cap", 0444,
+ bt_debugfs, NULL, &l2cap_debugfs_fops);
+ if (!l2cap_debugfs)
+ BT_ERR("Failed to create L2CAP debug file");
+ }
BT_INFO("L2CAP ver %s", VERSION);
BT_INFO("L2CAP socket layer initialized");
@@ -4044,7 +4053,7 @@ error:
static void __exit l2cap_exit(void)
{
- class_remove_file(bt_class, &class_attr_l2cap);
+ debugfs_remove(l2cap_debugfs);
if (bt_sock_unregister(BTPROTO_L2CAP) < 0)
BT_ERR("L2CAP socket unregistration failed");
diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
index cf16407..13f114e 100644
--- a/net/bluetooth/rfcomm/core.c
+++ b/net/bluetooth/rfcomm/core.c
@@ -33,6 +33,8 @@
#include <linux/init.h>
#include <linux/wait.h>
#include <linux/device.h>
+#include <linux/debugfs.h>
+#include <linux/seq_file.h>
#include <linux/net.h>
#include <linux/mutex.h>
#include <linux/kthread.h>
@@ -2098,14 +2100,10 @@ static struct hci_cb rfcomm_cb = {
.security_cfm = rfcomm_security_cfm
};
-static ssize_t rfcomm_dlc_sysfs_show(struct class *dev,
- struct class_attribute *attr,
- char *buf)
+static int rfcomm_dlc_debugfs_show(struct seq_file *f, void *x)
{
struct rfcomm_session *s;
struct list_head *pp, *p;
- char *str = buf;
- int size = PAGE_SIZE;
rfcomm_lock();
@@ -2114,29 +2112,33 @@ static ssize_t rfcomm_dlc_sysfs_show(struct class *dev,
list_for_each(pp, &s->dlcs) {
struct sock *sk = s->sock->sk;
struct rfcomm_dlc *d = list_entry(pp, struct rfcomm_dlc, list);
- int len;
- len = snprintf(str, size, "%s %s %ld %d %d %d %d\n",
- batostr(&bt_sk(sk)->src), batostr(&bt_sk(sk)->dst),
- d->state, d->dlci, d->mtu, d->rx_credits, d->tx_credits);
-
- size -= len;
- if (size <= 0)
- break;
-
- str += len;
+ seq_printf(f, "%s %s %ld %d %d %d %d\n",
+ batostr(&bt_sk(sk)->src),
+ batostr(&bt_sk(sk)->dst),
+ d->state, d->dlci, d->mtu,
+ d->rx_credits, d->tx_credits);
}
-
- if (size <= 0)
- break;
}
rfcomm_unlock();
- return (str - buf);
+ return 0;
+}
+
+static int rfcomm_dlc_debugfs_open(struct inode *inode, struct file *file)
+{
+ return single_open(file, rfcomm_dlc_debugfs_show, inode->i_private);
}
-static CLASS_ATTR(rfcomm_dlc, S_IRUGO, rfcomm_dlc_sysfs_show, NULL);
+static const struct file_operations rfcomm_dlc_debugfs_fops = {
+ .open = rfcomm_dlc_debugfs_open,
+ .read = seq_read,
+ .llseek = seq_lseek,
+ .release = single_release,
+};
+
+static struct dentry *rfcomm_dlc_debugfs;
/* ---- Initialization ---- */
static int __init rfcomm_init(void)
@@ -2153,8 +2155,12 @@ static int __init rfcomm_init(void)
goto unregister;
}
- if (class_create_file(bt_class, &class_attr_rfcomm_dlc) < 0)
- BT_ERR("Failed to create RFCOMM info file");
+ if (bt_debugfs) {
+ rfcomm_dlc_debugfs = debugfs_create_file("rfcomm_dlc", 0444,
+ bt_debugfs, NULL, &rfcomm_dlc_debugfs_fops);
+ if (!rfcomm_dlc_debugfs)
+ BT_ERR("Failed to create RFCOMM debug file");
+ }
err = rfcomm_init_ttys();
if (err < 0)
@@ -2182,7 +2188,7 @@ unregister:
static void __exit rfcomm_exit(void)
{
- class_remove_file(bt_class, &class_attr_rfcomm_dlc);
+ debugfs_remove(rfcomm_dlc_debugfs);
hci_unregister_cb(&rfcomm_cb);
diff --git a/net/bluetooth/rfcomm/sock.c b/net/bluetooth/rfcomm/sock.c
index 8d0ee0b..7f43976 100644
--- a/net/bluetooth/rfcomm/sock.c
+++ b/net/bluetooth/rfcomm/sock.c
@@ -40,6 +40,8 @@
#include <linux/skbuff.h>
#include <linux/list.h>
#include <linux/device.h>
+#include <linux/debugfs.h>
+#include <linux/seq_file.h>
#include <net/sock.h>
#include <asm/system.h>
@@ -1061,37 +1063,38 @@ done:
return result;
}
-static ssize_t rfcomm_sock_sysfs_show(struct class *dev,
- struct class_attribute *attr,
- char *buf)
+static int rfcomm_sock_debugfs_show(struct seq_file *f, void *p)
{
struct sock *sk;
struct hlist_node *node;
- char *str = buf;
- int size = PAGE_SIZE;
read_lock_bh(&rfcomm_sk_list.lock);
sk_for_each(sk, node, &rfcomm_sk_list.head) {
- int len;
-
- len = snprintf(str, size, "%s %s %d %d\n",
- batostr(&bt_sk(sk)->src), batostr(&bt_sk(sk)->dst),
+ seq_printf(f, "%s %s %d %d\n",
+ batostr(&bt_sk(sk)->src),
+ batostr(&bt_sk(sk)->dst),
sk->sk_state, rfcomm_pi(sk)->channel);
-
- size -= len;
- if (size <= 0)
- break;
-
- str += len;
}
read_unlock_bh(&rfcomm_sk_list.lock);
- return (str - buf);
+ return 0;
}
-static CLASS_ATTR(rfcomm, S_IRUGO, rfcomm_sock_sysfs_show, NULL);
+static int rfcomm_sock_debugfs_open(struct inode *inode, struct file *file)
+{
+ return single_open(file, rfcomm_sock_debugfs_show, inode->i_private);
+}
+
+static const struct file_operations rfcomm_sock_debugfs_fops = {
+ .open = rfcomm_sock_debugfs_open,
+ .read = seq_read,
+ .llseek = seq_lseek,
+ .release = single_release,
+};
+
+static struct dentry *rfcomm_sock_debugfs;
static const struct proto_ops rfcomm_sock_ops = {
.family = PF_BLUETOOTH,
@@ -1131,8 +1134,12 @@ int __init rfcomm_init_sockets(void)
if (err < 0)
goto error;
- if (class_create_file(bt_class, &class_attr_rfcomm) < 0)
- BT_ERR("Failed to create RFCOMM info file");
+ if (bt_debugfs) {
+ rfcomm_sock_debugfs = debugfs_create_file("rfcomm", 0444,
+ bt_debugfs, NULL, &rfcomm_sock_debugfs_fops);
+ if (!rfcomm_sock_debugfs)
+ BT_ERR("Failed to create RFCOMM debug file");
+ }
BT_INFO("RFCOMM socket layer initialized");
@@ -1146,7 +1153,7 @@ error:
void rfcomm_cleanup_sockets(void)
{
- class_remove_file(bt_class, &class_attr_rfcomm);
+ debugfs_remove(rfcomm_sock_debugfs);
if (bt_sock_unregister(BTPROTO_RFCOMM) < 0)
BT_ERR("RFCOMM socket layer unregistration failed");
diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index 967a751..e5b16b7 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -38,6 +38,8 @@
#include <linux/socket.h>
#include <linux/skbuff.h>
#include <linux/device.h>
+#include <linux/debugfs.h>
+#include <linux/seq_file.h>
#include <linux/list.h>
#include <net/sock.h>
@@ -953,37 +955,36 @@ drop:
return 0;
}
-static ssize_t sco_sysfs_show(struct class *dev,
- struct class_attribute *attr,
- char *buf)
+static int sco_debugfs_show(struct seq_file *f, void *p)
{
struct sock *sk;
struct hlist_node *node;
- char *str = buf;
- int size = PAGE_SIZE;
read_lock_bh(&sco_sk_list.lock);
sk_for_each(sk, node, &sco_sk_list.head) {
- int len;
-
- len = snprintf(str, size, "%s %s %d\n",
- batostr(&bt_sk(sk)->src), batostr(&bt_sk(sk)->dst),
- sk->sk_state);
-
- size -= len;
- if (size <= 0)
- break;
-
- str += len;
+ seq_printf(f, "%s %s %d\n", batostr(&bt_sk(sk)->src),
+ batostr(&bt_sk(sk)->dst), sk->sk_state);
}
read_unlock_bh(&sco_sk_list.lock);
- return (str - buf);
+ return 0;
}
-static CLASS_ATTR(sco, S_IRUGO, sco_sysfs_show, NULL);
+static int sco_debugfs_open(struct inode *inode, struct file *file)
+{
+ return single_open(file, sco_debugfs_show, inode->i_private);
+}
+
+static const struct file_operations sco_debugfs_fops = {
+ .open = sco_debugfs_open,
+ .read = seq_read,
+ .llseek = seq_lseek,
+ .release = single_release,
+};
+
+static struct dentry *sco_debugfs;
static const struct proto_ops sco_sock_ops = {
.family = PF_BLUETOOTH,
@@ -1041,8 +1042,12 @@ static int __init sco_init(void)
goto error;
}
- if (class_create_file(bt_class, &class_attr_sco) < 0)
- BT_ERR("Failed to create SCO info file");
+ if (bt_debugfs) {
+ sco_debugfs = debugfs_create_file("sco", 0444,
+ bt_debugfs, NULL, &sco_debugfs_fops);
+ if (!sco_debugfs)
+ BT_ERR("Failed to create SCO debug file");
+ }
BT_INFO("SCO (Voice Link) ver %s", VERSION);
BT_INFO("SCO socket layer initialized");
@@ -1056,7 +1061,7 @@ error:
static void __exit sco_exit(void)
{
- class_remove_file(bt_class, &class_attr_sco);
+ debugfs_remove(sco_debugfs);
if (bt_sock_unregister(BTPROTO_SCO) < 0)
BT_ERR("SCO socket unregistration failed");
--
1.6.6.1
^ permalink raw reply related
* [PATCH 1/3] Bluetooth: Fix potential bad memory access with sysfs files
From: Marcel Holtmann @ 2010-03-21 5:41 UTC (permalink / raw)
To: David Miller, Linus Torvalds; +Cc: netdev
In-Reply-To: <cover.1269149354.git.marcel@holtmann.org>
When creating a high number of Bluetooth sockets (L2CAP, SCO
and RFCOMM) it is possible to scribble repeatedly on arbitrary
pages of memory. Ensure that the content of these sysfs files is
always less than one page. Even if this means truncating. The
files in question are scheduled to be moved over to debugfs in
the future anyway.
Based on initial patches from Neil Brown and Linus Torvalds
Reported-by: Neil Brown <neilb@suse.de>
Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
---
net/bluetooth/l2cap.c | 10 +++++++++-
net/bluetooth/rfcomm/core.c | 13 ++++++++++++-
net/bluetooth/rfcomm/sock.c | 11 ++++++++++-
net/bluetooth/sco.c | 11 ++++++++++-
4 files changed, 41 insertions(+), 4 deletions(-)
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 4db7ae2..2755182 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -3944,16 +3944,24 @@ static ssize_t l2cap_sysfs_show(struct class *dev,
struct sock *sk;
struct hlist_node *node;
char *str = buf;
+ int size = PAGE_SIZE;
read_lock_bh(&l2cap_sk_list.lock);
sk_for_each(sk, node, &l2cap_sk_list.head) {
struct l2cap_pinfo *pi = l2cap_pi(sk);
+ int len;
- str += sprintf(str, "%s %s %d %d 0x%4.4x 0x%4.4x %d %d %d\n",
+ len = snprintf(str, size, "%s %s %d %d 0x%4.4x 0x%4.4x %d %d %d\n",
batostr(&bt_sk(sk)->src), batostr(&bt_sk(sk)->dst),
sk->sk_state, __le16_to_cpu(pi->psm), pi->scid,
pi->dcid, pi->imtu, pi->omtu, pi->sec_level);
+
+ size -= len;
+ if (size <= 0)
+ break;
+
+ str += len;
}
read_unlock_bh(&l2cap_sk_list.lock);
diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
index db8a68e..cf16407 100644
--- a/net/bluetooth/rfcomm/core.c
+++ b/net/bluetooth/rfcomm/core.c
@@ -2105,6 +2105,7 @@ static ssize_t rfcomm_dlc_sysfs_show(struct class *dev,
struct rfcomm_session *s;
struct list_head *pp, *p;
char *str = buf;
+ int size = PAGE_SIZE;
rfcomm_lock();
@@ -2113,11 +2114,21 @@ static ssize_t rfcomm_dlc_sysfs_show(struct class *dev,
list_for_each(pp, &s->dlcs) {
struct sock *sk = s->sock->sk;
struct rfcomm_dlc *d = list_entry(pp, struct rfcomm_dlc, list);
+ int len;
- str += sprintf(str, "%s %s %ld %d %d %d %d\n",
+ len = snprintf(str, size, "%s %s %ld %d %d %d %d\n",
batostr(&bt_sk(sk)->src), batostr(&bt_sk(sk)->dst),
d->state, d->dlci, d->mtu, d->rx_credits, d->tx_credits);
+
+ size -= len;
+ if (size <= 0)
+ break;
+
+ str += len;
}
+
+ if (size <= 0)
+ break;
}
rfcomm_unlock();
diff --git a/net/bluetooth/rfcomm/sock.c b/net/bluetooth/rfcomm/sock.c
index ca87d6a..8d0ee0b 100644
--- a/net/bluetooth/rfcomm/sock.c
+++ b/net/bluetooth/rfcomm/sock.c
@@ -1068,13 +1068,22 @@ static ssize_t rfcomm_sock_sysfs_show(struct class *dev,
struct sock *sk;
struct hlist_node *node;
char *str = buf;
+ int size = PAGE_SIZE;
read_lock_bh(&rfcomm_sk_list.lock);
sk_for_each(sk, node, &rfcomm_sk_list.head) {
- str += sprintf(str, "%s %s %d %d\n",
+ int len;
+
+ len = snprintf(str, size, "%s %s %d %d\n",
batostr(&bt_sk(sk)->src), batostr(&bt_sk(sk)->dst),
sk->sk_state, rfcomm_pi(sk)->channel);
+
+ size -= len;
+ if (size <= 0)
+ break;
+
+ str += len;
}
read_unlock_bh(&rfcomm_sk_list.lock);
diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index f93b939..967a751 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -960,13 +960,22 @@ static ssize_t sco_sysfs_show(struct class *dev,
struct sock *sk;
struct hlist_node *node;
char *str = buf;
+ int size = PAGE_SIZE;
read_lock_bh(&sco_sk_list.lock);
sk_for_each(sk, node, &sco_sk_list.head) {
- str += sprintf(str, "%s %s %d\n",
+ int len;
+
+ len = snprintf(str, size, "%s %s %d\n",
batostr(&bt_sk(sk)->src), batostr(&bt_sk(sk)->dst),
sk->sk_state);
+
+ size -= len;
+ if (size <= 0)
+ break;
+
+ str += len;
}
read_unlock_bh(&sco_sk_list.lock);
--
1.6.6.1
^ permalink raw reply related
* Pull request: bluetooth-2.6 2010-03-21
From: Marcel Holtmann @ 2010-03-21 5:41 UTC (permalink / raw)
To: David Miller, Linus Torvalds; +Cc: netdev
Hi Dave,
these patches fix a potential remote denial of service in the L2CAP
layer and a potential bad memory access with some sysfs files.
As agreed with Linus, I fixed the sysfs issue in two steps. The first
patch just avoids the invalid memory access while accepting that the
content might be truncated. The second patch moves everything over to
debugfs (where these files belong anyway) and switches to seq_file.
This two step approach allows an easy backport to -stable kernel by
only picking the first patch.
Linus, feel free to pull directly since both patches are security
related and should be merged as quickly as possible.
Regards
Marcel
Please pull from
git://git.kernel.org/pub/scm/linux/kernel/git/holtmann/bluetooth-2.6.git master
This will update the following files:
include/net/bluetooth/bluetooth.h | 2 +-
net/bluetooth/hci_sysfs.c | 3 +-
net/bluetooth/l2cap.c | 48 +++++++++++++++++++++++++++----------
net/bluetooth/rfcomm/core.c | 41 ++++++++++++++++++++++---------
net/bluetooth/rfcomm/sock.c | 38 ++++++++++++++++++++--------
net/bluetooth/sco.c | 38 ++++++++++++++++++++---------
6 files changed, 119 insertions(+), 51 deletions(-)
through these ChangeSets:
Andrei Emeltchenko (1):
Bluetooth: Fix kernel crash on L2CAP stress tests
Marcel Holtmann (2):
Bluetooth: Fix potential bad memory access with sysfs files
Bluetooth: Convert debug files to actually use debugfs instead of sysfs
^ permalink raw reply
* Re: [PATCH] xfrm: cache bundle lookup results in flow cache
From: Herbert Xu @ 2010-03-21 0:46 UTC (permalink / raw)
To: Timo Teräs; +Cc: netdev, David S. Miller
In-Reply-To: <4BA4F718.3020700@iki.fi>
On Sat, Mar 20, 2010 at 06:26:00PM +0200, Timo Teräs wrote:
>
> So should go ahead and:
> 1. modify flow cache to be more generic (have virtual put and get
> for each object; and remove the atomic_t pointer)
> 2. modify flow cache to have slow and fast resolvers so we can
> copy with the current sleeping requirement
I don't think we need either of these. To support the sleep
requirement, just return -EAGAIN from the resolver when the
template can't be resolved. Then the caller of flow_cache_lookup
can sleep as it does now. It simply has to repeat the flow
cache lookup afterwards.
> 3. cache bundles instead of policies for outgoing stuff
> 4. kill find_bundle and just instantiate new ones if we get cache
> miss
> 5. put all bundles to global hlist (since only place that walks
> through them is gc, and stale bundle can be dst_free'd right
> away); use genid's for policy to flush old bundles
> 6. dst_free and unlink bundle immediately if it's found to be stale
Sounds good.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH] TCP: check min TTL on received ICMP packets
From: Stephen Hemminger @ 2010-03-21 0:05 UTC (permalink / raw)
To: David Miller; +Cc: netdev, pekkas
In-Reply-To: <20100319.210801.39166378.davem@davemloft.net>
----- "David Miller" <davem@davemloft.net> wrote:
> From: Stephen Hemminger <shemminger@vyatta.com>
> Date: Thu, 18 Mar 2010 14:27:32 -0700
>
> > This adds RFC5082 checks for TTL on received ICMP packets.
> > It adds some security against spoofed ICMP packets
> > disrupting GTSM protected sessions.
> >
> > Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
>
> Applied.
>
> > Please apply to 2.6.33 since it basically a "follow correct RFC"
> > fix to original GTSM patch.
>
I meant 2.6.34... min_ttl is not in 2.6.33
^ permalink raw reply
* [PATCH] Fix bug in addrconf hlist conversion
From: David Miller @ 2010-03-20 23:19 UTC (permalink / raw)
To: shemminger; +Cc: netdev
Committed to net-next-2.6
ipv6: Fix bug in ipv6_chk_same_addr().
hlist_for_each_entry(p...) will not necessarily initialize 'p'
to anything if the hlist is empty. GCC notices this and emits
a warning.
Just return true explicitly when we hit a match, and return
false is we fall out of the loop without one.
Signed-off-by: David S. Miller <davem@davemloft.net>
---
net/ipv6/addrconf.c | 15 +++++++--------
1 files changed, 7 insertions(+), 8 deletions(-)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 7d7d4b1..68e5809 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -155,8 +155,8 @@ static void ipv6_ifa_notify(int event, struct inet6_ifaddr *ifa);
static void inet6_prefix_notify(int event, struct inet6_dev *idev,
struct prefix_info *pinfo);
-static int ipv6_chk_same_addr(struct net *net, const struct in6_addr *addr,
- struct net_device *dev);
+static bool ipv6_chk_same_addr(struct net *net, const struct in6_addr *addr,
+ struct net_device *dev);
static ATOMIC_NOTIFIER_HEAD(inet6addr_chain);
@@ -1295,23 +1295,22 @@ int ipv6_chk_addr(struct net *net, struct in6_addr *addr,
}
EXPORT_SYMBOL(ipv6_chk_addr);
-static
-int ipv6_chk_same_addr(struct net *net, const struct in6_addr *addr,
- struct net_device *dev)
+static bool ipv6_chk_same_addr(struct net *net, const struct in6_addr *addr,
+ struct net_device *dev)
{
+ unsigned int hash = ipv6_addr_hash(addr);
struct inet6_ifaddr *ifp;
struct hlist_node *node;
- unsigned int hash = ipv6_addr_hash(addr);
hlist_for_each_entry(ifp, node, &inet6_addr_lst[hash], addr_lst) {
if (!net_eq(dev_net(ifp->idev->dev), net))
continue;
if (ipv6_addr_equal(&ifp->addr, addr)) {
if (dev == NULL || ifp->idev->dev == dev)
- break;
+ return true;
}
}
- return ifp != NULL;
+ return false;
}
int ipv6_chk_prefix(struct in6_addr *addr, struct net_device *dev)
--
1.6.6.1
^ permalink raw reply related
* Re: [PATCH 9/9] IPv6: addrconf cleanup addrconf_verify
From: David Miller @ 2010-03-20 23:13 UTC (permalink / raw)
To: shemminger; +Cc: netdev
In-Reply-To: <20100318063123.441975388@vyatta.com>
From: Stephen Hemminger <shemminger@vyatta.com>
Date: Wed, 17 Mar 2010 23:31:17 -0700
> The variable regen_advance is only used in the privacy case.
> Move it to simplify code and eliminate ifdef's
>
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
Applied.
^ permalink raw reply
* Re: [PATCH 8/9] addrconf: checkpatch fixes
From: David Miller @ 2010-03-20 23:13 UTC (permalink / raw)
To: shemminger; +Cc: netdev
In-Reply-To: <20100318063123.372635144@vyatta.com>
From: Stephen Hemminger <shemminger@vyatta.com>
Date: Wed, 17 Mar 2010 23:31:16 -0700
> Fix some of the checkpatch complaints.
>
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
Applied.
^ permalink raw reply
* Re: [PATCH 7/9] IPv6: addrconf cleanups
From: David Miller @ 2010-03-20 23:12 UTC (permalink / raw)
To: shemminger; +Cc: netdev
In-Reply-To: <20100318063123.302950866@vyatta.com>
From: Stephen Hemminger <shemminger@vyatta.com>
Date: Wed, 17 Mar 2010 23:31:15 -0700
> Some minor stuff, reformat comments and add whitespace for clarity
>
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next-2.6] ipv6: Reduce timer events for addrconf_verify().
From: David Miller @ 2010-03-20 23:12 UTC (permalink / raw)
To: yoshfuji; +Cc: netdev, stephen.hemminger
In-Reply-To: <201003190635.o2J6Z17W016418@94.43.138.210.xn.2iij.net>
From: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
Date: Thu, 18 Mar 2010 18:01:11 +0900
> This patch reduces timer events while keeping accuracy by rounding
> our timer and/or batching several address validations in addrconf_verify().
>
> addrconf_verify() is called at earliest timeout among interface addresses'
> timeouts, but at maximum ADDR_CHECK_FREQUENCY (120 secs).
>
> In most cases, all of timeouts of interface addresses are long enough
> (e.g. several hours or days vs 2 minutes), this timer is usually called
> every ADDR_CHECK_FREQUENCY, and it is okay to be lazy.
> (Note this timer could be eliminated if all code paths which modifies
> variables related to timeouts call us manually, but it is another story.)
>
> However, in other least but important cases, we try keeping accuracy.
>
> When the real interface address timeout is coming, and the timeout
> is just before the rounded timeout, we accept some error.
>
> When a timeout has been reached, we also try batching other several
> events in very near future.
>
> Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
Applied.
^ permalink raw reply
* Re: [PATCH 5/9] ipv6: convert idev_list to list macros
From: David Miller @ 2010-03-20 23:05 UTC (permalink / raw)
To: shemminger; +Cc: netdev
In-Reply-To: <20100318063123.162276628@vyatta.com>
From: Stephen Hemminger <shemminger@vyatta.com>
Date: Wed, 17 Mar 2010 23:31:13 -0700
> Convert to list macro's for the list of addresses per interface
> in IPv6.
>
>
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
Applied.
^ permalink raw reply
* Re: [PATCH 4/9] ipv6: user better hash for addrconf
From: David Miller @ 2010-03-20 23:05 UTC (permalink / raw)
To: shemminger; +Cc: netdev
In-Reply-To: <20100318063123.092727548@vyatta.com>
From: Stephen Hemminger <shemminger@vyatta.com>
Date: Wed, 17 Mar 2010 23:31:12 -0700
> The existing hash function has a couple of issues:
> * it is hardwired to 16 for IN6_ADDR_HSIZE
> * limited to 256 and callers using int
> * use jhash2 rather than some old BSD algorithm
>
> No need for random seed since this is local only (based on assigned
> addresses) table.
>
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
Applied.
^ permalink raw reply
* Re: [PATCH 3/9] IPv6: convert addrconf hash list to RCU
From: David Miller @ 2010-03-20 23:05 UTC (permalink / raw)
To: shemminger; +Cc: netdev
In-Reply-To: <20100318063123.022778460@vyatta.com>
From: Stephen Hemminger <shemminger@vyatta.com>
Date: Wed, 17 Mar 2010 23:31:11 -0700
> Convert from reader/writer lock to RCU and spinlock for addrconf
> hash list.
>
> Adds an additional helper macro for hlist_for_each_entry_continue_rcu
> to handle the continue case.
>
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
Applied.
^ permalink raw reply
* Re: [PATCH 2/9] ipv6: convert addrconf list to hlist
From: David Miller @ 2010-03-20 23:05 UTC (permalink / raw)
To: shemminger; +Cc: netdev
In-Reply-To: <20100318063122.935061603@vyatta.com>
From: Stephen Hemminger <shemminger@vyatta.com>
Date: Wed, 17 Mar 2010 23:31:10 -0700
> Using hash list macros, simplifies code and helps later RCU.
>
> This patch includes some initialization that is not strictly necessary,
> since an empty hlist node/list is all zero; and list is in BSS
> and node is allocated with kzalloc.
>
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
Applied.
^ permalink raw reply
* Re: [PATCH 1/9] ipv6: convert temporary address list to list macros
From: David Miller @ 2010-03-20 23:05 UTC (permalink / raw)
To: shemminger; +Cc: netdev
In-Reply-To: <20100318063122.866367333@vyatta.com>
From: Stephen Hemminger <shemminger@vyatta.com>
Date: Wed, 17 Mar 2010 23:31:09 -0700
> Use list macros instead of open coded linked list.
>
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
Applied.
^ permalink raw reply
* [GIT]: Networking
From: David Miller @ 2010-03-20 22:23 UTC (permalink / raw)
To: torvalds; +Cc: akpm, netdev, linux-kernel
1) The TCP TTL limiting extension added to 2.6.34 needs to
check ICMP packets destined for such TCP sockets too.
Fix from Stephen Hemminger.
2) KS8851 regression, OOPS on multicast changes. Fix from
Abraham Arce.
3) TCP started setting POLLIN for pure OOB data accidently.
Noticed and fixed by Alexandra Kossovsky.
4) Add PCI IDs for new IGB device, from Carolyn Wyborny.
5) skb_bond_should_drop() should use ACCESS_ONCE() on
skb->dev_master to avoid potential oops, from Eric Dumazet.
6) TCP can mark the retransmit queue head SKB lost even when
->packets_out == 0, causing early retransmits. Fix from
Lennart Schulte.
7) NET_DMA can wedge a TCP socket if receive water marks are high
and the user does a large read. Fix by periodically submitting
and purging completed NET_DMA requests, instead of doing so only
when exiting tcp_recvmsg(). Fix from Steven J. Magnani.
8) Netlink fixes from Pablo Neira Ayuso:
a) nla_get_be64() needs to use nla_memcpy() since the object
will always be unaligned.
b) Fix NETLINK_RECV_NO_ENOBUFS to elide the error even when
it gets signalled via netlink_set_err().
c) Use the fix #b to cure ctnetlink from losing events.
9) VHOST bug fixes from Jeff Dike, Michael S. Tsirkin:
a) vhost_net_set_backend() can return with mutex held.
b) logic error causes interrupt mitigation to never trigger
c) several vring ioctls accidently return with mutex held
10) JME driver bug fixes from Guo-Fu Tseng:
a) VLAN RX code paths leaks memory
b) VLGRP acceses are racy
11) Fix Fibre link issues and and wake on lan in IXGBE driver.
From Mallikarjuna R Chilakala and Jeff Kirsher.
12) smsc95xx needs workaround for checksum offload of tiny UDP packets.
13) Recent bug fix unearthed an error in IPSEC route updating for
ipv4. We need to implement ipv4_dst_check() properly, otherwise
PMTU updates et al. never propagate any more to IPSEC routes.
Fix from Timo Teras, with help from Herbert Xu.
14) IXGBE indicates netdev feature changes too early, it must
occur after FCOE tx queue setup is done. Fix from Vasu Dev.
15) IXGBE can potentially double unmap DMA mappings, noticed by
Simon Horman and fixed by Mallikarjuna R Chilakala.
16) Multicast code can access vif_table[] outside of array bounds.
Fix from Patrick McHardy.
17) KS8695 updates descriptor head way too often, only hit it at
end of RX processing loop. From Yegor Yefremov with help
from Eric Dumazet.
18) IXGBEVF driver fixes from Greg Rose and Jeff Kirsher:
a) VF stats don't account properly across reset
b) recovery from PF resets take way too long, reduce
watchdog delay
Please pull, thanks a lot!
The following changes since commit 220bf991b0366cc50a94feede3d7341fa5710ee4:
Linus Torvalds (1):
Linux 2.6.34-rc2
are available in the git repository at:
master.kernel.org:/pub/scm/linux/kernel/git/davem/net-2.6.git master
Abraham Arce (1):
KS8851: Avoid NULL pointer in set rx mode
Adel Gadllah (1):
iwlwifi: Silence tfds_in_queue message
Alexandra Kossovsky (1):
tcp: Fix OOB POLLIN avoidance.
Carolyn Wyborny (1):
igb: Add support for 82576 ET2 Quad Port Server Adapter
David S. Miller (2):
Merge branch 'master' of git://git.kernel.org/.../linville/wireless-2.6
Merge branch 'vhost' of git://git.kernel.org/.../mst/vhost
Eric Dumazet (1):
net: Potential null skb->dev dereference
Felix Fietkau (1):
ath9k: fix BUG_ON triggered by PAE frames
Grazvydas Ignotas (1):
wl1251: fix potential crash
Greg Rose (3):
ixgbevf: Fix VF Stats accounting after reset
ixgbevf: Shorten up delay timer for watchdog task
ixgbevf: Message formatting cleanups
Guo-Fu Tseng (3):
jme: Fix VLAN memory leak
jme: Protect vlgrp structure by pause RX actions.
jme: Advance driver version number
Herbert Xu (1):
ipv6: Remove redundant dst NULL check in ip6_dst_check
Jeff Dike (1):
vhost: fix error path in vhost_net_set_backend
Lennart Schulte (1):
tcp: Fix tcp_mark_head_lost() with packets == 0
Mallikarjuna R Chilakala (3):
ixgbe: Fix 82599 multispeed fiber link issues due to Tx laser flapping
ixgbe: Fix 82599 KX4 Wake on LAN issue after an improper system shutdown
ixgbe: Set IXGBE_RSC_CB(skb)->DMA field to zero after unmapping the address
Michael S. Tsirkin (2):
vhost: fix interrupt mitigation with raw sockets
vhost: fix error handling in vring ioctls
Pablo Neira Ayuso (3):
netlink: fix unaligned access in nla_get_be64()
netlink: fix NETLINK_RECV_NO_ENOBUFS in netlink_set_err()
netfilter: ctnetlink: fix reliable event delivery if message building fails
Patrick McHardy (1):
net: ipmr/ip6mr: fix potential out-of-bounds vif_table access
Steve Glendinning (1):
smsc95xx: Fix tx checksum offload for small packets
Steven J. Magnani (1):
NET_DMA: free skbs periodically
Timo Teräs (1):
ipv4: check rt_genid in dst_check
Vasu Dev (1):
ixgbe: fix for real_num_tx_queues update issue
Yegor Yefremov (1):
KS8695: update ksp->next_rx_desc_read at the end of rx loop
stephen hemminger (1):
TCP: check min TTL on received ICMP packets
drivers/net/arm/ks8695net.c | 24 +++-----
drivers/net/igb/e1000_82575.c | 1 +
drivers/net/igb/e1000_hw.h | 1 +
drivers/net/igb/igb_main.c | 1 +
drivers/net/ixgbe/ixgbe_82599.c | 78 ++++++++++++++------------
drivers/net/ixgbe/ixgbe_fcoe.c | 6 +-
drivers/net/ixgbe/ixgbe_main.c | 22 ++++++--
drivers/net/ixgbe/ixgbe_type.h | 1 +
drivers/net/ixgbevf/ethtool.c | 42 +++++++++-----
drivers/net/ixgbevf/ixgbevf_main.c | 74 +++++++++++++++---------
drivers/net/ixgbevf/vf.h | 6 ++
drivers/net/jme.c | 35 ++++++++++++
drivers/net/jme.h | 2 +-
drivers/net/ks8851.c | 1 -
drivers/net/usb/smsc95xx.c | 18 +++++-
drivers/net/wireless/ath/ath9k/xmit.c | 21 +-------
drivers/net/wireless/iwlwifi/iwl-tx.c | 2 +-
drivers/net/wireless/wl12xx/wl1251_debugfs.c | 3 +-
drivers/vhost/net.c | 10 ++-
drivers/vhost/vhost.c | 18 ++++--
include/linux/netdevice.h | 8 +-
include/linux/netfilter/nfnetlink.h | 2 +-
include/linux/netlink.h | 2 +-
include/net/netlink.h | 6 ++-
net/8021q/vlan_core.c | 4 +-
net/core/dev.c | 8 ++-
net/ipv4/ipmr.c | 3 +
net/ipv4/route.c | 14 +++-
net/ipv4/tcp.c | 65 ++++++++++++++-------
net/ipv4/tcp_input.c | 3 +
net/ipv4/tcp_ipv4.c | 5 ++
net/ipv6/ip6mr.c | 3 +
net/ipv6/route.c | 2 +-
net/netfilter/nf_conntrack_netlink.c | 4 +-
net/netfilter/nfnetlink.c | 4 +-
net/netlink/af_netlink.c | 17 +++++-
36 files changed, 335 insertions(+), 181 deletions(-)
^ permalink raw reply
* Re: [GIT PULL] vhost-net fixes for issues in 2.6.34-rc1
From: David Miller @ 2010-03-20 21:41 UTC (permalink / raw)
To: mst
Cc: rusty, kvm, virtualization, netdev, linux-kernel, samudrala,
jirislaby, chavey, quintela, unai.uribarri, jdike
In-Reply-To: <20100318095355.GA24194@redhat.com>
From: "Michael S. Tsirkin" <mst@redhat.com>
Date: Thu, 18 Mar 2010 11:53:55 +0200
> The following tree includes patches fixing issues with vhost-net in
> 2.6.34-rc1. Please pull them for 2.6.34.
Pulled, thanks a lot.
^ permalink raw reply
* Re: [PATCH 1/3] netlink: fix NETLINK_RECV_NO_ENOBUFS in netlink_set_err()
From: David Miller @ 2010-03-20 21:30 UTC (permalink / raw)
To: pablo; +Cc: kaber, netdev
In-Reply-To: <4BA2C44A.1000100@netfilter.org>
From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Fri, 19 Mar 2010 01:24:42 +0100
> Patrick McHardy wrote:
>> Generally the logic seems inverted, you should return an error
>> to conntrack if userspace wasn't notified of the error.
>
> Indeed, thanks. Are you OK with this patch instead?
I went over all of this and now the patches #1 and #2 look
correct to me, so I've applied them to net-2.6
Patrick let me know if you think any follow-on tidy ups
are still necessary and we can add them.
Thanks Pablo!
^ permalink raw reply
* Re: [PATCH] NET_DMA: free skbs periodically
From: David Miller @ 2010-03-20 21:29 UTC (permalink / raw)
To: steve; +Cc: netdev, kuznet, pekkas, jmorris, yoshfuji, kaber, linux-kernel
In-Reply-To: <1268752964-4397-1-git-send-email-steve@digidescorp.com>
From: "Steven J. Magnani" <steve@digidescorp.com>
Date: Tue, 16 Mar 2010 10:22:44 -0500
> Under NET_DMA, data transfer can grind to a halt when userland issues a
> large read on a socket with a high RCVLOWAT (i.e., 512 KB for both).
> This appears to be because the NET_DMA design queues up lots of memcpy
> operations, but doesn't issue or wait for them (and thus free the
> associated skbs) until it is time for tcp_recvmesg() to return.
> The socket hangs when its TCP window goes to zero before enough data is
> available to satisfy the read.
>
> Periodically issue asynchronous memcpy operations, and free skbs for ones
> that have completed, to prevent sockets from going into zero-window mode.
>
> Signed-off-by: Steven J. Magnani <steve@digidescorp.com>
Applied, thanks for fixing this bug Steven.
^ permalink raw reply
* Re: [PATCH] drivers/net/wimax/i2400m/fw.c fix possible double free
From: David Miller @ 2010-03-20 21:24 UTC (permalink / raw)
To: darrenrjenkins
Cc: cindy.h.kao, wimax, netdev, kernel-janitors, linux-kernel,
linux-wimax
In-Reply-To: <1268902008.10536.41.camel@ICE-BOX>
From: Darren Jenkins <darrenrjenkins@gmail.com>
Date: Thu, 18 Mar 2010 19:46:48 +1100
>
> On Thu, Mar 18, 2010 at 9:10 AM, Inaky Perez-Gonzalez
> <inaky.perez-gonzalez@intel.com> wrote:
>
>> If krealloc() fails to aallocate a new pointer, the old block is
>> unmodified, so by doing this you are leaking a buffer allocation.
>
> It seems you are right.
> So now understanding correctly how krealloc() works I can see that the
> double kfree() can only actually happen if the el_size parameter to
> i2400m_zrealloc_2x() is zero, and it isn't at the two call sites.
>
> So this was a false positive and I am sorry for the noise.
No problem, at least the code got audited :-)
^ permalink raw reply
* Gianfar: RX Recycle skb->len error
From: Ben Menchaca (ben@bigfootnetworks.com) @ 2010-03-20 19:54 UTC (permalink / raw)
To: netdev@vger.kernel.org
We are seeing some random skb data length errors on RX after long-running, full-gigabit traffic. First, my debugging and solution are based on the following invariant assumption:
(skb->tail - skb->data) == skb->len
If this is wrong, please educate.
After some tracing, here is where the error packets seem to originate:
1. We are cleaning rx, in gfar_clean_rx_ring;
2. A new RX skb is drawn from the rx_recycle queue, and obey the above invariant (so, in gfar_new_skb(), __skb_dequeue returns an skb);
3. At this point skb_reserve is called, which moves data and tail by the same calculated alignamount;
4. So, newskb is not NULL. However, !(bdp->status & RXBD_LAST) || (bdp->status & RXBD_ERR)) is evaluates to true;
5. Since newskb is not NULL, we arrive at the else if (skb), which is true;
6. skb->data = skb->head + NET_SKB_PAD is applied, and then the skb is requeued for recycling.
At this point, skb->data != skb->tail, but skb->len == 0. When this skb is used for the next RX, it is causing issues later when we skb_put trailers, and then trust skb->len.
I would propose something like:
--- a/drivers/net/gianfar.c
+++ b/drivers/net/gianfar.c
@@ -2540,6 +2540,7 @@
* recycle list.
*/
skb->data = skb->head + NET_SKB_PAD;
+ skb_reset_tail_pointer(skb);
__skb_queue_head(&priv->rx_recycle, skb);
}
} else {
Ben Menchaca
Bigfoot Networks
^ permalink raw reply
* Re: [PATCH] xfrm: cache bundle lookup results in flow cache
From: Timo Teräs @ 2010-03-20 16:26 UTC (permalink / raw)
To: Herbert Xu; +Cc: netdev, David S. Miller
In-Reply-To: <20100320151751.GB2950@gondor.apana.org.au>
Herbert Xu wrote:
> On Fri, Mar 19, 2010 at 11:53:44AM +0200, Timo Teräs wrote:
>> But now, thinking more, it would probably make more sense to
>> just cache xfrm_dst's and keep ref to the policy on them. So
>> in general I agree with your recommendation. The only immediate
>> problem I can think now is that the resolved would need to
>> atomically check if xfrm_dst is valid, if not, resolve new one.
>> But creating new xfrm_dst involves taking locks and can sleep
>> so it cannot be inside the main resolver.
>
> OK this brings out my favourite topic :)
>
> The reason we have to sleep is to resolve the template. So if
> we had queueing for pending xfrm_dst objects, we wouldn't have
> to sleep at all when creating the top-level xfrm_dst.
>
> Packets using that xfrm_dst can wait in the queue until it is
> fully resolved. Now obviously this code doesn't exist so this
> is all just a wet dream.
Right. That sounds useful.
> Setting my favourite topic aside, I have to come to the conclusion
> that your patch still doesn't fully resolve the problem you set out
> to fix.
>
> The crux of the issue is the linked list of all bundles in a
> policy and the obvious problems stemming from walking a linked
> list that is unbounded.
>
> The reason I think it doesn't fully resolve this is because of
> the flow cache. Being a per-cpu cache, when you create the xfrm
> dst the first time around, you'll at most put it in one CPU's
> cache.
>
> The next CPU that comes along will still have to walk that same
> bundle linked list. So we're back to square one.
Not exactly, each CPU does one slow lookup after which it
finds it fast. But yes, it's not perfect solution. Especially,
if cpu happens to get switched between the initial lookup and
the update.
> Now Dave, my impression is that we picked the per-cpu design
> because it was the best data structure we had back in 2002,
> right?
>
> If so I'd like us to think about the possibility of switching
> over to a different design, in particular, an RCU-based hash
> table, similar to the one I just used for bridge multicasting.
>
> This would eliminate the need for walking the bundle list apart
> from the case when we're destroying the policy, which can be
> done in process context.
Right. This would speed the bundle lookup in all cases.
Except... we can have override policy on per-socket basis.
We should include the per-socket override in the flow lookups
so that those sockets get also boost from the cache. Though
usual use case is to disable all policies (so e.g. IKE can
be talked without policies applying).
> Actually I just realised that the other way we can fix this is
> to make xfrm_dst objects per-cpu just like IPv4 routes. That
> is, when you fail to find an xfrm_dst object in the per-cpu
> cache, you dont' bother calling xfrm_find_bundle but just make
> a new bundle.
>
> This is probably much easier than replacing the whole flow cache.
> Can any one think of any problems with duplicate xfrm_dst objects?
Sounds like a very good idea. If we instantiate new xfrm_dst,
all that it shares with others is xfrm_state and xfrm_policy
(inner objects will be unique). Since that's what happens anyway
I don't see any problem with this.
So should go ahead and:
1. modify flow cache to be more generic (have virtual put and get
for each object; and remove the atomic_t pointer)
2. modify flow cache to have slow and fast resolvers so we can
copy with the current sleeping requirement
3. cache bundles instead of policies for outgoing stuff
4. kill find_bundle and just instantiate new ones if we get cache
miss
5. put all bundles to global hlist (since only place that walks
through them is gc, and stale bundle can be dst_free'd right
away); use genid's for policy to flush old bundles
6. dst_free and unlink bundle immediately if it's found to be stale
- Timo
^ permalink raw reply
* Re: [PATCH] xfrm: cache bundle lookup results in flow cache
From: Herbert Xu @ 2010-03-20 15:17 UTC (permalink / raw)
To: Timo Teräs; +Cc: netdev, David S. Miller
In-Reply-To: <4BA349A8.9050105@iki.fi>
On Fri, Mar 19, 2010 at 11:53:44AM +0200, Timo Teräs wrote:
>
> But now, thinking more, it would probably make more sense to
> just cache xfrm_dst's and keep ref to the policy on them. So
> in general I agree with your recommendation. The only immediate
> problem I can think now is that the resolved would need to
> atomically check if xfrm_dst is valid, if not, resolve new one.
> But creating new xfrm_dst involves taking locks and can sleep
> so it cannot be inside the main resolver.
OK this brings out my favourite topic :)
The reason we have to sleep is to resolve the template. So if
we had queueing for pending xfrm_dst objects, we wouldn't have
to sleep at all when creating the top-level xfrm_dst.
Packets using that xfrm_dst can wait in the queue until it is
fully resolved. Now obviously this code doesn't exist so this
is all just a wet dream.
Setting my favourite topic aside, I have to come to the conclusion
that your patch still doesn't fully resolve the problem you set out
to fix.
The crux of the issue is the linked list of all bundles in a
policy and the obvious problems stemming from walking a linked
list that is unbounded.
The reason I think it doesn't fully resolve this is because of
the flow cache. Being a per-cpu cache, when you create the xfrm
dst the first time around, you'll at most put it in one CPU's
cache.
The next CPU that comes along will still have to walk that same
bundle linked list. So we're back to square one.
Now Dave, my impression is that we picked the per-cpu design
because it was the best data structure we had back in 2002,
right?
If so I'd like us to think about the possibility of switching
over to a different design, in particular, an RCU-based hash
table, similar to the one I just used for bridge multicasting.
This would eliminate the need for walking the bundle list apart
from the case when we're destroying the policy, which can be
done in process context.
Actually I just realised that the other way we can fix this is
to make xfrm_dst objects per-cpu just like IPv4 routes. That
is, when you fail to find an xfrm_dst object in the per-cpu
cache, you dont' bother calling xfrm_find_bundle but just make
a new bundle.
This is probably much easier than replacing the whole flow cache.
Can any one think of any problems with duplicate xfrm_dst objects?
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: Handling receive packets with limited buffer space
From: Andi Kleen @ 2010-03-20 15:01 UTC (permalink / raw)
To: Amit Uttamchandani; +Cc: netdev
In-Reply-To: <20100319094118.GA30249@debian>
Amit Uttamchandani <amit.uttam@gmail.com> writes:
> In my modified ethoc.c driver, I am using only one receive buffer
> descriptor due to limited space. This works fairly well for small
> packets (arp, pings, etc.).
>
> However, doing a 'wget' and downloading a file causes lots of drops and
> thus results in a terminated connection.
>
> How do I go about dealing with this? I tried stopping the queue
> (netif_stop_queue) while I'm processing the rx skb and then resuming it
> once I'm done, but this did not seem to help.
>
> Any ideas on this?
You'll need to allocate a new skb and copy into it before
passing the data up the stack.
-Andi
--
ak@linux.intel.com -- Speaking for myself only.
^ 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