* Re: netfilter: active obj WARN when cleaning up
From: Russell King - ARM Linux @ 2013-11-27 11:39 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Pablo Neira Ayuso, Sasha Levin, Patrick McHardy, kadlec,
David S. Miller, netfilter-devel, coreteam, netdev, LKML,
linux-mm, Andrew Morton, Christoph Lameter, Greg KH
In-Reply-To: <alpine.DEB.2.02.1311271106090.30673@ionos.tec.linutronix.de>
On Wed, Nov 27, 2013 at 11:45:17AM +0100, Thomas Gleixner wrote:
> On Wed, 27 Nov 2013, Pablo Neira Ayuso wrote:
>
> > On Tue, Nov 26, 2013 at 02:11:57PM -0500, Sasha Levin wrote:
> > > Ping? I still see this warning.
> >
> > Did your test include patch 0c3c6c00c6?
>
> And how is that patch supposed to help?
>
> > > >[ 418.312449] WARNING: CPU: 6 PID: 4178 at lib/debugobjects.c:260 debug_print_object+0x8d/0xb0()
> > > >[ 418.313243] ODEBUG: free active (active state 0) object type: timer_list hint:
> > > >delayed_work_timer_fn+0x0/0x20
>
> > > >[ 418.321101] [<ffffffff812874d7>] kmem_cache_free+0x197/0x340
> > > >[ 418.321101] [<ffffffff81249e76>] kmem_cache_destroy+0x86/0xe0
> > > >[ 418.321101] [<ffffffff83d5d681>] nf_conntrack_cleanup_net_list+0x131/0x170
>
> The debug code detects an active timer, which itself is part of a
> delayed work struct. The call comes from kmem_cache_destroy().
>
> kmem_cache_free(kmem_cache, s);
>
> So debug object says: s contains an active timer. s is the kmem_cache
> which is destroyed from nf_conntrack_cleanup_net_list.
>
> Now struct kmem_cache has in case of SLUB:
>
> struct kobject kobj; /* For sysfs */
>
> and struct kobject has:
>
> #ifdef CONFIG_DEBUG_KOBJECT_RELEASE
> struct delayed_work release;
> #endif
>
> So this is the thing you want to look at:
>
> commit c817a67ec (kobject: delayed kobject release: help find buggy
> drivers) added that delayed work thing.
>
> I fear that does not work for kobjects which are embedded into
> something else.
No, kobjects embedded into something else have their lifetime determined
by the embedded kobject. That's rule #1 of kobjects - or rather reference
counted objects.
The point at which the kobject gets destructed is when the release function
is called. If it is destructed before that time, that's a violation of
the reference counted nature of kobjects, and that's what the delay on
releasing is designed to catch.
It's designed to catch code which does this exact path:
put(obj)
free(obj)
rather than code which does it the right way:
put(obj)
-> refcount becomes 0
-> release function gets called
->free(obj)
The former is unsafe because obj may have other references.
^ permalink raw reply
* Re: [patch v2] net: heap overflow in __audit_sockaddr()
From: Eric Wong @ 2013-11-27 11:32 UTC (permalink / raw)
To: Dan Carpenter; +Cc: David S. Miller, netdev, security, Jüri Aedla, stable
In-Reply-To: <20131002212720.GA30492@elgon.mountain>
Dan Carpenter <dan.carpenter@oracle.com> wrote:
> --- a/net/socket.c
> +++ b/net/socket.c
> @@ -1964,6 +1964,16 @@ struct used_address {
> unsigned int name_len;
> };
>
> +static int copy_msghdr_from_user(struct msghdr *kmsg,
> + struct msghdr __user *umsg)
> +{
> + if (copy_from_user(kmsg, umsg, sizeof(struct msghdr)))
> + return -EFAULT;
> + if (kmsg->msg_namelen > sizeof(struct sockaddr_storage))
> + return -EINVAL;
> + return 0;
Crap, this seems to break Ruby trunk :x
https://bugs.ruby-lang.org/issues/9124
I'm inclined to think Ruby is wrong to use a gigantic buffer, but this
may also break some other existing userspace code. I'm not sure what
the best option since breaking userspace (even buggy userspace?) is not
taken lightly.
Is there a different way to fix this in the kernel?
Note: this doesn't affect a stable release of Ruby, yet.
(Not a Ruby developer myself, just occasional contributor).
^ permalink raw reply
* Re: [patch v2] net: heap overflow in __audit_sockaddr()
From: Hannes Frederic Sowa @ 2013-11-27 11:51 UTC (permalink / raw)
To: Eric Wong
Cc: Dan Carpenter, David S. Miller, netdev, security, Jüri Aedla,
stable
In-Reply-To: <20131127113218.GB1612@dcvr.yhbt.net>
On Wed, Nov 27, 2013 at 11:32:18AM +0000, Eric Wong wrote:
> Dan Carpenter <dan.carpenter@oracle.com> wrote:
> > --- a/net/socket.c
> > +++ b/net/socket.c
> > @@ -1964,6 +1964,16 @@ struct used_address {
> > unsigned int name_len;
> > };
> >
> > +static int copy_msghdr_from_user(struct msghdr *kmsg,
> > + struct msghdr __user *umsg)
> > +{
> > + if (copy_from_user(kmsg, umsg, sizeof(struct msghdr)))
> > + return -EFAULT;
> > + if (kmsg->msg_namelen > sizeof(struct sockaddr_storage))
> > + return -EINVAL;
> > + return 0;
>
> Crap, this seems to break Ruby trunk :x
> https://bugs.ruby-lang.org/issues/9124
>
> I'm inclined to think Ruby is wrong to use a gigantic buffer, but this
> may also break some other existing userspace code. I'm not sure what
> the best option since breaking userspace (even buggy userspace?) is not
> taken lightly.
>
> Is there a different way to fix this in the kernel?
>
> Note: this doesn't affect a stable release of Ruby, yet.
We have to clamp msg_namelen to max sizeof(struct sockaddr_storage).
The sendmsg handler will check msg_namelen again and error out correctly if
the size of msg_name is too short.
Greetings,
Hannes
^ permalink raw reply
* [PATCH] net: smc91: fix crash regression on the versatile
From: Linus Walleij @ 2013-11-27 11:51 UTC (permalink / raw)
To: netdev, Nicolas Pitre, David S. Miller
Cc: Linus Walleij, Russell King, Eric Miao, Jonathan Cameron, stable
The Versatile SMSC LAN91C111 is crashing like this:
------------[ cut here ]------------
kernel BUG at /home/linus/linux/drivers/net/ethernet/smsc/smc91x.c:599!
Internal error: Oops - BUG: 0 [#1] ARM
Modules linked in:
CPU: 0 PID: 43 Comm: udhcpc Not tainted 3.13.0-rc1+ #24
task: c6ccfaa0 ti: c6cd0000 task.ti: c6cd0000
PC is at smc_hardware_send_pkt+0x198/0x22c
LR is at smc_hardware_send_pkt+0x24/0x22c
pc : [<c01be324>] lr : [<c01be1b0>] psr: 20000013
sp : c6cd1d08 ip : 00000001 fp : 00000000
r10: c02adb08 r9 : 00000000 r8 : c6ced802
r7 : c786fba0 r6 : 00000146 r5 : c8800000 r4 : c78d6000
r3 : 0000000f r2 : 00000146 r1 : 00000000 r0 : 00000031
Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
Control: 0005317f Table: 06cf4000 DAC: 00000015
Process udhcpc (pid: 43, stack limit = 0xc6cd01c0)
Stack: (0xc6cd1d08 to 0xc6cd2000)
1d00: 00000010 c8800000 c78d6000 c786fba0 c78d6000 c01be868
1d20: c01be7a4 00004000 00000000 c786fba0 c6c12b80 c0208554 000004d0 c780fc60
1d40: 00000220 c01fb734 00000000 00000000 00000000 c6c9a440 c6c12b80 c78d6000
1d60: c786fba0 c6c9a440 00000000 c021d1d8 00000000 00000000 c6c12b80 c78d6000
1d80: c786fba0 00000001 c6c9a440 c02087f8 c6c9a4a0 00080008 00000000 00000000
1da0: c78d6000 c786fba0 c78d6000 00000138 00000000 00000000 00000000 00000000
1dc0: 00000000 c027ba74 00000138 00000138 00000001 00000010 c6cedc00 00000000
1de0: 00000008 c7404400 c6cd1eec c6cd1f14 c067a73c c065c0b8 00000000 c067a740
1e00: 01ffffff 002040d0 00000000 00000000 00000000 00000000 00000000 ffffffff
1e20: 43004400 00110022 c6cdef20 c027ae8c c6ccfaa0 be82d65c 00000014 be82d3cc
1e40: 00000000 00000000 00000000 c01f2870 00000000 00000000 00000000 c6cd1e88
1e60: c6ccfaa0 00000000 00000000 00000000 00000000 00000000 00000000 00000000
1e80: 00000000 00000000 00000031 c7802310 c7802300 00000138 c7404400 c0771da0
1ea0: 00000000 c6cd1eec c7800340 00000138 be82d65c 00000014 be82d3cc c6cd1f08
1ec0: 00000014 00000000 c7404400 c7404400 00000138 c01f4628 c78d6000 00000000
1ee0: 00000000 be82d3cc 00000138 c6cd1f08 00000014 c6cd1ee4 00000001 00000000
1f00: 00000000 00000000 00080011 00000002 06000000 ffffffff 0000ffff 00000002
1f20: 06000000 ffffffff 0000ffff c00928c8 c065c520 c6cd1f58 00000003 c009299c
1f40: 00000003 c065c520 c7404400 00000000 c7404400 c01f2218 c78106b0 c7441cb0
1f60: 00000000 00000006 c06799fc 00000000 00000000 00000006 00000000 c01f3ee0
1f80: 00000000 00000000 be82d678 be82d65c 00000014 00000001 00000122 c00139c8
1fa0: c6cd0000 c0013840 be82d65c 00000014 00000006 be82d3cc 00000138 00000000
1fc0: be82d65c 00000014 00000001 00000122 00000000 00000000 00018cb1 00000000
1fe0: 00003801 be82d3a8 0003a0c7 b6e9af08 60000010 00000006 00000000 00000000
[<c01be324>] (smc_hardware_send_pkt+0x198/0x22c) from [<c01be868>] (smc_hard_start_xmit+0xc4/0x1e8)
[<c01be868>] (smc_hard_start_xmit+0xc4/0x1e8) from [<c0208554>] (dev_hard_start_xmit+0x460/0x4cc)
[<c0208554>] (dev_hard_start_xmit+0x460/0x4cc) from [<c021d1d8>] (sch_direct_xmit+0x94/0x18c)
[<c021d1d8>] (sch_direct_xmit+0x94/0x18c) from [<c02087f8>] (dev_queue_xmit+0x238/0x42c)
[<c02087f8>] (dev_queue_xmit+0x238/0x42c) from [<c027ba74>] (packet_sendmsg+0xbe8/0xd28)
[<c027ba74>] (packet_sendmsg+0xbe8/0xd28) from [<c01f2870>] (sock_sendmsg+0x84/0xa8)
[<c01f2870>] (sock_sendmsg+0x84/0xa8) from [<c01f4628>] (SyS_sendto+0xb8/0xdc)
[<c01f4628>] (SyS_sendto+0xb8/0xdc) from [<c0013840>] (ret_fast_syscall+0x0/0x2c)
Code: e3130002 1a000001 e3130001 0affffcd (e7f001f2)
---[ end trace 81104fe70e8da7fe ]---
Kernel panic - not syncing: Fatal exception in interrupt
This is because the macro operations in smc91x.h are missing
necessary #defines for operations such as SMC_inbsw(),
SMC_outsw() and SMC_outsb() making these defined to BUG().
The Versatile needs and uses the same accessors as the other
platforms in the first if(...) clause, just switch it to using
that and we have one problem less to worry about.
This bug seems to have been around for some quite time, just
that few people test the Versatile on real hardware, and it
compiles.
Checkpatch complains about spacing, but I have opted to
follow the style of this .h-file.
Cc: Russell King <linux@arm.linux.org.uk>
Cc: Nicolas Pitre <nico@fluxnic.net>
Cc: Eric Miao <eric.y.miao@gmail.com>
Cc: Jonathan Cameron <jic23@cam.ac.uk>
Cc: stable@vger.kernel.org
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/net/ethernet/smsc/smc91x.h | 20 ++------------------
1 file changed, 2 insertions(+), 18 deletions(-)
diff --git a/drivers/net/ethernet/smsc/smc91x.h b/drivers/net/ethernet/smsc/smc91x.h
index c9d4c872e81d..902a7c268d82 100644
--- a/drivers/net/ethernet/smsc/smc91x.h
+++ b/drivers/net/ethernet/smsc/smc91x.h
@@ -46,7 +46,8 @@
defined(CONFIG_MACH_LITTLETON) ||\
defined(CONFIG_MACH_ZYLONITE2) ||\
defined(CONFIG_ARCH_VIPER) ||\
- defined(CONFIG_MACH_STARGATE2)
+ defined(CONFIG_MACH_STARGATE2) ||\
+ defined(CONFIG_ARCH_VERSATILE)
#include <asm/mach-types.h>
@@ -206,23 +207,6 @@ SMC_outw(u16 val, void __iomem *ioaddr, int reg)
#define RPC_LSA_DEFAULT RPC_LED_TX_RX
#define RPC_LSB_DEFAULT RPC_LED_100_10
-#elif defined(CONFIG_ARCH_VERSATILE)
-
-#define SMC_CAN_USE_8BIT 1
-#define SMC_CAN_USE_16BIT 1
-#define SMC_CAN_USE_32BIT 1
-#define SMC_NOWAIT 1
-
-#define SMC_inb(a, r) readb((a) + (r))
-#define SMC_inw(a, r) readw((a) + (r))
-#define SMC_inl(a, r) readl((a) + (r))
-#define SMC_outb(v, a, r) writeb(v, (a) + (r))
-#define SMC_outw(v, a, r) writew(v, (a) + (r))
-#define SMC_outl(v, a, r) writel(v, (a) + (r))
-#define SMC_insl(a, r, p, l) readsl((a) + (r), p, l)
-#define SMC_outsl(a, r, p, l) writesl((a) + (r), p, l)
-#define SMC_IRQ_FLAGS (-1) /* from resource */
-
#elif defined(CONFIG_MN10300)
/*
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH] net: smc91: fix crash regression on the versatile
From: Russell King - ARM Linux @ 2013-11-27 12:13 UTC (permalink / raw)
To: Linus Walleij
Cc: netdev, Nicolas Pitre, David S. Miller, Eric Miao,
Jonathan Cameron, stable
In-Reply-To: <1385553099-13874-1-git-send-email-linus.walleij@linaro.org>
On Wed, Nov 27, 2013 at 12:51:39PM +0100, Linus Walleij wrote:
> This is because the macro operations in smc91x.h are missing
> necessary #defines for operations such as SMC_inbsw(),
> SMC_outsw() and SMC_outsb() making these defined to BUG().
>
> The Versatile needs and uses the same accessors as the other
> platforms in the first if(...) clause, just switch it to using
> that and we have one problem less to worry about.
>
> This bug seems to have been around for some quite time, just
> that few people test the Versatile on real hardware, and it
> compiles.
That's a misconception. This bug is caused by:
commit e9e4ea74f06635f2ffc1dffe5ef40c854faa0a90
Author: Will Deacon <will.deacon@arm.com>
Date: Fri Oct 11 14:52:20 2013 +0100
net: smc91x: dont't use SMC_outw for fixing up halfword-aligned data
^ permalink raw reply
* Re: [PATCH] net: smc91: fix crash regression on the versatile
From: Linus Walleij @ 2013-11-27 12:28 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: netdev@vger.kernel.org, Nicolas Pitre, David S. Miller, Eric Miao,
Jonathan Cameron, stable
In-Reply-To: <20131127121303.GM16735@n2100.arm.linux.org.uk>
On Wed, Nov 27, 2013 at 1:13 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Wed, Nov 27, 2013 at 12:51:39PM +0100, Linus Walleij wrote:
>> This is because the macro operations in smc91x.h are missing
>> necessary #defines for operations such as SMC_inbsw(),
>> SMC_outsw() and SMC_outsb() making these defined to BUG().
>>
>> The Versatile needs and uses the same accessors as the other
>> platforms in the first if(...) clause, just switch it to using
>> that and we have one problem less to worry about.
>>
>> This bug seems to have been around for some quite time, just
>> that few people test the Versatile on real hardware, and it
>> compiles.
>
> That's a misconception. This bug is caused by:
>
> commit e9e4ea74f06635f2ffc1dffe5ef40c854faa0a90
> Author: Will Deacon <will.deacon@arm.com>
> Date: Fri Oct 11 14:52:20 2013 +0100
>
> net: smc91x: dont't use SMC_outw for fixing up halfword-aligned data
Ah, you're right, reverting that makes things work.
This patch should still be a viable solution, I'll fix up the
commit message and repost a v2.
Yours,
Linus Walleij
^ permalink raw reply
* [PATCH v2] net: smc91: fix crash regression on the versatile
From: Linus Walleij @ 2013-11-27 12:33 UTC (permalink / raw)
To: netdev, Nicolas Pitre, David S. Miller, Will Deacon
Cc: Linus Walleij, Russell King, Eric Miao, Jonathan Cameron, stable
After commit e9e4ea74f06635f2ffc1dffe5ef40c854faa0a90
"net: smc91x: dont't use SMC_outw for fixing up halfword-aligned data"
The Versatile SMSC LAN91C111 is crashing like this:
------------[ cut here ]------------
kernel BUG at /home/linus/linux/drivers/net/ethernet/smsc/smc91x.c:599!
Internal error: Oops - BUG: 0 [#1] ARM
Modules linked in:
CPU: 0 PID: 43 Comm: udhcpc Not tainted 3.13.0-rc1+ #24
task: c6ccfaa0 ti: c6cd0000 task.ti: c6cd0000
PC is at smc_hardware_send_pkt+0x198/0x22c
LR is at smc_hardware_send_pkt+0x24/0x22c
pc : [<c01be324>] lr : [<c01be1b0>] psr: 20000013
sp : c6cd1d08 ip : 00000001 fp : 00000000
r10: c02adb08 r9 : 00000000 r8 : c6ced802
r7 : c786fba0 r6 : 00000146 r5 : c8800000 r4 : c78d6000
r3 : 0000000f r2 : 00000146 r1 : 00000000 r0 : 00000031
Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
Control: 0005317f Table: 06cf4000 DAC: 00000015
Process udhcpc (pid: 43, stack limit = 0xc6cd01c0)
Stack: (0xc6cd1d08 to 0xc6cd2000)
1d00: 00000010 c8800000 c78d6000 c786fba0 c78d6000 c01be868
1d20: c01be7a4 00004000 00000000 c786fba0 c6c12b80 c0208554 000004d0 c780fc60
1d40: 00000220 c01fb734 00000000 00000000 00000000 c6c9a440 c6c12b80 c78d6000
1d60: c786fba0 c6c9a440 00000000 c021d1d8 00000000 00000000 c6c12b80 c78d6000
1d80: c786fba0 00000001 c6c9a440 c02087f8 c6c9a4a0 00080008 00000000 00000000
1da0: c78d6000 c786fba0 c78d6000 00000138 00000000 00000000 00000000 00000000
1dc0: 00000000 c027ba74 00000138 00000138 00000001 00000010 c6cedc00 00000000
1de0: 00000008 c7404400 c6cd1eec c6cd1f14 c067a73c c065c0b8 00000000 c067a740
1e00: 01ffffff 002040d0 00000000 00000000 00000000 00000000 00000000 ffffffff
1e20: 43004400 00110022 c6cdef20 c027ae8c c6ccfaa0 be82d65c 00000014 be82d3cc
1e40: 00000000 00000000 00000000 c01f2870 00000000 00000000 00000000 c6cd1e88
1e60: c6ccfaa0 00000000 00000000 00000000 00000000 00000000 00000000 00000000
1e80: 00000000 00000000 00000031 c7802310 c7802300 00000138 c7404400 c0771da0
1ea0: 00000000 c6cd1eec c7800340 00000138 be82d65c 00000014 be82d3cc c6cd1f08
1ec0: 00000014 00000000 c7404400 c7404400 00000138 c01f4628 c78d6000 00000000
1ee0: 00000000 be82d3cc 00000138 c6cd1f08 00000014 c6cd1ee4 00000001 00000000
1f00: 00000000 00000000 00080011 00000002 06000000 ffffffff 0000ffff 00000002
1f20: 06000000 ffffffff 0000ffff c00928c8 c065c520 c6cd1f58 00000003 c009299c
1f40: 00000003 c065c520 c7404400 00000000 c7404400 c01f2218 c78106b0 c7441cb0
1f60: 00000000 00000006 c06799fc 00000000 00000000 00000006 00000000 c01f3ee0
1f80: 00000000 00000000 be82d678 be82d65c 00000014 00000001 00000122 c00139c8
1fa0: c6cd0000 c0013840 be82d65c 00000014 00000006 be82d3cc 00000138 00000000
1fc0: be82d65c 00000014 00000001 00000122 00000000 00000000 00018cb1 00000000
1fe0: 00003801 be82d3a8 0003a0c7 b6e9af08 60000010 00000006 00000000 00000000
[<c01be324>] (smc_hardware_send_pkt+0x198/0x22c) from [<c01be868>] (smc_hard_start_xmit+0xc4/0x1e8)
[<c01be868>] (smc_hard_start_xmit+0xc4/0x1e8) from [<c0208554>] (dev_hard_start_xmit+0x460/0x4cc)
[<c0208554>] (dev_hard_start_xmit+0x460/0x4cc) from [<c021d1d8>] (sch_direct_xmit+0x94/0x18c)
[<c021d1d8>] (sch_direct_xmit+0x94/0x18c) from [<c02087f8>] (dev_queue_xmit+0x238/0x42c)
[<c02087f8>] (dev_queue_xmit+0x238/0x42c) from [<c027ba74>] (packet_sendmsg+0xbe8/0xd28)
[<c027ba74>] (packet_sendmsg+0xbe8/0xd28) from [<c01f2870>] (sock_sendmsg+0x84/0xa8)
[<c01f2870>] (sock_sendmsg+0x84/0xa8) from [<c01f4628>] (SyS_sendto+0xb8/0xdc)
[<c01f4628>] (SyS_sendto+0xb8/0xdc) from [<c0013840>] (ret_fast_syscall+0x0/0x2c)
Code: e3130002 1a000001 e3130001 0affffcd (e7f001f2)
---[ end trace 81104fe70e8da7fe ]---
Kernel panic - not syncing: Fatal exception in interrupt
This is because the macro operations in smc91x.h defined
for Versatile are missing SMC_outsw() as used in this
commit.
The Versatile needs and uses the same accessors as the other
platforms in the first if(...) clause, just switch it to using
that and we have one problem less to worry about.
Checkpatch complains about spacing, but I have opted to
follow the style of this .h-file.
Cc: Russell King <linux@arm.linux.org.uk>
Cc: Nicolas Pitre <nico@fluxnic.net>
Cc: Eric Miao <eric.y.miao@gmail.com>
Cc: Jonathan Cameron <jic23@cam.ac.uk>
Cc: Will Deacon <will.deacon@arm.com>
Cc: stable@vger.kernel.org
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v1->v2:
- Correct commit message to nail the actual commit causing
the bug, as pointed out by Russell.
---
drivers/net/ethernet/smsc/smc91x.h | 20 ++------------------
1 file changed, 2 insertions(+), 18 deletions(-)
diff --git a/drivers/net/ethernet/smsc/smc91x.h b/drivers/net/ethernet/smsc/smc91x.h
index c9d4c872e81d..902a7c268d82 100644
--- a/drivers/net/ethernet/smsc/smc91x.h
+++ b/drivers/net/ethernet/smsc/smc91x.h
@@ -46,7 +46,8 @@
defined(CONFIG_MACH_LITTLETON) ||\
defined(CONFIG_MACH_ZYLONITE2) ||\
defined(CONFIG_ARCH_VIPER) ||\
- defined(CONFIG_MACH_STARGATE2)
+ defined(CONFIG_MACH_STARGATE2) ||\
+ defined(CONFIG_ARCH_VERSATILE)
#include <asm/mach-types.h>
@@ -206,23 +207,6 @@ SMC_outw(u16 val, void __iomem *ioaddr, int reg)
#define RPC_LSA_DEFAULT RPC_LED_TX_RX
#define RPC_LSB_DEFAULT RPC_LED_100_10
-#elif defined(CONFIG_ARCH_VERSATILE)
-
-#define SMC_CAN_USE_8BIT 1
-#define SMC_CAN_USE_16BIT 1
-#define SMC_CAN_USE_32BIT 1
-#define SMC_NOWAIT 1
-
-#define SMC_inb(a, r) readb((a) + (r))
-#define SMC_inw(a, r) readw((a) + (r))
-#define SMC_inl(a, r) readl((a) + (r))
-#define SMC_outb(v, a, r) writeb(v, (a) + (r))
-#define SMC_outw(v, a, r) writew(v, (a) + (r))
-#define SMC_outl(v, a, r) writel(v, (a) + (r))
-#define SMC_insl(a, r, p, l) readsl((a) + (r), p, l)
-#define SMC_outsl(a, r, p, l) writesl((a) + (r), p, l)
-#define SMC_IRQ_FLAGS (-1) /* from resource */
-
#elif defined(CONFIG_MN10300)
/*
--
1.8.3.1
^ permalink raw reply related
* [patch] net: clamp ->msg_namelen instead of returning an error
From: Dan Carpenter @ 2013-11-27 12:40 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Eric Wong, Hannes Frederic Sowa
In-Reply-To: <20131127115120.GC20630@order.stressinduktion.org>
If kmsg->msg_namelen > sizeof(struct sockaddr_storage) then in the
original code that would lead to memory corruption in the kernel if you
had audit configured. If you didn't have audit configured it was
harmless.
There are some programs such as beta versions of Ruby which use too
large of a buffer and returning an error code breaks them. We should
clamp the ->msg_namelen value instead.
Reported-by: Eric Wong <normalperson@yhbt.net>
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/net/socket.c b/net/socket.c
index 0b18693f2be6..e83c416708af 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -1973,7 +1973,7 @@ static int copy_msghdr_from_user(struct msghdr *kmsg,
if (copy_from_user(kmsg, umsg, sizeof(struct msghdr)))
return -EFAULT;
if (kmsg->msg_namelen > sizeof(struct sockaddr_storage))
- return -EINVAL;
+ kmsg->msg_namelen = sizeof(struct sockaddr_storage);
return 0;
}
diff --git a/net/compat.c b/net/compat.c
index 618c6a8a911b..dd32e34c1e2c 100644
--- a/net/compat.c
+++ b/net/compat.c
@@ -72,7 +72,7 @@ int get_compat_msghdr(struct msghdr *kmsg, struct compat_msghdr __user *umsg)
__get_user(kmsg->msg_flags, &umsg->msg_flags))
return -EFAULT;
if (kmsg->msg_namelen > sizeof(struct sockaddr_storage))
- return -EINVAL;
+ kmsg->msg_namelen = sizeof(struct sockaddr_storage);
kmsg->msg_name = compat_ptr(tmp1);
kmsg->msg_iov = compat_ptr(tmp2);
kmsg->msg_control = compat_ptr(tmp3);
^ permalink raw reply related
* [PATCH] net: fec_main: dma_map() only the length of the skb
From: Sebastian Andrzej Siewior @ 2013-11-27 12:44 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, Fabio Estevam, Frank Li, Fugang Duan, Jim Baxter,
Marek Szyprowski, Sebastian Andrzej Siewior
On tx submit the driver always dma_map_single() FEC_ENET_TX_FRSIZE (=2048)
bytes. This works because we don't overwrite any memory after the data buffer,
we remove it from cache if it was there. So we hurt performace in case the
mapping of a smaller area makes a difference.
There is also a bug: If the data area starts shortly before the end of
RAM say 0xc7fffa10 and the RAM ends at 0xc8000000 then we have enough
space to fit the data area (according to skb->len) but we would map beyond
end of ram if we are using 2048. In v2.6.31 (against which kernel this patch
made) there is the following check in dma_cache_maint():
|BUG_ON(!virt_addr_valid(start) || !virt_addr_valid(start + size - 1));
Since the area starting at 0xc8000000 is no longer virt_addr_valid() we
BUG() during dma_map_single(). The BUG() statement was removed in v3.5-rc1 as
per 2dc6a016 ("ARM: dma-mapping: use asm-generic/dma-mapping-common.h").
This patch was tested on v2.6.31 and then forward-ported and compile
tested only against the net tree. I think it is still worth fixing
mainline even after the BUG() statement is gone.
Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
It would be nice if someone could test this on current kernel.
Is this worth pushing stable?
Marek: Was there a special reason why the check was removed? Would it make
sense to bring it back say under CONFIG_DMA_DEBUG?
drivers/net/ethernet/freescale/fec_main.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 4cbebf3..29d8dc4 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -385,7 +385,7 @@ fec_enet_start_xmit(struct sk_buff *skb, struct net_device *ndev)
* data.
*/
bdp->cbd_bufaddr = dma_map_single(&fep->pdev->dev, bufaddr,
- FEC_ENET_TX_FRSIZE, DMA_TO_DEVICE);
+ skb->len, DMA_TO_DEVICE);
if (dma_mapping_error(&fep->pdev->dev, bdp->cbd_bufaddr)) {
bdp->cbd_bufaddr = 0;
fep->tx_skbuff[index] = NULL;
@@ -779,11 +779,11 @@ fec_enet_tx(struct net_device *ndev)
else
index = bdp - fep->tx_bd_base;
- dma_unmap_single(&fep->pdev->dev, bdp->cbd_bufaddr,
- FEC_ENET_TX_FRSIZE, DMA_TO_DEVICE);
+ skb = fep->tx_skbuff[index];
+ dma_unmap_single(&fep->pdev->dev, bdp->cbd_bufaddr, skb->len,
+ DMA_TO_DEVICE);
bdp->cbd_bufaddr = 0;
- skb = fep->tx_skbuff[index];
/* Check for errors. */
if (status & (BD_ENET_TX_HB | BD_ENET_TX_LC |
--
1.8.4.4
^ permalink raw reply related
* Re: Supporting 4 way connections in LKSCTP
From: Neil Horman @ 2013-11-27 12:45 UTC (permalink / raw)
To: Sun Paul; +Cc: Vlad Yasevich, linux-sctp, netdev, linux-kernel
In-Reply-To: <CAFXGftLt1p6XpR2eGsMYKjEbbOHvch-LgcFw1sAXThcA2Rtjyw@mail.gmail.com>
On Wed, Nov 27, 2013 at 07:10:49AM +0800, Sun Paul wrote:
> Hi Vlad
>
> Thank for your reply. If it is based on the destination IP to find the
> best route, why the problem didn't happen on single-homing sample?
>
Because You only ever use one address from NODE A (12.1.1.1)
> In the single-homing sample that provided in the original email, both
> of the interfaces (eth1 and eth2) are presented on NODE-B during the
> test. However, the LKSCTP library know to use the interface eth1 to
> respond to the SCTP request.
>
Yes, because it does a route lookup to each of the two ip addresses to NODE B,
and in both lookups, the route indicates that only one source address should be
used (12.1.1.1). If you issue a ip route show command, you'll see that routes
to both address on NODE B match on a rule that specifies the same src address
and interface be used.
Neil
> - PS
>
> On Wed, Nov 27, 2013 at 7:09 AM, Sun Paul <paulrbk@gmail.com> wrote:
> > Hi Vlad
> >
> > Thank for your reply. If it is based on the destination IP to find the
> > best route, why the problem didn't happen on single-homing sample?
> >
> > In the single-homing sample that provided in the original email, both
> > of the interfaces (eth1 and eth2) are presented on NODE-B during the
> > test. However, the LKSCTP library know to use the interface eth1 to
> > respond to the SCTP request.
> >
> > - PS
> >
> > On Tue, Nov 26, 2013 at 11:19 PM, Vlad Yasevich <vyasevich@gmail.com> wrote:
> >> On 11/25/2013 08:03 PM, Sun Paul wrote:
> >>> Hi
> >>>
> >>> we have a problem on using LKSCTP to form a 4 ways multi-homing network.
> >>>
> >>> Configuration
> >>> - Node-A has 2 IP addresses in different subnets, known as IP-A (eth1),
> >>> IP-B (eth2)
> >>> - Node-B has 2 IP addresses in different subnets, known as IP-X (eth1),
> >>> IP-Y (eth2)
> >>>
> >>
> >> First of all, this is not a 4 way multi-homed network. As far as each
> >> SCTP association is concerned, it has only 2 destinations to send to
> >> so it has only 2 ways to get there. The fact that you have multiple
> >> local addresses doesn't mean that every local address can and should
> >> be used to connect to the remote.
> >>
> >>> the four way paths are shown below.
> >>> 1. IP-A (11.1.1.1) to IP-X (11.1.1.11)
> >>> 2. IP-B (12.1.1.1) to IP-Y (12.1.1.11)
> >>> 3. IP-A (11.1.1.1) to IP-Y (12.1.1.11)
> >>> 4. IP-B (12.1.1.1) to IP-X (11.1.1.11)
> >>
> >> No, actually you only have 2 paths: one to IPX and one to IP-Y.
> >> Which source address you choose is based on routing policy
> >> decisions and is outside the scope of SCTP.
> >>
> >>>
> >>> the HB/HB_ACK is normal for the paths " IP-A to IP-X" and "IP-B to
> >>> IP-Y", but it is not correct for the rest of two.
> >>
> >> Right, because linux is using a host addressing model, not an interface
> >> addressing model. SCTP stack simply finds the best source address
> >> that can be used to reach IP-X and it happens to be IP-A. So that
> >> is what it is going to use.
> >>
> >> The above explains why you are seeing what you describe below.
> >>
> >> In the end, linux SCTP implementation determines paths solely based
> >> on the destination address.
> >>
> >> -vlad
> >>
> >>>
> >>> First of all, we are using iproute2 to form 2 table such that when
> >>> IP-B arrives on IP-X, it will know how to route back to IP-B on the
> >>> same interface, i.e (eth1). Same logic for the path "IP-A to IP-X".
> >>>
> >>> What we observed here is that when 12.1.1.1 sends INIT to 11.1.1.11,
> >>> LKSCTP will send back the INIT_ACK to 12.1.1.1 using 12.1.1.11 but not
> >>> using the IP 11.1.1.11.
> >>>
> >>> The above operation makes the subsequence HB/HB_ACK in using wrong IP address.
> >>>
> >>> TCP trace on eth1
> >>> 18:02:41.058640 IP 12.1.1.1.2905 > 11.1.1.11.2905: sctp (1) [INIT]
> >>> [init tag: 19933036] [rwnd: 102400] [OS: 16] [MIS: 16] [init TSN: 0]
> >>> 18:02:41.061634 IP 12.1.1.1.2905 > 11.1.1.11.2905: sctp (1) [COOKIE ECHO]
> >>> 18:02:41.062642 IP 12.1.1.1.2905 > 11.1.1.11.2905: sctp (1) [HB REQ]
> >>> 18:02:41.062846 IP 11.1.1.11.2905 > 11.1.1.1.2905: sctp (1) [HB ACK]
> >>> 18:02:41.361811 IP 11.1.1.11.2905 > 11.1.1.1.2905: sctp (1) [HB ACK]
> >>> 18:02:41.661791 IP 11.1.1.11.2905 > 11.1.1.1.2905: sctp (1) [HB ACK]
> >>> 18:02:41.961791 IP 11.1.1.11.2905 > 11.1.1.1.2905: sctp (1) [HB ACK]
> >>>
> >>> TCP trace on eth2
> >>> 18:02:41.058755 IP 12.1.1.11.2905 > 12.1.1.1.2905: sctp (1) [INIT ACK]
> >>> [init tag: 424726157] [rwnd: 131072] [OS: 5] [MIS: 5] [init TSN:
> >>> 3340756356]
> >>> 18:02:41.061696 IP 12.1.1.11.2905 > 12.1.1.1.2905: sctp (1) [COOKIE ACK]
> >>> 18:02:41.062663 IP 12.1.1.11.2905 > 12.1.1.1.2905: sctp (1) [HB ACK]
> >>> 18:02:41.062791 IP 11.1.1.1.2905 > 12.1.1.11.2905: sctp (1) [HB REQ]
> >>> 18:02:41.361777 IP 11.1.1.1.2905 > 12.1.1.11.2905: sctp (1) [HB REQ]
> >>> 18:02:41.661772 IP 11.1.1.1.2905 > 12.1.1.11.2905: sctp (1) [HB REQ]
> >>> 18:02:41.961772 IP 11.1.1.1.2905 > 12.1.1.11.2905: sctp (1) [HB REQ]
> >>> 18:02:42.161771 IP 11.1.1.1.2905 > 12.1.1.11.2905: sctp (1) [HB REQ]
> >>> 18:02:42.461770 IP 11.1.1.1.2905 > 12.1.1.11.2905: sctp (1) [HB REQ]
> >>> 18:02:42.675770 IP 11.1.1.1.2905 > 12.1.1.11.2905: sctp (1) [HB REQ]
> >>>
> >>>
> >>> If we are using single homing, there is no problem on the SCTP
> >>> communication. Below is the TCP trace on eth1 using sctp_test
> >>>
> >>> 18:09:55.356727 IP 12.1.1.1.2905 > 11.1.1.11.2905: sctp (1) [INIT]
> >>> [init tag: 32516609] [rwnd: 102400] [OS: 16] [MIS: 16] [init TSN: 0]
> >>> 18:09:55.356811 IP 11.1.1.11.2905 > 12.1.1.1.2905: sctp (1) [INIT ACK]
> >>> [init tag: 3168861995] [rwnd: 131072] [OS: 10] [MIS: 16] [init TSN:
> >>> 1877695021]
> >>> 18:09:55.357727 IP 12.1.1.1.2905 > 11.1.1.11.2905: sctp (1) [COOKIE ECHO]
> >>> 18:09:55.357788 IP 11.1.1.11.2905 > 12.1.1.1.2905: sctp (1) [COOKIE ACK]
> >>> 18:09:55.358724 IP 12.1.1.1.2905 > 11.1.1.11.2905: sctp (1) [HB REQ]
> >>> 18:09:55.358740 IP 11.1.1.11.2905 > 12.1.1.1.2905: sctp (1) [HB ACK]
> >>> 18:09:55.379715 IP 12.1.1.1.2905 > 11.1.1.11.2905: sctp (1) [DATA]
> >>> (B)(E) [TSN: 0] [SID: 0] [SSEQ 0] [PPID 0x3]
> >>> 18:09:55.379735 IP 11.1.1.11.2905 > 12.1.1.1.2905: sctp (1) [SACK]
> >>> [cum ack 0] [a_rwnd 131064] [#gap acks 0] [#dup tsns 0]
> >>> 18:09:55.657716 IP 12.1.1.1.2905 > 11.1.1.11.2905: sctp (1) [HB REQ]
> >>> 18:09:55.657732 IP 11.1.1.11.2905 > 12.1.1.1.2905: sctp (1) [HB ACK]
> >>>
> >>> From the observations, it seems that the LKSCTP library is not able to
> >>> use the original local address when multi-homing is being used. Is
> >>> there anyway can be resolved it?
> >>>
> >>> Thanks
> >>>
> >>> PS
> >>> --
> >>> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> >>> the body of a message to majordomo@vger.kernel.org
> >>> More majordomo info at http://vger.kernel.org/majordomo-info.html
> >>>
> >>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* [PATCH net] af_packet: use spin_lock_bh() for sk_buff_head
From: Veaceslav Falico @ 2013-11-27 12:57 UTC (permalink / raw)
To: netdev
Cc: jstancek, Veaceslav Falico, David S. Miller, Daniel Borkmann,
Willem de Bruijn, Phil Sutter, Eric Dumazet
While running a debug kernel, a warning of inconsistent lock state showed
up for sk->sk_receive_queue->lock between packet_set_ring() (actually,
static prb_shutdown_retire_blk_timer(), which does spin_lock()) and
sock_queue_rcv_skb(), which does spin_lock_irqsave().
Fix this by converting the spin_lock() used in
prb_shutdown_retire_blk_timer() to spin_lock_bh(), as it is used in
packet_set_ring().
CC: "David S. Miller" <davem@davemloft.net>
CC: Daniel Borkmann <dborkman@redhat.com>
CC: Willem de Bruijn <willemb@google.com>
CC: Phil Sutter <phil@nwl.cc>
CC: Eric Dumazet <edumazet@google.com>
Reported-by: Jan Stancek <jstancek@redhat.com>
Tested-by: Jan Stancek <jstancek@redhat.com>
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---
Notes:
I'm not sure if it's the proper fix - as it might hide bigger problems,
and I'm not really comfortable with the current mixing spin_lock_irqsave()
and spin_lock_bh().
The complete warning is:
[ 1057.914735] =================================
[ 1057.919091] [ INFO: inconsistent lock state ]
[ 1057.923443] 3.13.0-rc1+ #1 Not tainted
[ 1057.927186] ---------------------------------
[ 1057.931536] inconsistent {IN-SOFTIRQ-W} -> {SOFTIRQ-ON-W} usage.
[ 1057.937541] dumpcap/9377 [HC0[0]:SC0[0]:HE1:SE1] takes:
[ 1057.942764] (&(&list->lock)->rlock){+.?...}, at: [<ffffffff81673ee0>] packet_set_ring+0x5a0/0x760
[ 1057.951782] {IN-SOFTIRQ-W} state was registered at:
[ 1057.956653] [<ffffffff810d4c7b>] __lock_acquire+0x59b/0x1a80
[ 1057.962509] [<ffffffff810d6952>] lock_acquire+0xa2/0x1d0
[ 1057.968001] [<ffffffff816a3916>] _raw_spin_lock_irqsave+0x56/0x90
[ 1057.974284] [<ffffffff81545d55>] sock_queue_rcv_skb+0xe5/0x3a0
[ 1057.980306] [<ffffffff8164a749>] __udpv6_queue_rcv_skb+0x49/0x250
[ 1057.986586] [<ffffffff8164c0cb>] udpv6_queue_rcv_skb+0x24b/0x4e0
[ 1057.992773] [<ffffffff8164c7e7>] __udp6_lib_rcv+0x147/0x650
[ 1057.998527] [<ffffffff8164cd05>] udpv6_rcv+0x15/0x20
[ 1058.003671] [<ffffffff8162cc99>] ip6_input_finish+0x1e9/0x810
[ 1058.009597] [<ffffffff8162db52>] ip6_input+0x22/0x60
[ 1058.014754] [<ffffffff8162c86f>] ip6_rcv_finish+0x3f/0x280
[ 1058.020419] [<ffffffff8162d758>] ipv6_rcv+0x498/0x870
[ 1058.025661] [<ffffffff8155c312>] __netif_receive_skb_core+0x9f2/0xdb0
[ 1058.032281] [<ffffffff8155c6e8>] __netif_receive_skb+0x18/0x60
[ 1058.038294] [<ffffffff8155ec8e>] process_backlog+0xbe/0x1a0
[ 1058.044048] [<ffffffff8155e9c2>] net_rx_action+0x162/0x370
[ 1058.049713] [<ffffffff8107ae84>] __do_softirq+0x104/0x480
[ 1058.055293] [<ffffffff816aedbc>] do_softirq_own_stack+0x1c/0x30
[ 1058.061393] [<ffffffff8107b2ed>] do_softirq+0x7d/0x90
[ 1058.066627] [<ffffffff8107b495>] local_bh_enable+0xc5/0xd0
[ 1058.072292] [<ffffffff816287ee>] ip6_finish_output2+0x1fe/0x810
[ 1058.078392] [<ffffffff8162c426>] ip6_finish_output+0x96/0x220
[ 1058.084317] [<ffffffff8162c5ff>] ip6_output+0x4f/0x1e0
[ 1058.089639] [<ffffffff8166fc19>] ip6_local_out+0x29/0x90
[ 1058.095139] [<ffffffff8162a060>] ip6_push_pending_frames+0x2f0/0x4e0
[ 1058.101680] [<ffffffff81649719>] udp_v6_push_pending_frames+0x129/0x3d0
[ 1058.108481] [<ffffffff8164a296>] udpv6_sendmsg+0x6f6/0xae0
[ 1058.114158] [<ffffffff815e5aa7>] inet_sendmsg+0x107/0x220
[ 1058.119738] [<ffffffff8154005b>] sock_sendmsg+0x8b/0xc0
[ 1058.125145] [<ffffffff81540204>] SYSC_sendto+0x124/0x1d0
[ 1058.130636] [<ffffffff8154149e>] SyS_sendto+0xe/0x10
[ 1058.135792] [<ffffffff816ad3e9>] system_call_fastpath+0x16/0x1b
[ 1058.141891] irq event stamp: 12417
[ 1058.145287] hardirqs last enabled at (12417): [<ffffffff816a16e0>] __mutex_unlock_slowpath+0xd0/0x1e0
[ 1058.154603] hardirqs last disabled at (12416): [<ffffffff816a1656>] __mutex_unlock_slowpath+0x46/0x1e0
[ 1058.163916] softirqs last enabled at (12412): [<ffffffff81673c36>] packet_set_ring+0x2f6/0x760
[ 1058.172616] softirqs last disabled at (12410): [<ffffffff816a2e28>] _raw_spin_lock_bh+0x18/0x80
[ 1058.181314]
[ 1058.181314] other info that might help us debug this:
[ 1058.187829] Possible unsafe locking scenario:
[ 1058.187829]
[ 1058.193739] CPU0
[ 1058.196188] ----
[ 1058.198634] lock(&(&list->lock)->rlock);
[ 1058.202756] <Interrupt>
[ 1058.205373] lock(&(&list->lock)->rlock);
[ 1058.209670]
[ 1058.209670] *** DEADLOCK ***
[ 1058.209670]
[ 1058.215580] 1 lock held by dumpcap/9377:
[ 1058.219495] #0: (sk_lock-AF_PACKET){+.+.+.}, at: [<ffffffff81673b88>] packet_set_ring+0x248/0x760
[ 1058.228612]
[ 1058.228612] stack backtrace:
[ 1058.232960] CPU: 1 PID: 9377 Comm: dumpcap Not tainted 3.13.0-rc1+ #1
[ 1058.239390] Hardware name: Dell Inc. PowerEdge R210 II/09T7VV, BIOS 2.0.4 02/29/2012
[ 1058.247117] ffffffff82688fe0 ffff880132fedb78 ffffffff81699e80 ffff880138348000
[ 1058.254569] ffff880132fedbc8 ffffffff81693c09 0000000000000000 ffff880100000001
[ 1058.262030] ffffffff00000001 0000000000000004 ffff880138348000 ffffffff810d3360
[ 1058.269488] Call Trace:
[ 1058.271933] [<ffffffff81699e80>] dump_stack+0x4d/0x66
[ 1058.277072] [<ffffffff81693c09>] print_usage_bug+0x1ec/0x1fd
[ 1058.282815] [<ffffffff810d3360>] ? print_shortest_lock_dependencies+0x1d0/0x1d0
[ 1058.290196] [<ffffffff810d3f5d>] mark_lock+0x21d/0x2a0
[ 1058.295413] [<ffffffff810d4ccf>] __lock_acquire+0x5ef/0x1a80
[ 1058.301149] [<ffffffff810d42fd>] ? trace_hardirqs_on+0xd/0x10
[ 1058.306981] [<ffffffff81022f83>] ? native_sched_clock+0x13/0x80
[ 1058.312983] [<ffffffff81022ff9>] ? sched_clock+0x9/0x10
[ 1058.318287] [<ffffffff81022f83>] ? native_sched_clock+0x13/0x80
[ 1058.324290] [<ffffffff81022ff9>] ? sched_clock+0x9/0x10
[ 1058.329594] [<ffffffff81022f83>] ? native_sched_clock+0x13/0x80
[ 1058.335597] [<ffffffff81022ff9>] ? sched_clock+0x9/0x10
[ 1058.340899] [<ffffffff810d6952>] lock_acquire+0xa2/0x1d0
[ 1058.346287] [<ffffffff81673ee0>] ? packet_set_ring+0x5a0/0x760
[ 1058.352199] [<ffffffff816a2dce>] _raw_spin_lock+0x3e/0x80
[ 1058.357683] [<ffffffff81673ee0>] ? packet_set_ring+0x5a0/0x760
[ 1058.363599] [<ffffffff81673ee0>] packet_set_ring+0x5a0/0x760
[ 1058.369337] [<ffffffff810d07dd>] ? trace_hardirqs_off+0xd/0x10
[ 1058.375254] [<ffffffff8167422d>] packet_release+0x18d/0x2e0
[ 1058.380910] [<ffffffff8153f55f>] sock_release+0x1f/0x90
[ 1058.386214] [<ffffffff8153f5e2>] sock_close+0x12/0x20
[ 1058.391352] [<ffffffff8120702d>] __fput+0xfd/0x310
[ 1058.396229] [<ffffffff8120728e>] ____fput+0xe/0x10
[ 1058.401101] [<ffffffff8109cdf4>] task_work_run+0xb4/0xe0
[ 1058.406497] [<ffffffff8101aaff>] do_notify_resume+0x9f/0xc0
[ 1058.412153] [<ffffffff816ad722>] int_signal+0x12/0x17
net/packet/af_packet.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index ac27c86..ba2548b 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -439,9 +439,9 @@ static void prb_shutdown_retire_blk_timer(struct packet_sock *po,
pkc = tx_ring ? &po->tx_ring.prb_bdqc : &po->rx_ring.prb_bdqc;
- spin_lock(&rb_queue->lock);
+ spin_lock_bh(&rb_queue->lock);
pkc->delete_blk_timer = 1;
- spin_unlock(&rb_queue->lock);
+ spin_unlock_bh(&rb_queue->lock);
prb_del_retire_blk_timer(pkc);
}
--
1.8.4
^ permalink raw reply related
* Re: [patch net-next RFC v2 2/2] ipv6 addrconf: introduce IFA_F_MANAGETEMPADDR to tell kernel to manage temporary addresses
From: Thomas Haller @ 2013-11-27 13:02 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, davem, kuznet, jmorris, yoshfuji, kaber, stephen, hannes,
vyasevich, dcbw, David.Laight
In-Reply-To: <1384857809-6573-3-git-send-email-jiri@resnulli.us>
[-- Attachment #1: Type: text/plain, Size: 1923 bytes --]
Hi Jiri,
just two comments:
> addrconf_verify(0);
> return 0;
> @@ -3609,6 +3617,7 @@ static int inet6_addr_modify(struct inet6_ifaddr *ifp, u32 ifa_flags,
> u32 flags;
> clock_t expires;
> unsigned long timeout;
> + bool was_managetempaddr;
>
> if (!valid_lft || (prefered_lft > valid_lft))
> return -EINVAL;
> @@ -3632,7 +3641,10 @@ static int inet6_addr_modify(struct inet6_ifaddr *ifp, u32 ifa_flags,
> }
>
> spin_lock_bh(&ifp->lock);
We should make sure here, not to allow IFA_F_MANAGETEMPADDR, if ifp has
prefixlen!=64 or is IFA_F_TEMPORARY itself. Note, that inet6_rtm_newaddr
does check for ifm->ifa_prefixlen != 64, but when modifying an existing
address, the ifm->ifa_prefixlen is not relevant.
> - ifp->flags = (ifp->flags & ~(IFA_F_DEPRECATED | IFA_F_PERMANENT | IFA_F_NODAD | IFA_F_HOMEADDRESS)) | ifa_flags;
> + was_managetempaddr = ifp->flags & IFA_F_MANAGETEMPADDR;
> + ifp->flags &= ~(IFA_F_DEPRECATED | IFA_F_PERMANENT | IFA_F_NODAD |
> + IFA_F_HOMEADDRESS | IFA_F_MANAGETEMPADDR);
> + ifp->flags |= ifa_flags;
> ifp->tstamp = jiffies;
> ifp->valid_lft = valid_lft;
> ifp->prefered_lft = prefered_lft;
> @@ -3643,6 +3655,14 @@ static int inet6_addr_modify(struct inet6_ifaddr *ifp, u32 ifa_flags,
>
> addrconf_prefix_route(&ifp->addr, ifp->prefix_len, ifp->idev->dev,
> expires, flags);
> +
> + if (was_managetempaddr || ifp->flags & IFA_F_MANAGETEMPADDR) {
> + if (was_managetempaddr && !(ifp->flags & IFA_F_MANAGETEMPADDR))
> + valid_lft = prefered_lft = 0;
> + manage_tempaddrs(ifp->idev, ifp, valid_lft, prefered_lft,
> + false, jiffies);
I think, the parameter "create" of manage_tempaddrs should be set to
"!was_managetempaddr", because we want to force creation of a new
temporary address, if ifp just gets the flag IFA_F_MANAGETEMPADDR
(without having it before).
Thomas
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* RE: [PATCH] net: fec_main: dma_map() only the length of the skb
From: Fugang Duan @ 2013-11-27 13:08 UTC (permalink / raw)
To: Sebastian Andrzej Siewior, netdev@vger.kernel.org
Cc: David S. Miller, Fabio Estevam, Frank Li, Jim Baxter,
Marek Szyprowski
In-Reply-To: <1385556253-4130-1-git-send-email-bigeasy@linutronix.de>
From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Data: Wednesday, November 27, 2013 8:44 PM
>To: netdev@vger.kernel.org
>Cc: David S. Miller; Estevam Fabio-R49496; Frank Li; Duan Fugang-B38611; Jim
>Baxter; Marek Szyprowski; Sebastian Andrzej Siewior
>Subject: [PATCH] net: fec_main: dma_map() only the length of the skb
>
>On tx submit the driver always dma_map_single() FEC_ENET_TX_FRSIZE (=2048)
>bytes. This works because we don't overwrite any memory after the data buffer,
>we remove it from cache if it was there. So we hurt performace in case the
>mapping of a smaller area makes a difference.
>There is also a bug: If the data area starts shortly before the end of RAM say
>0xc7fffa10 and the RAM ends at 0xc8000000 then we have enough space to fit the
>data area (according to skb->len) but we would map beyond end of ram if we are
>using 2048. In v2.6.31 (against which kernel this patch
>made) there is the following check in dma_cache_maint():
>
>|BUG_ON(!virt_addr_valid(start) || !virt_addr_valid(start + size - 1));
>
>Since the area starting at 0xc8000000 is no longer virt_addr_valid() we
>BUG() during dma_map_single(). The BUG() statement was removed in v3.5-rc1 as
>per 2dc6a016 ("ARM: dma-mapping: use asm-generic/dma-mapping-common.h").
>
>This patch was tested on v2.6.31 and then forward-ported and compile tested
>only against the net tree. I think it is still worth fixing mainline even after
>the BUG() statement is gone.
>
>Cc: Marek Szyprowski <m.szyprowski@samsung.com>
>Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
>---
>It would be nice if someone could test this on current kernel.
>Is this worth pushing stable?
>
>Marek: Was there a special reason why the check was removed? Would it make
>sense to bring it back say under CONFIG_DMA_DEBUG?
>
> drivers/net/ethernet/freescale/fec_main.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
>diff --git a/drivers/net/ethernet/freescale/fec_main.c
>b/drivers/net/ethernet/freescale/fec_main.c
>index 4cbebf3..29d8dc4 100644
>--- a/drivers/net/ethernet/freescale/fec_main.c
>+++ b/drivers/net/ethernet/freescale/fec_main.c
>@@ -385,7 +385,7 @@ fec_enet_start_xmit(struct sk_buff *skb, struct net_device
>*ndev)
> * data.
> */
> bdp->cbd_bufaddr = dma_map_single(&fep->pdev->dev, bufaddr,
>- FEC_ENET_TX_FRSIZE, DMA_TO_DEVICE);
>+ skb->len, DMA_TO_DEVICE);
> if (dma_mapping_error(&fep->pdev->dev, bdp->cbd_bufaddr)) {
> bdp->cbd_bufaddr = 0;
> fep->tx_skbuff[index] = NULL;
>@@ -779,11 +779,11 @@ fec_enet_tx(struct net_device *ndev)
> else
> index = bdp - fep->tx_bd_base;
>
>- dma_unmap_single(&fep->pdev->dev, bdp->cbd_bufaddr,
>- FEC_ENET_TX_FRSIZE, DMA_TO_DEVICE);
>+ skb = fep->tx_skbuff[index];
>+ dma_unmap_single(&fep->pdev->dev, bdp->cbd_bufaddr, skb->len,
>+ DMA_TO_DEVICE);
> bdp->cbd_bufaddr = 0;
>
>- skb = fep->tx_skbuff[index];
>
> /* Check for errors. */
> if (status & (BD_ENET_TX_HB | BD_ENET_TX_LC |
>--
>1.8.4.4
>
In fact, there have one memory copy for enet as below since enet have 16 bytes data buffer alignment request.
if (((unsigned long) bufaddr) & FEC_ALIGNMENT) {
memcpy(fep->tx_bounce[index], skb->data, skb->len);
bufaddr = fep->tx_bounce[index];
}
So, the bug you describe at commit log shouldn't exist.
Anyway, it is better to use the real packet size for the mapping.
I will do overnight stress test for the patch tomorrow.
Thanks,
Andy
^ permalink raw reply
* Re: netfilter: active obj WARN when cleaning up
From: Thomas Gleixner @ 2013-11-27 13:29 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Pablo Neira Ayuso, Sasha Levin, Patrick McHardy, kadlec,
David S. Miller, netfilter-devel, coreteam, netdev, LKML,
linux-mm, Andrew Morton, Christoph Lameter, Greg KH
In-Reply-To: <20131127113939.GL16735@n2100.arm.linux.org.uk>
On Wed, 27 Nov 2013, Russell King - ARM Linux wrote:
> On Wed, Nov 27, 2013 at 11:45:17AM +0100, Thomas Gleixner wrote:
> > On Wed, 27 Nov 2013, Pablo Neira Ayuso wrote:
> >
> > > On Tue, Nov 26, 2013 at 02:11:57PM -0500, Sasha Levin wrote:
> > > > Ping? I still see this warning.
> > >
> > > Did your test include patch 0c3c6c00c6?
> >
> > And how is that patch supposed to help?
> >
> > > > >[ 418.312449] WARNING: CPU: 6 PID: 4178 at lib/debugobjects.c:260 debug_print_object+0x8d/0xb0()
> > > > >[ 418.313243] ODEBUG: free active (active state 0) object type: timer_list hint:
> > > > >delayed_work_timer_fn+0x0/0x20
> >
> > > > >[ 418.321101] [<ffffffff812874d7>] kmem_cache_free+0x197/0x340
> > > > >[ 418.321101] [<ffffffff81249e76>] kmem_cache_destroy+0x86/0xe0
> > > > >[ 418.321101] [<ffffffff83d5d681>] nf_conntrack_cleanup_net_list+0x131/0x170
> >
> > The debug code detects an active timer, which itself is part of a
> > delayed work struct. The call comes from kmem_cache_destroy().
> >
> > kmem_cache_free(kmem_cache, s);
> >
> > So debug object says: s contains an active timer. s is the kmem_cache
> > which is destroyed from nf_conntrack_cleanup_net_list.
> >
> > Now struct kmem_cache has in case of SLUB:
> >
> > struct kobject kobj; /* For sysfs */
> >
> > and struct kobject has:
> >
> > #ifdef CONFIG_DEBUG_KOBJECT_RELEASE
> > struct delayed_work release;
> > #endif
> >
> > So this is the thing you want to look at:
> >
> > commit c817a67ec (kobject: delayed kobject release: help find buggy
> > drivers) added that delayed work thing.
> >
> > I fear that does not work for kobjects which are embedded into
> > something else.
>
> No, kobjects embedded into something else have their lifetime determined
> by the embedded kobject. That's rule #1 of kobjects - or rather reference
> counted objects.
>
> The point at which the kobject gets destructed is when the release function
> is called. If it is destructed before that time, that's a violation of
> the reference counted nature of kobjects, and that's what the delay on
> releasing is designed to catch.
>
> It's designed to catch code which does this exact path:
>
> put(obj)
> free(obj)
>
> rather than code which does it the right way:
>
> put(obj)
> -> refcount becomes 0
> -> release function gets called
> ->free(obj)
>
> The former is unsafe because obj may have other references.
Though the kobject is the only thing which has a delayed work embedded
inside struct kmem_cache. And the debug object splat points at the
kmem_cache_free() of the struct kmem_cache itself. That's why I
assumed the wreckage around that place. And indeed:
kmem_cache_destroy(s)
__kmem_cache_shutdown(s)
sysfs_slab_remove(s)
....
kobject_put(&s->kobj)
kref_put(&kobj->kref, kobject_release);
kobject_release(kref)
#ifdef CONFIG_DEBUG_KOBJECT_RELEASE
schedule_delayed_work(&kobj->release)
#else
kobject_cleanup(kobj)
#endif
So in the CONFIG_DEBUG_KOBJECT_RELEASE=y case, schedule_delayed_work()
_IS_ called which arms the timer. debugobjects catches the attempt to
free struct kmem_cache which contains the armed timer.
So much for rule #1
Thanks,
tglx
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: netfilter: active obj WARN when cleaning up
From: Russell King - ARM Linux @ 2013-11-27 13:32 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Pablo Neira Ayuso, Sasha Levin, Patrick McHardy, kadlec,
David S. Miller, netfilter-devel, coreteam, netdev, LKML,
linux-mm, Andrew Morton, Christoph Lameter, Greg KH
In-Reply-To: <alpine.DEB.2.02.1311271409280.30673@ionos.tec.linutronix.de>
On Wed, Nov 27, 2013 at 02:29:41PM +0100, Thomas Gleixner wrote:
> Though the kobject is the only thing which has a delayed work embedded
> inside struct kmem_cache. And the debug object splat points at the
> kmem_cache_free() of the struct kmem_cache itself. That's why I
> assumed the wreckage around that place. And indeed:
>
> kmem_cache_destroy(s)
> __kmem_cache_shutdown(s)
> sysfs_slab_remove(s)
> ....
> kobject_put(&s->kobj)
> kref_put(&kobj->kref, kobject_release);
> kobject_release(kref)
> #ifdef CONFIG_DEBUG_KOBJECT_RELEASE
> schedule_delayed_work(&kobj->release)
> #else
> kobject_cleanup(kobj)
> #endif
>
> So in the CONFIG_DEBUG_KOBJECT_RELEASE=y case, schedule_delayed_work()
> _IS_ called which arms the timer. debugobjects catches the attempt to
> free struct kmem_cache which contains the armed timer.
You fail to show where the free is in the above path.
^ permalink raw reply
* Re: [PATCH v2] net: smc91: fix crash regression on the versatile
From: Will Deacon @ 2013-11-27 13:35 UTC (permalink / raw)
To: Linus Walleij
Cc: netdev@vger.kernel.org, Nicolas Pitre, David S. Miller,
Russell King, Eric Miao, Jonathan Cameron, stable@vger.kernel.org
In-Reply-To: <1385555601-4497-1-git-send-email-linus.walleij@linaro.org>
Hi Linus,
On Wed, Nov 27, 2013 at 12:33:21PM +0000, Linus Walleij wrote:
> After commit e9e4ea74f06635f2ffc1dffe5ef40c854faa0a90
> "net: smc91x: dont't use SMC_outw for fixing up halfword-aligned data"
> The Versatile SMSC LAN91C111 is crashing like this:
[...]
> This is because the macro operations in smc91x.h defined
> for Versatile are missing SMC_outsw() as used in this
> commit.
>
> The Versatile needs and uses the same accessors as the other
> platforms in the first if(...) clause, just switch it to using
> that and we have one problem less to worry about.
[...]
> diff --git a/drivers/net/ethernet/smsc/smc91x.h b/drivers/net/ethernet/smsc/smc91x.h
> index c9d4c872e81d..902a7c268d82 100644
> --- a/drivers/net/ethernet/smsc/smc91x.h
> +++ b/drivers/net/ethernet/smsc/smc91x.h
> @@ -46,7 +46,8 @@
> defined(CONFIG_MACH_LITTLETON) ||\
> defined(CONFIG_MACH_ZYLONITE2) ||\
> defined(CONFIG_ARCH_VIPER) ||\
> - defined(CONFIG_MACH_STARGATE2)
> + defined(CONFIG_MACH_STARGATE2) ||\
> + defined(CONFIG_ARCH_VERSATILE)
>
> #include <asm/mach-types.h>
>
> @@ -206,23 +207,6 @@ SMC_outw(u16 val, void __iomem *ioaddr, int reg)
> #define RPC_LSA_DEFAULT RPC_LED_TX_RX
> #define RPC_LSB_DEFAULT RPC_LED_100_10
>
> -#elif defined(CONFIG_ARCH_VERSATILE)
> -
> -#define SMC_CAN_USE_8BIT 1
> -#define SMC_CAN_USE_16BIT 1
> -#define SMC_CAN_USE_32BIT 1
> -#define SMC_NOWAIT 1
> -
> -#define SMC_inb(a, r) readb((a) + (r))
> -#define SMC_inw(a, r) readw((a) + (r))
> -#define SMC_inl(a, r) readl((a) + (r))
> -#define SMC_outb(v, a, r) writeb(v, (a) + (r))
> -#define SMC_outw(v, a, r) writew(v, (a) + (r))
> -#define SMC_outl(v, a, r) writel(v, (a) + (r))
> -#define SMC_insl(a, r, p, l) readsl((a) + (r), p, l)
> -#define SMC_outsl(a, r, p, l) writesl((a) + (r), p, l)
> -#define SMC_IRQ_FLAGS (-1) /* from resource */
> -
> #elif defined(CONFIG_MN10300)
Ha! I was about to post a different fix for this issue, see below. I think
we may want a combination of the two...
Will
--->8
>From 2b4db3f7ddd560bc52be522098a96a6d18388f0f Mon Sep 17 00:00:00 2001
From: Will Deacon <will.deacon@arm.com>
Date: Wed, 27 Nov 2013 11:32:15 +0000
Subject: [PATCH] net: smc91x: ensure {in,out}sw are defined for platforms with
32-bit PIO
Commit e9e4ea74f066 ("net: smc91x: dont't use SMC_outw for fixing up
halfword-aligned data") introduced a requirement for all 32-bit PIO
capable platforms to implement the string variants for word accessors.
This patch adds the accessors to platforms where they are missing.
Reported-by: Russell King <linux@arm.linux.org.uk>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
drivers/net/ethernet/smsc/smc91x.h | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/net/ethernet/smsc/smc91x.h b/drivers/net/ethernet/smsc/smc91x.h
index c9d4c872e81d..cbe72022cb52 100644
--- a/drivers/net/ethernet/smsc/smc91x.h
+++ b/drivers/net/ethernet/smsc/smc91x.h
@@ -154,6 +154,8 @@ static inline void SMC_outw(u16 val, void __iomem *ioaddr, int reg)
#define SMC_outl(v, a, r) writel(v, (a) + (r))
#define SMC_insl(a, r, p, l) readsl((a) + (r), p, l)
#define SMC_outsl(a, r, p, l) writesl((a) + (r), p, l)
+#define SMC_insw(a, r, p, l) readsw((a) + (r), p, l)
+#define SMC_outsw(a, r, p, l) writesw((a) + (r), p, l)
#define SMC_IRQ_FLAGS (-1) /* from resource */
/* We actually can't write halfwords properly if not word aligned */
@@ -221,6 +223,8 @@ SMC_outw(u16 val, void __iomem *ioaddr, int reg)
#define SMC_outl(v, a, r) writel(v, (a) + (r))
#define SMC_insl(a, r, p, l) readsl((a) + (r), p, l)
#define SMC_outsl(a, r, p, l) writesl((a) + (r), p, l)
+#define SMC_insw(a, r, p, l) readsw((a) + (r), p, l)
+#define SMC_outsw(a, r, p, l) writesw((a) + (r), p, l)
#define SMC_IRQ_FLAGS (-1) /* from resource */
#elif defined(CONFIG_MN10300)
--
1.8.2.2
^ permalink raw reply related
* Re: [PATCH] net: fec_main: dma_map() only the length of the skb
From: Sebastian Andrzej Siewior @ 2013-11-27 13:38 UTC (permalink / raw)
To: Fugang Duan
Cc: netdev@vger.kernel.org, David S. Miller, Fabio Estevam, Frank Li,
Jim Baxter, Marek Szyprowski
In-Reply-To: <9848F2DB572E5649BA045B288BE08FBE01899801@039-SN2MPN1-023.039d.mgd.msft.net>
- <frank.li@freescale.net> because MTA complains
On 11/27/2013 02:08 PM, Fugang Duan wrote:
> In fact, there have one memory copy for enet as below since enet have 16 bytes data buffer alignment request.
> if (((unsigned long) bufaddr) & FEC_ALIGNMENT) {
> memcpy(fep->tx_bounce[index], skb->data, skb->len);
> bufaddr = fep->tx_bounce[index];
> }
This memcpy() is only executed if the buffer isn't properly aligned
which shouldn't be the rule but an exception. This aligment check is
also available in v2.6.31 where the BUG_ON() statement was triggered.
>
> So, the bug you describe at commit log shouldn't exist.
>
> Anyway, it is better to use the real packet size for the mapping.
>
>
> I will do overnight stress test for the patch tomorrow.
Thanks.
>
> Thanks,
> Andy
>
Sebastian
^ permalink raw reply
* Re: netfilter: active obj WARN when cleaning up
From: Russell King - ARM Linux @ 2013-11-27 13:40 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Pablo Neira Ayuso, Sasha Levin, Patrick McHardy, kadlec,
David S. Miller, netfilter-devel, coreteam, netdev, LKML,
linux-mm, Andrew Morton, Christoph Lameter, Greg KH
In-Reply-To: <20131127133231.GO16735@n2100.arm.linux.org.uk>
On Wed, Nov 27, 2013 at 01:32:31PM +0000, Russell King - ARM Linux wrote:
> On Wed, Nov 27, 2013 at 02:29:41PM +0100, Thomas Gleixner wrote:
> > Though the kobject is the only thing which has a delayed work embedded
> > inside struct kmem_cache. And the debug object splat points at the
> > kmem_cache_free() of the struct kmem_cache itself. That's why I
> > assumed the wreckage around that place. And indeed:
> >
> > kmem_cache_destroy(s)
> > __kmem_cache_shutdown(s)
> > sysfs_slab_remove(s)
> > ....
> > kobject_put(&s->kobj)
> > kref_put(&kobj->kref, kobject_release);
> > kobject_release(kref)
> > #ifdef CONFIG_DEBUG_KOBJECT_RELEASE
> > schedule_delayed_work(&kobj->release)
> > #else
> > kobject_cleanup(kobj)
> > #endif
> >
> > So in the CONFIG_DEBUG_KOBJECT_RELEASE=y case, schedule_delayed_work()
> > _IS_ called which arms the timer. debugobjects catches the attempt to
> > free struct kmem_cache which contains the armed timer.
>
> You fail to show where the free is in the above path.
Right, there's a kmem_cache_free(kmem_cache, s); by the slob code
after the above sequence, which is The Bug(tm).
As I said, a kobject has its own lifetime. If you embed that into
another structure, that structure inherits the lifetime of the kobject,
which is from the point at which it's created to the point at which the
kobject's release function is called.
So no, the code here is buggy. The kobject debugging has yet again
found a violation of the kobject lifetime rules. slub needs fixing.
^ permalink raw reply
* Re: netfilter: active obj WARN when cleaning up
From: Thomas Gleixner @ 2013-11-27 13:41 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Pablo Neira Ayuso, Sasha Levin, Patrick McHardy, kadlec,
David S. Miller, netfilter-devel, coreteam, netdev, LKML,
linux-mm, Andrew Morton, Christoph Lameter, Greg KH
In-Reply-To: <20131127133231.GO16735@n2100.arm.linux.org.uk>
On Wed, 27 Nov 2013, Russell King - ARM Linux wrote:
> On Wed, Nov 27, 2013 at 02:29:41PM +0100, Thomas Gleixner wrote:
> > Though the kobject is the only thing which has a delayed work embedded
> > inside struct kmem_cache. And the debug object splat points at the
> > kmem_cache_free() of the struct kmem_cache itself. That's why I
> > assumed the wreckage around that place. And indeed:
> >
> > kmem_cache_destroy(s)
> > __kmem_cache_shutdown(s)
{
> > sysfs_slab_remove(s)
> > ....
> > kobject_put(&s->kobj)
> > kref_put(&kobj->kref, kobject_release);
> > kobject_release(kref)
> > #ifdef CONFIG_DEBUG_KOBJECT_RELEASE
> > schedule_delayed_work(&kobj->release)
> > #else
> > kobject_cleanup(kobj)
> > #endif
}
kmem_cache_free(s);
> >
> > So in the CONFIG_DEBUG_KOBJECT_RELEASE=y case, schedule_delayed_work()
> > _IS_ called which arms the timer. debugobjects catches the attempt to
> > free struct kmem_cache which contains the armed timer.
>
> You fail to show where the free is in the above path.
So, yes. it's the issue you are trying to catch, but that code in
question made already sure, that there are no references held, because
it detached itself from sysfs before calling kobject_put().
Thanks,
tglx
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: netfilter: active obj WARN when cleaning up
From: Thomas Gleixner @ 2013-11-27 13:44 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Pablo Neira Ayuso, Sasha Levin, Patrick McHardy, kadlec,
David S. Miller, netfilter-devel, coreteam, netdev, LKML,
linux-mm, Andrew Morton, Christoph Lameter, Greg KH
In-Reply-To: <20131127134015.GA6011@n2100.arm.linux.org.uk>
On Wed, 27 Nov 2013, Russell King - ARM Linux wrote:
> On Wed, Nov 27, 2013 at 01:32:31PM +0000, Russell King - ARM Linux wrote:
> > On Wed, Nov 27, 2013 at 02:29:41PM +0100, Thomas Gleixner wrote:
> > > Though the kobject is the only thing which has a delayed work embedded
> > > inside struct kmem_cache. And the debug object splat points at the
> > > kmem_cache_free() of the struct kmem_cache itself. That's why I
> > > assumed the wreckage around that place. And indeed:
> > >
> > > kmem_cache_destroy(s)
> > > __kmem_cache_shutdown(s)
> > > sysfs_slab_remove(s)
> > > ....
> > > kobject_put(&s->kobj)
> > > kref_put(&kobj->kref, kobject_release);
> > > kobject_release(kref)
> > > #ifdef CONFIG_DEBUG_KOBJECT_RELEASE
> > > schedule_delayed_work(&kobj->release)
> > > #else
> > > kobject_cleanup(kobj)
> > > #endif
> > >
> > > So in the CONFIG_DEBUG_KOBJECT_RELEASE=y case, schedule_delayed_work()
> > > _IS_ called which arms the timer. debugobjects catches the attempt to
> > > free struct kmem_cache which contains the armed timer.
> >
> > You fail to show where the free is in the above path.
>
> Right, there's a kmem_cache_free(kmem_cache, s); by the slob code
> after the above sequence, which is The Bug(tm).
>
> As I said, a kobject has its own lifetime. If you embed that into
> another structure, that structure inherits the lifetime of the kobject,
> which is from the point at which it's created to the point at which the
> kobject's release function is called.
>
> So no, the code here is buggy. The kobject debugging has yet again
> found a violation of the kobject lifetime rules. slub needs fixing.
I leave that discussion to you, greg and the slub folks.
/me prepares deck chair, drinks and popcorn
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* RE: [patch v2] net: heap overflow in __audit_sockaddr()
From: David Laight @ 2013-11-27 13:56 UTC (permalink / raw)
To: Eric Wong, Dan Carpenter
Cc: David S. Miller, netdev, security, Jüri Aedla, stable
In-Reply-To: <20131127113218.GB1612@dcvr.yhbt.net>
> From: Eric Wong
> Dan Carpenter <dan.carpenter@oracle.com> wrote:
> > --- a/net/socket.c
> > +++ b/net/socket.c
> > @@ -1964,6 +1964,16 @@ struct used_address {
> > unsigned int name_len;
> > };
> >
> > +static int copy_msghdr_from_user(struct msghdr *kmsg,
> > + struct msghdr __user *umsg)
> > +{
> > + if (copy_from_user(kmsg, umsg, sizeof(struct msghdr)))
> > + return -EFAULT;
> > + if (kmsg->msg_namelen > sizeof(struct sockaddr_storage))
> > + return -EINVAL;
> > + return 0;
>
> Crap, this seems to break Ruby trunk :x
> https://bugs.ruby-lang.org/issues/9124
>
> I'm inclined to think Ruby is wrong to use a gigantic buffer, but this
> may also break some other existing userspace code. I'm not sure what
> the best option since breaking userspace (even buggy userspace?) is not
> taken lightly.
Well, 'struct sockaddr_storage' is a horrid item.
I think there are people who'll say that it should never be used
to allocate an actual buffer.
If the kernel is going to write an address into the buffer, it only
needs a buffer that is long enough.
If the kernel is going to read an address, the length needs to be
appropriate for the actual protocol.
Very portable code, and code that wants to be 'future proof' against
new protocols might well use a long buffer.
David
^ permalink raw reply
* Re: Kernel Panic in IPv6 vxlan
From: Ryan Whelan @ 2013-11-27 14:44 UTC (permalink / raw)
To: Fan Du; +Cc: netdev, amwang
In-Reply-To: <5295AC7A.8020502@windriver.com>
That isn't required with IPv4- if you don't specify an interface with
IPv4, it looks like the system will route the multicast traffic via
routing. Is this not an option with IPv6? I might be
misunderstanding something?
thanks
On Wed, Nov 27, 2013 at 3:25 AM, Fan Du <fan.du@windriver.com> wrote:
> Hallo,
>
>
> On 2013年11月27日 05:41, Ryan Whelan wrote:
>>
>> I'm not sure if this is the right place to report this- please correct
>> me if I'm mistaken.
>>
>> When creating a vxlan interface with an IPv6 multicast group but not
>> specifying an underlying device, the kernel panics when the vxlan
>> interface is brought up. This doesn't happen with an IPv4 multicast
>> group. Linux 3.12.1 x86_64
>>
>> `ip link add vxlan0 type vxlan id 42 group ff0e::110`
>
>
> You forgot to attach vxlan0 to a physical netdev ;) try:
>
> ip link add vxlan0 type vxlan id 42 group ff0e::110 dev ethX
>
> This will work anyway.
>
> --
> 浮沉随浪只记今朝笑
>
> --fan fan
^ permalink raw reply
* Re: [PATCH net] af_packet: use spin_lock_bh() for sk_buff_head
From: Eric Dumazet @ 2013-11-27 16:17 UTC (permalink / raw)
To: Veaceslav Falico
Cc: netdev, jstancek, David S. Miller, Daniel Borkmann,
Willem de Bruijn, Phil Sutter, Eric Dumazet
In-Reply-To: <1385557075-8896-1-git-send-email-vfalico@redhat.com>
On Wed, 2013-11-27 at 13:57 +0100, Veaceslav Falico wrote:
> While running a debug kernel, a warning of inconsistent lock state showed
> up for sk->sk_receive_queue->lock between packet_set_ring() (actually,
> static prb_shutdown_retire_blk_timer(), which does spin_lock()) and
> sock_queue_rcv_skb(), which does spin_lock_irqsave().
>
> Fix this by converting the spin_lock() used in
> prb_shutdown_retire_blk_timer() to spin_lock_bh(), as it is used in
> packet_set_ring().
Patch is good, but description is slightly misleading.
Problem is not because sock_queue_rcv_skb() uses spin_lock_irqsave().
Problem is prb_retire_rx_blk_timer_expired() is called from softirq
context.
So if we do not block BH in prb_shutdown_retire_blk_timer() :
spin_lock(&rb_queue->lock);
pkc->delete_blk_timer = 1;
spin_unlock(&rb_queue->lock);
Timer could fire right in the middle and would spin forever trying
to acquire the same lock.
Using spin_lock_bh() prevents timer being fired on this cpu.
Thanks !
^ permalink raw reply
* [PATCH 1/2] virtio_net: fix error handling for mergeable buffers
From: Michael S. Tsirkin @ 2013-11-27 16:31 UTC (permalink / raw)
To: linux-kernel; +Cc: Michael Dalton, netdev, virtualization, Eric Dumazet
Eric Dumazet noticed that if we encounter an error
when processing a mergeable buffer, we don't
dequeue all of the buffers from this packet,
the result is almost sure to be loss of networking.
Jason Wang noticed that we also leak a page and that we don't decrement
the rq buf count, so we won't repost buffers (a resource leak).
Fix both issues, and also make the logic a bit more
robust against device errors by not looping when e.g. because of a leak
like the one we are fixing here the number of buffers is 0.
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: Michael Dalton <mwdalton@google.com>
Reported-by: Eric Dumazet <edumazet@google.com>
Reported-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
Note: this bugfix is needed on stable too, but backport
might not be trivial.
I'll send a backport for stable separately.
drivers/net/virtio_net.c | 84 ++++++++++++++++++++++++++++++------------------
1 file changed, 52 insertions(+), 32 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 7bab4de..0e6ea69 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -299,41 +299,53 @@ static struct sk_buff *page_to_skb(struct receive_queue *rq,
return skb;
}
-static int receive_mergeable(struct receive_queue *rq, struct sk_buff *head_skb)
+static struct sk_buff *receive_mergeable(struct net_device *dev,
+ struct receive_queue *rq,
+ void *buf,
+ unsigned int len)
{
- struct skb_vnet_hdr *hdr = skb_vnet_hdr(head_skb);
+ struct skb_vnet_hdr *hdr = buf;
+ int num_buf = hdr->mhdr.num_buffers;
+ struct page *page = virt_to_head_page(buf);
+ int offset = buf - page_address(page);
+ struct sk_buff *head_skb = page_to_skb(rq, page, offset, len,
+ MERGE_BUFFER_LEN);
struct sk_buff *curr_skb = head_skb;
- char *buf;
- struct page *page;
- int num_buf, len, offset;
- num_buf = hdr->mhdr.num_buffers;
+ if (unlikely(!curr_skb))
+ goto err_skb;
+
while (--num_buf) {
- int num_skb_frags = skb_shinfo(curr_skb)->nr_frags;
+ int num_skb_frags;
+
buf = virtqueue_get_buf(rq->vq, &len);
if (unlikely(!buf)) {
- pr_debug("%s: rx error: %d buffers missing\n",
- head_skb->dev->name, hdr->mhdr.num_buffers);
- head_skb->dev->stats.rx_length_errors++;
- return -EINVAL;
+ pr_debug("%s: rx error: %d buffers out of %d missing\n",
+ dev->name, num_buf, hdr->mhdr.num_buffers);
+ dev->stats.rx_length_errors++;
+ goto err_buf;
}
if (unlikely(len > MERGE_BUFFER_LEN)) {
pr_debug("%s: rx error: merge buffer too long\n",
- head_skb->dev->name);
+ dev->name);
len = MERGE_BUFFER_LEN;
}
+
+ page = virt_to_head_page(buf);
+ --rq->num;
+
+ num_skb_frags = skb_shinfo(curr_skb)->nr_frags;
if (unlikely(num_skb_frags == MAX_SKB_FRAGS)) {
struct sk_buff *nskb = alloc_skb(0, GFP_ATOMIC);
- if (unlikely(!nskb)) {
- head_skb->dev->stats.rx_dropped++;
- return -ENOMEM;
- }
+
+ if (unlikely(!nskb))
+ goto err_skb;
if (curr_skb == head_skb)
skb_shinfo(curr_skb)->frag_list = nskb;
else
curr_skb->next = nskb;
- curr_skb = nskb;
head_skb->truesize += nskb->truesize;
+ curr_skb = nskb;
num_skb_frags = 0;
}
if (curr_skb != head_skb) {
@@ -341,8 +353,7 @@ static int receive_mergeable(struct receive_queue *rq, struct sk_buff *head_skb)
head_skb->len += len;
head_skb->truesize += MERGE_BUFFER_LEN;
}
- page = virt_to_head_page(buf);
- offset = buf - (char *)page_address(page);
+ offset = buf - page_address(page);
if (skb_can_coalesce(curr_skb, num_skb_frags, page, offset)) {
put_page(page);
skb_coalesce_rx_frag(curr_skb, num_skb_frags - 1,
@@ -351,9 +362,28 @@ static int receive_mergeable(struct receive_queue *rq, struct sk_buff *head_skb)
skb_add_rx_frag(curr_skb, num_skb_frags, page,
offset, len, MERGE_BUFFER_LEN);
}
+ }
+
+ return head_skb;
+
+err_skb:
+ put_page(page);
+err_buf:
+ dev->stats.rx_dropped++;
+ dev_kfree_skb(head_skb);
+ while (--num_buf) {
+ buf = virtqueue_get_buf(rq->vq, &len);
+ if (unlikely(!buf)) {
+ pr_debug("%s: rx error: %d buffers missing\n",
+ dev->name, num_buf);
+ dev->stats.rx_length_errors++;
+ break;
+ }
+ page = virt_to_head_page(buf);
+ put_page(page);
--rq->num;
}
- return 0;
+ return NULL;
}
static void receive_buf(struct receive_queue *rq, void *buf, unsigned int len)
@@ -382,19 +412,9 @@ static void receive_buf(struct receive_queue *rq, void *buf, unsigned int len)
len -= sizeof(struct virtio_net_hdr);
skb_trim(skb, len);
} else if (vi->mergeable_rx_bufs) {
- struct page *page = virt_to_head_page(buf);
- skb = page_to_skb(rq, page,
- (char *)buf - (char *)page_address(page),
- len, MERGE_BUFFER_LEN);
- if (unlikely(!skb)) {
- dev->stats.rx_dropped++;
- put_page(page);
+ skb = receive_mergeable(dev, rq, buf, len);
+ if (unlikely(!skb))
return;
- }
- if (receive_mergeable(rq, skb)) {
- dev_kfree_skb(skb);
- return;
- }
} else {
page = buf;
skb = page_to_skb(rq, page, 0, len, PAGE_SIZE);
--
MST
^ permalink raw reply related
* [PATCH 2/2] virtio-net: make all RX paths handle erors consistently
From: Michael S. Tsirkin @ 2013-11-27 16:31 UTC (permalink / raw)
To: linux-kernel; +Cc: Michael Dalton, netdev, virtualization, Eric Dumazet
In-Reply-To: <1385569684-26595-1-git-send-email-mst@redhat.com>
receive mergeable now handles errors internally.
Do same for big and small packet paths, otherwise
the logic is too hard to follow.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
While I can't point at a bug this fixes, I'm not sure
there's no bug in the existing logic.
So not exactly a bug fix bug I think it's justified for net.
drivers/net/virtio_net.c | 53 +++++++++++++++++++++++++++++++++---------------
1 file changed, 37 insertions(+), 16 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 0e6ea69..97c6212 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -299,6 +299,35 @@ static struct sk_buff *page_to_skb(struct receive_queue *rq,
return skb;
}
+static struct sk_buff *receive_small(void *buf, unsigned int len)
+{
+ struct sk_buff * skb = buf;
+
+ len -= sizeof(struct virtio_net_hdr);
+ skb_trim(skb, len);
+
+ return skb;
+}
+
+static struct sk_buff *receive_big(struct net_device *dev,
+ struct receive_queue *rq,
+ void *buf,
+ unsigned int len)
+{
+ struct page *page = buf;
+ struct sk_buff *skb = page_to_skb(rq, page, 0, len, PAGE_SIZE);
+
+ if (unlikely(!skb))
+ goto err;
+
+ return skb;
+
+err:
+ dev->stats.rx_dropped++;
+ give_pages(rq, page);
+ return NULL;
+}
+
static struct sk_buff *receive_mergeable(struct net_device *dev,
struct receive_queue *rq,
void *buf,
@@ -407,23 +436,15 @@ static void receive_buf(struct receive_queue *rq, void *buf, unsigned int len)
return;
}
- if (!vi->mergeable_rx_bufs && !vi->big_packets) {
- skb = buf;
- len -= sizeof(struct virtio_net_hdr);
- skb_trim(skb, len);
- } else if (vi->mergeable_rx_bufs) {
+ if (vi->mergeable_rx_bufs)
skb = receive_mergeable(dev, rq, buf, len);
- if (unlikely(!skb))
- return;
- } else {
- page = buf;
- skb = page_to_skb(rq, page, 0, len, PAGE_SIZE);
- if (unlikely(!skb)) {
- dev->stats.rx_dropped++;
- give_pages(rq, page);
- return;
- }
- }
+ else if (vi->big_packets)
+ skb = receive_big(dev, rq, buf, len);
+ else
+ skb = receive_small(buf, len);
+
+ if (unlikely(!skb))
+ return;
hdr = skb_vnet_hdr(skb);
--
MST
^ permalink raw reply related
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