* [PATCH] net: stmmac: mark PM functions as __maybe_unused
From: Arnd Bergmann @ 2018-08-13 21:50 UTC (permalink / raw)
To: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, David S. Miller
Cc: Arnd Bergmann, netdev, linux-kernel
The newly added suspend/resume functions cause a build warning
when CONFIG_PM is disabled:
drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c:324:12: error: 'stmmac_pci_resume' defined but not used [-Werror=unused-function]
drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c:306:12: error: 'stmmac_pci_suspend' defined but not used [-Werror=unused-function]
Mark them as __maybe_unused so gcc can drop them silently.
Fixes: b7d0f08e9129 ("net: stmmac: Fix WoL for PCI-based setups")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
index 6a393b16a1fc..c54a50dbd5ac 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
@@ -303,7 +303,7 @@ static void stmmac_pci_remove(struct pci_dev *pdev)
pci_disable_device(pdev);
}
-static int stmmac_pci_suspend(struct device *dev)
+static int __maybe_unused stmmac_pci_suspend(struct device *dev)
{
struct pci_dev *pdev = to_pci_dev(dev);
int ret;
@@ -321,7 +321,7 @@ static int stmmac_pci_suspend(struct device *dev)
return 0;
}
-static int stmmac_pci_resume(struct device *dev)
+static int __maybe_unused stmmac_pci_resume(struct device *dev)
{
struct pci_dev *pdev = to_pci_dev(dev);
int ret;
--
2.18.0
^ permalink raw reply related
* WARNING: refcount bug in sctp_transport_put
From: syzbot @ 2018-08-14 0:55 UTC (permalink / raw)
To: davem, linux-kernel, linux-sctp, marcelo.leitner, netdev, nhorman,
syzkaller-bugs, vyasevich
Hello,
syzbot found the following crash on:
HEAD commit: 112cbae26d18 Merge branch 'linus' of git://git.kernel.org/..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=12401622400000
kernel config: https://syzkaller.appspot.com/x/.config?x=152cb8ccd35b1f70
dashboard link: https://syzkaller.appspot.com/bug?extid=6a15c8ad0f0632ccd7f3
compiler: gcc (GCC) 8.0.1 20180413 (experimental)
Unfortunately, I don't have any reproducer for this crash yet.
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+6a15c8ad0f0632ccd7f3@syzkaller.appspotmail.com
------------[ cut here ]------------
refcount_t: underflow; use-after-free.
WARNING: CPU: 1 PID: 7203 at lib/refcount.c:187
refcount_sub_and_test+0x2e7/0x350 lib/refcount.c:187
Kernel panic - not syncing: panic_on_warn set ...
CPU: 1 PID: 7203 Comm: udevd Not tainted 4.18.0-rc8+ #182
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Call Trace:
<IRQ>
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x1c9/0x2b4 lib/dump_stack.c:113
panic+0x238/0x4e7 kernel/panic.c:184
__warn.cold.8+0x163/0x1ba kernel/panic.c:536
report_bug+0x252/0x2d0 lib/bug.c:186
fixup_bug arch/x86/kernel/traps.c:178 [inline]
do_error_trap+0x1fc/0x4d0 arch/x86/kernel/traps.c:296
do_invalid_op+0x1b/0x20 arch/x86/kernel/traps.c:316
invalid_op+0x14/0x20 arch/x86/entry/entry_64.S:992
RIP: 0010:refcount_sub_and_test+0x2e7/0x350 lib/refcount.c:187
Code: 89 de e8 fc b4 1c fe 84 db 74 07 31 db e9 46 ff ff ff e8 1c b4 1c fe
48 c7 c7 80 48 3a 87 c6 05 82 f2 25 05 01 e8 f9 cc e7 fd <0f> 0b 31 db e9
25 ff ff ff 48 8b bd 28 ff ff ff 89 85 34 ff ff ff
RSP: 0018:ffff8801db107598 EFLAGS: 00010286
RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000
RDX: 0000000000000100 RSI: ffffffff81632481 RDI: ffff8801db107270
RBP: ffff8801db107680 R08: ffff8801b9a22780 R09: 0000000000000002
R10: ffff8801b9a22780 R11: 0000000000000000 R12: 00000000ffffffff
R13: ffff8801db107658 R14: 0000000000000001 R15: ffff8801aefee088
refcount_dec_and_test+0x1a/0x20 lib/refcount.c:212
sctp_transport_put+0x76/0x200 net/sctp/transport.c:331
sctp_generate_heartbeat_event+0x2d7/0x450 net/sctp/sm_sideeffect.c:416
call_timer_fn+0x242/0x970 kernel/time/timer.c:1326
expire_timers kernel/time/timer.c:1363 [inline]
__run_timers+0x7a6/0xc70 kernel/time/timer.c:1666
run_timer_softirq+0x4c/0x70 kernel/time/timer.c:1692
__do_softirq+0x2e8/0xb17 kernel/softirq.c:292
invoke_softirq kernel/softirq.c:372 [inline]
irq_exit+0x1d4/0x210 kernel/softirq.c:412
exiting_irq arch/x86/include/asm/apic.h:527 [inline]
smp_apic_timer_interrupt+0x186/0x730 arch/x86/kernel/apic/apic.c:1055
apic_timer_interrupt+0xf/0x20 arch/x86/entry/entry_64.S:863
</IRQ>
RIP: 0010:update_stack_state+0xcc/0x690 arch/x86/kernel/unwind_frame.c:215
Code: 06 49 8d 7e 50 48 89 fa 89 85 24 ff ff ff 48 c1 ea 03 48 b8 00 00 00
00 00 fc ff df 80 3c 02 00 0f 85 04 05 00 00 49 8b 46 50 <48> 85 c0 0f 84
66 03 00 00 48 05 a8 00 00 00 48 89 85 10 ff ff ff
RSP: 0018:ffff88018d816a60 EFLAGS: 00000246 ORIG_RAX: ffffffffffffff13
RAX: 0000000000000000 RBX: ffff88018d816cc8 RCX: ffff88018d816c40
RDX: 1ffff10031b02da3 RSI: ffff88018d816d48 RDI: ffff88018d816d18
RBP: ffff88018d816b68 R08: ffff88018d816d00 R09: ffff8801b9a22780
R10: ffffed0031b02da3 R11: ffff88018d816d1f R12: 1ffff10031b02d54
R13: ffff88018d816d48 R14: ffff88018d816cc8 R15: ffff88018d816d18
unwind_next_frame.part.7+0x1ae/0x9e0 arch/x86/kernel/unwind_frame.c:329
unwind_next_frame arch/x86/include/asm/unwind.h:40 [inline]
__unwind_start+0x166/0x330 arch/x86/kernel/unwind_frame.c:414
unwind_start arch/x86/include/asm/unwind.h:54 [inline]
__save_stack_trace+0x59/0xf0 arch/x86/kernel/stacktrace.c:43
save_stack_trace+0x1a/0x20 arch/x86/kernel/stacktrace.c:60
save_stack+0x43/0xd0 mm/kasan/kasan.c:448
set_track mm/kasan/kasan.c:460 [inline]
kasan_kmalloc+0xc4/0xe0 mm/kasan/kasan.c:553
kasan_slab_alloc+0x12/0x20 mm/kasan/kasan.c:490
slab_post_alloc_hook mm/slab.h:444 [inline]
slab_alloc mm/slab.c:3392 [inline]
kmem_cache_alloc+0x11b/0x760 mm/slab.c:3552
anon_vma_chain_alloc mm/rmap.c:129 [inline]
anon_vma_clone+0x140/0x740 mm/rmap.c:269
anon_vma_fork+0xf0/0x960 mm/rmap.c:332
dup_mmap kernel/fork.c:498 [inline]
dup_mm kernel/fork.c:1266 [inline]
copy_mm kernel/fork.c:1320 [inline]
copy_process.part.39+0x4e53/0x70b0 kernel/fork.c:1826
copy_process kernel/fork.c:1639 [inline]
_do_fork+0x291/0x12a0 kernel/fork.c:2122
__do_sys_clone kernel/fork.c:2229 [inline]
__se_sys_clone kernel/fork.c:2223 [inline]
__x64_sys_clone+0xbf/0x150 kernel/fork.c:2223
do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x7f1b0bf66f46
Code: f7 d8 64 89 04 25 d4 02 00 00 64 4c 8b 14 25 10 00 00 00 31 d2 49 81
c2 d0 02 00 00 31 f6 bf 11 00 20 01 b8 38 00 00 00 0f 05 <48> 3d 00 f0 ff
ff 0f 87 31 01 00 00 85 c0 41 89 c4 0f 85 3b 01 00
RSP: 002b:00007ffc9eb9ddb0 EFLAGS: 00000246 ORIG_RAX: 0000000000000038
RAX: ffffffffffffffda RBX: 00007ffc9eb9ddb0 RCX: 00007f1b0bf66f46
RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000001200011
RBP: 00007ffc9eb9de10 R08: 0000000000001c23 R09: 0000000000001c23
R10: 00007f1b0c883a70 R11: 0000000000000246 R12: 0000000000000000
R13: 00007ffc9eb9ddd0 R14: 0000000000000005 R15: 0000000000000005
Dumping ftrace buffer:
(ftrace buffer empty)
Kernel Offset: disabled
Rebooting in 86400 seconds..
---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with
syzbot.
^ permalink raw reply
* Re: [PATCH] net: Change the layout of structure trace_event_raw_fib_table_lookup
From: Zong Li @ 2018-08-14 1:13 UTC (permalink / raw)
To: David Ahern
Cc: Zong Li, Steven Rostedt, mingo, netdev, Linux Kernel Mailing List,
greentime
In-Reply-To: <e9697bf5-11eb-68ec-238f-aedb724c8c17@gmail.com>
David Ahern <dsahern@gmail.com> 於 2018年8月13日 週一 下午10:40寫道：
>
> On 8/12/18 8:26 PM, Zong Li wrote:
> > There is an unalignment access about the structure
> > 'trace_event_raw_fib_table_lookup'.
> >
> > In include/trace/events/fib.h, there is a memory operation which casting
> > the 'src' data member to a pointer, and then store a value to this
> > pointer point to.
> >
> > p32 = (__be32 *) __entry->src;
> > *p32 = flp->saddr;
> >
> > The offset of 'src' in structure trace_event_raw_fib_table_lookup is not
> > four bytes alignment. On some architectures, they don't permit the
> > unalignment access, it need to pay the price to handle this situation in
> > exception handler.
> >
> > Adjust the layout of structure to avoid this case.
> >
> > Signed-off-by: Zong Li <zong@andestech.com>
> > ---
> > include/trace/events/fib.h | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
>
> Fixes: 9f323973c915 ("net/ipv4: Udate fib_table_lookup tracepoint")
> Acked-by: David Ahern <dsahern@gmail.com>
>
> Do you also need a similar change to fib6_table_lookup in
> include/trace/events/fib6.h?
Hi David,
Though I don't encounter difficulties on it for now, but I think it
has the same situation,
How do you think that also change the layout in include/trace/events/fib6.h?
^ permalink raw reply
* [RFC/PATCH]] net: nixge: Add support for 64-bit ZynqMP platform
From: Moritz Fischer @ 2018-08-13 22:24 UTC (permalink / raw)
To: davem
Cc: keescook, netdev, alex.williams, moritz.fischer, Moritz Fischer,
Moritz Fischer, Florian Fainelli
Add support for 64-bit (ZynqMP) platform to driver.
The hardware only supports 32-bit register accesses
so the accesses need to be split up into two writes
when setting the current and tail descriptor values.
Signed-off-by: Moritz Fischer <moritz@ettus.com>
Cc: Florian Fainelli <f.fainelli@gmail.com>
---
Hi all,
I'm working on making this work for X86_64, too (in fact I tested
a version where I got rid of the phy/mdio code on X86_64 over
PCIe) which will come as a follow up patch once I got the details
worked out. in the PCIe case this will be platform_device subdevice
of another device. I digress ...
For the generic case would my depends on look like
depends on HAS_MMIO && HAS_DMA ?
Am I lacking a depends on OF_MDIO?
This patch also intoduces a warning where with netdev_err()
in the IRQ handlers I print the address which for the 64bit case
is %llx vs %x for the 32-bit case.
I know %pad is the right format string to print dma_addr_t, but
that seemed to generate a warning, too. Ideas?
Thanks for your feedback,
Moritz
---
drivers/net/ethernet/ni/Kconfig | 2 +-
drivers/net/ethernet/ni/nixge.c | 170 ++++++++++++++++++++++----------
2 files changed, 117 insertions(+), 55 deletions(-)
diff --git a/drivers/net/ethernet/ni/Kconfig b/drivers/net/ethernet/ni/Kconfig
index aa41e5f6e437..bc6b29c7b0c1 100644
--- a/drivers/net/ethernet/ni/Kconfig
+++ b/drivers/net/ethernet/ni/Kconfig
@@ -18,7 +18,7 @@ if NET_VENDOR_NI
config NI_XGE_MANAGEMENT_ENET
tristate "National Instruments XGE management enet support"
- depends on ARCH_ZYNQ
+ depends on ARCH_ZYNQ || ARCH_ZYNQMP
select PHYLIB
help
Simple LAN device for debug or management purposes. Can
diff --git a/drivers/net/ethernet/ni/nixge.c b/drivers/net/ethernet/ni/nixge.c
index 76efed058f33..352d4a52191a 100644
--- a/drivers/net/ethernet/ni/nixge.c
+++ b/drivers/net/ethernet/ni/nixge.c
@@ -106,10 +106,10 @@
(NIXGE_JUMBO_MTU + NIXGE_HDR_SIZE + NIXGE_TRL_SIZE)
struct nixge_hw_dma_bd {
- u32 next;
- u32 reserved1;
- u32 phys;
- u32 reserved2;
+ u32 next_lo;
+ u32 next_hi;
+ u32 phys_lo;
+ u32 phys_hi;
u32 reserved3;
u32 reserved4;
u32 cntrl;
@@ -119,11 +119,39 @@ struct nixge_hw_dma_bd {
u32 app2;
u32 app3;
u32 app4;
- u32 sw_id_offset;
- u32 reserved5;
+ u32 sw_id_offset_lo;
+ u32 sw_id_offset_hi;
u32 reserved6;
};
+#ifdef CONFIG_PHYS_ADDR_T_64BIT
+#define nixge_hw_dma_bd_set_addr(bd, field, addr) \
+ do { \
+ (bd)->field##_lo = lower_32_bits(((u64)addr)); \
+ (bd)->field##_hi = upper_32_bits(((u64)addr)); \
+ } while (0)
+#else
+#define nixge_hw_dma_bd_set_addr(bd, field, addr) \
+ ((bd)->field##_lo = lower_32_bits((addr)))
+#endif
+
+#define nixge_hw_dma_bd_set_phys(bd, addr) \
+ nixge_hw_dma_bd_set_addr((bd), phys, (addr))
+
+#define nixge_hw_dma_bd_set_next(bd, addr) \
+ nixge_hw_dma_bd_set_addr((bd), next, (addr))
+
+#define nixge_hw_dma_bd_set_offset(bd, addr) \
+ nixge_hw_dma_bd_set_addr((bd), sw_id_offset, (addr))
+
+#ifdef CONFIG_PHYS_ADDR_T_64BIT
+#define nixge_hw_dma_bd_get_addr(bd, field) \
+ (dma_addr_t)((((u64)(bd)->field##_hi) << 32) | ((bd)->field##_lo))
+#else
+#define nixge_hw_dma_bd_get_addr(bd, field) \
+ (dma_addr_t)((bd)->field##_lo)
+#endif
+
struct nixge_tx_skb {
struct sk_buff *skb;
dma_addr_t mapping;
@@ -176,6 +204,15 @@ static void nixge_dma_write_reg(struct nixge_priv *priv, off_t offset, u32 val)
writel(val, priv->dma_regs + offset);
}
+static void nixge_dma_write_desc_reg(struct nixge_priv *priv, off_t offset,
+ dma_addr_t addr)
+{
+ writel(lower_32_bits(addr), priv->dma_regs + offset);
+#ifdef CONFIG_PHYS_ADDR_T_64BIT
+ writel(upper_32_bits(addr), priv->dma_regs + offset + 4);
+#endif
+}
+
static u32 nixge_dma_read_reg(const struct nixge_priv *priv, off_t offset)
{
return readl(priv->dma_regs + offset);
@@ -202,13 +239,22 @@ static u32 nixge_ctrl_read_reg(struct nixge_priv *priv, off_t offset)
static void nixge_hw_dma_bd_release(struct net_device *ndev)
{
struct nixge_priv *priv = netdev_priv(ndev);
+ dma_addr_t phys_addr;
+ struct sk_buff *skb;
int i;
for (i = 0; i < RX_BD_NUM; i++) {
- dma_unmap_single(ndev->dev.parent, priv->rx_bd_v[i].phys,
- NIXGE_MAX_JUMBO_FRAME_SIZE, DMA_FROM_DEVICE);
- dev_kfree_skb((struct sk_buff *)
- (priv->rx_bd_v[i].sw_id_offset));
+ phys_addr = nixge_hw_dma_bd_get_addr(&priv->rx_bd_v[i],
+ phys);
+
+ dma_unmap_single(ndev->dev.parent, phys_addr,
+ NIXGE_MAX_JUMBO_FRAME_SIZE,
+ DMA_FROM_DEVICE);
+
+ skb = (struct sk_buff *)
+ nixge_hw_dma_bd_get_addr(&priv->rx_bd_v[i],
+ sw_id_offset);
+ dev_kfree_skb(skb);
}
if (priv->rx_bd_v)
@@ -231,6 +277,7 @@ static int nixge_hw_dma_bd_init(struct net_device *ndev)
{
struct nixge_priv *priv = netdev_priv(ndev);
struct sk_buff *skb;
+ dma_addr_t phys;
u32 cr;
int i;
@@ -259,27 +306,30 @@ static int nixge_hw_dma_bd_init(struct net_device *ndev)
goto out;
for (i = 0; i < TX_BD_NUM; i++) {
- priv->tx_bd_v[i].next = priv->tx_bd_p +
- sizeof(*priv->tx_bd_v) *
- ((i + 1) % TX_BD_NUM);
+ nixge_hw_dma_bd_set_next(&priv->tx_bd_v[i],
+ priv->tx_bd_p +
+ sizeof(*priv->tx_bd_v) *
+ ((i + 1) % TX_BD_NUM));
}
for (i = 0; i < RX_BD_NUM; i++) {
- priv->rx_bd_v[i].next = priv->rx_bd_p +
- sizeof(*priv->rx_bd_v) *
- ((i + 1) % RX_BD_NUM);
+ nixge_hw_dma_bd_set_next(&priv->rx_bd_v[i],
+ priv->rx_bd_p
+ + sizeof(*priv->rx_bd_v) *
+ ((i + 1) % RX_BD_NUM));
skb = netdev_alloc_skb_ip_align(ndev,
NIXGE_MAX_JUMBO_FRAME_SIZE);
if (!skb)
goto out;
- priv->rx_bd_v[i].sw_id_offset = (u32)skb;
- priv->rx_bd_v[i].phys =
- dma_map_single(ndev->dev.parent,
- skb->data,
- NIXGE_MAX_JUMBO_FRAME_SIZE,
- DMA_FROM_DEVICE);
+ nixge_hw_dma_bd_set_offset(&priv->rx_bd_v[i], skb);
+ phys = dma_map_single(ndev->dev.parent, skb->data,
+ NIXGE_MAX_JUMBO_FRAME_SIZE,
+ DMA_FROM_DEVICE);
+
+ nixge_hw_dma_bd_set_phys(&priv->rx_bd_v[i], phys);
+
priv->rx_bd_v[i].cntrl = NIXGE_MAX_JUMBO_FRAME_SIZE;
}
@@ -312,18 +362,18 @@ static int nixge_hw_dma_bd_init(struct net_device *ndev)
/* Populate the tail pointer and bring the Rx Axi DMA engine out of
* halted state. This will make the Rx side ready for reception.
*/
- nixge_dma_write_reg(priv, XAXIDMA_RX_CDESC_OFFSET, priv->rx_bd_p);
+ nixge_dma_write_desc_reg(priv, XAXIDMA_RX_CDESC_OFFSET, priv->rx_bd_p);
cr = nixge_dma_read_reg(priv, XAXIDMA_RX_CR_OFFSET);
nixge_dma_write_reg(priv, XAXIDMA_RX_CR_OFFSET,
cr | XAXIDMA_CR_RUNSTOP_MASK);
- nixge_dma_write_reg(priv, XAXIDMA_RX_TDESC_OFFSET, priv->rx_bd_p +
+ nixge_dma_write_desc_reg(priv, XAXIDMA_RX_TDESC_OFFSET, priv->rx_bd_p +
(sizeof(*priv->rx_bd_v) * (RX_BD_NUM - 1)));
/* Write to the RS (Run-stop) bit in the Tx channel control register.
* Tx channel is now ready to run. But only after we write to the
* tail pointer register that the Tx channel will start transmitting.
*/
- nixge_dma_write_reg(priv, XAXIDMA_TX_CDESC_OFFSET, priv->tx_bd_p);
+ nixge_dma_write_desc_reg(priv, XAXIDMA_TX_CDESC_OFFSET, priv->tx_bd_p);
cr = nixge_dma_read_reg(priv, XAXIDMA_TX_CR_OFFSET);
nixge_dma_write_reg(priv, XAXIDMA_TX_CR_OFFSET,
cr | XAXIDMA_CR_RUNSTOP_MASK);
@@ -451,7 +501,7 @@ static int nixge_start_xmit(struct sk_buff *skb, struct net_device *ndev)
struct nixge_priv *priv = netdev_priv(ndev);
struct nixge_hw_dma_bd *cur_p;
struct nixge_tx_skb *tx_skb;
- dma_addr_t tail_p;
+ dma_addr_t tail_p, cur_phys;
skb_frag_t *frag;
u32 num_frag;
u32 ii;
@@ -466,15 +516,16 @@ static int nixge_start_xmit(struct sk_buff *skb, struct net_device *ndev)
return NETDEV_TX_OK;
}
- cur_p->phys = dma_map_single(ndev->dev.parent, skb->data,
- skb_headlen(skb), DMA_TO_DEVICE);
- if (dma_mapping_error(ndev->dev.parent, cur_p->phys))
+ cur_phys = dma_map_single(ndev->dev.parent, skb->data,
+ skb_headlen(skb), DMA_TO_DEVICE);
+ if (dma_mapping_error(ndev->dev.parent, cur_phys))
goto drop;
+ nixge_hw_dma_bd_set_phys(cur_p, cur_phys);
cur_p->cntrl = skb_headlen(skb) | XAXIDMA_BD_CTRL_TXSOF_MASK;
tx_skb->skb = NULL;
- tx_skb->mapping = cur_p->phys;
+ tx_skb->mapping = cur_phys;
tx_skb->size = skb_headlen(skb);
tx_skb->mapped_as_page = false;
@@ -485,16 +536,17 @@ static int nixge_start_xmit(struct sk_buff *skb, struct net_device *ndev)
tx_skb = &priv->tx_skb[priv->tx_bd_tail];
frag = &skb_shinfo(skb)->frags[ii];
- cur_p->phys = skb_frag_dma_map(ndev->dev.parent, frag, 0,
- skb_frag_size(frag),
- DMA_TO_DEVICE);
- if (dma_mapping_error(ndev->dev.parent, cur_p->phys))
+ cur_phys = skb_frag_dma_map(ndev->dev.parent, frag, 0,
+ skb_frag_size(frag),
+ DMA_TO_DEVICE);
+ if (dma_mapping_error(ndev->dev.parent, cur_phys))
goto frag_err;
+ nixge_hw_dma_bd_set_phys(cur_p, cur_phys);
cur_p->cntrl = skb_frag_size(frag);
tx_skb->skb = NULL;
- tx_skb->mapping = cur_p->phys;
+ tx_skb->mapping = cur_phys;
tx_skb->size = skb_frag_size(frag);
tx_skb->mapped_as_page = true;
}
@@ -506,7 +558,7 @@ static int nixge_start_xmit(struct sk_buff *skb, struct net_device *ndev)
tail_p = priv->tx_bd_p + sizeof(*priv->tx_bd_v) * priv->tx_bd_tail;
/* Start the transfer */
- nixge_dma_write_reg(priv, XAXIDMA_TX_TDESC_OFFSET, tail_p);
+ nixge_dma_write_desc_reg(priv, XAXIDMA_TX_TDESC_OFFSET, tail_p);
++priv->tx_bd_tail;
priv->tx_bd_tail %= TX_BD_NUM;
@@ -537,7 +589,7 @@ static int nixge_recv(struct net_device *ndev, int budget)
struct nixge_priv *priv = netdev_priv(ndev);
struct sk_buff *skb, *new_skb;
struct nixge_hw_dma_bd *cur_p;
- dma_addr_t tail_p = 0;
+ dma_addr_t tail_p = 0, cur_phys = 0;
u32 packets = 0;
u32 length = 0;
u32 size = 0;
@@ -549,13 +601,15 @@ static int nixge_recv(struct net_device *ndev, int budget)
tail_p = priv->rx_bd_p + sizeof(*priv->rx_bd_v) *
priv->rx_bd_ci;
- skb = (struct sk_buff *)(cur_p->sw_id_offset);
+ skb = (struct sk_buff *)nixge_hw_dma_bd_get_addr(cur_p,
+ sw_id_offset);
length = cur_p->status & XAXIDMA_BD_STS_ACTUAL_LEN_MASK;
if (length > NIXGE_MAX_JUMBO_FRAME_SIZE)
length = NIXGE_MAX_JUMBO_FRAME_SIZE;
- dma_unmap_single(ndev->dev.parent, cur_p->phys,
+ dma_unmap_single(ndev->dev.parent,
+ nixge_hw_dma_bd_get_addr(cur_p, phys),
NIXGE_MAX_JUMBO_FRAME_SIZE,
DMA_FROM_DEVICE);
@@ -579,16 +633,17 @@ static int nixge_recv(struct net_device *ndev, int budget)
if (!new_skb)
return packets;
- cur_p->phys = dma_map_single(ndev->dev.parent, new_skb->data,
- NIXGE_MAX_JUMBO_FRAME_SIZE,
- DMA_FROM_DEVICE);
- if (dma_mapping_error(ndev->dev.parent, cur_p->phys)) {
+ cur_phys = dma_map_single(ndev->dev.parent, new_skb->data,
+ NIXGE_MAX_JUMBO_FRAME_SIZE,
+ DMA_FROM_DEVICE);
+ if (dma_mapping_error(ndev->dev.parent, cur_phys)) {
/* FIXME: bail out and clean up */
netdev_err(ndev, "Failed to map ...\n");
}
+ nixge_hw_dma_bd_set_phys(cur_p, cur_phys);
cur_p->cntrl = NIXGE_MAX_JUMBO_FRAME_SIZE;
cur_p->status = 0;
- cur_p->sw_id_offset = (u32)new_skb;
+ nixge_hw_dma_bd_set_offset(cur_p, new_skb);
++priv->rx_bd_ci;
priv->rx_bd_ci %= RX_BD_NUM;
@@ -599,7 +654,7 @@ static int nixge_recv(struct net_device *ndev, int budget)
ndev->stats.rx_bytes += size;
if (tail_p)
- nixge_dma_write_reg(priv, XAXIDMA_RX_TDESC_OFFSET, tail_p);
+ nixge_dma_write_desc_reg(priv, XAXIDMA_RX_TDESC_OFFSET, tail_p);
return packets;
}
@@ -636,6 +691,7 @@ static irqreturn_t nixge_tx_irq(int irq, void *_ndev)
{
struct nixge_priv *priv = netdev_priv(_ndev);
struct net_device *ndev = _ndev;
+ dma_addr_t phys;
unsigned int status;
u32 cr;
@@ -650,9 +706,11 @@ static irqreturn_t nixge_tx_irq(int irq, void *_ndev)
return IRQ_NONE;
}
if (status & XAXIDMA_IRQ_ERROR_MASK) {
+ phys = nixge_hw_dma_bd_get_addr(&priv->tx_bd_v[priv->tx_bd_ci],
+ phys);
+
netdev_err(ndev, "DMA Tx error 0x%x\n", status);
- netdev_err(ndev, "Current BD is at: 0x%x\n",
- (priv->tx_bd_v[priv->tx_bd_ci]).phys);
+ netdev_err(ndev, "Current BD is at: 0x%x\n", phys);
cr = nixge_dma_read_reg(priv, XAXIDMA_TX_CR_OFFSET);
/* Disable coalesce, delay timer and error interrupts */
@@ -678,6 +736,7 @@ static irqreturn_t nixge_rx_irq(int irq, void *_ndev)
struct nixge_priv *priv = netdev_priv(_ndev);
struct net_device *ndev = _ndev;
unsigned int status;
+ dma_addr_t phys;
u32 cr;
status = nixge_dma_read_reg(priv, XAXIDMA_RX_SR_OFFSET);
@@ -697,9 +756,10 @@ static irqreturn_t nixge_rx_irq(int irq, void *_ndev)
return IRQ_NONE;
}
if (status & XAXIDMA_IRQ_ERROR_MASK) {
+ phys = nixge_hw_dma_bd_get_addr(&priv->rx_bd_v[priv->rx_bd_ci],
+ phys);
netdev_err(ndev, "DMA Rx error 0x%x\n", status);
- netdev_err(ndev, "Current BD is at: 0x%x\n",
- (priv->rx_bd_v[priv->rx_bd_ci]).phys);
+ netdev_err(ndev, "Current BD is at: 0x%x\n", phys);
cr = nixge_dma_read_reg(priv, XAXIDMA_TX_CR_OFFSET);
/* Disable coalesce, delay timer and error interrupts */
@@ -735,10 +795,12 @@ static void nixge_dma_err_handler(unsigned long data)
tx_skb = &lp->tx_skb[i];
nixge_tx_skb_unmap(lp, tx_skb);
- cur_p->phys = 0;
+ cur_p->phys_lo = 0;
+ cur_p->phys_hi = 0;
cur_p->cntrl = 0;
cur_p->status = 0;
- cur_p->sw_id_offset = 0;
+ cur_p->sw_id_offset_lo = 0;
+ cur_p->sw_id_offset_hi = 0;
}
for (i = 0; i < RX_BD_NUM; i++) {
@@ -779,18 +841,18 @@ static void nixge_dma_err_handler(unsigned long data)
/* Populate the tail pointer and bring the Rx Axi DMA engine out of
* halted state. This will make the Rx side ready for reception.
*/
- nixge_dma_write_reg(lp, XAXIDMA_RX_CDESC_OFFSET, lp->rx_bd_p);
+ nixge_dma_write_desc_reg(lp, XAXIDMA_RX_CDESC_OFFSET, lp->rx_bd_p);
cr = nixge_dma_read_reg(lp, XAXIDMA_RX_CR_OFFSET);
nixge_dma_write_reg(lp, XAXIDMA_RX_CR_OFFSET,
cr | XAXIDMA_CR_RUNSTOP_MASK);
- nixge_dma_write_reg(lp, XAXIDMA_RX_TDESC_OFFSET, lp->rx_bd_p +
+ nixge_dma_write_desc_reg(lp, XAXIDMA_RX_TDESC_OFFSET, lp->rx_bd_p +
(sizeof(*lp->rx_bd_v) * (RX_BD_NUM - 1)));
/* Write to the RS (Run-stop) bit in the Tx channel control register.
* Tx channel is now ready to run. But only after we write to the
* tail pointer register that the Tx channel will start transmitting
*/
- nixge_dma_write_reg(lp, XAXIDMA_TX_CDESC_OFFSET, lp->tx_bd_p);
+ nixge_dma_write_desc_reg(lp, XAXIDMA_TX_CDESC_OFFSET, lp->tx_bd_p);
cr = nixge_dma_read_reg(lp, XAXIDMA_TX_CR_OFFSET);
nixge_dma_write_reg(lp, XAXIDMA_TX_CR_OFFSET,
cr | XAXIDMA_CR_RUNSTOP_MASK);
--
2.18.0
^ permalink raw reply related
* Re: WARNING: refcount bug in sctp_transport_put
From: Marcelo Ricardo Leitner @ 2018-08-14 1:24 UTC (permalink / raw)
To: syzbot
Cc: davem, linux-kernel, linux-sctp, netdev, nhorman, syzkaller-bugs,
vyasevich
In-Reply-To: <000000000000fb3b1605735aa815@google.com>
On Mon, Aug 13, 2018 at 05:55:02PM -0700, syzbot wrote:
> ------------[ cut here ]------------
> refcount_t: underflow; use-after-free.
> WARNING: CPU: 1 PID: 7203 at lib/refcount.c:187
> refcount_sub_and_test+0x2e7/0x350 lib/refcount.c:187
> Kernel panic - not syncing: panic_on_warn set ...
>
...
> refcount_dec_and_test+0x1a/0x20 lib/refcount.c:212
> sctp_transport_put+0x76/0x200 net/sctp/transport.c:331
> sctp_generate_heartbeat_event+0x2d7/0x450 net/sctp/sm_sideeffect.c:416
> call_timer_fn+0x242/0x970 kernel/time/timer.c:1326
> expire_timers kernel/time/timer.c:1363 [inline]
> __run_timers+0x7a6/0xc70 kernel/time/timer.c:1666
> run_timer_softirq+0x4c/0x70 kernel/time/timer.c:1692
> __do_softirq+0x2e8/0xb17 kernel/softirq.c:292
> invoke_softirq kernel/softirq.c:372 [inline]
> irq_exit+0x1d4/0x210 kernel/softirq.c:412
> exiting_irq arch/x86/include/asm/apic.h:527 [inline]
> smp_apic_timer_interrupt+0x186/0x730 arch/x86/kernel/apic/apic.c:1055
> apic_timer_interrupt+0xf/0x20 arch/x86/entry/entry_64.S:863
> </IRQ>
Interesting, again on heartbeat timer handler.
Marcelo
^ permalink raw reply
* Re: [V9fs-developer] [PATCH 1/2] 9p: rename p9_free_req() function
From: piaojun @ 2018-08-14 1:34 UTC (permalink / raw)
To: Tomas Bortoli, asmadeus, ericvh, rminnich, lucho
Cc: Dominique Martinet, netdev, linux-kernel, syzkaller,
v9fs-developer, davem
In-Reply-To: <20180811144254.23665-1-tomasbortoli@gmail.com>
LGTM
On 2018/8/11 22:42, Tomas Bortoli wrote:
> In sight of the next patch to add a refcount in p9_req_t, rename
> the p9_free_req() function in p9_release_req().
>
> In the next patch the actual kfree will be moved to another function.
>
> Signed-off-by: Tomas Bortoli <tomasbortoli@gmail.com>
> Signed-off-by: Dominique Martinet <dominique.martinet@cea.fr>
Acked-by: Jun Piao <piaojun@huawei.com>
> ---
> net/9p/client.c | 100 ++++++++++++++++++++++++++++----------------------------
> 1 file changed, 50 insertions(+), 50 deletions(-)
>
> diff --git a/net/9p/client.c b/net/9p/client.c
> index 6c57ab1294d7..7942c0bfcc5b 100644
> --- a/net/9p/client.c
> +++ b/net/9p/client.c
> @@ -344,13 +344,13 @@ struct p9_req_t *p9_tag_lookup(struct p9_client *c, u16 tag)
> EXPORT_SYMBOL(p9_tag_lookup);
>
> /**
> - * p9_free_req - Free a request.
> + * p9_tag_remove - Remove a tag.
> * @c: Client session.
> - * @r: Request to free.
> + * @r: Request of reference.
> *
> * Context: Any context.
> */
> -static void p9_free_req(struct p9_client *c, struct p9_req_t *r)
> +static void p9_tag_remove(struct p9_client *c, struct p9_req_t *r)
> {
> unsigned long flags;
> u16 tag = r->tc.tag;
> @@ -379,7 +379,7 @@ static void p9_tag_cleanup(struct p9_client *c)
> rcu_read_lock();
> idr_for_each_entry(&c->reqs, req, id) {
> pr_info("Tag %d still in use\n", id);
> - p9_free_req(c, req);
> + p9_tag_remove(c, req);
> }
> rcu_read_unlock();
> }
> @@ -647,7 +647,7 @@ static int p9_client_flush(struct p9_client *c, struct p9_req_t *oldreq)
> if (c->trans_mod->cancelled)
> c->trans_mod->cancelled(c, oldreq);
>
> - p9_free_req(c, req);
> + p9_tag_remove(c, req);
> return 0;
> }
>
> @@ -681,7 +681,7 @@ static struct p9_req_t *p9_client_prepare_req(struct p9_client *c,
> trace_9p_client_req(c, type, req->tc.tag);
> return req;
> reterr:
> - p9_free_req(c, req);
> + p9_tag_remove(c, req);
> return ERR_PTR(err);
> }
>
> @@ -691,7 +691,7 @@ static struct p9_req_t *p9_client_prepare_req(struct p9_client *c,
> * @type: type of request
> * @fmt: protocol format string (see protocol.c)
> *
> - * Returns request structure (which client must free using p9_free_req)
> + * Returns request structure (which client must free using p9_tag_remove)
> */
>
> static struct p9_req_t *
> @@ -767,7 +767,7 @@ p9_client_rpc(struct p9_client *c, int8_t type, const char *fmt, ...)
> if (!err)
> return req;
> reterr:
> - p9_free_req(c, req);
> + p9_tag_remove(c, req);
> return ERR_PTR(safe_errno(err));
> }
>
> @@ -782,7 +782,7 @@ p9_client_rpc(struct p9_client *c, int8_t type, const char *fmt, ...)
> * @hdrlen: reader header size, This is the size of response protocol data
> * @fmt: protocol format string (see protocol.c)
> *
> - * Returns request structure (which client must free using p9_free_req)
> + * Returns request structure (which client must free using p9_tag_remove)
> */
> static struct p9_req_t *p9_client_zc_rpc(struct p9_client *c, int8_t type,
> struct iov_iter *uidata,
> @@ -849,7 +849,7 @@ static struct p9_req_t *p9_client_zc_rpc(struct p9_client *c, int8_t type,
> if (!err)
> return req;
> reterr:
> - p9_free_req(c, req);
> + p9_tag_remove(c, req);
> return ERR_PTR(safe_errno(err));
> }
>
> @@ -952,7 +952,7 @@ static int p9_client_version(struct p9_client *c)
>
> error:
> kfree(version);
> - p9_free_req(c, req);
> + p9_tag_remove(c, req);
>
> return err;
> }
> @@ -1094,7 +1094,7 @@ struct p9_fid *p9_client_attach(struct p9_client *clnt, struct p9_fid *afid,
> err = p9pdu_readf(&req->rc, clnt->proto_version, "Q", &qid);
> if (err) {
> trace_9p_protocol_dump(clnt, &req->rc);
> - p9_free_req(clnt, req);
> + p9_tag_remove(clnt, req);
> goto error;
> }
>
> @@ -1103,7 +1103,7 @@ struct p9_fid *p9_client_attach(struct p9_client *clnt, struct p9_fid *afid,
>
> memmove(&fid->qid, &qid, sizeof(struct p9_qid));
>
> - p9_free_req(clnt, req);
> + p9_tag_remove(clnt, req);
> return fid;
>
> error:
> @@ -1151,10 +1151,10 @@ struct p9_fid *p9_client_walk(struct p9_fid *oldfid, uint16_t nwname,
> err = p9pdu_readf(&req->rc, clnt->proto_version, "R", &nwqids, &wqids);
> if (err) {
> trace_9p_protocol_dump(clnt, &req->rc);
> - p9_free_req(clnt, req);
> + p9_tag_remove(clnt, req);
> goto clunk_fid;
> }
> - p9_free_req(clnt, req);
> + p9_tag_remove(clnt, req);
>
> p9_debug(P9_DEBUG_9P, "<<< RWALK nwqid %d:\n", nwqids);
>
> @@ -1229,7 +1229,7 @@ int p9_client_open(struct p9_fid *fid, int mode)
> fid->iounit = iounit;
>
> free_and_error:
> - p9_free_req(clnt, req);
> + p9_tag_remove(clnt, req);
> error:
> return err;
> }
> @@ -1274,7 +1274,7 @@ int p9_client_create_dotl(struct p9_fid *ofid, const char *name, u32 flags, u32
> ofid->iounit = iounit;
>
> free_and_error:
> - p9_free_req(clnt, req);
> + p9_tag_remove(clnt, req);
> error:
> return err;
> }
> @@ -1319,7 +1319,7 @@ int p9_client_fcreate(struct p9_fid *fid, const char *name, u32 perm, int mode,
> fid->iounit = iounit;
>
> free_and_error:
> - p9_free_req(clnt, req);
> + p9_tag_remove(clnt, req);
> error:
> return err;
> }
> @@ -1353,7 +1353,7 @@ int p9_client_symlink(struct p9_fid *dfid, const char *name,
> qid->type, (unsigned long long)qid->path, qid->version);
>
> free_and_error:
> - p9_free_req(clnt, req);
> + p9_tag_remove(clnt, req);
> error:
> return err;
> }
> @@ -1373,7 +1373,7 @@ int p9_client_link(struct p9_fid *dfid, struct p9_fid *oldfid, const char *newna
> return PTR_ERR(req);
>
> p9_debug(P9_DEBUG_9P, "<<< RLINK\n");
> - p9_free_req(clnt, req);
> + p9_tag_remove(clnt, req);
> return 0;
> }
> EXPORT_SYMBOL(p9_client_link);
> @@ -1397,7 +1397,7 @@ int p9_client_fsync(struct p9_fid *fid, int datasync)
>
> p9_debug(P9_DEBUG_9P, "<<< RFSYNC fid %d\n", fid->fid);
>
> - p9_free_req(clnt, req);
> + p9_tag_remove(clnt, req);
>
> error:
> return err;
> @@ -1432,7 +1432,7 @@ int p9_client_clunk(struct p9_fid *fid)
>
> p9_debug(P9_DEBUG_9P, "<<< RCLUNK fid %d\n", fid->fid);
>
> - p9_free_req(clnt, req);
> + p9_tag_remove(clnt, req);
> error:
> /*
> * Fid is not valid even after a failed clunk
> @@ -1466,7 +1466,7 @@ int p9_client_remove(struct p9_fid *fid)
>
> p9_debug(P9_DEBUG_9P, "<<< RREMOVE fid %d\n", fid->fid);
>
> - p9_free_req(clnt, req);
> + p9_tag_remove(clnt, req);
> error:
> if (err == -ERESTARTSYS)
> p9_client_clunk(fid);
> @@ -1493,7 +1493,7 @@ int p9_client_unlinkat(struct p9_fid *dfid, const char *name, int flags)
> }
> p9_debug(P9_DEBUG_9P, "<<< RUNLINKAT fid %d %s\n", dfid->fid, name);
>
> - p9_free_req(clnt, req);
> + p9_tag_remove(clnt, req);
> error:
> return err;
> }
> @@ -1545,7 +1545,7 @@ p9_client_read(struct p9_fid *fid, u64 offset, struct iov_iter *to, int *err)
> "D", &count, &dataptr);
> if (*err) {
> trace_9p_protocol_dump(clnt, &req->rc);
> - p9_free_req(clnt, req);
> + p9_tag_remove(clnt, req);
> break;
> }
> if (rsize < count) {
> @@ -1555,7 +1555,7 @@ p9_client_read(struct p9_fid *fid, u64 offset, struct iov_iter *to, int *err)
>
> p9_debug(P9_DEBUG_9P, "<<< RREAD count %d\n", count);
> if (!count) {
> - p9_free_req(clnt, req);
> + p9_tag_remove(clnt, req);
> break;
> }
>
> @@ -1565,7 +1565,7 @@ p9_client_read(struct p9_fid *fid, u64 offset, struct iov_iter *to, int *err)
> offset += n;
> if (n != count) {
> *err = -EFAULT;
> - p9_free_req(clnt, req);
> + p9_tag_remove(clnt, req);
> break;
> }
> } else {
> @@ -1573,7 +1573,7 @@ p9_client_read(struct p9_fid *fid, u64 offset, struct iov_iter *to, int *err)
> total += count;
> offset += count;
> }
> - p9_free_req(clnt, req);
> + p9_tag_remove(clnt, req);
> }
> return total;
> }
> @@ -1617,7 +1617,7 @@ p9_client_write(struct p9_fid *fid, u64 offset, struct iov_iter *from, int *err)
> *err = p9pdu_readf(&req->rc, clnt->proto_version, "d", &count);
> if (*err) {
> trace_9p_protocol_dump(clnt, &req->rc);
> - p9_free_req(clnt, req);
> + p9_tag_remove(clnt, req);
> break;
> }
> if (rsize < count) {
> @@ -1627,7 +1627,7 @@ p9_client_write(struct p9_fid *fid, u64 offset, struct iov_iter *from, int *err)
>
> p9_debug(P9_DEBUG_9P, "<<< RWRITE count %d\n", count);
>
> - p9_free_req(clnt, req);
> + p9_tag_remove(clnt, req);
> iov_iter_advance(from, count);
> total += count;
> offset += count;
> @@ -1661,7 +1661,7 @@ struct p9_wstat *p9_client_stat(struct p9_fid *fid)
> err = p9pdu_readf(&req->rc, clnt->proto_version, "wS", &ignored, ret);
> if (err) {
> trace_9p_protocol_dump(clnt, &req->rc);
> - p9_free_req(clnt, req);
> + p9_tag_remove(clnt, req);
> goto error;
> }
>
> @@ -1678,7 +1678,7 @@ struct p9_wstat *p9_client_stat(struct p9_fid *fid)
> from_kgid(&init_user_ns, ret->n_gid),
> from_kuid(&init_user_ns, ret->n_muid));
>
> - p9_free_req(clnt, req);
> + p9_tag_remove(clnt, req);
> return ret;
>
> error:
> @@ -1714,7 +1714,7 @@ struct p9_stat_dotl *p9_client_getattr_dotl(struct p9_fid *fid,
> err = p9pdu_readf(&req->rc, clnt->proto_version, "A", ret);
> if (err) {
> trace_9p_protocol_dump(clnt, &req->rc);
> - p9_free_req(clnt, req);
> + p9_tag_remove(clnt, req);
> goto error;
> }
>
> @@ -1739,7 +1739,7 @@ struct p9_stat_dotl *p9_client_getattr_dotl(struct p9_fid *fid,
> ret->st_ctime_nsec, ret->st_btime_sec, ret->st_btime_nsec,
> ret->st_gen, ret->st_data_version);
>
> - p9_free_req(clnt, req);
> + p9_tag_remove(clnt, req);
> return ret;
>
> error:
> @@ -1808,7 +1808,7 @@ int p9_client_wstat(struct p9_fid *fid, struct p9_wstat *wst)
>
> p9_debug(P9_DEBUG_9P, "<<< RWSTAT fid %d\n", fid->fid);
>
> - p9_free_req(clnt, req);
> + p9_tag_remove(clnt, req);
> error:
> return err;
> }
> @@ -1840,7 +1840,7 @@ int p9_client_setattr(struct p9_fid *fid, struct p9_iattr_dotl *p9attr)
> goto error;
> }
> p9_debug(P9_DEBUG_9P, "<<< RSETATTR fid %d\n", fid->fid);
> - p9_free_req(clnt, req);
> + p9_tag_remove(clnt, req);
> error:
> return err;
> }
> @@ -1868,7 +1868,7 @@ int p9_client_statfs(struct p9_fid *fid, struct p9_rstatfs *sb)
> &sb->files, &sb->ffree, &sb->fsid, &sb->namelen);
> if (err) {
> trace_9p_protocol_dump(clnt, &req->rc);
> - p9_free_req(clnt, req);
> + p9_tag_remove(clnt, req);
> goto error;
> }
>
> @@ -1879,7 +1879,7 @@ int p9_client_statfs(struct p9_fid *fid, struct p9_rstatfs *sb)
> sb->blocks, sb->bfree, sb->bavail, sb->files, sb->ffree,
> sb->fsid, (long int)sb->namelen);
>
> - p9_free_req(clnt, req);
> + p9_tag_remove(clnt, req);
> error:
> return err;
> }
> @@ -1907,7 +1907,7 @@ int p9_client_rename(struct p9_fid *fid,
>
> p9_debug(P9_DEBUG_9P, "<<< RRENAME fid %d\n", fid->fid);
>
> - p9_free_req(clnt, req);
> + p9_tag_remove(clnt, req);
> error:
> return err;
> }
> @@ -1937,7 +1937,7 @@ int p9_client_renameat(struct p9_fid *olddirfid, const char *old_name,
> p9_debug(P9_DEBUG_9P, "<<< RRENAMEAT newdirfid %d new name %s\n",
> newdirfid->fid, new_name);
>
> - p9_free_req(clnt, req);
> + p9_tag_remove(clnt, req);
> error:
> return err;
> }
> @@ -1974,10 +1974,10 @@ struct p9_fid *p9_client_xattrwalk(struct p9_fid *file_fid,
> err = p9pdu_readf(&req->rc, clnt->proto_version, "q", attr_size);
> if (err) {
> trace_9p_protocol_dump(clnt, &req->rc);
> - p9_free_req(clnt, req);
> + p9_tag_remove(clnt, req);
> goto clunk_fid;
> }
> - p9_free_req(clnt, req);
> + p9_tag_remove(clnt, req);
> p9_debug(P9_DEBUG_9P, "<<< RXATTRWALK fid %d size %llu\n",
> attr_fid->fid, *attr_size);
> return attr_fid;
> @@ -2011,7 +2011,7 @@ int p9_client_xattrcreate(struct p9_fid *fid, const char *name,
> goto error;
> }
> p9_debug(P9_DEBUG_9P, "<<< RXATTRCREATE fid %d\n", fid->fid);
> - p9_free_req(clnt, req);
> + p9_tag_remove(clnt, req);
> error:
> return err;
> }
> @@ -2074,11 +2074,11 @@ int p9_client_readdir(struct p9_fid *fid, char *data, u32 count, u64 offset)
> if (non_zc)
> memmove(data, dataptr, count);
>
> - p9_free_req(clnt, req);
> + p9_tag_remove(clnt, req);
> return count;
>
> free_and_error:
> - p9_free_req(clnt, req);
> + p9_tag_remove(clnt, req);
> error:
> return err;
> }
> @@ -2109,7 +2109,7 @@ int p9_client_mknod_dotl(struct p9_fid *fid, const char *name, int mode,
> (unsigned long long)qid->path, qid->version);
>
> error:
> - p9_free_req(clnt, req);
> + p9_tag_remove(clnt, req);
> return err;
>
> }
> @@ -2140,7 +2140,7 @@ int p9_client_mkdir_dotl(struct p9_fid *fid, const char *name, int mode,
> (unsigned long long)qid->path, qid->version);
>
> error:
> - p9_free_req(clnt, req);
> + p9_tag_remove(clnt, req);
> return err;
>
> }
> @@ -2173,7 +2173,7 @@ int p9_client_lock_dotl(struct p9_fid *fid, struct p9_flock *flock, u8 *status)
> }
> p9_debug(P9_DEBUG_9P, "<<< RLOCK status %i\n", *status);
> error:
> - p9_free_req(clnt, req);
> + p9_tag_remove(clnt, req);
> return err;
>
> }
> @@ -2208,7 +2208,7 @@ int p9_client_getlock_dotl(struct p9_fid *fid, struct p9_getlock *glock)
> "proc_id %d client_id %s\n", glock->type, glock->start,
> glock->length, glock->proc_id, glock->client_id);
> error:
> - p9_free_req(clnt, req);
> + p9_tag_remove(clnt, req);
> return err;
> }
> EXPORT_SYMBOL(p9_client_getlock_dotl);
> @@ -2234,7 +2234,7 @@ int p9_client_readlink(struct p9_fid *fid, char **target)
> }
> p9_debug(P9_DEBUG_9P, "<<< RREADLINK target %s\n", *target);
> error:
> - p9_free_req(clnt, req);
> + p9_tag_remove(clnt, req);
> return err;
> }
> EXPORT_SYMBOL(p9_client_readlink);
>
^ permalink raw reply
* Re: [PATCH net-next] net: sched: act_ife: disable bh when taking ife_mod_lock
From: David Miller @ 2018-08-13 22:53 UTC (permalink / raw)
To: xiyou.wangcong; +Cc: vladbu, netdev, jhs, jiri
In-Reply-To: <CAM_iQpXJn2sQoLB=sah9vDd0DtAm10HpwnUP4p4EEb6ExGaRng@mail.gmail.com>
From: Cong Wang <xiyou.wangcong@gmail.com>
Date: Mon, 13 Aug 2018 12:16:52 -0700
> Your fix doesn't make sense, because what ife_mod_lock protects
> is absolutely not touched in BH context, they have no race.
It does make sense, the problem is if you acquire ife_mod_lock and
take a software interrupt while you hold it.
If that software interrupt takes the tcfa_lock, we're setup for an
AB-BA deadlock.
And there is also no easy way to reverse the lock ordering to
avoid this either.
I therefore think his fix is perfectly fine and that's why I
applied it.
^ permalink raw reply
* Re: [V9fs-developer] [PATCH 2/2] 9p: Add refcount to p9_req_t
From: piaojun @ 2018-08-14 1:38 UTC (permalink / raw)
To: Tomas Bortoli, asmadeus, ericvh, rminnich, lucho
Cc: Dominique Martinet, netdev, linux-kernel, syzkaller,
v9fs-developer, davem
In-Reply-To: <20180811144254.23665-2-tomasbortoli@gmail.com>
Hi Tomas & Dominique,
On 2018/8/11 22:42, Tomas Bortoli wrote:
> To avoid use-after-free(s), use a refcount to keep track of the
> usable references to any instantiated struct p9_req_t.
>
> This commit adds p9_req_put(), p9_req_get() and p9_req_try_get() as
> wrappers to kref_put(), kref_get() and kref_get_unless_zero().
> These are used by the client and the transports to keep track of
> valid requests' references.
>
> p9_free_req() is added back and used as callback by kref_put().
>
> Add SLAB_TYPESAFE_BY_RCU as it ensures that the memory freed by
> kmem_cache_free() will not be reused for another type until the rcu
> synchronisation period is over, so an address gotten under rcu read
> lock is safe to inc_ref() without corrupting random memory while
> the lock is held.
>
> Co-developed-by: Dominique Martinet <dominique.martinet@cea.fr>
> Signed-off-by: Tomas Bortoli <tomasbortoli@gmail.com>
> Reported-by: syzbot+467050c1ce275af2a5b8@syzkaller.appspotmail.com
> Signed-off-by: Dominique Martinet <dominique.martinet@cea.fr>
> ---
> include/net/9p/client.h | 14 +++++++++++++
> net/9p/client.c | 54 +++++++++++++++++++++++++++++++++++++++++++------
> net/9p/trans_fd.c | 11 +++++++++-
> net/9p/trans_rdma.c | 1 +
> 4 files changed, 73 insertions(+), 7 deletions(-)
>
> diff --git a/include/net/9p/client.h b/include/net/9p/client.h
> index 735f3979d559..947a570307a6 100644
> --- a/include/net/9p/client.h
> +++ b/include/net/9p/client.h
> @@ -94,6 +94,7 @@ enum p9_req_status_t {
> struct p9_req_t {
> int status;
> int t_err;
> + struct kref refcount;
> wait_queue_head_t wq;
> struct p9_fcall tc;
> struct p9_fcall rc;
> @@ -233,6 +234,19 @@ int p9_client_lock_dotl(struct p9_fid *fid, struct p9_flock *flock, u8 *status);
> int p9_client_getlock_dotl(struct p9_fid *fid, struct p9_getlock *fl);
> void p9_fcall_fini(struct p9_fcall *fc);
> struct p9_req_t *p9_tag_lookup(struct p9_client *, u16);
> +
> +static inline void p9_req_get(struct p9_req_t *r)
> +{
> + kref_get(&r->refcount);
> +}
> +
> +static inline int p9_req_try_get(struct p9_req_t *r)
> +{
> + return kref_get_unless_zero(&r->refcount);
> +}
> +
> +int p9_req_put(struct p9_req_t *r);
> +
> void p9_client_cb(struct p9_client *c, struct p9_req_t *req, int status);
>
> int p9_parse_header(struct p9_fcall *, int32_t *, int8_t *, int16_t *, int);
> diff --git a/net/9p/client.c b/net/9p/client.c
> index 7942c0bfcc5b..83f2f0aadc14 100644
> --- a/net/9p/client.c
> +++ b/net/9p/client.c
> @@ -310,6 +310,18 @@ p9_tag_alloc(struct p9_client *c, int8_t type, unsigned int max_size)
> if (tag < 0)
> goto free;
>
> + /* Init ref to two because in the general case there is one ref
> + * that is put asynchronously by a writer thread, one ref
> + * temporarily given by p9_tag_lookup and put by p9_client_cb
> + * in the recv thread, and one ref put by p9_remove_tag in the
There is a spell mistake, p9_remove_tag->p9_tag_remove, and sorry for not
pointing this in last comment.
Thanks,
Jun
> + * main thread. The only exception is virtio that does not use
> + * p9_tag_lookup but does not have a writer thread either
> + * (the write happens synchronously in the request/zc_request
> + * callback), so p9_client_cb eats the second ref there
> + * as the pointer is duplicated directly by virtqueue_add_sgs()
> + */
> + refcount_set(&req->refcount.refcount, 2);
> +
> return req;
>
> free:
> @@ -333,10 +345,21 @@ struct p9_req_t *p9_tag_lookup(struct p9_client *c, u16 tag)
> struct p9_req_t *req;
>
> rcu_read_lock();
> +again:
> req = idr_find(&c->reqs, tag);
> - /* There's no refcount on the req; a malicious server could cause
> - * us to dereference a NULL pointer
> - */
> + if (req) {
> + /* We have to be careful with the req found under rcu_read_lock
> + * Thanks to SLAB_TYPESAFE_BY_RCU we can safely try to get the
> + * ref again without corrupting other data, then check again
> + * that the tag matches once we have the ref
> + */
> + if (!p9_req_try_get(req))
> + goto again;
> + if (req->tc.tag != tag) {
> + p9_req_put(req);
> + goto again;
> + }
> + }
> rcu_read_unlock();
>
> return req;
> @@ -350,7 +373,7 @@ EXPORT_SYMBOL(p9_tag_lookup);
> *
> * Context: Any context.
> */
> -static void p9_tag_remove(struct p9_client *c, struct p9_req_t *r)
> +static int p9_tag_remove(struct p9_client *c, struct p9_req_t *r)
> {
> unsigned long flags;
> u16 tag = r->tc.tag;
> @@ -359,11 +382,23 @@ static void p9_tag_remove(struct p9_client *c, struct p9_req_t *r)
> spin_lock_irqsave(&c->lock, flags);
> idr_remove(&c->reqs, tag);
> spin_unlock_irqrestore(&c->lock, flags);
> + return p9_req_put(r);
> +}
> +
> +static void p9_req_free(struct kref *ref)
> +{
> + struct p9_req_t *r = container_of(ref, struct p9_req_t, refcount);
> p9_fcall_fini(&r->tc);
> p9_fcall_fini(&r->rc);
> kmem_cache_free(p9_req_cache, r);
> }
>
> +int p9_req_put(struct p9_req_t *r)
> +{
> + return kref_put(&r->refcount, p9_req_free);
> +}
> +EXPORT_SYMBOL(p9_req_put);
> +
> /**
> * p9_tag_cleanup - cleans up tags structure and reclaims resources
> * @c: v9fs client struct
> @@ -379,7 +414,9 @@ static void p9_tag_cleanup(struct p9_client *c)
> rcu_read_lock();
> idr_for_each_entry(&c->reqs, req, id) {
> pr_info("Tag %d still in use\n", id);
> - p9_tag_remove(c, req);
> + if (p9_tag_remove(c, req) == 0)
> + pr_warn("Packet with tag %d has still references",
> + req->tc.tag);
> }
> rcu_read_unlock();
> }
> @@ -403,6 +440,7 @@ void p9_client_cb(struct p9_client *c, struct p9_req_t *req, int status)
>
> wake_up(&req->wq);
> p9_debug(P9_DEBUG_MUX, "wakeup: %d\n", req->tc.tag);
> + p9_req_put(req);
> }
> EXPORT_SYMBOL(p9_client_cb);
>
> @@ -682,6 +720,8 @@ static struct p9_req_t *p9_client_prepare_req(struct p9_client *c,
> return req;
> reterr:
> p9_tag_remove(c, req);
> + /* We have to put also the 2nd reference as it won't be used */
> + p9_req_put(req);
> return ERR_PTR(err);
> }
>
> @@ -716,6 +756,8 @@ p9_client_rpc(struct p9_client *c, int8_t type, const char *fmt, ...)
>
> err = c->trans_mod->request(c, req);
> if (err < 0) {
> + /* write won't happen */
> + p9_req_put(req);
> if (err != -ERESTARTSYS && err != -EFAULT)
> c->status = Disconnected;
> goto recalc_sigpending;
> @@ -2241,7 +2283,7 @@ EXPORT_SYMBOL(p9_client_readlink);
>
> int __init p9_client_init(void)
> {
> - p9_req_cache = KMEM_CACHE(p9_req_t, 0);
> + p9_req_cache = KMEM_CACHE(p9_req_t, SLAB_TYPESAFE_BY_RCU);
> return p9_req_cache ? 0 : -ENOMEM;
> }
>
> diff --git a/net/9p/trans_fd.c b/net/9p/trans_fd.c
> index 20f46f13fe83..686e24e355d0 100644
> --- a/net/9p/trans_fd.c
> +++ b/net/9p/trans_fd.c
> @@ -132,6 +132,7 @@ struct p9_conn {
> struct list_head req_list;
> struct list_head unsent_req_list;
> struct p9_req_t *req;
> + struct p9_req_t *wreq;
> char tmp_buf[7];
> struct p9_fcall rc;
> int wpos;
> @@ -383,6 +384,7 @@ static void p9_read_work(struct work_struct *work)
> m->rc.sdata = NULL;
> m->rc.offset = 0;
> m->rc.capacity = 0;
> + p9_req_put(m->req);
> m->req = NULL;
> }
>
> @@ -472,6 +474,8 @@ static void p9_write_work(struct work_struct *work)
> m->wbuf = req->tc.sdata;
> m->wsize = req->tc.size;
> m->wpos = 0;
> + p9_req_get(req);
> + m->wreq = req;
> spin_unlock(&m->client->lock);
> }
>
> @@ -492,8 +496,11 @@ static void p9_write_work(struct work_struct *work)
> }
>
> m->wpos += err;
> - if (m->wpos == m->wsize)
> + if (m->wpos == m->wsize) {
> m->wpos = m->wsize = 0;
> + p9_req_put(m->wreq);
> + m->wreq = NULL;
> + }
>
> end_clear:
> clear_bit(Wworksched, &m->wsched);
> @@ -694,6 +701,7 @@ static int p9_fd_cancel(struct p9_client *client, struct p9_req_t *req)
> if (req->status == REQ_STATUS_UNSENT) {
> list_del(&req->req_list);
> req->status = REQ_STATUS_FLSHD;
> + p9_req_put(req);
> ret = 0;
> }
> spin_unlock(&client->lock);
> @@ -711,6 +719,7 @@ static int p9_fd_cancelled(struct p9_client *client, struct p9_req_t *req)
> spin_lock(&client->lock);
> list_del(&req->req_list);
> spin_unlock(&client->lock);
> + p9_req_put(req);
>
> return 0;
> }
> diff --git a/net/9p/trans_rdma.c b/net/9p/trans_rdma.c
> index c60655c90c9e..8cff368a11e3 100644
> --- a/net/9p/trans_rdma.c
> +++ b/net/9p/trans_rdma.c
> @@ -365,6 +365,7 @@ send_done(struct ib_cq *cq, struct ib_wc *wc)
> c->busa, c->req->tc.size,
> DMA_TO_DEVICE);
> up(&rdma->sq_sem);
> + p9_req_put(c->req);
> kfree(c);
> }
>
>
^ permalink raw reply
* Re: [PATCH net-next v6 08/11] net: sched: don't release reference on action overwrite
From: Cong Wang @ 2018-08-13 23:00 UTC (permalink / raw)
To: Vlad Buslov
Cc: Linux Kernel Network Developers, David Miller, Jamal Hadi Salim,
Jiri Pirko, Alexei Starovoitov, Daniel Borkmann,
Yevgeny Kliteynik, Jiri Pirko
In-Reply-To: <1530800673-12280-9-git-send-email-vladbu@mellanox.com>
On Thu, Jul 5, 2018 at 7:24 AM Vlad Buslov <vladbu@mellanox.com> wrote:
> diff --git a/net/sched/act_ife.c b/net/sched/act_ife.c
> index 89a761395c94..acea3feae762 100644
> --- a/net/sched/act_ife.c
> +++ b/net/sched/act_ife.c
...
> @@ -548,6 +546,8 @@ static int tcf_ife_init(struct net *net, struct nlattr *nla,
>
> if (exists)
> spin_unlock_bh(&ife->tcf_lock);
> + tcf_idr_release(*a, bind);
> +
> kfree(p);
> return err;
> }
With this change, you seem release it twice when nla_parse_nested() fails
for ACT_P_CREATED case...?
Looks like what you want is the following?
if (err) {
tcf_idr_release(*a, bind);
kfree(p);
return err;
}
^ permalink raw reply
* Re: [PATCH net-next] net: sched: act_ife: disable bh when taking ife_mod_lock
From: Cong Wang @ 2018-08-13 23:01 UTC (permalink / raw)
To: David Miller
Cc: Vlad Buslov, Linux Kernel Network Developers, Jamal Hadi Salim,
Jiri Pirko
In-Reply-To: <20180813.155328.1817591638806945356.davem@davemloft.net>
On Mon, Aug 13, 2018 at 3:53 PM David Miller <davem@davemloft.net> wrote:
>
> From: Cong Wang <xiyou.wangcong@gmail.com>
> Date: Mon, 13 Aug 2018 12:16:52 -0700
>
> > Your fix doesn't make sense, because what ife_mod_lock protects
> > is absolutely not touched in BH context, they have no race.
>
> It does make sense, the problem is if you acquire ife_mod_lock and
> take a software interrupt while you hold it.
>
> If that software interrupt takes the tcfa_lock, we're setup for an
> AB-BA deadlock.
The lockdep does make sense, for sure. The fix does NOT.
>
> And there is also no easy way to reverse the lock ordering to
> avoid this either.
There is.
>
> I therefore think his fix is perfectly fine and that's why I
> applied it.
I will send a revert and a better fix.
Thanks.
^ permalink raw reply
* Re: [pull request][net-next V2 00/12] Mellanox, mlx5e updates 2018-08-10
From: David Miller @ 2018-08-13 23:04 UTC (permalink / raw)
To: saeedm; +Cc: netdev
In-Reply-To: <20180813204420.3342-1-saeedm@mellanox.com>
From: Saeed Mahameed <saeedm@mellanox.com>
Date: Mon, 13 Aug 2018 13:44:08 -0700
> This series provides some updates to mlx5e netdevice driver.
>
> For more information please see tag log below.
>
> Please pull and let me know if there's any problem.
>
> v1->v2:
> - Use l4_mask local function variable in validate_tcpudp4 rather than
> the passed paramter
> - Use ipv6_addr_any instead of explicitly implementing similar function
> - use ip6src_m mask provided by the user rather than generating a full
> mask.
Pulled, thanks.
^ permalink raw reply
* Re: [PATCH net-next] net: sched: act_ife: disable bh when taking ife_mod_lock
From: Cong Wang @ 2018-08-13 23:18 UTC (permalink / raw)
To: Vlad Buslov
Cc: Linux Kernel Network Developers, David Miller, Jamal Hadi Salim,
Jiri Pirko
In-Reply-To: <1534180811-10416-1-git-send-email-vladbu@mellanox.com>
Hi, Vlad,
Could you help to test my fixes?
I just pushed them into my own git repo:
https://github.com/congwang/linux/commits/net-sched-fixes
Particularly, this is the revert:
https://github.com/congwang/linux/commit/b3f51c4ab8272cc8d3244848e528fce1426c4659
and this is my fix for the lockdep warning you reported:
https://github.com/congwang/linux/commit/ecadcde94919183e9f0d5bc376f05e731baf2661
I don't have environment to test ife modules.
BTW, this is the fix for the deadlock I spotted:
https://github.com/congwang/linux/commit/44f3d7f5b6ed2d4a46177e6c658fa23b76141afa
Thanks!
^ permalink raw reply
* Re: [Question] bluetooth/{bnep,cmtp,hidp}: memory barriers
From: Brian Norris @ 2018-08-13 23:18 UTC (permalink / raw)
To: Andrea Parri
Cc: Marcel Holtmann, Johan Hedberg, David S. Miller, linux-bluetooth,
netdev, linux-kernel, Jeffy Chen, Brian Norris, AL Yu-Chen Cho
In-Reply-To: <20180730031030.GA9430@andrea>
On Mon, Jul 30, 2018 at 05:10:30AM +0200, Andrea Parri wrote:
> Hi,
Hi!
> I'm currently puzzled by the the three calls to smp_mb__before_atomic()
> in bnep_session(), cmtp_session() and hidp_session_run() respectively:
For the curious: I believe Jeffy Chen added all of those.
> On the one hand, these barriers provide no guarantee on the subsequent
> atomic_read(s->terminate) (as the comments preceding the barriers seem
> to suggest), because atomic_read() is not a read-modify-write.
I'll admit, I didn't notice that piece of the documentation when
reviewing this the first time:
Documentation/atomic_t.txt
<quote>
The barriers:
smp_mb__{before,after}_atomic()
only apply to the RMW ops and can be used to augment/upgrade the ordering
inherent to the used atomic op.
</quote>
> On the other hand, I'm currently unable to say *why such an "mb" would
> be required: not being too familiar with this code, I figured I should
> ask before sending a patch. ;-)
I can't fully speak for Jeffy, but I expect based on the initial
development of his patches like this one
commit 5da8e47d849d3d37b14129f038782a095b9ad049
Author: Jeffy Chen <jeffy.chen@rock-chips.com>
Date: Tue Jun 27 17:34:44 2017 +0800
Bluetooth: hidp: fix possible might sleep error in hidp_session_thread
that *some* kind of barrier was stuck in there simply as a response to
comments like this, that were going away:
- *
- * Note: set_current_state() performs any necessary
- * memory-barriers for us.
*/
- set_current_state(TASK_INTERRUPTIBLE);
+ /* Ensure session->terminate is updated */
+ smp_mb__before_atomic();
It was probably an attempt to fill in the gap for the
set_current_state() (and comment) which was being removed. I believe
Jeffy originally added more barriers in other places, but I convinced
him not to.
I have to say, I'm not really up-to-speed on the use of manual barriers
in Linux (it's much preferable when they're wrapped into higher-level
data structures already), but I believe the main intention here is to
ensure that any change to 'terminate' that happened during the previous
"wait_woken()" would be visible to our atomic_read().
Looking into wait_woken(), I'm feeling like none of these additional
barriers are necessary at all. I believe wait_woken() handles the
visibility issues we care about (that if we were woken for termination,
we'll see the terminating condition).
That's my two cents, even if it's only worth about two cents.
HTH,
Brian
^ permalink raw reply
* Re: [PATCH 1/1] NFC: Fix possible memory corruption when handling SHDLC I-Frame commands
From: Kees Cook @ 2018-08-14 0:28 UTC (permalink / raw)
To: Suren Baghdasaryan
Cc: Security Officers, kdeus, Samuel Ortiz, David S. Miller,
Allen Pais, linux-wireless, Network Development, LKML
In-Reply-To: <20180813223910.26276-1-surenb@google.com>
On Mon, Aug 13, 2018 at 3:39 PM, Suren Baghdasaryan <surenb@google.com> wrote:
> When handling SHDLC I-Frame commands "pipe" field used for indexing
> into an array should be checked before usage. If left unchecked it
> might access memory outside of the array of size NFC_HCI_MAX_PIPES(127).
>
> Malformed NFC HCI frames could be injected by a malicious NFC device
> communicating with the device being attacked (remote attack vector),
> or even by an attacker with physical access to the I2C bus such that
> they could influence the data transfers on that bus (local attack vector).
> skb->data is controlled by the attacker and has only been sanitized in
> the most trivial ways (CRC check), therefore we can consider the
> create_info struct and all of its members to tainted. 'create_info->pipe'
> with max value of 255 (uint8) is used to take an offset of the
> hdev->pipes array of 127 elements which can lead to OOB write.
>
> Suggested-by: Kevin Deus <kdeus@google.com>
> Signed-off-by: Suren Baghdasaryan <surenb@google.com>
Nice find!
Acked-by: Kees Cook <keescook@chromium.org>
> ---
> net/nfc/hci/core.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/net/nfc/hci/core.c b/net/nfc/hci/core.c
> index ac8030c4bcf8..19cb2e473ea6 100644
> --- a/net/nfc/hci/core.c
> +++ b/net/nfc/hci/core.c
> @@ -209,6 +209,11 @@ void nfc_hci_cmd_received(struct nfc_hci_dev *hdev, u8 pipe, u8 cmd,
> }
> create_info = (struct hci_create_pipe_resp *)skb->data;
>
> + if (create_info->pipe >= NFC_HCI_MAX_PIPES) {
> + status = NFC_HCI_ANY_E_NOK;
> + goto exit;
> + }
> +
> /* Save the new created pipe and bind with local gate,
> * the description for skb->data[3] is destination gate id
> * but since we received this cmd from host controller, we
> @@ -232,6 +237,11 @@ void nfc_hci_cmd_received(struct nfc_hci_dev *hdev, u8 pipe, u8 cmd,
> }
> delete_info = (struct hci_delete_pipe_noti *)skb->data;
>
> + if (delete_info->pipe >= NFC_HCI_MAX_PIPES) {
> + status = NFC_HCI_ANY_E_NOK;
> + goto exit;
> + }
> +
> hdev->pipes[delete_info->pipe].gate = NFC_HCI_INVALID_GATE;
> hdev->pipes[delete_info->pipe].dest_host = NFC_HCI_INVALID_HOST;
> break;
> --
> 2.18.0.597.ga71716f1ad-goog
>
--
Kees Cook
Pixel Security
^ permalink raw reply
* Re: [PATCH 2/2] net: lan743x: fix building without CONFIG_PTP_1588_CLOCK
From: David Miller @ 2018-08-14 3:44 UTC (permalink / raw)
To: arnd; +Cc: bryan.whitehead, UNGLinuxDriver, netdev, linux-kernel
In-Reply-To: <20180813211938.2375199-2-arnd@arndb.de>
From: Arnd Bergmann <arnd@arndb.de>
Date: Mon, 13 Aug 2018 23:19:22 +0200
> Building without CONFIG_PTP_1588_CLOCK results in multiple failures,
> this was obviously not well tested:
...
> Those #ifdef checks are hard to get right, replace them all with
> IS_ENABLED() checks that leave the same code visible to the compiler
> but let it optimize out the unused bits based on the configuration.
>
> Fixes: 07624df1c9ef ("lan743x: lan743x: Add PTP support")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Applied.
^ permalink raw reply
* Re: [PATCH] bnxt_en: avoid string overflow for record->system_name
From: David Miller @ 2018-08-14 3:46 UTC (permalink / raw)
To: arnd
Cc: michael.chan, vasundhara-v.volam, scott.branden, gospo, netdev,
linux-kernel
In-Reply-To: <20180813212700.2681499-1-arnd@arndb.de>
From: Arnd Bergmann <arnd@arndb.de>
Date: Mon, 13 Aug 2018 23:26:54 +0200
> The utsname()->nodename string may be 64 bytes long, and it gets
> copied without the trailing nul byte into the shorter record->system_name,
> as gcc now warns:
>
> In file included from include/linux/bitmap.h:9,
> from include/linux/ethtool.h:16,
> from drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c:13:
> In function 'strncpy',
> inlined from 'bnxt_fill_coredump_record' at drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c:2863:2:
> include/linux/string.h:254:9: error: '__builtin_strncpy' output truncated before terminating nul copying as many bytes from a string as its length [-Werror=stringop-truncation]
>
> Using strlcpy() at least avoids overflowing the destination buffer
> and adds proper nul-termination. It may still truncate long names
> though, which probably can't be solved here.
>
> Fixes: 6c5657d085ae ("bnxt_en: Add support for ethtool get dump.")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Applied.
^ permalink raw reply
* Re: [PATCH] l2tp: fix unused function warning
From: David Miller @ 2018-08-14 3:46 UTC (permalink / raw)
To: arnd; +Cc: jchapman, g.nault, lorenzo.bianconi, netdev, linux-kernel
In-Reply-To: <20180813214341.3333789-1-arnd@arndb.de>
From: Arnd Bergmann <arnd@arndb.de>
Date: Mon, 13 Aug 2018 23:43:05 +0200
> Removing one of the callers of pppol2tp_session_get_sock caused a harmless
> warning in some configurations:
>
> net/l2tp/l2tp_ppp.c:142:21: 'pppol2tp_session_get_sock' defined but not used [-Wunused-function]
>
> Rather than adding another #ifdef here, using a proper IS_ENABLED()
> check makes the code more readable and avoids those warnings while
> letting the compiler figure out for itself which code is needed.
>
> This adds one pointer for the unused show() callback in struct
> l2tp_session, but that seems harmless.
>
> Fixes: b0e29063dcb3 ("l2tp: remove pppol2tp_session_ioctl()")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Applied.
^ permalink raw reply
* Re: [PATCH] bnxt_en: take coredump_record structure off stack
From: David Miller @ 2018-08-14 3:46 UTC (permalink / raw)
To: arnd
Cc: michael.chan, vasundhara-v.volam, gospo, scott.branden, netdev,
linux-kernel
In-Reply-To: <20180813221256.299221-1-arnd@arndb.de>
From: Arnd Bergmann <arnd@arndb.de>
Date: Tue, 14 Aug 2018 00:12:45 +0200
> The bnxt_coredump_record structure is very long, causing a warning
> about possible stack overflow on 32-bit architectures:
>
> drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c: In function 'bnxt_get_coredump':
> drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c:2989:1: error: the frame size of 1188 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
>
> I could not see any reason to operate on an on-stack copy of the
> structure before copying it back into the caller-provided buffer, which
> also simplifies the code here.
>
> Fixes: 6c5657d085ae ("bnxt_en: Add support for ethtool get dump.")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Applied.
^ permalink raw reply
* [PATCH net] nfp: clean up return types in kdoc comments
From: Jakub Kicinski @ 2018-08-14 1:31 UTC (permalink / raw)
To: davem; +Cc: netdev, Jakub Kicinski
Remove 'Return:' information from functions which no longer
return a value. Also update name and return types of nfp_nffw_info
access functions.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Dirk van der Merwe <dirk.vandermerwe@netronome.com>
---
drivers/net/ethernet/netronome/nfp/nfp_net_common.c | 2 --
drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nffw.c | 6 ++----
2 files changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
index 0817c69fc568..a8b9fbab5f73 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
@@ -949,8 +949,6 @@ static int nfp_net_tx(struct sk_buff *skb, struct net_device *netdev)
* nfp_net_tx_complete() - Handled completed TX packets
* @tx_ring: TX ring structure
* @budget: NAPI budget (only used as bool to determine if in NAPI context)
- *
- * Return: Number of completed TX descriptors
*/
static void nfp_net_tx_complete(struct nfp_net_tx_ring *tx_ring, int budget)
{
diff --git a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nffw.c b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nffw.c
index 37a6d7822a38..40510860341b 100644
--- a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nffw.c
+++ b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nffw.c
@@ -207,7 +207,7 @@ nffw_res_fwinfos(struct nfp_nffw_info_data *fwinf, struct nffw_fwinfo **arr)
* nfp_nffw_info_open() - Acquire the lock on the NFFW table
* @cpp: NFP CPP handle
*
- * Return: 0, or -ERRNO
+ * Return: pointer to nfp_nffw_info object or ERR_PTR()
*/
struct nfp_nffw_info *nfp_nffw_info_open(struct nfp_cpp *cpp)
{
@@ -253,10 +253,8 @@ struct nfp_nffw_info *nfp_nffw_info_open(struct nfp_cpp *cpp)
}
/**
- * nfp_nffw_info_release() - Release the lock on the NFFW table
+ * nfp_nffw_info_close() - Release the lock on the NFFW table and free state
* @state: NFP FW info state
- *
- * Return: 0, or -ERRNO
*/
void nfp_nffw_info_close(struct nfp_nffw_info *state)
{
--
2.17.1
^ permalink raw reply related
* Re: [Question] bluetooth/{bnep,cmtp,hidp}: memory barriers
From: JeffyChen @ 2018-08-14 4:26 UTC (permalink / raw)
To: Brian Norris, Andrea Parri
Cc: Marcel Holtmann, Johan Hedberg, David S. Miller, linux-bluetooth,
netdev, linux-kernel, Brian Norris, AL Yu-Chen Cho
In-Reply-To: <20180813231854.GA173912@ban.mtv.corp.google.com>
Hi guys,
Thanks for your mails, and sorry for the late response..
On 08/14/2018 07:18 AM, Brian Norris wrote:
>
> commit 5da8e47d849d3d37b14129f038782a095b9ad049
> Author: Jeffy Chen<jeffy.chen@rock-chips.com>
> Date: Tue Jun 27 17:34:44 2017 +0800
>
> Bluetooth: hidp: fix possible might sleep error in hidp_session_thread
>
> that*some* kind of barrier was stuck in there simply as a response to
> comments like this, that were going away:
>
> - *
> - * Note: set_current_state() performs any necessary
> - * memory-barriers for us.
> */
> - set_current_state(TASK_INTERRUPTIBLE);
>
> + /* Ensure session->terminate is updated */
> + smp_mb__before_atomic();
>
>
> It was probably an attempt to fill in the gap for the
> set_current_state() (and comment) which was being removed. I believe
> Jeffy originally added more barriers in other places, but I convinced
> him not to.
right, i was trying to avoid losing memory-barriers when removing
set_current_state and changing wake_up_process to wake_up_interruptible.
and checking these code again, it's true the smp_mb__before_atomic
before atomic_read is not needed, the smp_mb after
atomic_inc(&session->terminate) should be enough.
and as Brian point out, there's already an smp_store_mb at the end of
wait_woken, i agree we can remove all the
smp_mb__{before,after}_atomic() i wrongly added :)
>
> I have to say, I'm not really up-to-speed on the use of manual barriers
> in Linux (it's much preferable when they're wrapped into higher-level
> data structures already), but I believe the main intention here is to
> ensure that any change to 'terminate' that happened during the previous
> "wait_woken()" would be visible to our atomic_read().
>
> Looking into wait_woken(), I'm feeling like none of these additional
> barriers are necessary at all. I believe wait_woken() handles the
> visibility issues we care about (that if we were woken for termination,
> we'll see the terminating condition).
^ permalink raw reply
* Re: [PATCH net 1/2] net_sched: fix NULL pointer dereference when delete tcindex filter
From: Cong Wang @ 2018-08-14 1:46 UTC (permalink / raw)
To: Hangbin Liu; +Cc: Linux Kernel Network Developers, David Miller
In-Reply-To: <1534157044-19753-2-git-send-email-liuhangbin@gmail.com>
On Mon, Aug 13, 2018 at 3:44 AM Hangbin Liu <liuhangbin@gmail.com> wrote:
>
> This is because in tcindex_set_parms, when there is no old_r, we set new
> exts to cr.exts. And we didn't set it to filter when r == &new_filter_result.
>
> Then in tcindex_delete() -> tcf_exts_get_net(), we will get NULL pointer
> dereference as we didn't init exts.
>
> Fix it by moving tcf_exts_change() after "if (old_r && old_r != r)" check.
> Then we don't need "cr" as there is no errout after that.
Acked-by: Cong Wang <xiyou.wangcong@gmail.com>
^ permalink raw reply
* Re: [PATCH net 2/2] net_sched: Fix missing res info when create new tc_index filter
From: Cong Wang @ 2018-08-14 1:46 UTC (permalink / raw)
To: Hangbin Liu; +Cc: Linux Kernel Network Developers, David Miller
In-Reply-To: <1534157044-19753-3-git-send-email-liuhangbin@gmail.com>
On Mon, Aug 13, 2018 at 3:44 AM Hangbin Liu <liuhangbin@gmail.com> wrote:
> This is because we didn't update f->result.res when create new filter. Then in
> tcindex_delete() -> tcf_unbind_filter(), we will failed to find out the res
> and unbind filter, which will trigger the WARN_ON() in cbq_destroy_class().
>
> Fix it by updating f->result.res when create new filter.
>
> Fixes: 6e0565697a106 ("net_sched: fix another crash in cls_tcindex")
> Reported-by: Li Shuang <shuali@redhat.com>
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
Acked-by: Cong Wang <xiyou.wangcong@gmail.com>
^ permalink raw reply
* inconsistent lock state in ila_xlat_nl_cmd_add_mapping
From: syzbot @ 2018-08-14 4:40 UTC (permalink / raw)
To: davem, gregkh, keescook, ktkhai, kuznet, linux-kernel, netdev,
pombredanne, stephen, syzkaller-bugs, tom, yoshfuji
Hello,
syzbot found the following crash on:
HEAD commit: 78cbac647e61 Merge branch 'ip-faster-in-order-IP-fragments'
git tree: net-next
console output: https://syzkaller.appspot.com/x/log.txt?x=14df4828400000
kernel config: https://syzkaller.appspot.com/x/.config?x=9100338df26ab75
dashboard link: https://syzkaller.appspot.com/bug?extid=eaaf6c4a6a8cb1869d86
compiler: gcc (GCC) 8.0.1 20180413 (experimental)
syzkaller repro:https://syzkaller.appspot.com/x/repro.syz?x=13069ad2400000
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+eaaf6c4a6a8cb1869d86@syzkaller.appspotmail.com
8021q: adding VLAN 0 to HW filter on device team0
8021q: adding VLAN 0 to HW filter on device team0
8021q: adding VLAN 0 to HW filter on device team0
================================
WARNING: inconsistent lock state
4.18.0-rc8+ #183 Not tainted
--------------------------------
inconsistent {IN-SOFTIRQ-W} -> {SOFTIRQ-ON-W} usage.
syz-executor2/6403 [HC0[0]:SC0[0]:HE1:SE1] takes:
(____ptrval____) (&(&tlocks[i])->rlock){+.?.}, at: spin_lock
include/linux/spinlock.h:310 [inline]
(____ptrval____) (&(&tlocks[i])->rlock){+.?.}, at: ila_add_mapping
net/ipv6/ila/ila_xlat.c:233 [inline]
(____ptrval____) (&(&tlocks[i])->rlock){+.?.}, at:
ila_xlat_nl_cmd_add_mapping+0x6bb/0x17e0 net/ipv6/ila/ila_xlat.c:355
{IN-SOFTIRQ-W} state was registered at:
lock_acquire+0x1e4/0x540 kernel/locking/lockdep.c:3924
__raw_spin_lock_bh include/linux/spinlock_api_smp.h:135 [inline]
_raw_spin_lock_bh+0x31/0x40 kernel/locking/spinlock.c:168
spin_lock_bh include/linux/spinlock.h:315 [inline]
__rhashtable_insert_fast include/linux/rhashtable.h:596 [inline]
rhashtable_lookup_insert_fast include/linux/rhashtable.h:784 [inline]
fdb_create+0x5cc/0x1710 net/bridge/br_fdb.c:508
br_fdb_update+0x4e7/0xd40 net/bridge/br_fdb.c:605
br_handle_frame_finish+0xa23/0x1960 net/bridge/br_input.c:97
br_nf_hook_thresh+0x48d/0x5f0 net/bridge/br_netfilter_hooks.c:1011
br_nf_pre_routing_finish_ipv6+0x7bc/0xef0
net/bridge/br_netfilter_ipv6.c:209
NF_HOOK include/linux/netfilter.h:287 [inline]
br_nf_pre_routing_ipv6+0x4af/0xac0 net/bridge/br_netfilter_ipv6.c:237
br_nf_pre_routing+0xb33/0x17d0 net/bridge/br_netfilter_hooks.c:494
nf_hook_entry_hookfn include/linux/netfilter.h:119 [inline]
nf_hook_slow+0xc2/0x1c0 net/netfilter/core.c:511
nf_hook include/linux/netfilter.h:242 [inline]
NF_HOOK include/linux/netfilter.h:285 [inline]
br_handle_frame+0xc0d/0x1a20 net/bridge/br_input.c:303
__netif_receive_skb_core+0x1455/0x3af0 net/core/dev.c:4821
__netif_receive_skb_one_core+0xd0/0x200 net/core/dev.c:4890
__netif_receive_skb+0x2c/0x1e0 net/core/dev.c:5002
process_backlog+0x219/0x760 net/core/dev.c:5808
napi_poll net/core/dev.c:6228 [inline]
net_rx_action+0x7a5/0x1920 net/core/dev.c:6294
__do_softirq+0x2e8/0xb17 kernel/softirq.c:292
do_softirq_own_stack+0x2a/0x40 arch/x86/entry/entry_64.S:1046
do_softirq.part.18+0x155/0x1a0 kernel/softirq.c:336
do_softirq arch/x86/include/asm/preempt.h:23 [inline]
__local_bh_enable_ip+0x1ec/0x230 kernel/softirq.c:189
local_bh_enable include/linux/bottom_half.h:32 [inline]
rcu_read_unlock_bh include/linux/rcupdate.h:725 [inline]
ip6_finish_output2+0xce8/0x2820 net/ipv6/ip6_output.c:121
ip6_finish_output+0x5fe/0xbc0 net/ipv6/ip6_output.c:154
NF_HOOK_COND include/linux/netfilter.h:276 [inline]
ip6_output+0x234/0x9d0 net/ipv6/ip6_output.c:171
dst_output include/net/dst.h:444 [inline]
NF_HOOK include/linux/netfilter.h:287 [inline]
ndisc_send_skb+0x100d/0x1570 net/ipv6/ndisc.c:491
ndisc_send_ns+0x3c1/0x8d0 net/ipv6/ndisc.c:633
addrconf_dad_work+0xbf2/0x1310 net/ipv6/addrconf.c:4060
process_one_work+0xc73/0x1ba0 kernel/workqueue.c:2153
worker_thread+0x189/0x13c0 kernel/workqueue.c:2296
kthread+0x345/0x410 kernel/kthread.c:246
ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:412
irq event stamp: 113
hardirqs last enabled at (113): [<ffffffff86b11fe4>]
__raw_spin_unlock_irqrestore include/linux/spinlock_api_smp.h:160 [inline]
hardirqs last enabled at (113): [<ffffffff86b11fe4>]
_raw_spin_unlock_irqrestore+0x74/0xc0 kernel/locking/spinlock.c:184
hardirqs last disabled at (112): [<ffffffff86b120a4>]
__raw_spin_lock_irqsave include/linux/spinlock_api_smp.h:108 [inline]
hardirqs last disabled at (112): [<ffffffff86b120a4>]
_raw_spin_lock_irqsave+0x74/0xc0 kernel/locking/spinlock.c:152
softirqs last enabled at (64): [<ffffffff85079b7c>] spin_unlock_bh
include/linux/spinlock.h:355 [inline]
softirqs last enabled at (64): [<ffffffff85079b7c>]
release_sock+0x1ec/0x2c0 net/core/sock.c:2860
softirqs last disabled at (62): [<ffffffff85079a0d>] spin_lock_bh
include/linux/spinlock.h:315 [inline]
softirqs last disabled at (62): [<ffffffff85079a0d>]
release_sock+0x7d/0x2c0 net/core/sock.c:2847
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0
----
lock(&(&tlocks[i])->rlock);
<Interrupt>
lock(&(&tlocks[i])->rlock);
*** DEADLOCK ***
1 lock held by syz-executor2/6403:
#0: (____ptrval____) (cb_lock){++++}, at: genl_rcv+0x19/0x40
net/netlink/genetlink.c:636
stack backtrace:
CPU: 0 PID: 6403 Comm: syz-executor2 Not tainted 4.18.0-rc8+ #183
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x1c9/0x2b4 lib/dump_stack.c:113
print_usage_bug.cold.63+0x320/0x41a kernel/locking/lockdep.c:2546
valid_state kernel/locking/lockdep.c:2559 [inline]
mark_lock_irq kernel/locking/lockdep.c:2753 [inline]
mark_lock+0x1048/0x19f0 kernel/locking/lockdep.c:3151
mark_irqflags kernel/locking/lockdep.c:3047 [inline]
__lock_acquire+0x7ca/0x5020 kernel/locking/lockdep.c:3392
lock_acquire+0x1e4/0x540 kernel/locking/lockdep.c:3924
__raw_spin_lock include/linux/spinlock_api_smp.h:142 [inline]
_raw_spin_lock+0x2a/0x40 kernel/locking/spinlock.c:144
spin_lock include/linux/spinlock.h:310 [inline]
ila_add_mapping net/ipv6/ila/ila_xlat.c:233 [inline]
ila_xlat_nl_cmd_add_mapping+0x6bb/0x17e0 net/ipv6/ila/ila_xlat.c:355
genl_family_rcv_msg+0x8a3/0x1140 net/netlink/genetlink.c:601
genl_rcv_msg+0xc6/0x168 net/netlink/genetlink.c:626
netlink_rcv_skb+0x172/0x440 net/netlink/af_netlink.c:2454
genl_rcv+0x28/0x40 net/netlink/genetlink.c:637
netlink_unicast_kernel net/netlink/af_netlink.c:1317 [inline]
netlink_unicast+0x5a0/0x760 net/netlink/af_netlink.c:1343
netlink_sendmsg+0xa18/0xfc0 net/netlink/af_netlink.c:1908
sock_sendmsg_nosec net/socket.c:640 [inline]
sock_sendmsg+0xd5/0x120 net/socket.c:650
___sys_sendmsg+0x7fd/0x930 net/socket.c:2133
__sys_sendmsg+0x11d/0x290 net/socket.c:2171
__do_sys_sendmsg net/socket.c:2180 [inline]
__se_sys_sendmsg net/socket.c:2178 [inline]
__x64_sys_sendmsg+0x78/0xb0 net/socket.c:2178
do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x457089
Code: fd b4 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7
48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff
ff 0f 83 cb b4 fb ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007fcc9357bc78 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
RAX: ffffffffffffffda RBX: 00007fcc9357c6d4 RCX: 0000000000457089
RDX: 0000000000000000 RSI: 0000000020000080 RDI: 0000000000000003
RBP: 00000000009300a0 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 00000000ffffffff
R13: 00000000004d3c18 R14: 00000000004c88f5 R15: 0000000000000000
---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with
syzbot.
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches
^ permalink raw reply
* serdev: How to attach serdev devices to USB based tty devices?
From: Andreas Färber @ 2018-08-14 2:28 UTC (permalink / raw)
To: Rob Herring, linux-serial@vger.kernel.org, linux-usb
Cc: linux-arm-kernel@lists.infradead.org, Linux-MIPS, Stefan Rehm,
Xue Liu, LoRa_Community_Support@semtech.com, Oliver Neukum,
Alexander Graf, Ben Whitten, devicetree, Jian-Hong Pan,
netdev@vger.kernel.org
Hi Rob et al.,
For my LoRa network driver project [1] I have found your serdev
framework to be a valuable help for dealing with hardware modules
exposing some textual or binary UART interface.
In particular on arm(64) and mips this allows to define an unlimited
number of serdev drivers [2] that are associated via their Device Tree
compatible string and can optionally be configured via DT properties.
And in theory it seems serdev has also grown support for ACPI.
Now, a growing number of vendors are placing such modules on a USB stick
for easy evaluation on x86_64 PC hardware, or are designing mPCIe or M.2
cards using their USB pins. While I do not yet have access to such a
device myself, it is my understanding that devices with USB-UART bridge
chipsets (e.g., FTDI) will show up as /dev/ttyUSBx and devices with an
MCU implementing the CDC USB protocol (e.g., Pico-cell gateway = picoGW)
will show up as /dev/ttyACMx.
On the Raspberry Pi I've seen that Device Tree nodes can be used to pass
information to on-board devices such as MAC address to Ethernet chipset,
but that does not seem all that useful for passing a serdev child node
to hot-plugged devices at unpredictable hub/port location (where it
should not interfere with regular USB-UART cables for debugging), nor
would it help ACPI based platforms such as x86_64.
My idea then was that if we had some unique criteria like vendor and
product IDs (or whatever is supported in usb_device_id), we could write
a usb_driver with suitable USB_DEVICE*() macro. In its probe function we
could call into the existing tty driver's probe function and afterwards
try creating and attaching the appropriate serdev device, i.e. a fixed
USB-to-serdev driver mapping. Problem is that most devices don't seem to
implement any unique identifier I could make this depend on - either by
using a standard FT232/FT2232/CH340G chip or by using STMicroelectronics
virtual com port identifiers in CDC firmware and only differing in the
textual description [3] the usb_device_id does not seem to match on.
The obvious solution would of course be if hardware vendors could revise
their designs to configure FTDI/etc. chips uniquely. I hear that that
may involve exchanging the chipset, increasing costs, and may impact
existing drivers. Wouldn't help for devices out there today either.
For the picoGW CDC firmware, Semtech does appear to own a USB vendor ID,
so it would seem possible to allocate their own product IDs for SX1301
and SX1308 respectively to replace the generic STMicroelectronics IDs,
which the various vendors could offer as firmware updates.
All outside my control though.
Oliver therefore suggested to not mess with USB drivers and instead use
a line discipline (ldisc). It seems that for example the userspace tool
slattach takes a tty device and performs an ioctl to switch the generic
tty device into a special N_SLIP protocol mode, implemented in [4].
However, the existing number of such ldisc modes appears to be below 30,
with hardly any vendor-specific implementation, so polluting its number
space seems undesirable? And in some cases I would like to use the same
protocol implementation over direct UART and over USB, so would like to
avoid duplicate serdev_device_driver and tty_ldisc_ops implementations.
Long story short, has there been any thinking about a userspace
interface to attach a given serdev driver to a tty device?
Or is there, on OF_DYNAMIC platforms, a way from userspace to associate
a DT fragment (!= DT Overlay) with a given USB device dynamically, to
attach a serdev node with sub-nodes?
Any other ideas how to cleanly solve this?
In some cases we're talking about a "simple" AT-like command interface;
the picoGW implements a semi-generic USB-SPI bridge that may host a
choice of 2+ chipsets, which in turn has two further sub-devices with 3+
chipset choices (theoretically clk output and rx/tx options etc.) each.
(For the latter I'm thinking we'll need a serdev driver exposing a
regmap_bus and then implement regmap_bus based versions of the SPI
drivers like Ben and I refactored SX1257 in [2] last weekend.)
Thanks,
Andreas
[1] https://patchwork.ozlabs.org/cover/937545/
[2]
https://git.kernel.org/pub/scm/linux/kernel/git/afaerber/linux-lora.git/tree/drivers/net/lora?h=lora-next
[3]
https://github.com/Lora-net/picoGW_mcu/blob/master/src/usb_cdc/Src/usbd_desc.cpp#L59
[4]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/slip/slip.c#n1281
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
^ permalink raw reply
* Re: [PATCH net] nfp: clean up return types in kdoc comments
From: David Miller @ 2018-08-14 2:36 UTC (permalink / raw)
To: jakub.kicinski; +Cc: netdev
In-Reply-To: <20180814013105.19180-1-jakub.kicinski@netronome.com>
From: Jakub Kicinski <jakub.kicinski@netronome.com>
Date: Mon, 13 Aug 2018 18:31:05 -0700
> Remove 'Return:' information from functions which no longer
> return a value. Also update name and return types of nfp_nffw_info
> access functions.
>
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> Reviewed-by: Dirk van der Merwe <dirk.vandermerwe@netronome.com>
Applied, thanks Jakub.
^ 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