* Re: [PATCH] flexcan: Fix CAN_RAW_RECV_OWN_MSGS and CAN_RAW_LOOPBACK
From: Kurt Van Dijck @ 2011-11-02 8:04 UTC (permalink / raw)
To: Reuben Dowle; +Cc: netdev, linux-can
In-Reply-To: <70F6AAAFDC054F41B9994A9BCD3DF64E16FAA1DD@exch01-aklnz.MARINE.NET.INT>
On Tue, Nov 01, 2011 at 11:18:03AM +1300, Reuben Dowle wrote:
> Currently the flexcan driver uses hardware local echo. This blindly echos all transmitted frames to all receiving sockets, regardless what CAN_RAW_RECV_OWN_MSGS and CAN_RAW_LOOPBACK are set to.
>
> This patch now submits transmitted frames to be echoed in the transmit complete interrupt, preserving the reference to the sending socket. This allows the can protocol to correctly handle the local echo.
>
> Signed-off-by: Reuben Dowle <reuben.dowle@navico.com>
>
> ---
> drivers/net/can/flexcan.c | 19 +++++++++++++++----
> 1 files changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index e023379..542ada8 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -302,7 +302,7 @@ static int flexcan_start_xmit(struct sk_buff *skb, struct net_device *dev)
> flexcan_write(can_id, ®s->cantxfg[FLEXCAN_TX_BUF_ID].can_id);
> flexcan_write(ctrl, ®s->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl);
>
> - kfree_skb(skb);
> + can_put_echo_skb(skb, dev, 0);
>
> /* tx_packets is incremented in flexcan_irq */
> stats->tx_bytes += cf->can_dlc;
Why not move the statistics to the place where the echo_skb is popped?
When no frame gets out (due to whatever reason), the statistics may have
incremented.
Or maybe this needs a seperate patch?
The rest looks good.
Kurt
^ permalink raw reply
* Re: [PATCH] IPv6 - support for NLM_F_* flags at IPv6 routing requests
From: David Miller @ 2011-11-02 7:28 UTC (permalink / raw)
To: matti.vaittinen; +Cc: netdev, shemminger
In-Reply-To: <1320217791.12052.12.camel@lakki>
From: Matti Vaittinen <matti.vaittinen@nsn.com>
Date: Wed, 02 Nov 2011 09:09:51 +0200
> On Tue, 2011-11-01 at 16:27 +0200, Matti Vaittinen wrote:
>> Hi dee Ho again.
>>
>> Here's the support for NLM_F_* flags at IPv6 routing requests once again.
>>
>> This time if no NLM_F_CREATE flag is not defined for RTM_NEWROUTE request,
>> warning is printed, but no error is returned. Instead new route is added.
>>
>> Exception is when NLM_F_REPLACE flag is given without NLM_F_CREATE, and
>> no matching route is found. In this case it should be safe to assume
>> that the request issuer is familiar with NLM_F_* flags, and does really
>> not want route to be created.
>>
>> Specifying NLM_F_REPLACE flag will now make the kernel to search for
>> matching route, and replace it with new one. If no route is found and
>> NLM_F_CREATE is specified as well, then new route is created.
>>
>> Also, specifying NLM_F_EXCL will yield returning of error if matching route
>> is found.
>>
>> Patch is created against linux-3.1-rc4
>>
>
> New patch where the definition of new error is removed as Stephen suggested.
>
> Signed-off-by: Matti Vaittinen <Mazziesaccount@gmail.com>
Please do not submit new versions of patches in this way by replying
and quoting your original commit log message. That makes for lots
of work for me.
Instead, submit a fresh new full patch posting and prefix your subject
with something like "[PATCH v2]" to indicate it's a new version.
^ permalink raw reply
* Re: [PATCH] IPv6 - support for NLM_F_* flags at IPv6 routing requests
From: Matti Vaittinen @ 2011-11-02 7:09 UTC (permalink / raw)
To: davem; +Cc: netdev, shemminger
In-Reply-To: <1320157347.11816.3.camel@lakki>
On Tue, 2011-11-01 at 16:27 +0200, Matti Vaittinen wrote:
> Hi dee Ho again.
>
> Here's the support for NLM_F_* flags at IPv6 routing requests once again.
>
> This time if no NLM_F_CREATE flag is not defined for RTM_NEWROUTE request,
> warning is printed, but no error is returned. Instead new route is added.
>
> Exception is when NLM_F_REPLACE flag is given without NLM_F_CREATE, and
> no matching route is found. In this case it should be safe to assume
> that the request issuer is familiar with NLM_F_* flags, and does really
> not want route to be created.
>
> Specifying NLM_F_REPLACE flag will now make the kernel to search for
> matching route, and replace it with new one. If no route is found and
> NLM_F_CREATE is specified as well, then new route is created.
>
> Also, specifying NLM_F_EXCL will yield returning of error if matching route
> is found.
>
> Patch is created against linux-3.1-rc4
>
New patch where the definition of new error is removed as Stephen suggested.
Signed-off-by: Matti Vaittinen <Mazziesaccount@gmail.com>
---
diff -uNr linux-3.1-rc4.orig/net/ipv6/ip6_fib.c linux-3.1-rc4.new/net/ipv6/ip6_fib.c
--- linux-3.1-rc4.orig/net/ipv6/ip6_fib.c 2011-11-01 14:01:55.000000000 +0200
+++ linux-3.1-rc4.new/net/ipv6/ip6_fib.c 2011-11-02 08:37:21.000000000 +0200
@@ -429,17 +429,34 @@
static struct fib6_node * fib6_add_1(struct fib6_node *root, void *addr,
int addrlen, int plen,
- int offset)
+ int offset, struct nl_info *info)
{
struct fib6_node *fn, *in, *ln;
struct fib6_node *pn = NULL;
struct rt6key *key;
int bit;
+
+
+ int allow_create = 1;
+ int replace_required = 0;
+
+
__be32 dir = 0;
__u32 sernum = fib6_new_sernum();
RT6_TRACE("fib6_add_1\n");
+ if (NULL != info &&
+ NULL != info->nlh &&
+ (info->nlh->nlmsg_flags&NLM_F_REPLACE)) {
+ replace_required = 1;
+ }
+ if (NULL != info &&
+ NULL != info->nlh &&
+ !(info->nlh->nlmsg_flags&NLM_F_CREATE)) {
+ allow_create = 0;
+ }
+
/* insert node in tree */
fn = root;
@@ -451,8 +468,12 @@
* Prefix match
*/
if (plen < fn->fn_bit ||
- !ipv6_prefix_equal(&key->addr, addr, fn->fn_bit))
+ !ipv6_prefix_equal(&key->addr, addr, fn->fn_bit)) {
+ if (!allow_create)
+ printk(KERN_WARNING
+ "NLM_F_CREATE should be specified when creating new rt\n");
goto insert_above;
+ }
/*
* Exact match ?
@@ -481,10 +502,27 @@
fn = dir ? fn->right: fn->left;
} while (fn);
+
+ if (replace_required && !allow_create) {
+ /* We should not create new node because
+ * NLM_F_REPLACE was specified without NLM_F_CREATE
+ * I assume it is safe to require NLM_F_CREATE when
+ * REPLACE flag is used! Later we may want to remove the
+ * check for replace_required, because according
+ * to netlink specification, NLM_F_CREATE
+ * MUST be specified if new route is created.
+ * That would keep IPv6 consistent with IPv4
+ */
+ printk(KERN_WARNING
+ "NLM_F_CREATE should be specified when creating new rt - ignoring request\n");
+ return ERR_PTR(-ENOENT);
+ }
/*
* We walked to the bottom of tree.
* Create new leaf node without children.
*/
+ if (!allow_create)
+ printk(KERN_WARNING "NLM_F_CREATE should be specified when creating new rt\n");
ln = node_alloc();
@@ -567,7 +605,6 @@
fn->parent = in;
ln->fn_sernum = sernum;
-
if (addr_bit_set(addr, bit)) {
in->right = ln;
in->left = fn;
@@ -585,6 +622,7 @@
ln = node_alloc();
+
if (ln == NULL)
return NULL;
@@ -618,6 +656,12 @@
{
struct rt6_info *iter = NULL;
struct rt6_info **ins;
+ int replace = (NULL != info &&
+ NULL != info->nlh &&
+ (info->nlh->nlmsg_flags&NLM_F_REPLACE));
+ int add = ((NULL == info || NULL == info->nlh) ||
+ (info->nlh->nlmsg_flags&NLM_F_CREATE));
+ int found = 0;
ins = &fn->leaf;
@@ -630,6 +674,13 @@
/*
* Same priority level
*/
+ if (NULL != info->nlh &&
+ (info->nlh->nlmsg_flags&NLM_F_EXCL))
+ return -EEXIST;
+ if (replace) {
+ found++;
+ break;
+ }
if (iter->rt6i_dev == rt->rt6i_dev &&
iter->rt6i_idev == rt->rt6i_idev &&
@@ -659,19 +710,41 @@
/*
* insert node
*/
+ if (!replace) {
+ if (!add)
+ printk(KERN_WARNING "NLM_F_CREATE should be specified when creating new rt\n");
+
+add:
+ rt->dst.rt6_next = iter;
+ *ins = rt;
+ rt->rt6i_node = fn;
+ atomic_inc(&rt->rt6i_ref);
+ inet6_rt_notify(RTM_NEWROUTE, rt, info);
+ info->nl_net->ipv6.rt6_stats->fib_rt_entries++;
+
+ if ((fn->fn_flags & RTN_RTINFO) == 0) {
+ info->nl_net->ipv6.rt6_stats->fib_route_nodes++;
+ fn->fn_flags |= RTN_RTINFO;
+ }
- rt->dst.rt6_next = iter;
- *ins = rt;
- rt->rt6i_node = fn;
- atomic_inc(&rt->rt6i_ref);
- inet6_rt_notify(RTM_NEWROUTE, rt, info);
- info->nl_net->ipv6.rt6_stats->fib_rt_entries++;
-
- if ((fn->fn_flags & RTN_RTINFO) == 0) {
- info->nl_net->ipv6.rt6_stats->fib_route_nodes++;
- fn->fn_flags |= RTN_RTINFO;
+ } else {
+ if (!found) {
+ if (add)
+ goto add;
+ printk(KERN_WARNING "add rtinfo to node - NLM_F_REPLACE specified, but no existing node found! bailing out\n");
+ return -ENOENT;
+ }
+ *ins = rt;
+ rt->rt6i_node = fn;
+ rt->dst.rt6_next = iter->dst.rt6_next;
+ atomic_inc(&rt->rt6i_ref);
+ inet6_rt_notify(RTM_NEWROUTE, rt, info);
+ rt6_release(iter);
+ if ((fn->fn_flags & RTN_RTINFO) == 0) {
+ info->nl_net->ipv6.rt6_stats->fib_route_nodes++;
+ fn->fn_flags |= RTN_RTINFO;
+ }
}
-
return 0;
}
@@ -700,10 +773,29 @@
{
struct fib6_node *fn, *pn = NULL;
int err = -ENOMEM;
+ int allow_create = 1;
+ int allow_replace = 1;
+ if (NULL != info &&
+ NULL != info->nlh &&
+ !(info->nlh->nlmsg_flags&NLM_F_REPLACE)) {
+ allow_replace = 0;
+ }
+ if (NULL != info &&
+ NULL != info->nlh &&
+ !(info->nlh->nlmsg_flags&NLM_F_CREATE)) {
+ allow_create = 0;
+ }
+ if (!allow_create && !allow_replace)
+ printk(KERN_WARNING "RTM_NEWROUTE with no NLM_F_CREATE or NLM_F_REPLACE\n");
fn = fib6_add_1(root, &rt->rt6i_dst.addr, sizeof(struct in6_addr),
- rt->rt6i_dst.plen, offsetof(struct rt6_info, rt6i_dst));
+ rt->rt6i_dst.plen, offsetof(struct rt6_info, rt6i_dst),
+ info);
+ if (-ENOENT == PTR_ERR(fn)) {
+ err = -EINVAL;
+ fn = NULL;
+ }
if (fn == NULL)
goto out;
@@ -716,6 +808,8 @@
if (fn->subtree == NULL) {
struct fib6_node *sfn;
+ if (!allow_create)
+ printk(KERN_WARNING "NLM_F_CREATE should be specified when creating new rt\n");
/*
* Create subtree.
*
@@ -740,7 +834,8 @@
sn = fib6_add_1(sfn, &rt->rt6i_src.addr,
sizeof(struct in6_addr), rt->rt6i_src.plen,
- offsetof(struct rt6_info, rt6i_src));
+ offsetof(struct rt6_info, rt6i_src),
+ info);
if (sn == NULL) {
/* If it is failed, discard just allocated
@@ -757,8 +852,13 @@
} else {
sn = fib6_add_1(fn->subtree, &rt->rt6i_src.addr,
sizeof(struct in6_addr), rt->rt6i_src.plen,
- offsetof(struct rt6_info, rt6i_src));
+ offsetof(struct rt6_info, rt6i_src),
+ info);
+ if (-ENOENT == PTR_ERR(sn)) {
+ err = -EINVAL;
+ sn = NULL;
+ }
if (sn == NULL)
goto st_failure;
}
diff -uNr linux-3.1-rc4.orig/net/ipv6/route.c linux-3.1-rc4.new/net/ipv6/route.c
--- linux-3.1-rc4.orig/net/ipv6/route.c 2011-11-01 14:01:55.000000000 +0200
+++ linux-3.1-rc4.new/net/ipv6/route.c 2011-10-27 10:45:05.000000000 +0300
@@ -1223,9 +1223,18 @@
if (cfg->fc_metric == 0)
cfg->fc_metric = IP6_RT_PRIO_USER;
- table = fib6_new_table(net, cfg->fc_table);
+ err = -ENOBUFS;
+ if (NULL != cfg->fc_nlinfo.nlh &&
+ !(cfg->fc_nlinfo.nlh->nlmsg_flags&NLM_F_CREATE)) {
+ table = fib6_get_table(net, cfg->fc_table);
+ if (table == NULL) {
+ printk(KERN_WARNING "NLM_F_CREATE should be specified when creating new rt\n");
+ table = fib6_new_table(net, cfg->fc_table);
+ }
+ } else {
+ table = fib6_new_table(net, cfg->fc_table);
+ }
if (table == NULL) {
- err = -ENOBUFS;
goto out;
}
^ permalink raw reply
* Re: PROBLEM: pppol2tp over pppoe NULL pointer dereference
From: Eric Dumazet @ 2011-11-02 7:07 UTC (permalink / raw)
To: Misha Labjuk; +Cc: netdev
In-Reply-To: <CAKpUy7ZTpXtUDXB+NCxa4QBr3qiwo7aBW9DmUY8ADqO8FgrUpw@mail.gmail.com>
Le mercredi 02 novembre 2011 à 09:04 +0400, Misha Labjuk a écrit :
> 2011/11/2 Eric Dumazet <eric.dumazet@gmail.com>:
> >
> > On what kind of NIC this is happening ?
> >
>
> Realtek Semiconductor Co., Ltd. RTL8111/8168B PCI Express Gigabit
> Ethernet controller (rev 02)
> Kernel driver in use: r8169
OK thanks, could you try the following patch as well ?
If we release reorder_q.lock, we must not keep a dangling pointer (tmp)
and restart the whole loop.
diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
index 34b2dde..bf8d50c 100644
--- a/net/l2tp/l2tp_core.c
+++ b/net/l2tp/l2tp_core.c
@@ -397,6 +397,7 @@ static void l2tp_recv_dequeue(struct l2tp_session *session)
* expect to send up next, dequeue it and any other
* in-sequence packets behind it.
*/
+start:
spin_lock_bh(&session->reorder_q.lock);
skb_queue_walk_safe(&session->reorder_q, skb, tmp) {
if (time_after(jiffies, L2TP_SKB_CB(skb)->expires)) {
@@ -433,7 +434,7 @@ static void l2tp_recv_dequeue(struct l2tp_session *session)
*/
spin_unlock_bh(&session->reorder_q.lock);
l2tp_recv_dequeue_skb(session, skb);
- spin_lock_bh(&session->reorder_q.lock);
+ goto start;
}
out:
^ permalink raw reply related
* Re: [PATCH] IPv6 - support for NLM_F_* flags at IPv6 routing requests
From: Maz The Northener @ 2011-11-02 6:28 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Matti Vaittinen, davem, netdev
In-Reply-To: <20111101224302.6d370068@s6510.linuxnetplumber.net>
On Tue, Nov 1, 2011 at 11:43 PM, Stephen Hemminger
<shemminger@vyatta.com> wrote:
> On Tue, 01 Nov 2011 16:22:27 +0200
> Matti Vaittinen <matti.vaittinen@nsn.com> wrote:
>
>>
>> +#define RT6_CANT_CREATE ((int)-1)
>> #define RT6_DEBUG 2
>
> Rather than introduce a new error flag, why not convert the code
> to use the kernel standard PTR_ERR() macros?
> --
Thanks for comment Stephen, I admitt I felt there was something rotten
in adding a new define. And as to why not use PTR_ERR() - I did not
know about such a facility. I'll fix this to be
return ERR_PTR(-ENOENT);
and at calling function check for
if (PTR_ERR(fn) == -ENOENT)
I guess that is correct way?
And finally couple of questions about sending patches (I'm a new guy
in this "house" of yours):
Is it Ok to add patch as attachment? I'm having difficulties in using
a client which allows sending plain text.
And is it Ok to create patch against linux 3.1-rc4? I've never used
git, and thus obtaining 3.1-rc4 from kernel.org is the easiest way for
me.
--Matti Vaittinen
^ permalink raw reply
* Re: linux-next: build failure after merge of the final tree (net tree related)
From: David Miller @ 2011-11-02 5:50 UTC (permalink / raw)
To: sfr; +Cc: netdev, linux-next, linux-kernel, eric.dumazet
In-Reply-To: <20111102163708.75d719e765dea20e0c4c5a04@canb.auug.org.au>
From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Wed, 2 Nov 2011 16:37:08 +1100
> Subject: [PATCH] net: fix typo in drivers/net/ethernet/xilinx/ll_temac_main.c
>
> Commit 9e903e085262 ("net: add skb frag size accessors") used frag_size
> instead of skb_frag_size in this file.
>
> Fixes this build error:
>
> drivers/net/ethernet/xilinx/ll_temac_main.c: In function 'temac_start_xmit':
> drivers/net/ethernet/xilinx/ll_temac_main.c:717:3: error: implicit declaration of function 'frag_size' [-Werror=implicit-function-declaration]
>
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
Applied, thanks!
^ permalink raw reply
* linux-next: build failure after merge of the final tree (net tree related)
From: Stephen Rothwell @ 2011-11-02 5:37 UTC (permalink / raw)
To: David Miller, netdev; +Cc: linux-next, linux-kernel, Eric Dumazet
[-- Attachment #1: Type: text/plain, Size: 2019 bytes --]
Hi all,
[I thought I had reported this, but it was a similar problem in another
driver.]
After merging the final tree, today's linux-next build (powerpc
allyesconfig) failed like this:
drivers/net/ethernet/xilinx/ll_temac_main.c: In function 'temac_start_xmit':
drivers/net/ethernet/xilinx/ll_temac_main.c:717:3: error: implicit declaration of function 'frag_size' [-Werror=implicit-function-declaration]
Caused by commit 9e903e085262 ("net: add skb frag size accessors").
I have added this patch for today:
From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Wed, 2 Nov 2011 16:31:37 +1100
Subject: [PATCH] net: fix typo in drivers/net/ethernet/xilinx/ll_temac_main.c
Commit 9e903e085262 ("net: add skb frag size accessors") used frag_size
instead of skb_frag_size in this file.
Fixes this build error:
drivers/net/ethernet/xilinx/ll_temac_main.c: In function 'temac_start_xmit':
drivers/net/ethernet/xilinx/ll_temac_main.c:717:3: error: implicit declaration of function 'frag_size' [-Werror=implicit-function-declaration]
Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
drivers/net/ethernet/xilinx/ll_temac_main.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/xilinx/ll_temac_main.c b/drivers/net/ethernet/xilinx/ll_temac_main.c
index 4d1658e..caf3659 100644
--- a/drivers/net/ethernet/xilinx/ll_temac_main.c
+++ b/drivers/net/ethernet/xilinx/ll_temac_main.c
@@ -716,8 +716,8 @@ static int temac_start_xmit(struct sk_buff *skb, struct net_device *ndev)
cur_p = &lp->tx_bd_v[lp->tx_bd_tail];
cur_p->phys = dma_map_single(ndev->dev.parent,
skb_frag_address(frag),
- frag_size(frag), DMA_TO_DEVICE);
- cur_p->len = frag_size(frag);
+ skb_frag_size(frag), DMA_TO_DEVICE);
+ cur_p->len = skb_frag_size(frag);
cur_p->app0 = 0;
frag++;
}
--
1.7.7
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply related
* Re: PROBLEM: pppol2tp over pppoe NULL pointer dereference
From: Misha Labjuk @ 2011-11-02 5:04 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1320186916.4728.1.camel@edumazet-laptop>
2011/11/2 Eric Dumazet <eric.dumazet@gmail.com>:
>
> On what kind of NIC this is happening ?
>
Realtek Semiconductor Co., Ltd. RTL8111/8168B PCI Express Gigabit
Ethernet controller (rev 02)
Kernel driver in use: r8169
^ permalink raw reply
* Re: [PATCH net-next] bnx2x: stop using on-stack napi struct
From: David Miller @ 2011-11-02 5:00 UTC (permalink / raw)
To: ariele; +Cc: davem, netdev, eilong
In-Reply-To: <1320163450.11011.19.camel@lb-tlvb-ariel.il.broadcom.com>
From: "Ariel Elior" <ariele@broadcom.com>
Date: Tue, 1 Nov 2011 18:04:10 +0200
> Napi structure was allocated on stack to hold temporary value of copied
> fastpath. This can be avoided by using the source fastpath as a
> scratchpad thus saving stack space and code.
> Signed-off-by: Ariel Elior <ariele@broadcom.com>
> Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
Your patch has been severely corrupted by your email client.
I'm sure your colleagues at Broadcom can help you fix this
so that you can submit this patch properly. :-)
^ permalink raw reply
* Re: [PATCH] net: ucc_geth, increase no. of HW RX descriptors
From: David Miller @ 2011-11-02 4:56 UTC (permalink / raw)
To: Joakim.Tjernlund; +Cc: avorontsov, netdev
In-Reply-To: <1319548630-6561-1-git-send-email-Joakim.Tjernlund@transmode.se>
From: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
Date: Tue, 25 Oct 2011 15:17:10 +0200
> In a busy network we see ucc_geth is dropping RX pkgs every now
> and then. Increase the first RX queues HW descriptors from
> 16 to 32 to deal with this.
>
> Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
1) This patch doesn't apply, because in the current tree this
dirver lives under drivers/net/ethernet/freescale/ not
drivers/net
2) Why not increase RX_BD_RING_LEN and thus all the ring sizes,
is there a special reason only the first ring's size should
be increased?
Because the way you've made this change is confusing, someone
will see RX_BD_RING_LEN's definition and believe that this is
the size of the primary ring unless they happen to notice
how .bdRingLenRx is initialized in this special way.
Thus, this patch needs changes before it can even be considered
for inclusion.
^ permalink raw reply
* Re: [PATCH] udp: fix a race in encap_rcv handling
From: David Miller @ 2011-11-02 4:51 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <1320188219.4728.7.camel@edumazet-laptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 01 Nov 2011 23:56:59 +0100
> udp_queue_rcv_skb() has a possible race in encap_rcv handling, since
> this pointer can be changed anytime.
>
> We should use ACCESS_ONCE() to close the race.
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied and queued up for -stable, thanks!
^ permalink raw reply
* Re: [v2] Fix NULL dereference in x25_recvmsg
From: David Miller @ 2011-11-02 4:50 UTC (permalink / raw)
To: eric.dumazet; +Cc: davej, netdev, mattjd
In-Reply-To: <1320201017.4728.31.camel@edumazet-laptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 02 Nov 2011 03:30:17 +0100
> Le mardi 01 novembre 2011 à 22:26 -0400, Dave Jones a écrit :
>> commit cb101ed2 in 3.0 introduced a bug in x25_recvmsg()
>> When passed bogus junk from userspace, x25->neighbour can be NULL,
>> as shown in this oops..
>>
> ...
>> Signed-off-by: Dave Jones <davej@redhat.com>
...
> Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied and queued up for -stable, thanks everyone.
^ permalink raw reply
* Re: ARP garbage collection issues in 3.1?
From: David Miller @ 2011-11-02 4:48 UTC (permalink / raw)
To: anton; +Cc: netdev
In-Reply-To: <20111102153443.38cc1e5c@kryten>
From: Anton Blanchard <anton@samba.org>
Date: Wed, 2 Nov 2011 15:34:43 +1100
> Any ideas how we could make this behave a bit better? I know setting
> gc_thresh3 higher is the ultimate solution, but if gc_thresh1 and
> gc_thresh2 are always below the route threshold we should either fix
> this issue or remove them completely.
The solution is to do refcount'less RCU lookups into the neigh
tables on every packet send, and long term that's what I intend
to implement.
That's what's behind making the recent change to make the ARP hash
cheaper etc.
See slides 5, 6, and 7 in:
http://vger.kernel.org/netconf2011_slides/davem_netconf2011.pdf
Once that's done you can trim whatever neigh entries you want,
whenever you want.
You are right that the current situation is silly, because if
we're willing to commit to N routing table entries we might as
well be willing to commit to N arp table entries as well.
^ permalink raw reply
* Re: on bnx2x firmware...
From: Ben Hutchings @ 2011-11-02 4:36 UTC (permalink / raw)
To: Maciej Żenczykowski; +Cc: Linux NetDev
In-Reply-To: <CAHo-OowF1LNmL0P_Rt=oKmhZMt=CVaVsAYkh_XqVJEOzBPXV7w@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1007 bytes --]
On Tue, 2011-11-01 at 18:01 -0700, Maciej Żenczykowski wrote:
> ((v3.1))$ cat drivers/net/bnx2x/bnx2x_hsi.h | egrep BCM_5710_FW_
>
> #define BCM_5710_FW_MAJOR_VERSION 7
> #define BCM_5710_FW_MINOR_VERSION 0
> #define BCM_5710_FW_REVISION_VERSION 23
> #define BCM_5710_FW_ENGINEERING_VERSION 0
>
> So the desired firmware is version 7.0.23.0
>
> ((v3.1))$ find | egrep bnx2x | egrep fw
> ./firmware/bnx2x/bnx2x-e1-6.2.9.0.fw.ihex
> ./firmware/bnx2x/bnx2x-e2-6.2.9.0.fw.ihex
> ./firmware/bnx2x/bnx2x-e1h-6.2.9.0.fw.ihex
> ./drivers/net/bnx2x/bnx2x_fw_defs.h
> ./drivers/net/bnx2x/bnx2x_fw_file_hdr.h
>
> So it doesn't look like the in-tree fw matches the bnx2x driver in
> released v3.1.
> I can find this firmware in the firmware repository.
[...]
That's where it belongs. But there is some confusion/disagreement on
this point.
Ben.
--
Ben Hutchings
Sturgeon's Law: Ninety percent of everything is crap.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]
^ permalink raw reply
* ARP garbage collection issues in 3.1?
From: Anton Blanchard @ 2011-11-02 4:34 UTC (permalink / raw)
To: netdev
Hi,
I've been getting complaints about machines not garbage collecting
ARP entries. The network in question has a lot of MAC addresses on it
(probably more than gc_thresh3 - 1024), but this machine is only
talking over the network to a few targets.
Over a period of hours or days the ARP table slowly grows until it hits
gc_thresh3 and at that stage we get "Neighbour table overflow" errors.
Increasing gc_thresh3 makes the issue go away as expected. The question
is, why isn't this slow growth of the ARP table being managed by the
gc_thresh1 and gc_thresh2 watermarks?
I tried this simple test on 3.1-git*:
machine A:
for i in `seq 2 254`; do ifconfig eth0:$i 192.168.111.$i; done
machine B:
echo 64 > /proc/sys/net/ipv4/neigh/default/gc_thresh1
echo 128 > /proc/sys/net/ipv4/neigh/default/gc_thresh2
echo 192 > /proc/sys/net/ipv4/neigh/default/gc_thresh3
ifconfig eth0:1 192.168.111.1
for i in `seq 2 254`; do ping -c 1 192.168.111.$i; done
I was expecting to get the "Neighbour table overflow" errors and then
after a period of time the route and ARP garbage collection would kick
in. I waited for over an hour and this had not happened, we had
gc_thresh3 (192) ARP entries and no new ARP entries could be added to
the table.
The problem seems to be that our neighbour garbage collection thresholds
are set low (1024) and our route threshold is set high (524288). When we
get into trouble we do try to garbage collect routes but
rt_garbage_collect never sees enough entries to invoke garbage
collection. In effect the ARP entries are pinned, and gc_thresh1
gc_thresh2 are useless.
My first thought was the patch below. It does help but it's Russian
Roulette since we shoot one entry down at random and it might not be
one in the neighbour table. As such, it often takes a number of goes to
clear a stale ARP entry.
Any ideas how we could make this behave a bit better? I know setting
gc_thresh3 higher is the ultimate solution, but if gc_thresh1 and
gc_thresh2 are always below the route threshold we should either fix
this issue or remove them completely.
Anton
---
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 155138d..8104d41 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -876,7 +876,7 @@ static void rt_emergency_hash_rebuild(struct net *net)
and when load increases it reduces to limit cache size.
*/
-static int rt_garbage_collect(struct dst_ops *ops)
+static int __rt_garbage_collect(struct dst_ops *ops, int force)
{
static unsigned long expire = RT_GC_TIMEOUT;
static unsigned long last_gc;
@@ -895,7 +895,7 @@ static int rt_garbage_collect(struct dst_ops *ops)
RT_CACHE_STAT_INC(gc_total);
- if (now - last_gc < ip_rt_gc_min_interval &&
+ if (!force && now - last_gc < ip_rt_gc_min_interval &&
entries < ip_rt_max_size) {
RT_CACHE_STAT_INC(gc_ignored);
goto out;
@@ -920,6 +920,9 @@ static int rt_garbage_collect(struct dst_ops *ops)
equilibrium = entries - goal;
}
+ if (force)
+ goal = 1;
+
if (now - last_gc >= ip_rt_gc_min_interval)
last_gc = now;
@@ -996,6 +999,11 @@ work_done:
out: return 0;
}
+static int rt_garbage_collect(struct dst_ops *ops)
+{
+ return __rt_garbage_collect(ops, 0);
+}
+
/*
* Returns number of entries in a hash chain that have different hash_inputs
*/
@@ -1192,7 +1200,7 @@ restart:
int saved_int = ip_rt_gc_min_interval;
ip_rt_gc_elasticity = 1;
ip_rt_gc_min_interval = 0;
- rt_garbage_collect(&ipv4_dst_ops);
+ __rt_garbage_collect(&ipv4_dst_ops, 1);
ip_rt_gc_min_interval = saved_int;
ip_rt_gc_elasticity = saved_elasticity;
goto restart;
^ permalink raw reply related
* Contact UPS Via {ups_despatch.ng@dgoh.org} For Claims
From: Boerner, Hunter @ 2011-11-02 3:49 UTC (permalink / raw)
After much attempts to reach you on phone, I deemed it necessary and urgent to contact you via your e-mail and to notify you finally about your outstanding compensation payment.
During our last annual calculation of your banking and Internet activities we realized that you are eligible to receive a compensation payment of $2,811,041.00 USD.This compensation is being made to all of you who have suffered losses as a result of fraud, accident or illness.
For more information, contact the UPS agent for the delivery of your cashier check ($2,811,041.00 USD).
United Parcel Service (UPS)
Contact Name: Collins Abah
Tel:+2348168636983
E-mail: ups_despatch.ng@dgoh.org <mailto:ups_despatch.ng@dgoh.org>
Please take note that you will pay a shipping/handling fee of $245.00 USD to UPS.
Thanks for your patience.
Boernar Hunter
Programme Manager
United Nations Human Settlements Programme
^ permalink raw reply
* Re: [v2] Fix NULL dereference in x25_recvmsg
From: Eric Dumazet @ 2011-11-02 2:30 UTC (permalink / raw)
To: Dave Jones; +Cc: netdev, Matthew Daley
In-Reply-To: <20111102022644.GB8512@redhat.com>
Le mardi 01 novembre 2011 à 22:26 -0400, Dave Jones a écrit :
> commit cb101ed2 in 3.0 introduced a bug in x25_recvmsg()
> When passed bogus junk from userspace, x25->neighbour can be NULL,
> as shown in this oops..
>
...
> Signed-off-by: Dave Jones <davej@redhat.com>
>
> diff --git a/net/x25/af_x25.c b/net/x25/af_x25.c
> index 5f03e4e..3e16c6a 100644
> --- a/net/x25/af_x25.c
> +++ b/net/x25/af_x25.c
> @@ -1261,14 +1261,19 @@ static int x25_recvmsg(struct kiocb *iocb, struct socket *sock,
> struct x25_sock *x25 = x25_sk(sk);
> struct sockaddr_x25 *sx25 = (struct sockaddr_x25 *)msg->msg_name;
> size_t copied;
> - int qbit, header_len = x25->neighbour->extended ?
> - X25_EXT_MIN_LEN : X25_STD_MIN_LEN;
> -
> + int qbit, header_len;
> struct sk_buff *skb;
> unsigned char *asmptr;
> int rc = -ENOTCONN;
>
> lock_sock(sk);
> +
> + if (x25->neighbour == NULL)
> + goto out;
> +
> + header_len = x25->neighbour->extended ?
> + X25_EXT_MIN_LEN : X25_STD_MIN_LEN;
> +
> /*
> * This works for seqpacket too. The receiver has ordered the queue for
> * us! We do one quick check first though
Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
^ permalink raw reply
* [v2] Fix NULL dereference in x25_recvmsg
From: Dave Jones @ 2011-11-02 2:26 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, Matthew Daley
In-Reply-To: <1320200165.4728.25.camel@edumazet-laptop>
commit cb101ed2 in 3.0 introduced a bug in x25_recvmsg()
When passed bogus junk from userspace, x25->neighbour can be NULL,
as shown in this oops..
BUG: unable to handle kernel NULL pointer dereference at 000000000000001c
IP: [<ffffffffa05482bd>] x25_recvmsg+0x4d/0x280 [x25]
PGD 1015f3067 PUD 105072067 PMD 0
Oops: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC
CPU 0
Pid: 27928, comm: iknowthis Not tainted 3.1.0+ #2 Gigabyte Technology Co., Ltd. GA-MA78GM-S2H/GA-MA78GM-S2H
RIP: 0010:[<ffffffffa05482bd>] [<ffffffffa05482bd>] x25_recvmsg+0x4d/0x280 [x25]
RSP: 0018:ffff88010c0b7cc8 EFLAGS: 00010282
RAX: 0000000000000000 RBX: ffff88010c0b7d78 RCX: 0000000000000c02
RDX: ffff88010c0b7d78 RSI: ffff88011c93dc00 RDI: ffff880103f667b0
RBP: ffff88010c0b7d18 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000000 R12: ffff880103f667b0
R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
FS: 00007f479ce7f700(0000) GS:ffff88012a600000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
CR2: 000000000000001c CR3: 000000010529e000 CR4: 00000000000006f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
Process iknowthis (pid: 27928, threadinfo ffff88010c0b6000, task ffff880103faa4f0)
Stack:
0000000000000c02 0000000000000c02 ffff88010c0b7d18 ffffff958153cb37
ffffffff8153cb60 0000000000000c02 ffff88011c93dc00 0000000000000000
0000000000000c02 ffff88010c0b7e10 ffff88010c0b7de8 ffffffff815372c2
Call Trace:
[<ffffffff8153cb60>] ? sock_update_classid+0xb0/0x180
[<ffffffff815372c2>] sock_aio_read.part.10+0x142/0x150
[<ffffffff812d6752>] ? inode_has_perm+0x62/0xa0
[<ffffffff815372fd>] sock_aio_read+0x2d/0x40
[<ffffffff811b05e2>] do_sync_read+0xd2/0x110
[<ffffffff812d3796>] ? security_file_permission+0x96/0xb0
[<ffffffff811b0a91>] ? rw_verify_area+0x61/0x100
[<ffffffff811b103d>] vfs_read+0x16d/0x180
[<ffffffff811b109d>] sys_read+0x4d/0x90
[<ffffffff81657282>] system_call_fastpath+0x16/0x1b
Code: 8b 66 20 4c 8b 32 48 89 d3 48 89 4d b8 45 89 c7 c7 45 cc 95 ff ff ff 4d 85 e4 0f 84 ed 01 00 00 49 8b 84 24 18 05 00 00 4c 89 e7
78 1c 01 45 19 ed 31 f6 e8 d5 37 ff e0 41 0f b6 44 24 0e 41
Signed-off-by: Dave Jones <davej@redhat.com>
diff --git a/net/x25/af_x25.c b/net/x25/af_x25.c
index 5f03e4e..3e16c6a 100644
--- a/net/x25/af_x25.c
+++ b/net/x25/af_x25.c
@@ -1261,14 +1261,19 @@ static int x25_recvmsg(struct kiocb *iocb, struct socket *sock,
struct x25_sock *x25 = x25_sk(sk);
struct sockaddr_x25 *sx25 = (struct sockaddr_x25 *)msg->msg_name;
size_t copied;
- int qbit, header_len = x25->neighbour->extended ?
- X25_EXT_MIN_LEN : X25_STD_MIN_LEN;
-
+ int qbit, header_len;
struct sk_buff *skb;
unsigned char *asmptr;
int rc = -ENOTCONN;
lock_sock(sk);
+
+ if (x25->neighbour == NULL)
+ goto out;
+
+ header_len = x25->neighbour->extended ?
+ X25_EXT_MIN_LEN : X25_STD_MIN_LEN;
+
/*
* This works for seqpacket too. The receiver has ordered the queue for
* us! We do one quick check first though
^ permalink raw reply related
* Re: Fix NULL dereference in x25_recvmsg
From: Eric Dumazet @ 2011-11-02 2:21 UTC (permalink / raw)
To: Dave Jones; +Cc: netdev, Matthew Daley
In-Reply-To: <1320200358.4728.28.camel@edumazet-laptop>
Le mercredi 02 novembre 2011 à 03:19 +0100, Eric Dumazet a écrit :
> Le mardi 01 novembre 2011 à 22:15 -0400, Dave Jones a écrit :
> > On Wed, Nov 02, 2011 at 03:10:45AM +0100, Eric Dumazet wrote:
> > > Le mardi 01 novembre 2011 à 21:53 -0400, Dave Jones a écrit :
> > > > commit cb101ed2 in 3.0 introduced a bug in x25_recvmsg()
> > > > When passed bogus junk from userspace, x25->neighbour can be NULL,
> > > > as shown in this oops..
> > > >
> > >
> > > Your patch seems fine but :
> > >
> > > Are you sure this bug is not present on previous kernels ?
> > >
> > > It seems we had prior to this commit :
> > >
> > > skb_pull(skb, x25->neighbour->extended ?
> > > X25_EXT_MIN_LEN : X25_STD_MIN_LEN);
> >
> > It might have been possible with a specifically crafted set of arguments.
> >
> > It never showed up in testing before now, probably because we were
> > returning from the function before we got to that skb_pull
> > via all the other tests that get performed.
> >
>
> neighbour is not an x25_recvmsg() argument, but related to x25 socket
> state.
>
> Maybe your tests dont try to use x25_recvmsg() while socket has no
> neighbour...
>
> This bug was there before the cb101ed2 commit.
>
Ah ok, I see now, we exited early because of
if (sk->sk_state != TCP_ESTABLISHED)
goto out;
So yes, commit cb101ed2 is the bug origin, sorry for the noise.
^ permalink raw reply
* Re: Fix NULL dereference in x25_recvmsg
From: Eric Dumazet @ 2011-11-02 2:19 UTC (permalink / raw)
To: Dave Jones; +Cc: netdev, Matthew Daley
In-Reply-To: <20111102021525.GA8512@redhat.com>
Le mardi 01 novembre 2011 à 22:15 -0400, Dave Jones a écrit :
> On Wed, Nov 02, 2011 at 03:10:45AM +0100, Eric Dumazet wrote:
> > Le mardi 01 novembre 2011 à 21:53 -0400, Dave Jones a écrit :
> > > commit cb101ed2 in 3.0 introduced a bug in x25_recvmsg()
> > > When passed bogus junk from userspace, x25->neighbour can be NULL,
> > > as shown in this oops..
> > >
> >
> > Your patch seems fine but :
> >
> > Are you sure this bug is not present on previous kernels ?
> >
> > It seems we had prior to this commit :
> >
> > skb_pull(skb, x25->neighbour->extended ?
> > X25_EXT_MIN_LEN : X25_STD_MIN_LEN);
>
> It might have been possible with a specifically crafted set of arguments.
>
> It never showed up in testing before now, probably because we were
> returning from the function before we got to that skb_pull
> via all the other tests that get performed.
>
neighbour is not an x25_recvmsg() argument, but related to x25 socket
state.
Maybe your tests dont try to use x25_recvmsg() while socket has no
neighbour...
This bug was there before the cb101ed2 commit.
^ permalink raw reply
* Re: Fix NULL dereference in x25_recvmsg
From: Eric Dumazet @ 2011-11-02 2:16 UTC (permalink / raw)
To: Dave Jones; +Cc: netdev, Matthew Daley
In-Reply-To: <20111102015315.GA6569@redhat.com>
Le mardi 01 novembre 2011 à 21:53 -0400, Dave Jones a écrit :
> commit cb101ed2 in 3.0 introduced a bug in x25_recvmsg()
> When passed bogus junk from userspace, x25->neighbour can be NULL,
> as shown in this oops..
>
> BUG: unable to handle kernel NULL pointer dereference at 000000000000001c
> IP: [<ffffffffa05482bd>] x25_recvmsg+0x4d/0x280 [x25]
> PGD 1015f3067 PUD 105072067 PMD 0
> Oops: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC
> CPU 0
> Pid: 27928, comm: iknowthis Not tainted 3.1.0+ #2 Gigabyte Technology Co., Ltd. GA-MA78GM-S2H/GA-MA78GM-S2H
> RIP: 0010:[<ffffffffa05482bd>] [<ffffffffa05482bd>] x25_recvmsg+0x4d/0x280 [x25]
> RSP: 0018:ffff88010c0b7cc8 EFLAGS: 00010282
> RAX: 0000000000000000 RBX: ffff88010c0b7d78 RCX: 0000000000000c02
> RDX: ffff88010c0b7d78 RSI: ffff88011c93dc00 RDI: ffff880103f667b0
> RBP: ffff88010c0b7d18 R08: 0000000000000000 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000000 R12: ffff880103f667b0
> R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
> FS: 00007f479ce7f700(0000) GS:ffff88012a600000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
> CR2: 000000000000001c CR3: 000000010529e000 CR4: 00000000000006f0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
> Process iknowthis (pid: 27928, threadinfo ffff88010c0b6000, task ffff880103faa4f0)
> Stack:
> 0000000000000c02 0000000000000c02 ffff88010c0b7d18 ffffff958153cb37
> ffffffff8153cb60 0000000000000c02 ffff88011c93dc00 0000000000000000
> 0000000000000c02 ffff88010c0b7e10 ffff88010c0b7de8 ffffffff815372c2
> Call Trace:
> [<ffffffff8153cb60>] ? sock_update_classid+0xb0/0x180
> [<ffffffff815372c2>] sock_aio_read.part.10+0x142/0x150
> [<ffffffff812d6752>] ? inode_has_perm+0x62/0xa0
> [<ffffffff815372fd>] sock_aio_read+0x2d/0x40
> [<ffffffff811b05e2>] do_sync_read+0xd2/0x110
> [<ffffffff812d3796>] ? security_file_permission+0x96/0xb0
> [<ffffffff811b0a91>] ? rw_verify_area+0x61/0x100
> [<ffffffff811b103d>] vfs_read+0x16d/0x180
> [<ffffffff811b109d>] sys_read+0x4d/0x90
> [<ffffffff81657282>] system_call_fastpath+0x16/0x1b
> Code: 8b 66 20 4c 8b 32 48 89 d3 48 89 4d b8 45 89 c7 c7 45 cc 95 ff ff ff 4d 85 e4 0f 84 ed 01 00 00 49 8b 84 24 18 05 00 00 4c 89 e7
> 78 1c 01 45 19 ed 31 f6 e8 d5 37 ff e0 41 0f b6 44 24 0e 41
>
> Signed-off-by: Dave Jones <davej@redhat.com>
>
> diff --git a/net/x25/af_x25.c b/net/x25/af_x25.c
> index 5f03e4e..291b2e0 100644
> --- a/net/x25/af_x25.c
> +++ b/net/x25/af_x25.c
> @@ -1261,13 +1261,17 @@ static int x25_recvmsg(struct kiocb *iocb, struct socket *sock,
> struct x25_sock *x25 = x25_sk(sk);
> struct sockaddr_x25 *sx25 = (struct sockaddr_x25 *)msg->msg_name;
> size_t copied;
> - int qbit, header_len = x25->neighbour->extended ?
> - X25_EXT_MIN_LEN : X25_STD_MIN_LEN;
> -
> + int qbit, header_len;
> struct sk_buff *skb;
> unsigned char *asmptr;
> int rc = -ENOTCONN;
>
> + if (x25->neighbour == NULL)
> + return rc;
> +
> + header_len = x25->neighbour->extended ?
> + X25_EXT_MIN_LEN : X25_STD_MIN_LEN;
> +
> lock_sock(sk);
BTW, you probably want to check x25->neighbour while sk is locked, not
before.
lock_sock(sk);
if (x25->neighbour == NULL)
goto out;
header_len = x25->neighbour->extended ?
X25_EXT_MIN_LEN : X25_STD_MIN_LEN;
^ permalink raw reply
* Re: Fix NULL dereference in x25_recvmsg
From: Dave Jones @ 2011-11-02 2:15 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, Matthew Daley
In-Reply-To: <1320199845.4728.23.camel@edumazet-laptop>
On Wed, Nov 02, 2011 at 03:10:45AM +0100, Eric Dumazet wrote:
> Le mardi 01 novembre 2011 à 21:53 -0400, Dave Jones a écrit :
> > commit cb101ed2 in 3.0 introduced a bug in x25_recvmsg()
> > When passed bogus junk from userspace, x25->neighbour can be NULL,
> > as shown in this oops..
> >
>
> Your patch seems fine but :
>
> Are you sure this bug is not present on previous kernels ?
>
> It seems we had prior to this commit :
>
> skb_pull(skb, x25->neighbour->extended ?
> X25_EXT_MIN_LEN : X25_STD_MIN_LEN);
It might have been possible with a specifically crafted set of arguments.
It never showed up in testing before now, probably because we were
returning from the function before we got to that skb_pull
via all the other tests that get performed.
Dave
^ permalink raw reply
* Re: Fix NULL dereference in x25_recvmsg
From: Eric Dumazet @ 2011-11-02 2:10 UTC (permalink / raw)
To: Dave Jones; +Cc: netdev, Matthew Daley
In-Reply-To: <20111102015315.GA6569@redhat.com>
Le mardi 01 novembre 2011 à 21:53 -0400, Dave Jones a écrit :
> commit cb101ed2 in 3.0 introduced a bug in x25_recvmsg()
> When passed bogus junk from userspace, x25->neighbour can be NULL,
> as shown in this oops..
>
Your patch seems fine but :
Are you sure this bug is not present on previous kernels ?
It seems we had prior to this commit :
skb_pull(skb, x25->neighbour->extended ?
X25_EXT_MIN_LEN : X25_STD_MIN_LEN);
> Signed-off-by: Dave Jones <davej@redhat.com>
>
> diff --git a/net/x25/af_x25.c b/net/x25/af_x25.c
> index 5f03e4e..291b2e0 100644
> --- a/net/x25/af_x25.c
> +++ b/net/x25/af_x25.c
> @@ -1261,13 +1261,17 @@ static int x25_recvmsg(struct kiocb *iocb, struct socket *sock,
> struct x25_sock *x25 = x25_sk(sk);
> struct sockaddr_x25 *sx25 = (struct sockaddr_x25 *)msg->msg_name;
> size_t copied;
> - int qbit, header_len = x25->neighbour->extended ?
> - X25_EXT_MIN_LEN : X25_STD_MIN_LEN;
> -
> + int qbit, header_len;
> struct sk_buff *skb;
> unsigned char *asmptr;
> int rc = -ENOTCONN;
>
> + if (x25->neighbour == NULL)
> + return rc;
> +
> + header_len = x25->neighbour->extended ?
> + X25_EXT_MIN_LEN : X25_STD_MIN_LEN;
> +
> lock_sock(sk);
^ permalink raw reply
* Fix NULL dereference in x25_recvmsg
From: Dave Jones @ 2011-11-02 1:53 UTC (permalink / raw)
To: netdev; +Cc: Matthew Daley
commit cb101ed2 in 3.0 introduced a bug in x25_recvmsg()
When passed bogus junk from userspace, x25->neighbour can be NULL,
as shown in this oops..
BUG: unable to handle kernel NULL pointer dereference at 000000000000001c
IP: [<ffffffffa05482bd>] x25_recvmsg+0x4d/0x280 [x25]
PGD 1015f3067 PUD 105072067 PMD 0
Oops: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC
CPU 0
Pid: 27928, comm: iknowthis Not tainted 3.1.0+ #2 Gigabyte Technology Co., Ltd. GA-MA78GM-S2H/GA-MA78GM-S2H
RIP: 0010:[<ffffffffa05482bd>] [<ffffffffa05482bd>] x25_recvmsg+0x4d/0x280 [x25]
RSP: 0018:ffff88010c0b7cc8 EFLAGS: 00010282
RAX: 0000000000000000 RBX: ffff88010c0b7d78 RCX: 0000000000000c02
RDX: ffff88010c0b7d78 RSI: ffff88011c93dc00 RDI: ffff880103f667b0
RBP: ffff88010c0b7d18 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000000 R12: ffff880103f667b0
R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
FS: 00007f479ce7f700(0000) GS:ffff88012a600000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
CR2: 000000000000001c CR3: 000000010529e000 CR4: 00000000000006f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
Process iknowthis (pid: 27928, threadinfo ffff88010c0b6000, task ffff880103faa4f0)
Stack:
0000000000000c02 0000000000000c02 ffff88010c0b7d18 ffffff958153cb37
ffffffff8153cb60 0000000000000c02 ffff88011c93dc00 0000000000000000
0000000000000c02 ffff88010c0b7e10 ffff88010c0b7de8 ffffffff815372c2
Call Trace:
[<ffffffff8153cb60>] ? sock_update_classid+0xb0/0x180
[<ffffffff815372c2>] sock_aio_read.part.10+0x142/0x150
[<ffffffff812d6752>] ? inode_has_perm+0x62/0xa0
[<ffffffff815372fd>] sock_aio_read+0x2d/0x40
[<ffffffff811b05e2>] do_sync_read+0xd2/0x110
[<ffffffff812d3796>] ? security_file_permission+0x96/0xb0
[<ffffffff811b0a91>] ? rw_verify_area+0x61/0x100
[<ffffffff811b103d>] vfs_read+0x16d/0x180
[<ffffffff811b109d>] sys_read+0x4d/0x90
[<ffffffff81657282>] system_call_fastpath+0x16/0x1b
Code: 8b 66 20 4c 8b 32 48 89 d3 48 89 4d b8 45 89 c7 c7 45 cc 95 ff ff ff 4d 85 e4 0f 84 ed 01 00 00 49 8b 84 24 18 05 00 00 4c 89 e7
78 1c 01 45 19 ed 31 f6 e8 d5 37 ff e0 41 0f b6 44 24 0e 41
Signed-off-by: Dave Jones <davej@redhat.com>
diff --git a/net/x25/af_x25.c b/net/x25/af_x25.c
index 5f03e4e..291b2e0 100644
--- a/net/x25/af_x25.c
+++ b/net/x25/af_x25.c
@@ -1261,13 +1261,17 @@ static int x25_recvmsg(struct kiocb *iocb, struct socket *sock,
struct x25_sock *x25 = x25_sk(sk);
struct sockaddr_x25 *sx25 = (struct sockaddr_x25 *)msg->msg_name;
size_t copied;
- int qbit, header_len = x25->neighbour->extended ?
- X25_EXT_MIN_LEN : X25_STD_MIN_LEN;
-
+ int qbit, header_len;
struct sk_buff *skb;
unsigned char *asmptr;
int rc = -ENOTCONN;
+ if (x25->neighbour == NULL)
+ return rc;
+
+ header_len = x25->neighbour->extended ?
+ X25_EXT_MIN_LEN : X25_STD_MIN_LEN;
+
lock_sock(sk);
/*
* This works for seqpacket too. The receiver has ordered the queue for
^ permalink raw reply related
* on bnx2x firmware...
From: Maciej Żenczykowski @ 2011-11-02 1:01 UTC (permalink / raw)
To: Linux NetDev
((v3.1))$ cat drivers/net/bnx2x/bnx2x_hsi.h | egrep BCM_5710_FW_
#define BCM_5710_FW_MAJOR_VERSION 7
#define BCM_5710_FW_MINOR_VERSION 0
#define BCM_5710_FW_REVISION_VERSION 23
#define BCM_5710_FW_ENGINEERING_VERSION 0
So the desired firmware is version 7.0.23.0
((v3.1))$ find | egrep bnx2x | egrep fw
./firmware/bnx2x/bnx2x-e1-6.2.9.0.fw.ihex
./firmware/bnx2x/bnx2x-e2-6.2.9.0.fw.ihex
./firmware/bnx2x/bnx2x-e1h-6.2.9.0.fw.ihex
./drivers/net/bnx2x/bnx2x_fw_defs.h
./drivers/net/bnx2x/bnx2x_fw_file_hdr.h
So it doesn't look like the in-tree fw matches the bnx2x driver in
released v3.1.
I can find this firmware in the firmware repository.
My questions are:
- is this a bug (and the firmware repo will be merged into a stable
3.1 update),
- is this a feature, and the 6.2.9.0 firmwares will be removed from the tree?
I guess I'm just not clear on what the current firmware policy is, we
seem to be in some weird half-state.
Thanks,
Maciej.
^ 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