* [GIT] Networking
From: David Miller @ 2014-10-11 21:59 UTC (permalink / raw)
To: torvalds; +Cc: akpm, netdev, linux-kernel
This set fixes a bunch of fallout from the changes that went in
during this merge window, particularly:
1) Fix fsl_pq_mdio (Claudiu Manoil) and fm10k (Pranith Kumar) build
failures.
2) Several networking drivers do atomic_set() on page counts where
that's not exactly legal. From Eric Dumazet.
3) Make __skb_flow_get_ports() work cleanly with unaligned data,
from Alexander Duyck.
4) Fix some kernel-doc buglets in rfkill and netlabel, from Fabian
Frederick.
5) Unbalanced enable_irq_wake usage in bcmgenet and systemport
drivers, from Florian Fainelli.
6) pxa168_eth needs to depend on HAS_DMA, from Geert Uytterhoeven.
7) Multi-dequeue in the qdisc layer severely bypasses the fairness
limits the previous code used to enforce, reintroduce in a way that
at the same time doesn't compromise bulk dequeue opportunities.
From Jesper Dangaard Brouer.
8) macvlan receive path unnecessarily hops through a softirq by
using netif_rx() instead of netif_receive_skb(). From Jason
Baron.
Please pull, thanks a lot!
The following changes since commit 35a9ad8af0bb0fa3525e6d0d20e32551d226f38e:
Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next (2014-10-08 21:40:54 -0400)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git master
for you to fetch changes up to 01d2d484e49e9bc0ed9b5fdaf345a0e2bf35ffed:
Merge branch 'bcmgenet_systemport' (2014-10-10 15:39:22 -0400)
----------------------------------------------------------------
Alexander Duyck (1):
flow-dissector: Fix alignment issue in __skb_flow_get_ports
Alexei Starovoitov (1):
net: bpf: fix bpf syscall dependence on anon_inodes
Andrea Merello (1):
rtl818x_pci: fix response rate may be incorrect.
Claudiu Manoil (8):
net/fsl_pq_mdio: Fix asm/ucc.h compile error for ARM
net/fsl_pq_mdio: Use ioread/iowrite32be() portable accessors
net/fsl_pq_mdio: Replace spin_event_timeout() with arch independent
gianfar: Include missing headers for ARM builds
gianfar: Exclude PPC specific errata handling from ARM builds
gianfar: Make MAC addr setup endian safe, cleanup
gianfar: Replace spin_event_timeout() with arch independent
gianfar: Replace eieio with wmb for non-PPC archs
David S. Miller (9):
Merge branch 'gianfar'
Merge branch 'cxgb4'
Merge branch 'r8152'
Merge tag 'master-2014-10-08' of git://git.kernel.org/.../linville/wireless-next
Merge git://git.kernel.org/.../pablo/nf-next
Merge branch 'xgene'
Merge branch 'macvlan'
Merge branch 'net-drivers-pgcnt'
Merge branch 'bcmgenet_systemport'
Eric Dumazet (5):
fm10k: fix race accessing page->_count
igb: fix race accessing page->_count
ixgbe: fix race accessing page->_count
mlx4: fix race accessing page->_count
net: fix races in page->_count manipulation
Fabian Frederick (2):
net: rfkill: kernel-doc warning fixes
netlabel: kernel-doc warning fix
Florian Fainelli (3):
net: bcmgenet: fix off-by-one in incrementing read pointer
net: bcmgenet: avoid unbalanced enable_irq_wake calls
net: systemport: avoid unbalanced enable_irq_wake calls
Geert Uytterhoeven (1):
net: pxa168_eth: PXA168_ETH should depend on HAS_DMA
Hariprasad Shenai (3):
cxgb4/cxgb4vf: Updated the LSO transfer length in CPL_TX_PKT_LSO for T5
cxgb4vf: Add 40G support for cxgb4vf driver
cxgb4: Wait for device to get ready before reading any register
Iyappan Subramanian (6):
MAINTAINERS: Update APM X-Gene section
Documentation: dts: Update section header for APM X-Gene
dtb: Add 10GbE node to APM X-Gene SoC device tree
drivers: net: xgene: Preparing for adding 10GbE support
drivers: net: xgene: Add 10GbE support
drivers: net: xgene: Add 10GbE ethtool support
Jesper Dangaard Brouer (1):
net_sched: restore qdisc quota fairness limits after bulk dequeue
John W. Linville (1):
Merge branch 'master' of git://git.kernel.org/.../linville/wireless
LEROY Christophe (1):
net: fs_enet: error: 'SCCE_ENET_TXF' undeclared
Larry Finger (1):
rtlwifi: Fix possible unaligned array in ether_addr_copy()
Li RongQing (2):
Documentation: replace __sk_run_filter with __bpf_prog_run
net: filter: fix the comments
Marek Puzyniak (1):
ath9k_htc: avoid kernel panic in ath9k_hw_reset
Masanari Iida (1):
net: Missing @ before descriptions cause make xmldocs warning
Pablo Neira Ayuso (2):
netfilter: kill nf_send_reset6() from include/net/netfilter/ipv6/nf_reject.h
netfilter: fix wrong arithmetics regarding NFT_REJECT_ICMPX_MAX
Pranith Kumar (1):
networking: fm10k: Fix build failure
Sascha Hauer (1):
net/phy: micrel: Add clock support for KSZ8021/KSZ8031
Sujith Manoharan (3):
ath: Fix smatch warning
ath9k: Fix crash in MCC mode
ath9k: Fix sequence number assignment
Vince Bridgers (1):
stmmac: correct mc_filter local variable in set_filter and set_mac_addr call
hayeswang (3):
r8152: autoresume before setting feature
r8152: adjust usb_autopm_xxx
r8152: add mutex for hw settings
jbaron@akamai.com (2):
macvlan: pass 'bool' type to macvlan_count_rx()
macvlan: optimize the receive path
Documentation/devicetree/bindings/net/apm-xgene-enet.txt | 4 +-
Documentation/devicetree/bindings/net/micrel.txt | 6 +
Documentation/networking/filter.txt | 4 +-
MAINTAINERS | 1 -
arch/arm64/boot/dts/apm-mustang.dts | 4 +
arch/arm64/boot/dts/apm-storm.dtsi | 29 ++++-
drivers/net/ethernet/apm/xgene/Makefile | 3 +-
drivers/net/ethernet/apm/xgene/xgene_enet_ethtool.c | 28 ++++-
drivers/net/ethernet/apm/xgene/xgene_enet_hw.c | 44 ++++---
drivers/net/ethernet/apm/xgene/xgene_enet_hw.h | 30 ++---
drivers/net/ethernet/apm/xgene/xgene_enet_main.c | 86 ++++++++++----
drivers/net/ethernet/apm/xgene/xgene_enet_main.h | 24 +++-
drivers/net/ethernet/apm/xgene/xgene_enet_xgmac.c | 331 ++++++++++++++++++++++++++++++++++++++++++++++++++++
drivers/net/ethernet/apm/xgene/xgene_enet_xgmac.h | 57 +++++++++
drivers/net/ethernet/broadcom/bcmsysport.c | 3 +-
drivers/net/ethernet/broadcom/genet/bcmgenet.c | 9 +-
drivers/net/ethernet/broadcom/genet/bcmgenet_wol.c | 4 +-
drivers/net/ethernet/chelsio/cxgb4/cxgb4.h | 2 +-
drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c | 6 +-
drivers/net/ethernet/chelsio/cxgb4/sge.c | 5 +-
drivers/net/ethernet/chelsio/cxgb4/t4_hw.c | 17 +--
drivers/net/ethernet/chelsio/cxgb4/t4_msg.h | 1 +
drivers/net/ethernet/chelsio/cxgb4/t4_regs.h | 5 +-
drivers/net/ethernet/chelsio/cxgb4vf/cxgb4vf_main.c | 12 +-
drivers/net/ethernet/chelsio/cxgb4vf/sge.c | 5 +-
drivers/net/ethernet/chelsio/cxgb4vf/t4vf_common.h | 6 +
drivers/net/ethernet/chelsio/cxgb4vf/t4vf_hw.c | 10 +-
drivers/net/ethernet/freescale/fs_enet/mac-fcc.c | 2 +-
drivers/net/ethernet/freescale/fs_enet/mac-scc.c | 2 +-
drivers/net/ethernet/freescale/fsl_pq_mdio.c | 56 +++++----
drivers/net/ethernet/freescale/gianfar.c | 68 ++++++-----
drivers/net/ethernet/freescale/gianfar.h | 31 +++++
drivers/net/ethernet/intel/Kconfig | 1 +
drivers/net/ethernet/intel/fm10k/fm10k_main.c | 7 +-
drivers/net/ethernet/intel/igb/igb_main.c | 7 +-
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 8 +-
drivers/net/ethernet/marvell/Kconfig | 3 +-
drivers/net/ethernet/mellanox/mlx4/en_rx.c | 6 +-
drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c | 4 +-
drivers/net/macvlan.c | 21 ++--
drivers/net/phy/micrel.c | 31 ++++-
drivers/net/usb/r8152.c | 98 +++++++++++++---
drivers/net/wireless/ath/ath9k/ath9k.h | 4 +-
drivers/net/wireless/ath/ath9k/beacon.c | 12 +-
drivers/net/wireless/ath/ath9k/htc_drv_init.c | 1 +
drivers/net/wireless/ath/ath9k/main.c | 2 +-
drivers/net/wireless/ath/ath9k/tx99.c | 8 +-
drivers/net/wireless/ath/ath9k/xmit.c | 34 ++++--
drivers/net/wireless/ath/main.c | 8 +-
drivers/net/wireless/rtl818x/rtl8180/dev.c | 36 ++++--
drivers/net/wireless/rtlwifi/wifi.h | 2 +-
include/linux/micrel_phy.h | 1 +
include/net/netfilter/ipv6/nf_reject.h | 157 +------------------------
include/uapi/linux/netfilter/nf_tables.h | 2 +-
net/Kconfig | 1 +
net/core/filter.c | 9 +-
net/core/flow_dissector.c | 36 +++---
net/core/skbuff.c | 35 ++++--
net/netfilter/nft_reject.c | 10 +-
net/netlabel/netlabel_kapi.c | 1 -
net/rfkill/core.c | 4 +-
net/sched/sch_generic.c | 20 ++--
62 files changed, 1017 insertions(+), 447 deletions(-)
create mode 100644 drivers/net/ethernet/apm/xgene/xgene_enet_xgmac.c
create mode 100644 drivers/net/ethernet/apm/xgene/xgene_enet_xgmac.h
^ permalink raw reply
* Re: Question regarding netpoll_info != NULL check.
From: Eric W. Biederman @ 2014-10-11 21:39 UTC (permalink / raw)
To: Tetsuo Handa; +Cc: nhorman, davem, netdev
In-Reply-To: <201410112036.CGG26070.FOHSFMtOQOVLFJ@I-love.SAKURA.ne.jp>
Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> writes:
> Hello.
>
> A 2.6.32-220.4.2.el6 system crashed at poll_one_napi() when the administrator
> was updating network configuration and by error issued ifconfig command with
> a wrong argument.
>
> His system was using a bonding device as netconsole's the ethernet device to
> send console messages out of, and poll_one_napi() was called due to printk()
> by changing status of a bonding device.
>
> ---------- linux-2.6.32-220.4.2.el6.x86_64/net/core/netpoll.c ----------
> 128:static int poll_one_napi(struct netpoll_info *npinfo,
> 129: struct napi_struct *napi, int budget)
> 130:{
> 131: int work;
> 132:
> 133: /* net_rx_action's ->poll() invocations and our's are
> 134: * synchronized by this test which is only made while
> 135: * holding the napi->poll_lock.
> 136: */
> 137: if (!test_bit(NAPI_STATE_SCHED, &napi->state))
> 138: return budget;
> 139:
> 140: npinfo->rx_flags |= NETPOLL_RX_DROP;
> 141: atomic_inc(&trapped);
> 142: set_bit(NAPI_STATE_NPSVC, &napi->state);
> 143:
> 144: work = napi->poll(napi, budget);
> 145: trace_napi_poll(napi);
> 146:
> 147: clear_bit(NAPI_STATE_NPSVC, &napi->state);
> 148: atomic_dec(&trapped);
> 149: npinfo->rx_flags &= ~NETPOLL_RX_DROP;
> 150:
> 151: return budget - work;
> 152:}
> 153:
> 154:static void poll_napi(struct net_device *dev)
> 155:{
> 156: struct napi_struct *napi;
> 157: int budget = 16;
> 158:
> 159: list_for_each_entry(napi, &dev->napi_list, dev_list) {
> 160: if (napi->poll_owner != smp_processor_id() &&
> 161: spin_trylock(&napi->poll_lock)) {
> 162: budget = poll_one_napi(dev->npinfo, napi, budget);
> 163: spin_unlock(&napi->poll_lock);
> 164:
> 165: if (!budget)
> 166: break;
> 167: }
> 168: }
> 169:}
> 170:
> 171:static void service_arp_queue(struct netpoll_info *npi)
> 172:{
> 173: if (npi) {
> 174: struct sk_buff *skb;
> 175:
> 176: while ((skb = skb_dequeue(&npi->arp_tx)))
> 177: arp_reply(skb);
> 178: }
> 179:}
> 180:
> 181:void netpoll_poll_dev(struct net_device *dev)
> 182:{
> 183: const struct net_device_ops *ops;
> 184:
> 185: if (!dev || !netif_running(dev))
> 186: return;
> 187:
> 188: ops = dev->netdev_ops;
> 189: if (!ops->ndo_poll_controller)
> 190: return;
> 191:
> 192: /* Process pending work on NIC */
> 193: ops->ndo_poll_controller(dev);
> 194:
> 195: poll_napi(dev);
> 196:
> 197: if (dev->priv_flags & IFF_SLAVE) {
> 198: if (dev->npinfo) {
> 199: struct net_device *bond_dev = dev->master;
> 200: struct sk_buff *skb;
> 201: while ((skb = skb_dequeue(&dev->npinfo->arp_tx))) {
> 202: skb->dev = bond_dev;
> 203: skb_queue_tail(&bond_dev->npinfo->arp_tx, skb);
> 204: }
> 205: }
> 206: }
> 207:
> 208: service_arp_queue(dev->npinfo);
> 209:
> 210: zap_completion_queue();
> 211:}
> ---------- linux-2.6.32-220.4.2.el6.x86_64/net/core/netpoll.c ----------
>
> We can see that while line 198 did dev->npinfo != NULL check, line 162 called
> by line 195 did not do dev->npinfo != NULL check and thus npinfo was NULL at
> line 140 called by line 162.
>
> What is strange, we kept this dev->npinfo != NULL check until commit 18b37535
> ("netpoll: Consolidate neigh_tx processing in service_neigh_queue") and that
> commit depends on dev->npinfo != NULL assumption which commit ca99ca14
> ("netpoll: protect napi_poll and poll_controller during dev_[open|close]")
> is also assuming. But the above-mentioned system crashed due to
> dev->npinfo == NULL. Why we don't do dev->npinfo != NULL check?
The short version is that netpoll methods are not supposed to be called
on network devices where npinfo is NULL, and that check at line 198 has
always been dody. The poll_napi check started dereferencing
dev->npinfo in 2.6.13. While the IFF_SLAVE code came in in 2.6.39.
The current code in netpoll_poll_dev does the exclusion against races
that was happening in poll_napi at the begining of netpoll_poll_dev.
In the current code and I expect el6 is similar the code flow of all
of this starts with netpoll_send_udp. netpoll_send_udp is passed a
netpoll structure. The device we send to is obtained from that netpoll
structure.
That npinfo structure should be allocated in __netpoll_setup,
and freed in netpoll_cleanup.
I would recommend looking at where your bonding driver calls
netpoll_setup and netpoll_cleanup and the current exclusing in the
netpoll driver against races.
> Excuse me, I didn't do a full-fledged study because the size of vmcore was
> too huge to receive. Therefore I asked the administrator to do a superficial
> analysis by feeding predefined commands to the crash utility. If the reason
> we don't need dev->npinfo != NULL check is that dev->npinfo != NULL is always
> true, how should I verify that dev->npinfo == NULL was a race
> condition?
If possible look at the bonding event that was happening and see how
that interacts with the rest of the code.
There really isn't a lot of netpoll code. So it should be too hard to
read through all of the relevant bits and understand what is going on.
Eric
^ permalink raw reply
* Re: [PATCH] net: fec: Fix sparse warnings with different lock contexts for basic block
From: Fabio Estevam @ 2014-10-11 21:22 UTC (permalink / raw)
To: Fugang Duan
Cc: David S. Miller, netdev@vger.kernel.org, sparse, Li Frank-B20596
In-Reply-To: <1412996429-2253-1-git-send-email-b38611@freescale.com>
Hi Fugang,
On Sat, Oct 11, 2014 at 12:00 AM, Fugang Duan <b38611@freescale.com> wrote:
> reproduce:
> make ARCH=arm C=1 2>fec.txt drivers/net/ethernet/freescale/fec_main.o
> cat fec.txt
>
> sparse warnings:
> drivers/net/ethernet/freescale/fec_main.c:2916:12: warning: context imbalance
> in 'fec_set_features' - different lock contexts for basic block
>
> Christopher Li suggest to change as below:
> if (need_lock) {
> lock();
> do_something_real();
> unlock();
> } else {
> do_something_real();
> }
>
> Reported-by: Fabio Estevam <festevam@gmail.com>
> Signed-off-by: Fugang Duan <B38611@freescale.com>
> Signed-off-by: Christopher Li <sparse@chrisli.org>
It looks like Suggested-by is a more appropriate tag for Christopher.
^ permalink raw reply
* Re: [bisected] e341694e3eb5 netlink_lookup() rcu conversion causes latencies
From: Eric Dumazet @ 2014-10-11 19:32 UTC (permalink / raw)
To: Heiko Carstens, Sasha Levin, paulmck
Cc: Thomas Graf, Nikolay Aleksandrov, David S. Miller, netdev,
linux-kernel, Ursula Braun
In-Reply-To: <20141011083627.GB5074@osiris>
On Sat, 2014-10-11 at 10:36 +0200, Heiko Carstens wrote:
> Hi all,
>
> it just came to my attention that commit e341694e3eb5
> "netlink: Convert netlink_lookup() to use RCU protected hash table"
> causes network latencies for me on s390.
>
> The testcase is quite simple and 100% reproducible on s390:
>
> Simply login via ssh to a remote system which has the above mentioned
> patch applied. Any action like pressing return now has significant
> latencies. Or in other words, working via such a connection becomes
> a pain ;)
>
> I haven't debugged it, however I assume the problem is that a) the
> commit introduces a synchronize_net() call und b) s390 kernels
> usually get compiled with CONFIG_HZ_100 while most other architectures
> use CONFIG_HZ_1000.
> If I change the kernel config to CONFIG_HZ_1000 the problem goes away,
> however I don't consider this a fix...
>
> Another reason why this hasn't been observed on x86 may or may not be
> that we haven't implemented CONFIG_HAVE_CONTEXT_TRACKING on s390 (yet).
> But that's just guessing...
CC Paul and Sasha
^ permalink raw reply
* Re: [PATCH V3.18] rtlwifi: rtl8192ee: Prevent log spamming for switch statements
From: Joe Perches @ 2014-10-11 18:14 UTC (permalink / raw)
To: Larry Finger; +Cc: linville, linux-wireless, troy_tan, netdev
In-Reply-To: <1413050393-6257-1-git-send-email-Larry.Finger@lwfinger.net>
On Sat, 2014-10-11 at 12:59 -0500, Larry Finger wrote:
> The driver logs a message when the default branch of switch statements are
> taken. Such information is useful when debugging, but these log items should
> not be seen for standard usage.
Hey Larry.
At some point, it'd be good to make RT_TRACE use
the generic dynamic_debug facility.
Something like the below, but I believe there are
some issues with include ordering and #define DEBUG
when dynamic_debug is not #defined.
---
drivers/net/wireless/rtlwifi/debug.h | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/net/wireless/rtlwifi/debug.h b/drivers/net/wireless/rtlwifi/debug.h
index fc794b3..fbc8185 100644
--- a/drivers/net/wireless/rtlwifi/debug.h
+++ b/drivers/net/wireless/rtlwifi/debug.h
@@ -178,17 +178,14 @@ do { \
do { \
if (unlikely(((comp) & rtlpriv->dbg.global_debugcomponents) && \
((level) <= rtlpriv->dbg.global_debuglevel))) { \
- printk(KERN_DEBUG KBUILD_MODNAME ":%s():<%lx-%x> " fmt, \
- __func__, in_interrupt(), in_atomic(), \
- ##__VA_ARGS__); \
+ pr_debug(fmt, ##__VA_ARGS__); \
} \
} while (0)
#define RTPRINT(rtlpriv, dbgtype, dbgflag, fmt, ...) \
do { \
if (unlikely(rtlpriv->dbg.dbgp_type[dbgtype] & dbgflag)) { \
- printk(KERN_DEBUG KBUILD_MODNAME ": " fmt, \
- ##__VA_ARGS__); \
+ pr_debug(fmt, ##__VA_ARGS__); \
} \
} while (0)
^ permalink raw reply related
* [PATCH V3.18] rtlwifi: rtl8192ee: Prevent log spamming for switch statements
From: Larry Finger @ 2014-10-11 17:59 UTC (permalink / raw)
To: linville-2XuSBdqkA4R54TAoqtyWWQ
Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA,
troy_tan-kXabqFNEczNtrwSWzY7KCg, Larry Finger,
netdev-u79uwXL29TY76Z2rM5mHXA
The driver logs a message when the default branch of switch statements are
taken. Such information is useful when debugging, but these log items should
not be seen for standard usage.
Signed-off-by: Larry Finger <Larry.Finger-tQ5ms3gMjBLk1uMJSBkQmQ@public.gmane.org>
---
drivers/net/wireless/rtlwifi/rtl8192ee/hw.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/wireless/rtlwifi/rtl8192ee/hw.c b/drivers/net/wireless/rtlwifi/rtl8192ee/hw.c
index dfdc9b2..1a87edc 100644
--- a/drivers/net/wireless/rtlwifi/rtl8192ee/hw.c
+++ b/drivers/net/wireless/rtlwifi/rtl8192ee/hw.c
@@ -362,7 +362,7 @@ void rtl92ee_get_hw_reg(struct ieee80211_hw *hw, u8 variable, u8 *val)
}
break;
default:
- RT_TRACE(rtlpriv, COMP_ERR, DBG_EMERG,
+ RT_TRACE(rtlpriv, COMP_ERR, DBG_DMESG,
"switch case not process %x\n", variable);
break;
}
@@ -591,7 +591,7 @@ void rtl92ee_set_hw_reg(struct ieee80211_hw *hw, u8 variable, u8 *val)
acm_ctrl &= (~ACMHW_BEQEN);
break;
default:
- RT_TRACE(rtlpriv, COMP_ERR, DBG_EMERG,
+ RT_TRACE(rtlpriv, COMP_ERR, DBG_DMESG,
"switch case not process\n");
break;
}
@@ -710,7 +710,7 @@ void rtl92ee_set_hw_reg(struct ieee80211_hw *hw, u8 variable, u8 *val)
}
break;
default:
- RT_TRACE(rtlpriv, COMP_ERR, DBG_EMERG,
+ RT_TRACE(rtlpriv, COMP_ERR, DBG_DMESG,
"switch case not process %x\n", variable);
break;
}
@@ -2424,7 +2424,7 @@ void rtl92ee_set_key(struct ieee80211_hw *hw, u32 key_index,
enc_algo = CAM_AES;
break;
default:
- RT_TRACE(rtlpriv, COMP_ERR, DBG_EMERG,
+ RT_TRACE(rtlpriv, COMP_ERR, DBG_DMESG,
"switch case not process\n");
enc_algo = CAM_TKIP;
break;
--
1.8.4.5
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH iproute2] netlink: extend buffers to 16K
From: Joe Perches @ 2014-10-11 17:00 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Stephen Hemminger, netdev
In-Reply-To: <1413045793.9362.48.camel@edumazet-glaptop2.roam.corp.google.com>
On Sat, 2014-10-11 at 09:43 -0700, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> Starting from linux-3.15 (commit 9063e21fb026, "netlink: autosize skb
> lengths"), kernel is able to send up to 16K in netlink replies.
>
> This change enables iproute2 commands to get bigger chunks,
> without breaking compatibility with old kernels.
Maybe a #define for the next time it's increased?
^ permalink raw reply
* [PATCH iproute2] netlink: extend buffers to 16K
From: Eric Dumazet @ 2014-10-11 16:43 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
From: Eric Dumazet <edumazet@google.com>
Starting from linux-3.15 (commit 9063e21fb026, "netlink: autosize skb
lengths"), kernel is able to send up to 16K in netlink replies.
This change enables iproute2 commands to get bigger chunks,
without breaking compatibility with old kernels.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
lib/libnetlink.c | 4 ++--
misc/ss.c | 8 ++++----
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/lib/libnetlink.c b/lib/libnetlink.c
index 9e2a795..8d504a9 100644
--- a/lib/libnetlink.c
+++ b/lib/libnetlink.c
@@ -420,7 +420,7 @@ int rtnl_listen(struct rtnl_handle *rtnl,
.msg_iov = &iov,
.msg_iovlen = 1,
};
- char buf[8192];
+ char buf[16384];
memset(&nladdr, 0, sizeof(nladdr));
nladdr.nl_family = AF_NETLINK;
@@ -486,7 +486,7 @@ int rtnl_from_file(FILE *rtnl, rtnl_filter_t handler,
{
int status;
struct sockaddr_nl nladdr;
- char buf[8192];
+ char buf[16384];
struct nlmsghdr *h = (void*)buf;
memset(&nladdr, 0, sizeof(nladdr));
diff --git a/misc/ss.c b/misc/ss.c
index 2420b51..d185040 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -1864,7 +1864,7 @@ static int inet_show_netlink(struct filter *f, FILE *dump_fp, int protocol)
int fd, family;
struct sockaddr_nl nladdr;
struct msghdr msg;
- char buf[8192];
+ char buf[16384];
struct iovec iov[3];
if ((fd = socket(AF_NETLINK, SOCK_RAW, NETLINK_INET_DIAG)) < 0)
@@ -1980,7 +1980,7 @@ done:
static int tcp_show_netlink_file(struct filter *f)
{
FILE *fp;
- char buf[8192];
+ char buf[16384];
if ((fp = fopen(getenv("TCPDIAG_FILE"), "r")) == NULL) {
perror("fopen($TCPDIAG_FILE)");
@@ -2497,7 +2497,7 @@ static int handle_netlink_request(struct filter *f, FILE *dump_fp,
int (* show_one_sock)(struct nlmsghdr *nlh, struct filter *f))
{
int fd;
- char buf[8192];
+ char buf[16384];
if ((fd = socket(AF_NETLINK, SOCK_RAW, NETLINK_INET_DIAG)) < 0)
return -1;
@@ -2782,7 +2782,7 @@ static int packet_show_netlink(struct filter *f, FILE *dump_fp)
struct nlmsghdr nlh;
struct packet_diag_req r;
} req;
- char buf[8192];
+ char buf[16384];
if ((fd = socket(AF_NETLINK, SOCK_RAW, NETLINK_INET_DIAG)) < 0)
return -1;
^ permalink raw reply related
* [PATCH 06/12] net: ks8842: use dmaengine_terminate_all() API
From: Vinod Koul @ 2014-10-11 15:40 UTC (permalink / raw)
To: dmaengine
Cc: Vinod Koul, Mark Brown, Bartlomiej Zolnierkiewicz, Kyungmin Park,
Dan Williams, netdev, linux-kernel
In-Reply-To: <1413041973-28146-1-git-send-email-vinod.koul@intel.com>
The drivers should use dmaengine_terminate_all() API instead of
accessing the device_control which will be deprecated soon
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
---
drivers/net/ethernet/micrel/ks8842.c | 6 ++----
1 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/micrel/ks8842.c b/drivers/net/ethernet/micrel/ks8842.c
index 822616e..0c33b92 100644
--- a/drivers/net/ethernet/micrel/ks8842.c
+++ b/drivers/net/ethernet/micrel/ks8842.c
@@ -875,13 +875,11 @@ static void ks8842_stop_dma(struct ks8842_adapter *adapter)
tx_ctl->adesc = NULL;
if (tx_ctl->chan)
- tx_ctl->chan->device->device_control(tx_ctl->chan,
- DMA_TERMINATE_ALL, 0);
+ dmaengine_terminate_all(tx_ctl->chan);
rx_ctl->adesc = NULL;
if (rx_ctl->chan)
- rx_ctl->chan->device->device_control(rx_ctl->chan,
- DMA_TERMINATE_ALL, 0);
+ dmaengine_terminate_all(rx_ctl->chan);
if (sg_dma_address(&rx_ctl->sg))
dma_unmap_single(adapter->dev, sg_dma_address(&rx_ctl->sg),
--
1.7.0.4
^ permalink raw reply related
* [PATCH 00/12] dmaengine: remove users of device_control
From: Vinod Koul @ 2014-10-11 15:39 UTC (permalink / raw)
To: dmaengine
Cc: Vinod Koul, Viresh Kumar, Tejun Heo, Linus Walleij, Dan Williams,
Guennadi Liakhovetski, Mauro Carvalho Chehab, David Woodhouse,
Brian Norris, Nicolas Ferre, Mark Brown, Greg Kroah-Hartman,
Jiri Slaby, Felipe Balbi, Jean-Christophe Plagniol-Villard,
Tomi Valkeinen, Liam Girdwood, Jaroslav Kysela, Takashi Iwai,
Jingoo Han, Bartlomiej Zolnierkiewicz, Laurent Pinchart
The recent discussion [1] on the API have resulted in moving away from
device_control ioctl method to proper channel APIs.
There are still few users on the device_control which should use the wrappers
existing rather than access device_control.
This will aid us in deprecating and removing device_control, possibly after
the merge window.
These can be merged thru respective subsystem tree or dmaengine tree. Either
way please just let me know.
Feng's kbuild has tested these as well [2]
[1]: http://www.spinics.net/lists/dmaengine/msg02212.html
[2]: http://git.infradead.org/users/vkoul/slave-dma.git/shortlog/refs/heads/topic/dma_control_cleanup
Vinod Koul (12):
pata_arasan_cf: use dmaengine_terminate_all() API
dmaengine: coh901318: use dmaengine_terminate_all() API
[media] V4L2: mx3_camer: use dmaengine_pause() API
mtd: fsmc_nand: use dmaengine_terminate_all() API
mtd: sh_flctl: use dmaengine_terminate_all() API
net: ks8842: use dmaengine_terminate_all() API
spi/atmel: use dmaengine_terminate_all() API
spi/spi-dw-mid.c: use dmaengine_slave_config() API
serial: sh-sci: use dmaengine_terminate_all() API
usb: musb: ux500_dma: use dmaengine_xxx() APIs
ASoC: txx9: use dmaengine_terminate_all() API
video: mx3fb: use dmaengine_terminate_all() API
drivers/ata/pata_arasan_cf.c | 5 ++---
drivers/dma/coh901318.c | 2 +-
drivers/media/platform/soc_camera/mx3_camera.c | 6 ++----
drivers/mtd/nand/fsmc_nand.c | 2 +-
drivers/mtd/nand/sh_flctl.c | 2 +-
drivers/net/ethernet/micrel/ks8842.c | 6 ++----
drivers/spi/spi-atmel.c | 6 ++----
drivers/spi/spi-dw-mid.c | 6 ++----
drivers/tty/serial/sh-sci.c | 2 +-
drivers/usb/musb/ux500_dma.c | 7 ++-----
drivers/video/fbdev/mx3fb.c | 3 +--
sound/soc/txx9/txx9aclc.c | 7 +++----
12 files changed, 20 insertions(+), 34 deletions(-)
^ permalink raw reply
* Re: [PATCH net-next RFC 3/3] virtio-net: conditionally enable tx interrupt
From: Eric Dumazet @ 2014-10-11 14:48 UTC (permalink / raw)
To: Jason Wang; +Cc: kvm, mst, netdev, linux-kernel, virtualization, linux-api
In-Reply-To: <1413011806-3813-4-git-send-email-jasowang@redhat.com>
On Sat, 2014-10-11 at 15:16 +0800, Jason Wang wrote:
> We free transmitted packets in ndo_start_xmit() in the past to get better
> performance in the past. One side effect is that skb_orphan() needs to be
> called in ndo_start_xmit() which makes sk_wmem_alloc not accurate in
> fact. For TCP protocol, this means several optimization could not work well
> such as TCP small queue and auto corking. This can lead extra low
> throughput of small packets stream.
>
> Thanks to the urgent descriptor support. This patch tries to solve this
> issue by enable the tx interrupt selectively for stream packets. This means
> we don't need to orphan TCP stream packets in ndo_start_xmit() but enable
> tx interrupt for those packets. After we get tx interrupt, a tx napi was
> scheduled to free those packets.
>
> With this method, sk_wmem_alloc of TCP socket were more accurate than in
> the past which let TCP can batch more through TSQ and auto corking.
>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
> drivers/net/virtio_net.c | 164 ++++++++++++++++++++++++++++++++++++-----------
> 1 file changed, 128 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 5810841..b450fc4 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -72,6 +72,8 @@ struct send_queue {
>
> /* Name of the send queue: output.$index */
> char name[40];
> +
> + struct napi_struct napi;
> };
>
> /* Internal representation of a receive virtqueue */
> @@ -217,15 +219,40 @@ static struct page *get_a_page(struct receive_queue *rq, gfp_t gfp_mask)
> return p;
> }
>
> +static int free_old_xmit_skbs(struct send_queue *sq, int budget)
> +{
> + struct sk_buff *skb;
> + unsigned int len;
> + struct virtnet_info *vi = sq->vq->vdev->priv;
> + struct virtnet_stats *stats = this_cpu_ptr(vi->stats);
> + int sent = 0;
> +
> + while (sent < budget &&
> + (skb = virtqueue_get_buf(sq->vq, &len)) != NULL) {
> + pr_debug("Sent skb %p\n", skb);
> +
> + u64_stats_update_begin(&stats->tx_syncp);
> + stats->tx_bytes += skb->len;
> + stats->tx_packets++;
> + u64_stats_update_end(&stats->tx_syncp);
> +
> + dev_kfree_skb_any(skb);
> + sent++;
> + }
> +
You could accumulate skb->len in a totlen var, and perform a single
u64_stats_update_begin(&stats->tx_syncp);
stats->tx_bytes += totlen;
stats->tx_packets += sent;
u64_stats_update_end(&stats->tx_syncp);
after the loop.
> + return sent;
> +}
> +
...
> +
> +static bool virtnet_skb_needs_intr(struct sk_buff *skb)
> +{
> + union {
> + unsigned char *network;
> + struct iphdr *ipv4;
> + struct ipv6hdr *ipv6;
> + } hdr;
> + struct tcphdr *th = tcp_hdr(skb);
> + u16 payload_len;
> +
> + hdr.network = skb_network_header(skb);
> +
> + /* Only IPv4/IPv6 with TCP is supported */
Oh well, yet another packet flow dissector :)
If most packets were caught by your implementation, you could use it
for fast patj and fallback to skb_flow_dissect() for encapsulated
traffic.
struct flow_keys keys;
if (!skb_flow_dissect(skb, &keys))
return false;
if (keys.ip_proto != IPPROTO_TCP)
return false;
then check __skb_get_poff() how to get th, and check if there is some
payload...
> + if ((skb->protocol == htons(ETH_P_IP)) &&
> + hdr.ipv4->protocol == IPPROTO_TCP) {
> + payload_len = ntohs(hdr.ipv4->tot_len) - hdr.ipv4->ihl * 4 -
> + th->doff * 4;
> + } else if ((skb->protocol == htons(ETH_P_IPV6) ||
> + hdr.ipv6->nexthdr == IPPROTO_TCP)) {
> + payload_len = ntohs(hdr.ipv6->payload_len) - th->doff * 4;
> + } else {
> + return false;
> + }
> +
> + /* We don't want to dealy packet with PUSH bit and pure ACK packet */
> + if (!th->psh && payload_len)
> + return true;
> +
> + return false;
> }
^ permalink raw reply
* Question/Problem - Connection reset - Ralink RT3290 - HP Split 13 x2
From: Vincent Fortier @ 2014-10-11 12:24 UTC (permalink / raw)
To: netdev
Hopefully I'm at the right place to ask for help with the problem I
have with my Wifi connection: It stops really really really often.
Sometimes it works for up to
an hour but usually within 10-15 minutes it just stop/reset and then
does it every few minutes... My presumption is that networkmanager is
able of restarting it most of the time until I get tired of it and
end-up restarting my laptop. Note that the issue is not happening
under win8.1.
Tested using 3.16 & 3.17.
Device:
03:00.0 Network controller: Ralink corp. RT3290 Wireless 802.11n 1T/1R PCIe
Subsystem: Hewlett-Packard Company Ralink RT3290LE 802.11bgn 1x1
Wi-Fi and Bluetooth 4.0 Combo Adapter
Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop-
ParErr- Stepping- SERR- FastB2B- DisINTx-
Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort-
<TAbort- <MAbort- >SERR- <PERR- INTx-
Latency: 0, Cache Line Size: 64 bytes
Interrupt: pin A routed to IRQ 18
Region 0: Memory at b0410000 (32-bit, non-prefetchable) [size=64K]
Capabilities: <access denied>
Kernel driver in use: rt2800pci
Getting the following messsages in syslog which I presume may be related:
Oct 10 17:45:36 brutus kernel: [ 4.937845] ieee80211 phy0:
rt2x00_set_rt: Info - RT chipset 3290, rev 0015 detected
Oct 10 17:45:36 brutus kernel: [ 4.941563] ieee80211 phy0:
rt2x00_set_rf: Info - RF chipset 3290 detected
Oct 10 17:45:36 brutus kernel: [ 4.983868] ieee80211 phy0: Selected
rate control algorithm 'minstrel_ht'
Oct 10 17:45:37 brutus NetworkManager[877]: <info> rfkill0: found WiFi
radio killswitch (at
/sys/devices/pci0000:00/0000:
00:1c.2/0000:03:00.0/ieee80211/phy0/rfkill0)
(driver rt2800pci)
Oct 10 17:45:37 brutus kernel: [ 5.610165] ieee80211 phy0:
rt2x00lib_request_firmware: Info - Loading firmware file 'rt3290.bin'
Oct 10 17:45:37 brutus kernel: [ 5.611453] ieee80211 phy0:
rt2x00lib_request_firmware: Info - Firmware detected - version: 0.37
Oct 10 17:46:01 brutus kernel: [ 29.621979] ieee80211 phy0:
rt2x00queue_flush_queue: Warning - Queue 2 failed to flush
Oct 10 17:46:01 brutus kernel: [ 30.070125] ieee80211 phy0:
rt2x00queue_flush_queue: Warning - Queue 0 failed to flush
Oct 10 17:46:01 brutus kernel: [ 30.230153] ieee80211 phy0:
rt2x00queue_flush_queue: Warning - Queue 2 failed to flush
Oct 10 17:46:02 brutus kernel: [ 30.678194] ieee80211 phy0:
rt2x00queue_flush_queue: Warning - Queue 2 failed to flush
Oct 10 17:46:02 brutus kernel: [ 31.190166] ieee80211 phy0:
rt2x00queue_flush_queue: Warning - Queue 0 failed to flush
Oct 10 17:46:02 brutus kernel: [ 31.350260] ieee80211 phy0:
rt2x00queue_flush_queue: Warning - Queue 2 failed to flush
Oct 10 17:46:03 brutus kernel: [ 31.798309] ieee80211 phy0:
rt2x00queue_flush_queue: Warning - Queue 2 failed to flush
Oct 10 17:46:03 brutus kernel: [ 32.246352] ieee80211 phy0:
rt2x00queue_flush_queue: Warning - Queue 0 failed to flush
Oct 10 17:46:03 brutus kernel: [ 32.406369] ieee80211 phy0:
....
It eventually renegociate network every few minutes:
[ 6.987927] cfg80211: Calling CRDA for country: CA
[ 6.991802] cfg80211: Regulatory domain changed to country: CA
[ 6.991809] cfg80211: DFS Master region: unset
[ 6.991813] cfg80211: (start_freq - end_freq @ bandwidth),
(max_antenna_gain, max_eirp), (dfs_cac_time)
[ 6.991819] cfg80211: (2402000 KHz - 2472000 KHz @ 40000 KHz),
(300 mBi, 2700 mBm), (N/A)
[ 6.991824] cfg80211: (5170000 KHz - 5250000 KHz @ 40000 KHz),
(300 mBi, 1700 mBm), (N/A)
[ 6.991828] cfg80211: (5250000 KHz - 5330000 KHz @ 40000 KHz),
(300 mBi, 2000 mBm), (0 s)
[ 6.991832] cfg80211: (5490000 KHz - 5710000 KHz @ 40000 KHz),
(300 mBi, 2000 mBm), (0 s)
[ 6.991836] cfg80211: (5735000 KHz - 5835000 KHz @ 40000 KHz),
(300 mBi, 3000 mBm), (N/A)
Help much appreciated. Thnx in advance.
- vin
^ permalink raw reply
* [PATCH 10/12] isdn/capi: handle CAPI 2.0 message parser failures
From: Tilman Schmidt @ 2014-10-11 11:46 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Dave Jones, Hansjoerg Lipp, Karsten Keil,
isdn4linux
In-Reply-To: <cover.1413021630.git.tilman@imap.cc>
Have callers of capi_cmsg2message and capi_message2cmsg handle
non-zero return values indicating failure.
Signed-off-by: Tilman Schmidt <tilman@imap.cc>
---
Note: The issue about printk(KERN_ERR which checkpatch is complaining
about is present in many more places in the original source file. I
deliberately preferred keeping the style consistent within the file.
drivers/isdn/capi/capidrv.c | 24 +++++--
drivers/isdn/gigaset/capi.c | 148 +++++++++++++++++++++++++++++++++++++-------
2 files changed, 145 insertions(+), 27 deletions(-)
diff --git a/drivers/isdn/capi/capidrv.c b/drivers/isdn/capi/capidrv.c
index fd6d28f..1cc6ca8 100644
--- a/drivers/isdn/capi/capidrv.c
+++ b/drivers/isdn/capi/capidrv.c
@@ -506,7 +506,10 @@ static void send_message(capidrv_contr *card, _cmsg *cmsg)
struct sk_buff *skb;
size_t len;
- capi_cmsg2message(cmsg, cmsg->buf);
+ if (capi_cmsg2message(cmsg, cmsg->buf)) {
+ printk(KERN_ERR "capidrv::send_message: parser failure\n");
+ return;
+ }
len = CAPIMSG_LEN(cmsg->buf);
skb = alloc_skb(len, GFP_ATOMIC);
if (!skb) {
@@ -1578,7 +1581,12 @@ static _cmsg s_cmsg;
static void capidrv_recv_message(struct capi20_appl *ap, struct sk_buff *skb)
{
- capi_message2cmsg(&s_cmsg, skb->data);
+ if (capi_message2cmsg(&s_cmsg, skb->data)) {
+ printk(KERN_ERR "capidrv: applid=%d: received invalid message\n",
+ ap->applid);
+ kfree_skb(skb);
+ return;
+ }
if (debugmode > 3) {
_cdebbuf *cdb = capi_cmsg2str(&s_cmsg);
@@ -1903,7 +1911,11 @@ static int capidrv_command(isdn_ctrl *c, capidrv_contr *card)
NULL, /* Useruserdata */
NULL /* Facilitydataarray */
);
- capi_cmsg2message(&cmdcmsg, cmdcmsg.buf);
+ if (capi_cmsg2message(&cmdcmsg, cmdcmsg.buf)) {
+ printk(KERN_ERR "capidrv-%d: capidrv_command: parser failure\n",
+ card->contrnr);
+ return -EINVAL;
+ }
plci_change_state(card, bchan->plcip, EV_PLCI_CONNECT_RESP);
send_message(card, &cmdcmsg);
return 0;
@@ -2090,7 +2102,11 @@ static int if_sendbuf(int id, int channel, int doack, struct sk_buff *skb)
if (capidrv_add_ack(nccip, datahandle, doack ? (int)skb->len : -1) < 0)
return 0;
- capi_cmsg2message(&sendcmsg, sendcmsg.buf);
+ if (capi_cmsg2message(&sendcmsg, sendcmsg.buf)) {
+ printk(KERN_ERR "capidrv-%d: if_sendbuf: parser failure\n",
+ card->contrnr);
+ return -EINVAL;
+ }
msglen = CAPIMSG_LEN(sendcmsg.buf);
if (skb_headroom(skb) < msglen) {
struct sk_buff *nskb = skb_realloc_headroom(skb, msglen);
diff --git a/drivers/isdn/gigaset/capi.c b/drivers/isdn/gigaset/capi.c
index 47e2a91..ccec777 100644
--- a/drivers/isdn/gigaset/capi.c
+++ b/drivers/isdn/gigaset/capi.c
@@ -647,7 +647,13 @@ int gigaset_isdn_icall(struct at_state_t *at_state)
__func__);
break;
}
- capi_cmsg2message(&iif->hcmsg, __skb_put(skb, msgsize));
+ if (capi_cmsg2message(&iif->hcmsg,
+ __skb_put(skb, msgsize))) {
+ dev_err(cs->dev, "%s: message parser failure\n",
+ __func__);
+ dev_kfree_skb_any(skb);
+ break;
+ }
dump_cmsg(DEBUG_CMD, __func__, &iif->hcmsg);
/* add to listeners on this B channel, update state */
@@ -693,7 +699,12 @@ static void send_disconnect_ind(struct bc_state *bcs,
dev_err(cs->dev, "%s: out of memory\n", __func__);
return;
}
- capi_cmsg2message(&iif->hcmsg, __skb_put(skb, CAPI_DISCONNECT_IND_LEN));
+ if (capi_cmsg2message(&iif->hcmsg,
+ __skb_put(skb, CAPI_DISCONNECT_IND_LEN))) {
+ dev_err(cs->dev, "%s: message parser failure\n", __func__);
+ dev_kfree_skb_any(skb);
+ return;
+ }
dump_cmsg(DEBUG_CMD, __func__, &iif->hcmsg);
capi_ctr_handle_message(&iif->ctr, ap->id, skb);
}
@@ -723,8 +734,12 @@ static void send_disconnect_b3_ind(struct bc_state *bcs,
dev_err(cs->dev, "%s: out of memory\n", __func__);
return;
}
- capi_cmsg2message(&iif->hcmsg,
- __skb_put(skb, CAPI_DISCONNECT_B3_IND_BASELEN));
+ if (capi_cmsg2message(&iif->hcmsg,
+ __skb_put(skb, CAPI_DISCONNECT_B3_IND_BASELEN))) {
+ dev_err(cs->dev, "%s: message parser failure\n", __func__);
+ dev_kfree_skb_any(skb);
+ return;
+ }
dump_cmsg(DEBUG_CMD, __func__, &iif->hcmsg);
capi_ctr_handle_message(&iif->ctr, ap->id, skb);
}
@@ -789,7 +804,11 @@ void gigaset_isdn_connD(struct bc_state *bcs)
dev_err(cs->dev, "%s: out of memory\n", __func__);
return;
}
- capi_cmsg2message(&iif->hcmsg, __skb_put(skb, msgsize));
+ if (capi_cmsg2message(&iif->hcmsg, __skb_put(skb, msgsize))) {
+ dev_err(cs->dev, "%s: message parser failure\n", __func__);
+ dev_kfree_skb_any(skb);
+ return;
+ }
dump_cmsg(DEBUG_CMD, __func__, &iif->hcmsg);
capi_ctr_handle_message(&iif->ctr, ap->id, skb);
}
@@ -889,7 +908,11 @@ void gigaset_isdn_connB(struct bc_state *bcs)
dev_err(cs->dev, "%s: out of memory\n", __func__);
return;
}
- capi_cmsg2message(&iif->hcmsg, __skb_put(skb, msgsize));
+ if (capi_cmsg2message(&iif->hcmsg, __skb_put(skb, msgsize))) {
+ dev_err(cs->dev, "%s: message parser failure\n", __func__);
+ dev_kfree_skb_any(skb);
+ return;
+ }
dump_cmsg(DEBUG_CMD, __func__, &iif->hcmsg);
capi_ctr_handle_message(&iif->ctr, ap->id, skb);
}
@@ -1096,13 +1119,19 @@ static void send_conf(struct gigaset_capi_ctr *iif,
struct sk_buff *skb,
u16 info)
{
+ struct cardstate *cs = iif->ctr.driverdata;
+
/*
* _CONF replies always only have NCCI and Info parameters
* so they'll fit into the _REQ message skb
*/
capi_cmsg_answer(&iif->acmsg);
iif->acmsg.Info = info;
- capi_cmsg2message(&iif->acmsg, skb->data);
+ if (capi_cmsg2message(&iif->acmsg, skb->data)) {
+ dev_err(cs->dev, "%s: message parser failure\n", __func__);
+ dev_kfree_skb_any(skb);
+ return;
+ }
__skb_trim(skb, CAPI_STDCONF_LEN);
dump_cmsg(DEBUG_CMD, __func__, &iif->acmsg);
capi_ctr_handle_message(&iif->ctr, ap->id, skb);
@@ -1124,7 +1153,11 @@ static void do_facility_req(struct gigaset_capi_ctr *iif,
static u8 confparam[10]; /* max. 9 octets + length byte */
/* decode message */
- capi_message2cmsg(cmsg, skb->data);
+ if (capi_message2cmsg(cmsg, skb->data)) {
+ dev_err(cs->dev, "%s: message parser failure\n", __func__);
+ dev_kfree_skb_any(skb);
+ return;
+ }
dump_cmsg(DEBUG_CMD, __func__, cmsg);
/*
@@ -1223,6 +1256,7 @@ static void do_facility_req(struct gigaset_capi_ctr *iif,
}
/* send FACILITY_CONF with given Info and confirmation parameter */
+ dev_kfree_skb_any(skb);
capi_cmsg_answer(cmsg);
cmsg->Info = info;
cmsg->FacilityConfirmationParameter = confparam;
@@ -1232,7 +1266,11 @@ static void do_facility_req(struct gigaset_capi_ctr *iif,
dev_err(cs->dev, "%s: out of memory\n", __func__);
return;
}
- capi_cmsg2message(cmsg, __skb_put(cskb, msgsize));
+ if (capi_cmsg2message(cmsg, __skb_put(cskb, msgsize))) {
+ dev_err(cs->dev, "%s: message parser failure\n", __func__);
+ dev_kfree_skb_any(cskb);
+ return;
+ }
dump_cmsg(DEBUG_CMD, __func__, cmsg);
capi_ctr_handle_message(&iif->ctr, ap->id, cskb);
}
@@ -1246,8 +1284,14 @@ static void do_listen_req(struct gigaset_capi_ctr *iif,
struct gigaset_capi_appl *ap,
struct sk_buff *skb)
{
+ struct cardstate *cs = iif->ctr.driverdata;
+
/* decode message */
- capi_message2cmsg(&iif->acmsg, skb->data);
+ if (capi_message2cmsg(&iif->acmsg, skb->data)) {
+ dev_err(cs->dev, "%s: message parser failure\n", __func__);
+ dev_kfree_skb_any(skb);
+ return;
+ }
dump_cmsg(DEBUG_CMD, __func__, &iif->acmsg);
/* store listening parameters */
@@ -1264,8 +1308,14 @@ static void do_alert_req(struct gigaset_capi_ctr *iif,
struct gigaset_capi_appl *ap,
struct sk_buff *skb)
{
+ struct cardstate *cs = iif->ctr.driverdata;
+
/* decode message */
- capi_message2cmsg(&iif->acmsg, skb->data);
+ if (capi_message2cmsg(&iif->acmsg, skb->data)) {
+ dev_err(cs->dev, "%s: message parser failure\n", __func__);
+ dev_kfree_skb_any(skb);
+ return;
+ }
dump_cmsg(DEBUG_CMD, __func__, &iif->acmsg);
send_conf(iif, ap, skb, CapiAlertAlreadySent);
}
@@ -1290,7 +1340,11 @@ static void do_connect_req(struct gigaset_capi_ctr *iif,
u16 info;
/* decode message */
- capi_message2cmsg(cmsg, skb->data);
+ if (capi_message2cmsg(cmsg, skb->data)) {
+ dev_err(cs->dev, "%s: message parser failure\n", __func__);
+ dev_kfree_skb_any(skb);
+ return;
+ }
dump_cmsg(DEBUG_CMD, __func__, cmsg);
/* get free B channel & construct PLCI */
@@ -1577,7 +1631,11 @@ static void do_connect_resp(struct gigaset_capi_ctr *iif,
int channel;
/* decode message */
- capi_message2cmsg(cmsg, skb->data);
+ if (capi_message2cmsg(cmsg, skb->data)) {
+ dev_err(cs->dev, "%s: message parser failure\n", __func__);
+ dev_kfree_skb_any(skb);
+ return;
+ }
dump_cmsg(DEBUG_CMD, __func__, cmsg);
dev_kfree_skb_any(skb);
@@ -1743,7 +1801,11 @@ static void do_connect_b3_req(struct gigaset_capi_ctr *iif,
int channel;
/* decode message */
- capi_message2cmsg(cmsg, skb->data);
+ if (capi_message2cmsg(cmsg, skb->data)) {
+ dev_err(cs->dev, "%s: message parser failure\n", __func__);
+ dev_kfree_skb_any(skb);
+ return;
+ }
dump_cmsg(DEBUG_CMD, __func__, cmsg);
/* extract and check channel number from PLCI */
@@ -1788,7 +1850,11 @@ static void do_connect_b3_resp(struct gigaset_capi_ctr *iif,
u8 command;
/* decode message */
- capi_message2cmsg(cmsg, skb->data);
+ if (capi_message2cmsg(cmsg, skb->data)) {
+ dev_err(cs->dev, "%s: message parser failure\n", __func__);
+ dev_kfree_skb_any(skb);
+ return;
+ }
dump_cmsg(DEBUG_CMD, __func__, cmsg);
/* extract and check channel number and NCCI */
@@ -1828,7 +1894,11 @@ static void do_connect_b3_resp(struct gigaset_capi_ctr *iif,
capi_cmsg_header(cmsg, ap->id, command, CAPI_IND,
ap->nextMessageNumber++, cmsg->adr.adrNCCI);
__skb_trim(skb, msgsize);
- capi_cmsg2message(cmsg, skb->data);
+ if (capi_cmsg2message(cmsg, skb->data)) {
+ dev_err(cs->dev, "%s: message parser failure\n", __func__);
+ dev_kfree_skb_any(skb);
+ return;
+ }
dump_cmsg(DEBUG_CMD, __func__, cmsg);
capi_ctr_handle_message(&iif->ctr, ap->id, skb);
}
@@ -1850,7 +1920,11 @@ static void do_disconnect_req(struct gigaset_capi_ctr *iif,
int channel;
/* decode message */
- capi_message2cmsg(cmsg, skb->data);
+ if (capi_message2cmsg(cmsg, skb->data)) {
+ dev_err(cs->dev, "%s: message parser failure\n", __func__);
+ dev_kfree_skb_any(skb);
+ return;
+ }
dump_cmsg(DEBUG_CMD, __func__, cmsg);
/* extract and check channel number from PLCI */
@@ -1906,8 +1980,14 @@ static void do_disconnect_req(struct gigaset_capi_ctr *iif,
kfree(b3cmsg);
return;
}
- capi_cmsg2message(b3cmsg,
- __skb_put(b3skb, CAPI_DISCONNECT_B3_IND_BASELEN));
+ if (capi_cmsg2message(b3cmsg,
+ __skb_put(b3skb, CAPI_DISCONNECT_B3_IND_BASELEN))) {
+ dev_err(cs->dev, "%s: message parser failure\n",
+ __func__);
+ kfree(b3cmsg);
+ dev_kfree_skb_any(b3skb);
+ return;
+ }
dump_cmsg(DEBUG_CMD, __func__, b3cmsg);
kfree(b3cmsg);
capi_ctr_handle_message(&iif->ctr, ap->id, b3skb);
@@ -1938,7 +2018,11 @@ static void do_disconnect_b3_req(struct gigaset_capi_ctr *iif,
int channel;
/* decode message */
- capi_message2cmsg(cmsg, skb->data);
+ if (capi_message2cmsg(cmsg, skb->data)) {
+ dev_err(cs->dev, "%s: message parser failure\n", __func__);
+ dev_kfree_skb_any(skb);
+ return;
+ }
dump_cmsg(DEBUG_CMD, __func__, cmsg);
/* extract and check channel number and NCCI */
@@ -2055,8 +2139,14 @@ static void do_reset_b3_req(struct gigaset_capi_ctr *iif,
struct gigaset_capi_appl *ap,
struct sk_buff *skb)
{
+ struct cardstate *cs = iif->ctr.driverdata;
+
/* decode message */
- capi_message2cmsg(&iif->acmsg, skb->data);
+ if (capi_message2cmsg(&iif->acmsg, skb->data)) {
+ dev_err(cs->dev, "%s: message parser failure\n", __func__);
+ dev_kfree_skb_any(skb);
+ return;
+ }
dump_cmsg(DEBUG_CMD, __func__, &iif->acmsg);
send_conf(iif, ap, skb,
CapiResetProcedureNotSupportedByCurrentProtocol);
@@ -2069,8 +2159,14 @@ static void do_unsupported(struct gigaset_capi_ctr *iif,
struct gigaset_capi_appl *ap,
struct sk_buff *skb)
{
+ struct cardstate *cs = iif->ctr.driverdata;
+
/* decode message */
- capi_message2cmsg(&iif->acmsg, skb->data);
+ if (capi_message2cmsg(&iif->acmsg, skb->data)) {
+ dev_err(cs->dev, "%s: message parser failure\n", __func__);
+ dev_kfree_skb_any(skb);
+ return;
+ }
dump_cmsg(DEBUG_CMD, __func__, &iif->acmsg);
send_conf(iif, ap, skb, CapiMessageNotSupportedInCurrentState);
}
@@ -2082,8 +2178,14 @@ static void do_nothing(struct gigaset_capi_ctr *iif,
struct gigaset_capi_appl *ap,
struct sk_buff *skb)
{
+ struct cardstate *cs = iif->ctr.driverdata;
+
/* decode message */
- capi_message2cmsg(&iif->acmsg, skb->data);
+ if (capi_message2cmsg(&iif->acmsg, skb->data)) {
+ dev_err(cs->dev, "%s: message parser failure\n", __func__);
+ dev_kfree_skb_any(skb);
+ return;
+ }
dump_cmsg(DEBUG_CMD, __func__, &iif->acmsg);
dev_kfree_skb_any(skb);
}
--
1.9.2.459.g68773ac
^ permalink raw reply related
* [PATCH 04/12] isdn/gigaset: fix NULL pointer dereference
From: Tilman Schmidt @ 2014-10-11 11:46 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Dave Jones, Hansjoerg Lipp, Karsten Keil,
isdn4linux
In-Reply-To: <cover.1413021630.git.tilman@imap.cc>
In do_action, a NULL pointer might be passed to function start_dial
which will dereference it.
Fix by adding a check for NULL before the call.
Spotted with Coverity.
Signed-off-by: Tilman Schmidt <tilman@imap.cc>
---
drivers/isdn/gigaset/ev-layer.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/isdn/gigaset/ev-layer.c b/drivers/isdn/gigaset/ev-layer.c
index dcae14a..0f699eb 100644
--- a/drivers/isdn/gigaset/ev-layer.c
+++ b/drivers/isdn/gigaset/ev-layer.c
@@ -1380,6 +1380,11 @@ static void do_action(int action, struct cardstate *cs,
/* events from the LL */
case ACT_DIAL:
+ if (!ev->ptr) {
+ *p_genresp = 1;
+ *p_resp_code = RSP_ERROR;
+ break;
+ }
start_dial(at_state, ev->ptr, ev->parameter);
break;
case ACT_ACCEPT:
--
1.9.2.459.g68773ac
^ permalink raw reply related
* [PATCH 05/12] isdn/gigaset: fix non-heap pointer deallocation
From: Tilman Schmidt @ 2014-10-11 11:46 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Dave Jones, Hansjoerg Lipp, Karsten Keil,
isdn4linux
In-Reply-To: <cover.1413021630.git.tilman@imap.cc>
at_state structures may be allocated individually or as part of a
cardstate or bc_state structure. The disconnect() function handled
both cases, creating a risk that it might try to deallocate an
at_state structure that had not been allocated individually.
Fix by splitting disconnect() into two variants handling cases
with and without an associated B channel separately, and adding
an explicit check.
Spotted with Coverity.
Signed-off-by: Tilman Schmidt <tilman@imap.cc>
---
drivers/isdn/gigaset/ev-layer.c | 111 +++++++++++++++++++++++++---------------
1 file changed, 70 insertions(+), 41 deletions(-)
diff --git a/drivers/isdn/gigaset/ev-layer.c b/drivers/isdn/gigaset/ev-layer.c
index 0f699eb..c8ced12 100644
--- a/drivers/isdn/gigaset/ev-layer.c
+++ b/drivers/isdn/gigaset/ev-layer.c
@@ -604,14 +604,14 @@ void gigaset_handle_modem_response(struct cardstate *cs)
}
EXPORT_SYMBOL_GPL(gigaset_handle_modem_response);
-/* disconnect
+/* disconnect_nobc
* process closing of connection associated with given AT state structure
+ * without B channel
*/
-static void disconnect(struct at_state_t **at_state_p)
+static void disconnect_nobc(struct at_state_t **at_state_p,
+ struct cardstate *cs)
{
unsigned long flags;
- struct bc_state *bcs = (*at_state_p)->bcs;
- struct cardstate *cs = (*at_state_p)->cs;
spin_lock_irqsave(&cs->lock, flags);
++(*at_state_p)->seq_index;
@@ -622,23 +622,44 @@ static void disconnect(struct at_state_t **at_state_p)
gig_dbg(DEBUG_EVENT, "Scheduling PC_UMMODE");
cs->commands_pending = 1;
}
- spin_unlock_irqrestore(&cs->lock, flags);
- if (bcs) {
- /* B channel assigned: invoke hardware specific handler */
- cs->ops->close_bchannel(bcs);
- /* notify LL */
- if (bcs->chstate & (CHS_D_UP | CHS_NOTIFY_LL)) {
- bcs->chstate &= ~(CHS_D_UP | CHS_NOTIFY_LL);
- gigaset_isdn_hupD(bcs);
- }
- } else {
- /* no B channel assigned: just deallocate */
- spin_lock_irqsave(&cs->lock, flags);
+ /* check for and deallocate temporary AT state */
+ if (!list_empty(&(*at_state_p)->list)) {
list_del(&(*at_state_p)->list);
kfree(*at_state_p);
*at_state_p = NULL;
- spin_unlock_irqrestore(&cs->lock, flags);
+ }
+
+ spin_unlock_irqrestore(&cs->lock, flags);
+}
+
+/* disconnect_bc
+ * process closing of connection associated with given AT state structure
+ * and B channel
+ */
+static void disconnect_bc(struct at_state_t *at_state,
+ struct cardstate *cs, struct bc_state *bcs)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&cs->lock, flags);
+ ++at_state->seq_index;
+
+ /* revert to selected idle mode */
+ if (!cs->cidmode) {
+ cs->at_state.pending_commands |= PC_UMMODE;
+ gig_dbg(DEBUG_EVENT, "Scheduling PC_UMMODE");
+ cs->commands_pending = 1;
+ }
+ spin_unlock_irqrestore(&cs->lock, flags);
+
+ /* invoke hardware specific handler */
+ cs->ops->close_bchannel(bcs);
+
+ /* notify LL */
+ if (bcs->chstate & (CHS_D_UP | CHS_NOTIFY_LL)) {
+ bcs->chstate &= ~(CHS_D_UP | CHS_NOTIFY_LL);
+ gigaset_isdn_hupD(bcs);
}
}
@@ -646,7 +667,7 @@ static void disconnect(struct at_state_t **at_state_p)
* get a free AT state structure: either one of those associated with the
* B channels of the Gigaset device, or if none of those is available,
* a newly allocated one with bcs=NULL
- * The structure should be freed by calling disconnect() after use.
+ * The structure should be freed by calling disconnect_nobc() after use.
*/
static inline struct at_state_t *get_free_channel(struct cardstate *cs,
int cid)
@@ -1057,7 +1078,7 @@ static void do_action(int action, struct cardstate *cs,
struct event_t *ev)
{
struct at_state_t *at_state = *p_at_state;
- struct at_state_t *at_state2;
+ struct bc_state *bcs2;
unsigned long flags;
int channel;
@@ -1156,8 +1177,8 @@ static void do_action(int action, struct cardstate *cs,
break;
case ACT_RING:
/* get fresh AT state structure for new CID */
- at_state2 = get_free_channel(cs, ev->parameter);
- if (!at_state2) {
+ at_state = get_free_channel(cs, ev->parameter);
+ if (!at_state) {
dev_warn(cs->dev,
"RING ignored: could not allocate channel structure\n");
break;
@@ -1166,16 +1187,16 @@ static void do_action(int action, struct cardstate *cs,
/* initialize AT state structure
* note that bcs may be NULL if no B channel is free
*/
- at_state2->ConState = 700;
+ at_state->ConState = 700;
for (i = 0; i < STR_NUM; ++i) {
- kfree(at_state2->str_var[i]);
- at_state2->str_var[i] = NULL;
+ kfree(at_state->str_var[i]);
+ at_state->str_var[i] = NULL;
}
- at_state2->int_var[VAR_ZCTP] = -1;
+ at_state->int_var[VAR_ZCTP] = -1;
spin_lock_irqsave(&cs->lock, flags);
- at_state2->timer_expires = RING_TIMEOUT;
- at_state2->timer_active = 1;
+ at_state->timer_expires = RING_TIMEOUT;
+ at_state->timer_active = 1;
spin_unlock_irqrestore(&cs->lock, flags);
break;
case ACT_ICALL:
@@ -1213,14 +1234,17 @@ static void do_action(int action, struct cardstate *cs,
case ACT_DISCONNECT:
cs->cur_at_seq = SEQ_NONE;
at_state->cid = -1;
- if (bcs && cs->onechannel && cs->dle) {
+ if (!bcs) {
+ disconnect_nobc(p_at_state, cs);
+ } else if (cs->onechannel && cs->dle) {
/* Check for other open channels not needed:
* DLE only used for M10x with one B channel.
*/
at_state->pending_commands |= PC_DLE0;
cs->commands_pending = 1;
- } else
- disconnect(p_at_state);
+ } else {
+ disconnect_bc(at_state, cs, bcs);
+ }
break;
case ACT_FAKEDLE0:
at_state->int_var[VAR_ZDLE] = 0;
@@ -1228,25 +1252,27 @@ static void do_action(int action, struct cardstate *cs,
/* fall through */
case ACT_DLE0:
cs->cur_at_seq = SEQ_NONE;
- at_state2 = &cs->bcs[cs->curchannel].at_state;
- disconnect(&at_state2);
+ bcs2 = cs->bcs + cs->curchannel;
+ disconnect_bc(&bcs2->at_state, cs, bcs2);
break;
case ACT_ABORTHUP:
cs->cur_at_seq = SEQ_NONE;
dev_warn(cs->dev, "Could not hang up.\n");
at_state->cid = -1;
- if (bcs && cs->onechannel)
+ if (!bcs)
+ disconnect_nobc(p_at_state, cs);
+ else if (cs->onechannel)
at_state->pending_commands |= PC_DLE0;
else
- disconnect(p_at_state);
+ disconnect_bc(at_state, cs, bcs);
schedule_init(cs, MS_RECOVER);
break;
case ACT_FAILDLE0:
cs->cur_at_seq = SEQ_NONE;
dev_warn(cs->dev, "Error leaving DLE mode.\n");
cs->dle = 0;
- at_state2 = &cs->bcs[cs->curchannel].at_state;
- disconnect(&at_state2);
+ bcs2 = cs->bcs + cs->curchannel;
+ disconnect_bc(&bcs2->at_state, cs, bcs2);
schedule_init(cs, MS_RECOVER);
break;
case ACT_FAILDLE1:
@@ -1275,14 +1301,14 @@ static void do_action(int action, struct cardstate *cs,
if (reinit_and_retry(cs, channel) < 0) {
dev_warn(cs->dev,
"Could not get a call ID. Cannot dial.\n");
- at_state2 = &cs->bcs[channel].at_state;
- disconnect(&at_state2);
+ bcs2 = cs->bcs + channel;
+ disconnect_bc(&bcs2->at_state, cs, bcs2);
}
break;
case ACT_ABORTCID:
cs->cur_at_seq = SEQ_NONE;
- at_state2 = &cs->bcs[cs->curchannel].at_state;
- disconnect(&at_state2);
+ bcs2 = cs->bcs + cs->curchannel;
+ disconnect_bc(&bcs2->at_state, cs, bcs2);
break;
case ACT_DIALING:
@@ -1291,7 +1317,10 @@ static void do_action(int action, struct cardstate *cs,
break;
case ACT_ABORTACCEPT: /* hangup/error/timeout during ICALL procssng */
- disconnect(p_at_state);
+ if (bcs)
+ disconnect_bc(at_state, cs, bcs);
+ else
+ disconnect_nobc(p_at_state, cs);
break;
case ACT_ABORTDIAL: /* error/timeout during dial preparation */
--
1.9.2.459.g68773ac
^ permalink raw reply related
* [PATCH 07/12] isdn/capi: prevent index overrun from command_2_index()
From: Tilman Schmidt @ 2014-10-11 11:46 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Dave Jones, Hansjoerg Lipp, Karsten Keil,
isdn4linux
In-Reply-To: <cover.1413021630.git.tilman@imap.cc>
The result of the function command_2_index() is used to index two
arrays mnames[] and cpars[] with max. index 0x4e but in its current
form that function can produce results up to 3*(0x9+0x9)+0x7f =
0xb5.
Fix by clamping all result values potentially overrunning the arrays
to zero which is already handled as an invalid value.
Re-spotted with Coverity.
Signed-off-by: Tilman Schmidt <tilman@imap.cc>
---
drivers/isdn/capi/capiutil.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/isdn/capi/capiutil.c b/drivers/isdn/capi/capiutil.c
index 4073d16..b501d76 100644
--- a/drivers/isdn/capi/capiutil.c
+++ b/drivers/isdn/capi/capiutil.c
@@ -207,6 +207,8 @@ static unsigned command_2_index(unsigned c, unsigned sc)
c = 0x9 + (c & 0x0f);
else if (c == 0x41)
c = 0x9 + 0x1;
+ if (c > 0x18)
+ c = 0x00;
return (sc & 3) * (0x9 + 0x9) + c;
}
--
1.9.2.459.g68773ac
^ permalink raw reply related
* [PATCH 00/12] Coverity patches for drivers/isdn
From: Tilman Schmidt @ 2014-10-11 11:46 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Dave Jones, Hansjoerg Lipp, Karsten Keil,
isdn4linux
Here's a series of patches for the ISDN CAPI subsystem and the
Gigaset ISDN driver.
Patches 1 to 7 are specific fixes for Coverity warnings.
Patches 8 to 11 fix related problems with the handling of invalid
CAPI command codes I noticed while working on this.
Patch 12 fixes an unrelated problem I noticed during the subsequent
regression tests.
It would be great if these could still be merged.
Thanks,
Tilman
Tilman Schmidt (12):
isdn/gigaset: missing break in do_facility_req
isdn/gigaset: make sure controller name is null terminated
isdn/gigaset: limit raw CAPI message dump length
isdn/gigaset: fix NULL pointer dereference
isdn/gigaset: fix non-heap pointer deallocation
isdn/capi: correct capi20_manufacturer argument type mismatch
isdn/capi: prevent index overrun from command_2_index()
isdn/capi: refactor command/subcommand table accesses
isdn/capi: prevent NULL pointer dereference on invalid CAPI command
isdn/capi: handle CAPI 2.0 message parser failures
isdn/capi: don't return NULL from capi_cmd2str()
isdn/gigaset: fix usb_gigaset write_cmd result race
drivers/isdn/capi/capidrv.c | 24 +++++-
drivers/isdn/capi/capiutil.c | 41 ++++++++--
drivers/isdn/capi/kcapi.c | 4 +-
drivers/isdn/gigaset/capi.c | 155 +++++++++++++++++++++++++++++++------
drivers/isdn/gigaset/ev-layer.c | 116 +++++++++++++++++----------
drivers/isdn/gigaset/usb-gigaset.c | 4 +-
include/linux/kernelcapi.h | 2 +-
7 files changed, 265 insertions(+), 81 deletions(-)
--
1.9.2.459.g68773ac
^ permalink raw reply
* [PATCH 06/12] isdn/capi: correct capi20_manufacturer argument type mismatch
From: Tilman Schmidt @ 2014-10-11 11:46 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Dave Jones, Hansjoerg Lipp, Karsten Keil,
isdn4linux
In-Reply-To: <cover.1413021630.git.tilman@imap.cc>
Function capi20_manufacturer() is declared with unsigned int cmd
argument but called with unsigned long.
Fix by correcting the function prototype since the actual argument
is part of the user visible API.
Spotted with Coverity.
Signed-off-by: Tilman Schmidt <tilman@imap.cc>
---
Note: The issue about printk(KERN_ERR which checkpatch is complaining
about is already present in the original file in this and many other
places. Cleaning it up is outside the scope of this patch.
drivers/isdn/capi/kcapi.c | 4 ++--
include/linux/kernelcapi.h | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/isdn/capi/kcapi.c b/drivers/isdn/capi/kcapi.c
index c123709..823f698 100644
--- a/drivers/isdn/capi/kcapi.c
+++ b/drivers/isdn/capi/kcapi.c
@@ -1184,7 +1184,7 @@ static int old_capi_manufacturer(unsigned int cmd, void __user *data)
* Return value: CAPI result code
*/
-int capi20_manufacturer(unsigned int cmd, void __user *data)
+int capi20_manufacturer(unsigned long cmd, void __user *data)
{
struct capi_ctr *ctr;
int retval;
@@ -1259,7 +1259,7 @@ int capi20_manufacturer(unsigned int cmd, void __user *data)
}
default:
- printk(KERN_ERR "kcapi: manufacturer command %d unknown.\n",
+ printk(KERN_ERR "kcapi: manufacturer command %lu unknown.\n",
cmd);
break;
diff --git a/include/linux/kernelcapi.h b/include/linux/kernelcapi.h
index 9be37da..e985ba6 100644
--- a/include/linux/kernelcapi.h
+++ b/include/linux/kernelcapi.h
@@ -41,7 +41,7 @@ u16 capi20_get_manufacturer(u32 contr, u8 buf[CAPI_MANUFACTURER_LEN]);
u16 capi20_get_version(u32 contr, struct capi_version *verp);
u16 capi20_get_serial(u32 contr, u8 serial[CAPI_SERIAL_LEN]);
u16 capi20_get_profile(u32 contr, struct capi_profile *profp);
-int capi20_manufacturer(unsigned int cmd, void __user *data);
+int capi20_manufacturer(unsigned long cmd, void __user *data);
#define CAPICTR_UP 0
#define CAPICTR_DOWN 1
--
1.9.2.459.g68773ac
^ permalink raw reply related
* [PATCH 11/12] isdn/capi: don't return NULL from capi_cmd2str()
From: Tilman Schmidt @ 2014-10-11 11:46 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Dave Jones, Hansjoerg Lipp, Karsten Keil,
isdn4linux
In-Reply-To: <cover.1413021630.git.tilman@imap.cc>
capi_cmd2str() is used in many places to build log messages.
None of them is prepared to handle NULL as a result.
Change the function to return printable string "INVALID_COMMAND"
instead.
Signed-off-by: Tilman Schmidt <tilman@imap.cc>
---
drivers/isdn/capi/capiutil.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/isdn/capi/capiutil.c b/drivers/isdn/capi/capiutil.c
index 36835ef..36c1b37 100644
--- a/drivers/isdn/capi/capiutil.c
+++ b/drivers/isdn/capi/capiutil.c
@@ -489,12 +489,17 @@ static char *mnames[] =
* @cmd: command number
* @subcmd: subcommand number
*
- * Return value: static string, NULL if command/subcommand unknown
+ * Return value: static string
*/
char *capi_cmd2str(u8 cmd, u8 subcmd)
{
- return mnames[command_2_index(cmd, subcmd)];
+ char *result;
+
+ result = mnames[command_2_index(cmd, subcmd)];
+ if (result == NULL)
+ result = "INVALID_COMMAND";
+ return result;
}
--
1.9.2.459.g68773ac
^ permalink raw reply related
* [PATCH 01/12] isdn/gigaset: missing break in do_facility_req
From: Tilman Schmidt @ 2014-10-11 11:46 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Dave Jones, Hansjoerg Lipp, Karsten Keil,
isdn4linux
In-Reply-To: <cover.1413021630.git.tilman@imap.cc>
If we take the unsupported supplementary service notification mask
path, we end up falling through and overwriting the error code.
Insert a break statement to skip the remainder of the switch case
and proceed to sending the reply message.
Spotted with Coverity.
Reported-by: Dave Jones <davej@redhat.com>
Signed-off-by: Tilman Schmidt <tilman@imap.cc>
---
drivers/isdn/gigaset/capi.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/isdn/gigaset/capi.c b/drivers/isdn/gigaset/capi.c
index 3286903..a2eabe9 100644
--- a/drivers/isdn/gigaset/capi.c
+++ b/drivers/isdn/gigaset/capi.c
@@ -1180,6 +1180,7 @@ static void do_facility_req(struct gigaset_capi_ctr *iif,
confparam[3] = 2; /* length */
capimsg_setu16(confparam, 4,
CapiSupplementaryServiceNotSupported);
+ break;
}
info = CapiSuccess;
confparam[3] = 2; /* length */
--
1.9.2.459.g68773ac
^ permalink raw reply related
* [PATCH 02/12] isdn/gigaset: make sure controller name is null terminated
From: Tilman Schmidt @ 2014-10-11 11:46 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Dave Jones, Hansjoerg Lipp, Karsten Keil,
isdn4linux
In-Reply-To: <cover.1413021630.git.tilman@imap.cc>
In gigaset_isdn_regdev, the name field may not have a null terminator
if the source string's length is equal to the buffer size.
Fix by zero filling the structure and excluding the last byte of the
name field from the copy.
Spotted with Coverity.
Signed-off-by: Tilman Schmidt <tilman@imap.cc>
---
drivers/isdn/gigaset/capi.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/isdn/gigaset/capi.c b/drivers/isdn/gigaset/capi.c
index a2eabe9..044392c 100644
--- a/drivers/isdn/gigaset/capi.c
+++ b/drivers/isdn/gigaset/capi.c
@@ -2358,7 +2358,7 @@ int gigaset_isdn_regdev(struct cardstate *cs, const char *isdnid)
struct gigaset_capi_ctr *iif;
int rc;
- iif = kmalloc(sizeof(*iif), GFP_KERNEL);
+ iif = kzalloc(sizeof(*iif), GFP_KERNEL);
if (!iif) {
pr_err("%s: out of memory\n", __func__);
return -ENOMEM;
@@ -2367,7 +2367,7 @@ int gigaset_isdn_regdev(struct cardstate *cs, const char *isdnid)
/* prepare controller structure */
iif->ctr.owner = THIS_MODULE;
iif->ctr.driverdata = cs;
- strncpy(iif->ctr.name, isdnid, sizeof(iif->ctr.name));
+ strncpy(iif->ctr.name, isdnid, sizeof(iif->ctr.name) - 1);
iif->ctr.driver_name = "gigaset";
iif->ctr.load_firmware = NULL;
iif->ctr.reset_ctr = NULL;
--
1.9.2.459.g68773ac
^ permalink raw reply related
* [PATCH 12/12] isdn/gigaset: fix usb_gigaset write_cmd result race
From: Tilman Schmidt @ 2014-10-11 11:46 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Dave Jones, Hansjoerg Lipp, Karsten Keil,
isdn4linux
In-Reply-To: <cover.1413021630.git.tilman@imap.cc>
In usb_gigaset function gigaset_write_cmd(), the length field of
the command buffer structure could be cleared by the transmit
tasklet before it was used for the function's return value.
Fix by copying to a local variable before scheduling the tasklet.
Signed-off-by: Tilman Schmidt <tilman@imap.cc>
---
drivers/isdn/gigaset/usb-gigaset.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/isdn/gigaset/usb-gigaset.c b/drivers/isdn/gigaset/usb-gigaset.c
index 82e91ba..a8e652d 100644
--- a/drivers/isdn/gigaset/usb-gigaset.c
+++ b/drivers/isdn/gigaset/usb-gigaset.c
@@ -497,6 +497,7 @@ static int send_cb(struct cardstate *cs, struct cmdbuf_t *cb)
static int gigaset_write_cmd(struct cardstate *cs, struct cmdbuf_t *cb)
{
unsigned long flags;
+ int len;
gigaset_dbg_buffer(cs->mstate != MS_LOCKED ?
DEBUG_TRANSCMD : DEBUG_LOCKCMD,
@@ -515,10 +516,11 @@ static int gigaset_write_cmd(struct cardstate *cs, struct cmdbuf_t *cb)
spin_unlock_irqrestore(&cs->cmdlock, flags);
spin_lock_irqsave(&cs->lock, flags);
+ len = cb->len;
if (cs->connected)
tasklet_schedule(&cs->write_tasklet);
spin_unlock_irqrestore(&cs->lock, flags);
- return cb->len;
+ return len;
}
static int gigaset_write_room(struct cardstate *cs)
--
1.9.2.459.g68773ac
^ permalink raw reply related
* [PATCH 08/12] isdn/capi: refactor command/subcommand table accesses
From: Tilman Schmidt @ 2014-10-11 11:46 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Dave Jones, Hansjoerg Lipp, Karsten Keil,
isdn4linux
In-Reply-To: <cover.1413021630.git.tilman@imap.cc>
Encapsulate accesses to the CAPI 2.0 command/subcommand name and
parameter tables in a single place in preparation for redesign.
Signed-off-by: Tilman Schmidt <tilman@imap.cc>
---
drivers/isdn/capi/capiutil.c | 23 ++++++++++++++++++-----
1 file changed, 18 insertions(+), 5 deletions(-)
diff --git a/drivers/isdn/capi/capiutil.c b/drivers/isdn/capi/capiutil.c
index b501d76..8e401ed 100644
--- a/drivers/isdn/capi/capiutil.c
+++ b/drivers/isdn/capi/capiutil.c
@@ -212,6 +212,19 @@ static unsigned command_2_index(unsigned c, unsigned sc)
return (sc & 3) * (0x9 + 0x9) + c;
}
+/**
+ * capi_cmd2par() - find parameter string for CAPI 2.0 command/subcommand
+ * @cmd: command number
+ * @subcmd: subcommand number
+ *
+ * Return value: static string, NULL if command/subcommand unknown
+ */
+
+static unsigned char *capi_cmd2par(u8 cmd, u8 subcmd)
+{
+ return cpars[command_2_index(cmd, subcmd)];
+}
+
/*-------------------------------------------------------*/
#define TYP (cdef[cmsg->par[cmsg->p]].typ)
#define OFF (((u8 *)cmsg) + cdef[cmsg->par[cmsg->p]].off)
@@ -304,7 +317,7 @@ unsigned capi_cmsg2message(_cmsg *cmsg, u8 *msg)
cmsg->m = msg;
cmsg->l = 8;
cmsg->p = 0;
- cmsg->par = cpars[command_2_index(cmsg->Command, cmsg->Subcommand)];
+ cmsg->par = capi_cmd2par(cmsg->Command, cmsg->Subcommand);
pars_2_message(cmsg);
@@ -377,7 +390,7 @@ unsigned capi_message2cmsg(_cmsg *cmsg, u8 *msg)
cmsg->p = 0;
byteTRcpy(cmsg->m + 4, &cmsg->Command);
byteTRcpy(cmsg->m + 5, &cmsg->Subcommand);
- cmsg->par = cpars[command_2_index(cmsg->Command, cmsg->Subcommand)];
+ cmsg->par = capi_cmd2par(cmsg->Command, cmsg->Subcommand);
message_2_pars(cmsg);
@@ -761,10 +774,10 @@ _cdebbuf *capi_message2str(u8 *msg)
cmsg->p = 0;
byteTRcpy(cmsg->m + 4, &cmsg->Command);
byteTRcpy(cmsg->m + 5, &cmsg->Subcommand);
- cmsg->par = cpars[command_2_index(cmsg->Command, cmsg->Subcommand)];
+ cmsg->par = capi_cmd2par(cmsg->Command, cmsg->Subcommand);
cdb = bufprint(cdb, "%-26s ID=%03d #0x%04x LEN=%04d\n",
- mnames[command_2_index(cmsg->Command, cmsg->Subcommand)],
+ capi_cmd2str(cmsg->Command, cmsg->Subcommand),
((unsigned short *) msg)[1],
((unsigned short *) msg)[3],
((unsigned short *) msg)[0]);
@@ -798,7 +811,7 @@ _cdebbuf *capi_cmsg2str(_cmsg *cmsg)
cmsg->l = 8;
cmsg->p = 0;
cdb = bufprint(cdb, "%s ID=%03d #0x%04x LEN=%04d\n",
- mnames[command_2_index(cmsg->Command, cmsg->Subcommand)],
+ capi_cmd2str(cmsg->Command, cmsg->Subcommand),
((u16 *) cmsg->m)[1],
((u16 *) cmsg->m)[3],
((u16 *) cmsg->m)[0]);
--
1.9.2.459.g68773ac
^ permalink raw reply related
* [PATCH 09/12] isdn/capi: prevent NULL pointer dereference on invalid CAPI command
From: Tilman Schmidt @ 2014-10-11 11:46 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Dave Jones, Hansjoerg Lipp, Karsten Keil,
isdn4linux
In-Reply-To: <cover.1413021630.git.tilman@imap.cc>
An invalid CAPI 2.0 command/subcommand combination may retrieve a
NULL pointer from the cpars[] array which will later be dereferenced
by the parser routines.
Fix by adding NULL pointer checks in strategic places.
Signed-off-by: Tilman Schmidt <tilman@imap.cc>
---
drivers/isdn/capi/capiutil.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/isdn/capi/capiutil.c b/drivers/isdn/capi/capiutil.c
index 8e401ed..36835ef 100644
--- a/drivers/isdn/capi/capiutil.c
+++ b/drivers/isdn/capi/capiutil.c
@@ -318,6 +318,8 @@ unsigned capi_cmsg2message(_cmsg *cmsg, u8 *msg)
cmsg->l = 8;
cmsg->p = 0;
cmsg->par = capi_cmd2par(cmsg->Command, cmsg->Subcommand);
+ if (!cmsg->par)
+ return 1; /* invalid command/subcommand */
pars_2_message(cmsg);
@@ -391,6 +393,8 @@ unsigned capi_message2cmsg(_cmsg *cmsg, u8 *msg)
byteTRcpy(cmsg->m + 4, &cmsg->Command);
byteTRcpy(cmsg->m + 5, &cmsg->Subcommand);
cmsg->par = capi_cmd2par(cmsg->Command, cmsg->Subcommand);
+ if (!cmsg->par)
+ return 1; /* invalid command/subcommand */
message_2_pars(cmsg);
@@ -640,6 +644,9 @@ static _cdebbuf *printstruct(_cdebbuf *cdb, u8 *m)
static _cdebbuf *protocol_message_2_pars(_cdebbuf *cdb, _cmsg *cmsg, int level)
{
+ if (!cmsg->par)
+ return NULL; /* invalid command/subcommand */
+
for (; TYP != _CEND; cmsg->p++) {
int slen = 29 + 3 - level;
int i;
--
1.9.2.459.g68773ac
^ permalink raw reply related
* [PATCH 03/12] isdn/gigaset: limit raw CAPI message dump length
From: Tilman Schmidt @ 2014-10-11 11:46 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Dave Jones, Hansjoerg Lipp, Karsten Keil,
isdn4linux
In-Reply-To: <cover.1413021630.git.tilman@imap.cc>
In dump_rawmsg, the length field from a received data package was
used unscrutinized, allowing an attacker to control the size of the
allocated buffer and the number of times the output loop iterates.
Fix by limiting to a reasonable value.
Spotted with Coverity.
Signed-off-by: Tilman Schmidt <tilman@imap.cc>
---
drivers/isdn/gigaset/capi.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/isdn/gigaset/capi.c b/drivers/isdn/gigaset/capi.c
index 044392c..47e2a91 100644
--- a/drivers/isdn/gigaset/capi.c
+++ b/drivers/isdn/gigaset/capi.c
@@ -250,6 +250,8 @@ static inline void dump_rawmsg(enum debuglevel level, const char *tag,
l -= 12;
if (l <= 0)
return;
+ if (l > 64)
+ l = 64; /* arbitrary limit */
dbgline = kmalloc(3 * l, GFP_ATOMIC);
if (!dbgline)
return;
--
1.9.2.459.g68773ac
^ 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