* [PATCH] cw1200: Fix a sleep-in-atomic bug in cw1200_tx_confirm_cb and cw1200_cqm_bssloss_sm
From: Jia-Ju Bai @ 2017-06-01 2:36 UTC (permalink / raw)
To: pizza, kvalo; +Cc: linux-wireless, netdev, linux-kernel, Jia-Ju Bai
The driver may sleep under a spin lock, and the function call path is:
cw1200_tx_confirm_cb (acquire the lock by spin_lock)
__cw1200_cqm_bssloss_sm
cancel_work_sync --> may sleep
cw1200_cqm_bssloss_sm
__cw1200_cqm_bssloss_sm
cancel_work_sync --> may sleep
To fix it, the lock is released before cancel_work_sync, and the lock
is acquired again after this function.
Signed-off-by: Jia-Ju Bai <baijiaju1990@163.com>
---
drivers/net/wireless/st/cw1200/sta.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/wireless/st/cw1200/sta.c b/drivers/net/wireless/st/cw1200/sta.c
index a522248..d5f7698 100644
--- a/drivers/net/wireless/st/cw1200/sta.c
+++ b/drivers/net/wireless/st/cw1200/sta.c
@@ -154,7 +154,9 @@ void __cw1200_cqm_bssloss_sm(struct cw1200_common *priv,
int tx = 0;
priv->delayed_link_loss = 0;
+ spin_unlock(&priv->bss_loss_lock);
cancel_work_sync(&priv->bss_params_work);
+ spin_lock(&priv->bss_loss_lock);
pr_debug("[STA] CQM BSSLOSS_SM: state: %d init %d good %d bad: %d txlock: %d uj: %d\n",
priv->bss_loss_state,
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH v2 net-next 0/8] Introduce bpf ID
From: Jakub Kicinski @ 2017-06-01 2:22 UTC (permalink / raw)
To: Martin KaFai Lau, Hannes Frederic Sowa
Cc: netdev, Alexei Starovoitov, Daniel Borkmann, kernel-team
In-Reply-To: <cover.1496256722.git.kafai@fb.com>
On Wed, 31 May 2017 11:58:54 -0700, Martin KaFai Lau wrote:
> This patch series:
> 1) Introduce ID for both bpf_prog and bpf_map.
> 2) Add bpf commands to iterate the prog IDs and map
> IDs of the system.
> 3) Add bpf commands to get a prog/map fd from an ID
> 4) Add bpf command to get prog/map info from a fd.
> The prog/map info is a jump start in this patchset
> and it is not meant to be a complete list. They can
> be extended in the future patches.
Hi! IIUC there were plans at some point to store the BPF byte code in
the kernel, the way it was loaded from the user space and before it
went through the verifier. Are you still planning on implementing that?
It would be handy for offloads to have the original byte code, things
like call inlining make translations of maps calls a bit challenging,
since we try to run through the verifier again..
^ permalink raw reply
* Re: [PATCH v2] net: fec: add post PHY reset delay DT property
From: Florian Fainelli @ 2017-06-01 1:52 UTC (permalink / raw)
To: Andy Duan, Rob Herring, Quentin Schulz
Cc: mark.rutland@arm.com, netdev@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
thomas.petazzoni@free-electrons.com
In-Reply-To: <AM4PR0401MB226073F4A403F31FA0FF2BE0FFF60@AM4PR0401MB2260.eurprd04.prod.outlook.com>
Le 05/31/17 à 18:39, Andy Duan a écrit :
> From: Rob Herring <robh@kernel.org> Sent: Thursday, June 01, 2017 12:44 AM
>> On Tue, May 23, 2017 at 11:48:08AM +0200, Quentin Schulz wrote:
>>> Some PHY require to wait for a bit after the reset GPIO has been
>>> toggled. This adds support for the DT property `phy-reset-post-delay`
>>> which gives the delay in milliseconds to wait after reset.
>>>
>>> If the DT property is not given, no delay is observed. Post reset
>>> delay greater than 1000ms are invalid.
>>>
>>> Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
>>> ---
>>>
>>> v2:
>>> - return -EINVAL when phy-reset-post-delay is greater than 1000ms
>>> instead of defaulting to 1ms,
>>> - remove `default to 1ms` when phy-reset-post-delay > 1000Ms from DT
>>> binding doc and commit log,
>>> - move phy-reset-post-delay property reading before
>>> devm_gpio_request_one(),
>>>
>>> Documentation/devicetree/bindings/net/fsl-fec.txt | 4 ++++
>>> drivers/net/ethernet/freescale/fec_main.c | 16 +++++++++++++++-
>>> 2 files changed, 19 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/net/fsl-fec.txt
>>> b/Documentation/devicetree/bindings/net/fsl-fec.txt
>>> index a1e3693cca16..6f55bdd52f8a 100644
>>> --- a/Documentation/devicetree/bindings/net/fsl-fec.txt
>>> +++ b/Documentation/devicetree/bindings/net/fsl-fec.txt
>>> @@ -15,6 +15,10 @@ Optional properties:
>>> - phy-reset-active-high : If present then the reset sequence using the GPIO
>>> specified in the "phy-reset-gpios" property is reversed (H=reset state,
>>> L=operation state).
>>> +- phy-reset-post-delay : Post reset delay in milliseconds. If present
>>> +then
>>
>> This needs unit suffix minimally. It should also have a vendor prefix or be
>> made generic.
>>
>> But really, this is a property of the phy and should be in the phy node as
>> should phy-reset-gpios, phy-reset-active-high, phy-supply, etc.
>>
> Yes, it is better to make it general.
> Last year, Uwe Kleine-König's patch "Commit da47b4572056 ("phy: add support for a reset-gpio specification")" did this, but it was reverted by commit 948350140ef0 (Revert "phy: add support for a reset-gpio specification"). And in all phy device driver, only at803x.c add the gpio reset in currently.
Getting the binding correct does not prevent us from later moving this
reset code into PHYLIB where it's appropriate. In fact; a correct and
generic binding proposed for FEC here could be used as a basis for all
other MAC and PHY drivers.
--
Florian
^ permalink raw reply
* RE: [PATCH v2] net: fec: add post PHY reset delay DT property
From: Andy Duan @ 2017-06-01 1:39 UTC (permalink / raw)
To: Rob Herring, Quentin Schulz
Cc: mark.rutland-5wv7dgnIgG8@public.gmane.org,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org
In-Reply-To: <20170531164416.ka3hk6a2lybrlngd@rob-hp-laptop>
From: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> Sent: Thursday, June 01, 2017 12:44 AM
>On Tue, May 23, 2017 at 11:48:08AM +0200, Quentin Schulz wrote:
>> Some PHY require to wait for a bit after the reset GPIO has been
>> toggled. This adds support for the DT property `phy-reset-post-delay`
>> which gives the delay in milliseconds to wait after reset.
>>
>> If the DT property is not given, no delay is observed. Post reset
>> delay greater than 1000ms are invalid.
>>
>> Signed-off-by: Quentin Schulz <quentin.schulz-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>> ---
>>
>> v2:
>> - return -EINVAL when phy-reset-post-delay is greater than 1000ms
>> instead of defaulting to 1ms,
>> - remove `default to 1ms` when phy-reset-post-delay > 1000Ms from DT
>> binding doc and commit log,
>> - move phy-reset-post-delay property reading before
>> devm_gpio_request_one(),
>>
>> Documentation/devicetree/bindings/net/fsl-fec.txt | 4 ++++
>> drivers/net/ethernet/freescale/fec_main.c | 16 +++++++++++++++-
>> 2 files changed, 19 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/net/fsl-fec.txt
>> b/Documentation/devicetree/bindings/net/fsl-fec.txt
>> index a1e3693cca16..6f55bdd52f8a 100644
>> --- a/Documentation/devicetree/bindings/net/fsl-fec.txt
>> +++ b/Documentation/devicetree/bindings/net/fsl-fec.txt
>> @@ -15,6 +15,10 @@ Optional properties:
>> - phy-reset-active-high : If present then the reset sequence using the GPIO
>> specified in the "phy-reset-gpios" property is reversed (H=reset state,
>> L=operation state).
>> +- phy-reset-post-delay : Post reset delay in milliseconds. If present
>> +then
>
>This needs unit suffix minimally. It should also have a vendor prefix or be
>made generic.
>
>But really, this is a property of the phy and should be in the phy node as
>should phy-reset-gpios, phy-reset-active-high, phy-supply, etc.
>
Yes, it is better to make it general.
Last year, Uwe Kleine-König's patch "Commit da47b4572056 ("phy: add support for a reset-gpio specification")" did this, but it was reverted by commit 948350140ef0 (Revert "phy: add support for a reset-gpio specification"). And in all phy device driver, only at803x.c add the gpio reset in currently.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" 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
* linux-next: manual merge of the net-next tree with the net tree
From: Stephen Rothwell @ 2017-06-01 1:30 UTC (permalink / raw)
To: David Miller, Networking
Cc: Linux-Next Mailing List, Linux Kernel Mailing List, Russell King,
Andrew Lunn
Hi all,
Today's linux-next merge of the net-next tree got a conflict in:
drivers/net/phy/marvell.c
between commit:
898805e0cdf7 ("net: phy: fix marvell phy status reading")
from the net tree and commit:
e1dde8dc5b27 ("net: phy: marvell: Refactor some bigger functions")
from the net-next tree.
I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging. You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.
--
Cheers,
Stephen Rothwell
diff --cc drivers/net/phy/marvell.c
index 57297ba23987,1a72bebc588a..000000000000
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@@ -1082,119 -1133,106 +1133,104 @@@ static int marvell_update_link(struct p
return 0;
}
- /* marvell_read_status_page
- *
- * Description:
- * Check the link, then figure out the current state
- * by comparing what we advertise with what the link partner
- * advertises. Start by checking the gigabit possibilities,
- * then move on to 10/100.
- */
- static int marvell_read_status_page(struct phy_device *phydev, int page)
+ static int marvell_read_status_page_an(struct phy_device *phydev,
+ int fiber)
{
- int adv;
- int err;
+ int status;
int lpa;
int lpagb;
- int status = 0;
- int fiber;
+ int adv;
- /* Detect and update the link, but return if there
- * was an error */
- if (page == MII_M1111_FIBER)
- fiber = 1;
- else
- fiber = 0;
+ status = phy_read(phydev, MII_M1011_PHY_STATUS);
+ if (status < 0)
+ return status;
- err = marvell_update_link(phydev, fiber);
- if (err)
- return err;
+ lpa = phy_read(phydev, MII_LPA);
+ if (lpa < 0)
+ return lpa;
- if (AUTONEG_ENABLE == phydev->autoneg) {
- status = phy_read(phydev, MII_M1011_PHY_STATUS);
- if (status < 0)
- return status;
+ lpagb = phy_read(phydev, MII_STAT1000);
+ if (lpagb < 0)
+ return lpagb;
- lpa = phy_read(phydev, MII_LPA);
- if (lpa < 0)
- return lpa;
+ adv = phy_read(phydev, MII_ADVERTISE);
+ if (adv < 0)
+ return adv;
- lpagb = phy_read(phydev, MII_STAT1000);
- if (lpagb < 0)
- return lpagb;
- lpa &= adv;
-
+ if (status & MII_M1011_PHY_STATUS_FULLDUPLEX)
+ phydev->duplex = DUPLEX_FULL;
+ else
+ phydev->duplex = DUPLEX_HALF;
- adv = phy_read(phydev, MII_ADVERTISE);
- if (adv < 0)
- return adv;
+ status = status & MII_M1011_PHY_STATUS_SPD_MASK;
+ phydev->pause = 0;
+ phydev->asym_pause = 0;
- if (status & MII_M1011_PHY_STATUS_FULLDUPLEX)
- phydev->duplex = DUPLEX_FULL;
- else
- phydev->duplex = DUPLEX_HALF;
+ switch (status) {
+ case MII_M1011_PHY_STATUS_1000:
+ phydev->speed = SPEED_1000;
+ break;
- status = status & MII_M1011_PHY_STATUS_SPD_MASK;
- phydev->pause = phydev->asym_pause = 0;
+ case MII_M1011_PHY_STATUS_100:
+ phydev->speed = SPEED_100;
+ break;
- switch (status) {
- case MII_M1011_PHY_STATUS_1000:
- phydev->speed = SPEED_1000;
- break;
+ default:
+ phydev->speed = SPEED_10;
+ break;
+ }
- case MII_M1011_PHY_STATUS_100:
- phydev->speed = SPEED_100;
- break;
+ if (!fiber) {
+ phydev->lp_advertising =
+ mii_stat1000_to_ethtool_lpa_t(lpagb) |
+ mii_lpa_to_ethtool_lpa_t(lpa);
- default:
- phydev->speed = SPEED_10;
- break;
+ if (phydev->duplex == DUPLEX_FULL) {
+ phydev->pause = lpa & LPA_PAUSE_CAP ? 1 : 0;
+ phydev->asym_pause = lpa & LPA_PAUSE_ASYM ? 1 : 0;
}
-
- if (!fiber) {
- phydev->lp_advertising = mii_stat1000_to_ethtool_lpa_t(lpagb) |
- mii_lpa_to_ethtool_lpa_t(lpa);
-
- if (phydev->duplex == DUPLEX_FULL) {
- phydev->pause = lpa & LPA_PAUSE_CAP ? 1 : 0;
- phydev->asym_pause = lpa & LPA_PAUSE_ASYM ? 1 : 0;
- }
- } else {
- /* The fiber link is only 1000M capable */
- phydev->lp_advertising = fiber_lpa_to_ethtool_lpa_t(lpa);
-
- if (phydev->duplex == DUPLEX_FULL) {
- if (!(lpa & LPA_PAUSE_FIBER)) {
- phydev->pause = 0;
- phydev->asym_pause = 0;
- } else if ((lpa & LPA_PAUSE_ASYM_FIBER)) {
- phydev->pause = 1;
- phydev->asym_pause = 1;
- } else {
- phydev->pause = 1;
- phydev->asym_pause = 0;
- }
+ } else {
+ /* The fiber link is only 1000M capable */
+ phydev->lp_advertising = fiber_lpa_to_ethtool_lpa_t(lpa);
+
+ if (phydev->duplex == DUPLEX_FULL) {
+ if (!(lpa & LPA_PAUSE_FIBER)) {
+ phydev->pause = 0;
+ phydev->asym_pause = 0;
+ } else if ((lpa & LPA_PAUSE_ASYM_FIBER)) {
+ phydev->pause = 1;
+ phydev->asym_pause = 1;
+ } else {
+ phydev->pause = 1;
+ phydev->asym_pause = 0;
}
}
- } else {
- int bmcr = phy_read(phydev, MII_BMCR);
+ }
+ return 0;
+ }
- if (bmcr < 0)
- return bmcr;
+ static int marvell_read_status_page_fixed(struct phy_device *phydev)
+ {
+ int bmcr = phy_read(phydev, MII_BMCR);
- if (bmcr & BMCR_FULLDPLX)
- phydev->duplex = DUPLEX_FULL;
- else
- phydev->duplex = DUPLEX_HALF;
+ if (bmcr < 0)
+ return bmcr;
- if (bmcr & BMCR_SPEED1000)
- phydev->speed = SPEED_1000;
- else if (bmcr & BMCR_SPEED100)
- phydev->speed = SPEED_100;
- else
- phydev->speed = SPEED_10;
+ if (bmcr & BMCR_FULLDPLX)
+ phydev->duplex = DUPLEX_FULL;
+ else
+ phydev->duplex = DUPLEX_HALF;
- phydev->pause = phydev->asym_pause = 0;
- phydev->lp_advertising = 0;
- }
+ if (bmcr & BMCR_SPEED1000)
+ phydev->speed = SPEED_1000;
+ else if (bmcr & BMCR_SPEED100)
+ phydev->speed = SPEED_100;
+ else
+ phydev->speed = SPEED_10;
+
+ phydev->pause = 0;
+ phydev->asym_pause = 0;
+ phydev->lp_advertising = 0;
return 0;
}
^ permalink raw reply
* [PATCH V4] neigh: Really delete an arp/neigh entry on "ip neigh delete" or "arp -d"
From: Sowmini Varadhan @ 2017-06-01 1:24 UTC (permalink / raw)
To: netdev, sowmini.varadhan; +Cc: davem, stephen, sowmini.varadhan, ja
The command
# arp -s 62.2.0.1 a:b:c:d:e:f dev eth2
adds an entry like the following (listed by "arp -an")
? (62.2.0.1) at 0a:0b:0c:0d:0e:0f [ether] PERM on eth2
but the symmetric deletion command
# arp -i eth2 -d 62.2.0.1
does not remove the PERM entry from the table, and instead leaves behind
? (62.2.0.1) at <incomplete> on eth2
The reason is that there is a refcnt of 1 for the arp_tbl itself
(neigh_alloc starts off the entry with a refcnt of 1), thus
the neigh_release() call from arp_invalidate() will (at best) just
decrement the ref to 1, but will never actually free it from the
table.
To fix this, we need to do something like neigh_forced_gc: if
the refcnt is 1 (i.e., on the table's ref), remove the entry from
the table and free it. This patch refactors and shares common code
between neigh_forced_gc and the newly added neigh_remove_one.
A similar issue exists for IPv6 Neighbor Cache entries, and is fixed
in a similar manner by this patch.
Signed-off-by: Sowmini Varadhan <sowmini.varadhan@oracle.com>
---
v2: kbuild-test-robot compile error
v3: do not export_symbol neigh_remove_one (David Miller comment)
v4: rearrange locking for tbl->lock (Julian Anastasov comment)
include/net/neighbour.h | 1 +
net/core/neighbour.c | 61 ++++++++++++++++++++++++++++++++++++++--------
net/ipv4/arp.c | 4 +++
3 files changed, 55 insertions(+), 11 deletions(-)
diff --git a/include/net/neighbour.h b/include/net/neighbour.h
index e4dd3a2..639b675 100644
--- a/include/net/neighbour.h
+++ b/include/net/neighbour.h
@@ -317,6 +317,7 @@ struct neighbour *__neigh_create(struct neigh_table *tbl, const void *pkey,
int neigh_update(struct neighbour *neigh, const u8 *lladdr, u8 new, u32 flags,
u32 nlmsg_pid);
void __neigh_set_probe_once(struct neighbour *neigh);
+bool neigh_remove_one(struct neighbour *ndel, struct neigh_table *tbl);
void neigh_changeaddr(struct neigh_table *tbl, struct net_device *dev);
int neigh_ifdown(struct neigh_table *tbl, struct net_device *dev);
int neigh_resolve_output(struct neighbour *neigh, struct sk_buff *skb);
diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index d274f81..b473f9f 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -118,6 +118,51 @@ unsigned long neigh_rand_reach_time(unsigned long base)
EXPORT_SYMBOL(neigh_rand_reach_time);
+static bool neigh_del(struct neighbour *n, __u8 state,
+ struct neighbour __rcu **np, struct neigh_table *tbl)
+{
+ bool retval = false;
+
+ write_lock(&n->lock);
+ if (atomic_read(&n->refcnt) == 1 && !(n->nud_state & state)) {
+ rcu_assign_pointer(*np,
+ rcu_dereference_protected(n->next,
+ lockdep_is_held(&tbl->lock)));
+ n->dead = 1;
+ retval = true;
+ }
+ write_unlock(&n->lock);
+ if (retval)
+ neigh_cleanup_and_release(n);
+ return retval;
+}
+
+bool neigh_remove_one(struct neighbour *ndel, struct neigh_table *tbl)
+{
+ struct neigh_hash_table *nht;
+ void *pkey = ndel->primary_key;
+ u32 hash_val;
+ struct neighbour *n;
+ struct neighbour __rcu **np;
+ bool retval = false;
+
+ nht = rcu_dereference_protected(tbl->nht,
+ lockdep_is_held(&tbl->lock));
+ hash_val = tbl->hash(pkey, ndel->dev, nht->hash_rnd);
+ hash_val = hash_val >> (32 - nht->hash_shift);
+
+ np = &nht->hash_buckets[hash_val];
+ while ((n = rcu_dereference_protected(*np,
+ lockdep_is_held(&tbl->lock))) != NULL) {
+ if (n == ndel) {
+ retval = neigh_del(n, 0, np, tbl);
+ return retval;
+ }
+ np = &n->next;
+ }
+ return retval;
+}
+
static int neigh_forced_gc(struct neigh_table *tbl)
{
int shrunk = 0;
@@ -140,19 +185,10 @@ static int neigh_forced_gc(struct neigh_table *tbl)
* - nobody refers to it.
* - it is not permanent
*/
- write_lock(&n->lock);
- if (atomic_read(&n->refcnt) == 1 &&
- !(n->nud_state & NUD_PERMANENT)) {
- rcu_assign_pointer(*np,
- rcu_dereference_protected(n->next,
- lockdep_is_held(&tbl->lock)));
- n->dead = 1;
- shrunk = 1;
- write_unlock(&n->lock);
- neigh_cleanup_and_release(n);
+ if (neigh_del(n, NUD_PERMANENT, np, tbl)) {
+ shrunk = 1;
continue;
}
- write_unlock(&n->lock);
np = &n->next;
}
}
@@ -1649,7 +1685,10 @@ static int neigh_delete(struct sk_buff *skb, struct nlmsghdr *nlh,
NEIGH_UPDATE_F_OVERRIDE |
NEIGH_UPDATE_F_ADMIN,
NETLINK_CB(skb).portid);
+ write_lock_bh(&tbl->lock);
neigh_release(neigh);
+ neigh_remove_one(neigh, tbl);
+ write_unlock_bh(&tbl->lock);
out:
return err;
diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
index e9f3386..a651c53 100644
--- a/net/ipv4/arp.c
+++ b/net/ipv4/arp.c
@@ -1113,13 +1113,17 @@ static int arp_invalidate(struct net_device *dev, __be32 ip)
{
struct neighbour *neigh = neigh_lookup(&arp_tbl, &ip, dev);
int err = -ENXIO;
+ struct neigh_table *tbl = &arp_tbl;
if (neigh) {
if (neigh->nud_state & ~NUD_NOARP)
err = neigh_update(neigh, NULL, NUD_FAILED,
NEIGH_UPDATE_F_OVERRIDE|
NEIGH_UPDATE_F_ADMIN, 0);
+ write_lock_bh(&tbl->lock);
neigh_release(neigh);
+ neigh_remove_one(neigh, tbl);
+ write_unlock_bh(&tbl->lock);
}
return err;
--
1.7.1
^ permalink raw reply related
* Re: [PATCH V3] neigh: Really delete an arp/neigh entry on "ip neigh delete" or "arp -d"
From: Sowmini Varadhan @ 2017-06-01 1:21 UTC (permalink / raw)
To: Julian Anastasov; +Cc: netdev, davem, stephen
In-Reply-To: <alpine.LFD.2.20.1706010059370.10214@ja.home.ssi.bg>
On (06/01/17 01:41), Julian Anastasov wrote:
> Another problem is that neigh_update() changes the
> state but before we go and unlink the entry another CPU
> can reactivate the entry, i.e. NUD_INCOMPLETE entered
> in __neigh_event_send(). So, there will be always some
> small window where surprises can happen and the entry is
> not really deleted.
but that would be ok- it's the same as if I did
arp -d <addr>
ping <addr>
I think the only danger is that, once we drop the ref with
neigh_release(), we are relying on the table's lock for the neigh
to not disappear under us. Thus as you correctly pointed out,
we need the tbl->lock to make sure we sync with all paths that
can pull the neigh out of the table (and release the table's ref
along the way).
Thanks for catching that, patch v4 (with correct smtp timestamp!)
will be sent out shortly.
--Sowmini
^ permalink raw reply
* Re: net/ipv6: use-after-free in ip6_dst_ifdown
From: David Ahern @ 2017-06-01 1:17 UTC (permalink / raw)
To: Cong Wang, Eric Dumazet
Cc: Andrey Konovalov, David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, netdev, LKML, David Ahern,
Dmitry Vyukov, Kostya Serebryany, syzkaller
In-Reply-To: <CAM_iQpV3NyvTLbg3S0_Eq6WYpz0MbPfcGt=A7Vu6=ttAuY1V3Q@mail.gmail.com>
On 5/31/17 4:49 PM, Cong Wang wrote:
>>>> ==================================================================
>>>> BUG: KASAN: use-after-free in ip6_dst_ifdown+0x3cc/0x400 net/ipv6/route.c:422
>>>> Read of size 8 at addr ffff88006afa4ad8 by task syz-executor6/23554
>>>
>>>
>>> This one is very interesting.
>>>
>>> Here we are at:
>>>
>>> if (dev != loopback_dev) {
>>> if (idev && idev->dev == dev) {
>>> struct inet6_dev *loopback_idev =
>>> in6_dev_get(loopback_dev);
>>> if (loopback_idev) {
>>> rt->rt6i_idev = loopback_idev;
>>> in6_dev_put(idev);
>>> }
>>> }
>>> }
>>>
>>> clearly no skb involved, it looks like idev is the one used-after-free.
>>>
>>> But below it is actually skb which is allocated and freed...
>>>
>>
>> skb->head was a kmalloc(X) with X = 1024 in this case.
>>
>> So it is very possible the two different objects (skb->head and idev )
>> were accidentally using the same slab (1024 bytes).
>>
>> KASAN only remember the last pair of alloc/free for a particular memory zone.
>
> I see. So that memory area was freed for idev and then allocated
> and freed again for skb->head, this happened so quick that the
> use-after-free happened after it... Therefore we lost the track on where
> we free the idev.
>
Andrey: can you add this to your runs? If it triggers again, we can see
if this use-after-free is another problem where the dst hit the gc list
and came back into the IPv6 FIB. The location of the idev entry in
rt6_info is after the size of rtable so if this is an IPv4 dst on the
IPv6 list it could trigger that warning.
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 9d9b5bbea153..237f42144b3e 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -418,6 +418,7 @@ static void ip6_dst_ifdown(struct dst_entry *dst,
struct net_device *dev,
struct net_device *loopback_dev =
dev_net(dev)->loopback_dev;
+WARN_ON(dst->ops->family != AF_INET6);
if (dev != loopback_dev) {
if (idev && idev->dev == dev) {
struct inet6_dev *loopback_idev =
^ permalink raw reply related
* [PATCH net-next v2 2/2] bpf: Remove the capability check for cgroup skb eBPF program
From: Chenbo Feng @ 2017-06-01 1:16 UTC (permalink / raw)
To: netdev, David Miller; +Cc: Lorenzo Colitti, Chenbo Feng
In-Reply-To: <1496279760-20996-1-git-send-email-chenbofeng.kernel@gmail.com>
From: Chenbo Feng <fengc@google.com>
Currently loading a cgroup skb eBPF program require a CAP_SYS_ADMIN
capability while attaching the program to a cgroup only requires the
user have CAP_NET_ADMIN privilege. We can escape the capability
check when load the program just like socket filter program to make
the capability requirement consistent.
Change since v1:
Change the code style in order to be compliant with checkpatch.pl
preference
Signed-off-by: Chenbo Feng <fengc@google.com>
---
kernel/bpf/syscall.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 265a0d8..59da103 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -815,7 +815,9 @@ static int bpf_prog_load(union bpf_attr *attr)
attr->kern_version != LINUX_VERSION_CODE)
return -EINVAL;
- if (type != BPF_PROG_TYPE_SOCKET_FILTER && !capable(CAP_SYS_ADMIN))
+ if (type != BPF_PROG_TYPE_SOCKET_FILTER &&
+ type != BPF_PROG_TYPE_CGROUP_SKB &&
+ !capable(CAP_SYS_ADMIN))
return -EPERM;
/* plain bpf_prog allocation */
--
2.7.4
^ permalink raw reply related
* [PATCH net-next v2 1/2] bpf: Allow CGROUP_SKB eBPF program to access sk_buff
From: Chenbo Feng @ 2017-06-01 1:15 UTC (permalink / raw)
To: netdev, David Miller; +Cc: Lorenzo Colitti, Chenbo Feng
From: Chenbo Feng <fengc@google.com>
This allows cgroup eBPF program to classify packet based on their
protocol or other detail information. Currently program need
CAP_NET_ADMIN privilege to attach a cgroup eBPF program, and A
process with CAP_NET_ADMIN can already see all packets on the system,
for example, by creating an iptables rules that causes the packet to
be passed to userspace via NFLOG.
Signed-off-by: Chenbo Feng <fengc@google.com>
---
kernel/bpf/verifier.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 339c8a1..94a9bc9 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -2419,6 +2419,7 @@ static bool may_access_skb(enum bpf_prog_type type)
case BPF_PROG_TYPE_SOCKET_FILTER:
case BPF_PROG_TYPE_SCHED_CLS:
case BPF_PROG_TYPE_SCHED_ACT:
+ case BPF_PROG_TYPE_CGROUP_SKB:
return true;
default:
return false;
--
2.7.4
^ permalink raw reply related
* [PATCH net-next v2] bpf: Remove the capability check for cgroup skb eBPF program
From: Chenbo Feng @ 2017-06-01 1:12 UTC (permalink / raw)
To: netdev, David Miller; +Cc: Lorenzo Colitti, Chenbo Feng
From: Chenbo Feng <fengc@google.com>
Currently loading a cgroup skb eBPF program require a CAP_SYS_ADMIN
capability while attaching the program to a cgroup only requires the
user have CAP_NET_ADMIN privilege. We can escape the capability
check when load the program just like socket filter program to make
the capability requirement consistent.
Change since v1:
Change the code style in order to be compliant with checkpatch.pl
preference
Signed-off-by: Chenbo Feng <fengc@google.com>
---
kernel/bpf/syscall.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 265a0d8..59da103 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -815,7 +815,9 @@ static int bpf_prog_load(union bpf_attr *attr)
attr->kern_version != LINUX_VERSION_CODE)
return -EINVAL;
- if (type != BPF_PROG_TYPE_SOCKET_FILTER && !capable(CAP_SYS_ADMIN))
+ if (type != BPF_PROG_TYPE_SOCKET_FILTER &&
+ type != BPF_PROG_TYPE_CGROUP_SKB &&
+ !capable(CAP_SYS_ADMIN))
return -EPERM;
/* plain bpf_prog allocation */
--
2.7.4
^ permalink raw reply related
* Re: [PATCH] b43legacy: Fix a sleep-in-atomic bug in b43legacy_op_bss_info_changed
From: Jia-Ju Bai @ 2017-06-01 1:07 UTC (permalink / raw)
To: Larry Finger
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
b43-dev-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Michael Büsch,
Kalle Valo
In-Reply-To: <284d6d5d-d548-9e05-eafd-a6b521af5a04-tQ5ms3gMjBLk1uMJSBkQmQ@public.gmane.org>
On 06/01/2017 08:07 AM, Larry Finger wrote:
> On 05/31/2017 10:32 AM, Michael Büsch wrote:
>> On Wed, 31 May 2017 13:26:43 +0300
>> Kalle Valo <kvalo-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> wrote:
>>
>>> Jia-Ju Bai <baijiaju1990-9Onoh4P/yGk@public.gmane.org> writes:
>>>
>>>> The driver may sleep under a spin lock, and the function call path is:
>>>> b43legacy_op_bss_info_changed (acquire the lock by spin_lock_irqsave)
>>>> b43legacy_synchronize_irq
>>>> synchronize_irq --> may sleep
>>>>
>>>> To fix it, the lock is released before b43legacy_synchronize_irq,
>>>> and the
>>>> lock is acquired again after this function.
>>>>
>>>> Signed-off-by: Jia-Ju Bai <baijiaju1990-9Onoh4P/yGk@public.gmane.org>
>>>> ---
>>>> drivers/net/wireless/broadcom/b43legacy/main.c | 2 ++
>>>> 1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/drivers/net/wireless/broadcom/b43legacy/main.c
>>>> b/drivers/net/wireless/broadcom/b43legacy/main.c
>>>> index f1e3dad..31ead21 100644
>>>> --- a/drivers/net/wireless/broadcom/b43legacy/main.c
>>>> +++ b/drivers/net/wireless/broadcom/b43legacy/main.c
>>>> @@ -2859,7 +2859,9 @@ static void
>>>> b43legacy_op_bss_info_changed(struct ieee80211_hw *hw,
>>>> b43legacy_write32(dev, B43legacy_MMIO_GEN_IRQ_MASK, 0);
>>>> if (changed & BSS_CHANGED_BSSID) {
>>>> + spin_unlock_irqrestore(&wl->irq_lock, flags);
>>>> b43legacy_synchronize_irq(dev);
>>>> + spin_lock_irqsave(&wl->irq_lock, flags);
>>>
>>> To me this looks like a fragile workaround and not a real fix. You can
>>> easily add new race conditions with releasing the lock like this.
>>>
>>
>>
>> I think releasing the lock possibly is fine. It certainly is better than
>> sleeping with a lock held.
>> We disabled the device interrupts just before this line.
>>
>> However I think the synchronize_irq should be outside of the
>> conditional right after the write to B43legacy_MMIO_GEN_IRQ_MASK. (So
>> two lines above)
>> I don't think it makes sense to only synchronize if BSS_CHANGED_BSSID
>> is set.
>>
>>
>> On the other hand b43 does not have this irq-disabling foobar anymore.
>> So somebody must have removed it. Maybe you can find the commit that
>> removed this stuff from b43 and port it to b43legacy?
>>
>>
>> So I would vote for moving the synchronize_irq up outside of the
>> conditional and put the unlock/lock sequence around it.
>> And as a second patch on top of that try to remove this stuff
>> altogether like b43 did.
>
> The patch that removed it in b43 is
>
> commit 36dbd9548e92268127b0c31b0e121e63e9207108
> Author: Michael Buesch <mb-fseUSCV1ubazQB+pC5nmwQ@public.gmane.org>
> Date: Fri Sep 4 22:51:29 2009 +0200
>
> b43: Use a threaded IRQ handler
>
> Use a threaded IRQ handler to allow locking the mutex and
> sleeping while executing an interrupt.
> This removes usage of the irq_lock spinlock, but introduces
> a new hardirq_lock, which is _only_ used for the PCI/SSB lowlevel
> hard-irq handler. Sleeping busses (SDIO) will use mutex instead.
>
> Signed-off-by: Michael Buesch <mb-fseUSCV1ubazQB+pC5nmwQ@public.gmane.org>
> Tested-by: Larry Finger <Larry.Finger-tQ5ms3gMjBLk1uMJSBkQmQ@public.gmane.org>
> Signed-off-by: John W. Linville <linville-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
>
> I vaguely remember this patch. Although it is roughly a 1000-line fix,
> I will try to port it to b43legacy. I still have an old BCM4306 PCMCIA
> card that I can test in a PowerBook G4.
>
> I agree with Michael that this is the way to go. Both of Jia-Ju's
> patches should be rejected.
>
> Larry
>
>
It is fine to me to fix the bug by porting this former patch.
Thanks,
Jia-Ju Bai
^ permalink raw reply
* Re: [PATCH] b43legacy: Fix a sleep-in-atomic bug in b43legacy_attr_interfmode_store
From: Jia-Ju Bai @ 2017-06-01 1:05 UTC (permalink / raw)
To: Larry Finger
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
b43-dev-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
kvalo-sgV2jX0FEOL9JmXXK+q4OQ, linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <85905124-7167-aeb0-8aff-4ceec09e9542-tQ5ms3gMjBLk1uMJSBkQmQ@public.gmane.org>
On 06/01/2017 01:33 AM, Larry Finger wrote:
> On 05/31/2017 05:29 AM, Jia-Ju Bai wrote:
>> The driver may sleep under a spin lock, and the function call path is:
>> b43legacy_attr_interfmode_store (acquire the lock by spin_lock_irqsave)
>> b43legacy_radio_set_interference_mitigation
>> b43legacy_radio_interference_mitigation_disable
>> b43legacy_calc_nrssi_slope
>> b43legacy_synth_pu_workaround
>> might_sleep and msleep --> may sleep
>>
>> Fixing it may be complex, and a possible way is to remove
>> spin_lock_irqsave and spin_lock_irqrestore in
>> b43legacy_attr_interfmode_store, and the code has been protected by
>> mutex_lock and mutex_unlock.
>>
>> Signed-off-by: Jia-Ju Bai <baijiaju1990-9Onoh4P/yGk@public.gmane.org>
>> ---
>> drivers/net/wireless/broadcom/b43legacy/sysfs.c | 2 --
>> 1 file changed, 2 deletions(-)
>>
>> diff --git a/drivers/net/wireless/broadcom/b43legacy/sysfs.c
>> b/drivers/net/wireless/broadcom/b43legacy/sysfs.c
>> index 2a1da15..9ede143 100644
>> --- a/drivers/net/wireless/broadcom/b43legacy/sysfs.c
>> +++ b/drivers/net/wireless/broadcom/b43legacy/sysfs.c
>> @@ -137,14 +137,12 @@ static ssize_t
>> b43legacy_attr_interfmode_store(struct device *dev,
>> }
>> mutex_lock(&wldev->wl->mutex);
>> - spin_lock_irqsave(&wldev->wl->irq_lock, flags);
>> err = b43legacy_radio_set_interference_mitigation(wldev, mode);
>> if (err)
>> b43legacyerr(wldev->wl, "Interference Mitigation not "
>> "supported by device\n");
>> mmiowb();
>> - spin_unlock_irqrestore(&wldev->wl->irq_lock, flags);
>> mutex_unlock(&wldev->wl->mutex);
>> return err ? err : count;
>>
>
> Jia-Ju,
>
> Did you actually observe the attempt to sleep under the spin lock, or
> did you discover this using some tool? In other words, have either of
> your patches been tested?
>
> Larry
>
Hi,
In fact, my reported bugs are found by a static analysis tool written by
me, and they are checked by my review of the driver code.
I admit my patches are not well tested, and they may not well fix the bugs.
I am looking forward to opinions and suggestions :)
Thanks,
Jia-Ju Bai
^ permalink raw reply
* Re: [PATCH v5 net-next 0/5] dsa: add Microchip KSZ9477 DSA driver
From: David Miller @ 2017-06-01 0:57 UTC (permalink / raw)
To: Woojung.Huh; +Cc: netdev, andrew, f.fainelli, UNGLinuxDriver
In-Reply-To: <9235D6609DB808459E95D78E17F2E43D40AA5556@CHN-SV-EXMX02.mchp-main.com>
From: <Woojung.Huh@microchip.com>
Date: Wed, 31 May 2017 20:19:00 +0000
> From: Woojung Huh <Woojung.Huh@microchip.com>
>
> This series of patches is for Microchip KSZ9477 DSA driver.
> KSZ9477 is 7 ports GigE switch with numerous advanced features.
> 5 ports are 10/100/1000 Mbps internal PHYs and 2 ports have
> Interfaces to SGMII, RGMII, MII or RMII.
>
> This patch supports VLAN, MDB, FDB and port mirroring offloads.
>
> Welcome reviews and comments from community.
>
> Note: Tests are performed on internal development board.
>
> V5
> - add missing MODULE_LICENSE
Series applied, thanks.
^ permalink raw reply
* [PATCH] xfrm: fix xfrm_dev_event() missing when compile without CONFIG_XFRM_OFFLOAD
From: Hangbin Liu @ 2017-06-01 0:38 UTC (permalink / raw)
To: netdev; +Cc: Guy Shapiro, Steffen Klassert, David Miller, Hangbin Liu
In commit d77e38e612a0 ("xfrm: Add an IPsec hardware offloading API") we
make xfrm_device.o only compiled when enable option CONFIG_XFRM_OFFLOAD.
But this will make xfrm_dev_event() missing if we only enable default XFRM
options.
So when we set down and unregister an interface with IPsec on it. there
will no xfrm_garbage_collect(), which will cause dev usage count hold and
get error like:
unregister_netdevice: waiting for <dev> to become free. Usage count = 4
Fixes: d77e38e612a0 ("xfrm: Add an IPsec hardware offloading API")
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
net/xfrm/Makefile | 3 +--
net/xfrm/xfrm_device.c | 2 ++
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/net/xfrm/Makefile b/net/xfrm/Makefile
index abf81b3..55b2ac3 100644
--- a/net/xfrm/Makefile
+++ b/net/xfrm/Makefile
@@ -4,8 +4,7 @@
obj-$(CONFIG_XFRM) := xfrm_policy.o xfrm_state.o xfrm_hash.o \
xfrm_input.o xfrm_output.o \
- xfrm_sysctl.o xfrm_replay.o
-obj-$(CONFIG_XFRM_OFFLOAD) += xfrm_device.o
+ xfrm_sysctl.o xfrm_replay.o xfrm_device.o
obj-$(CONFIG_XFRM_STATISTICS) += xfrm_proc.o
obj-$(CONFIG_XFRM_ALGO) += xfrm_algo.o
obj-$(CONFIG_XFRM_USER) += xfrm_user.o
diff --git a/net/xfrm/xfrm_device.c b/net/xfrm/xfrm_device.c
index 574e6f3..5aba036 100644
--- a/net/xfrm/xfrm_device.c
+++ b/net/xfrm/xfrm_device.c
@@ -22,6 +22,7 @@
#include <net/xfrm.h>
#include <linux/notifier.h>
+#ifdef CONFIG_XFRM_OFFLOAD
int validate_xmit_xfrm(struct sk_buff *skb, netdev_features_t features)
{
int err;
@@ -137,6 +138,7 @@ bool xfrm_dev_offload_ok(struct sk_buff *skb, struct xfrm_state *x)
return true;
}
EXPORT_SYMBOL_GPL(xfrm_dev_offload_ok);
+#endif
int xfrm_dev_register(struct net_device *dev)
{
--
2.5.5
^ permalink raw reply related
* Re: [PATCH net-next] liquidio: Fix checkpatch errors with references crossing single line
From: Joe Perches @ 2017-06-01 0:31 UTC (permalink / raw)
To: Felix Manlunas, davem
Cc: netdev, raghu.vatsavayi, derek.chickles, satananda.burla
In-Reply-To: <20170531174515.GA1539@felix-thinkpad.cavium.com>
On Wed, 2017-05-31 at 10:45 -0700, Felix Manlunas wrote:
> > From: Satanand Burla <satananda.burla@cavium.com>
>
> > Signed-off-by: Satanand Burla <satananda.burla@cavium.com>
> > Signed-off-by: Derek Chickles <derek.chickles@cavium.com>
> > Signed-off-by: Felix Manlunas <felix.manlunas@cavium.com>
> ---
> drivers/net/ethernet/cavium/liquidio/lio_ethtool.c | 8 ++++----
Much better would be not to separate out the object onto
multiple lines, but use temporaries to make the code "volume"
much smaller and it also saves a bit of object code.
For this file, perhaps
Subject: [PATCH] liquidio: Reduce dereferences
Multiple dereferences of the same object are hard to read.
Use temporaries to reduce the code volume and also reduce
overall object size a bit.
$ size drivers/net/ethernet/cavium/liquidio/lio_ethtool.o*
text data bss dec hex filename
22038 0 0 22038 5616 drivers/net/ethernet/cavium/liquidio/lio_ethtool.o.new
22246 0 0 22246 56e6 drivers/net/ethernet/cavium/liquidio/lio_ethtool.o.old
Signed-off-by: Joe Perches <joe@perches.com>
---
drivers/net/ethernet/cavium/liquidio/lio_ethtool.c | 450 ++++++++++-----------
1 file changed, 215 insertions(+), 235 deletions(-)
diff --git a/drivers/net/ethernet/cavium/liquidio/lio_ethtool.c b/drivers/net/ethernet/cavium/liquidio/lio_ethtool.c
index 579dc7336f58..bd2db6382377 100644
--- a/drivers/net/ethernet/cavium/liquidio/lio_ethtool.c
+++ b/drivers/net/ethernet/cavium/liquidio/lio_ethtool.c
@@ -786,257 +786,245 @@ lio_get_ethtool_stats(struct net_device *netdev,
struct lio *lio = GET_LIO(netdev);
struct octeon_device *oct_dev = lio->oct_dev;
struct net_device_stats *netstats = &netdev->stats;
+ struct nic_tx_stats *tstats = &oct_dev->link_stats.fromhost;
+ struct nic_rx_stats *rstats = &oct_dev->link_stats.fromwire;
int i = 0, j;
netdev->netdev_ops->ndo_get_stats(netdev);
octnet_get_link_stats(netdev);
- /*sum of oct->droq[oq_no]->stats->rx_pkts_received */
+ /* sum of oct->droq[oq_no]->stats->rx_pkts_received */
data[i++] = CVM_CAST64(netstats->rx_packets);
- /*sum of oct->instr_queue[iq_no]->stats.tx_done */
+ /* sum of oct->instr_queue[iq_no]->stats.tx_done */
data[i++] = CVM_CAST64(netstats->tx_packets);
- /*sum of oct->droq[oq_no]->stats->rx_bytes_received */
+ /* sum of oct->droq[oq_no]->stats->rx_bytes_received */
data[i++] = CVM_CAST64(netstats->rx_bytes);
- /*sum of oct->instr_queue[iq_no]->stats.tx_tot_bytes */
+ /* sum of oct->instr_queue[iq_no]->stats.tx_tot_bytes */
data[i++] = CVM_CAST64(netstats->tx_bytes);
data[i++] = CVM_CAST64(netstats->rx_errors);
data[i++] = CVM_CAST64(netstats->tx_errors);
- /*sum of oct->droq[oq_no]->stats->rx_dropped +
- *oct->droq[oq_no]->stats->dropped_nodispatch +
- *oct->droq[oq_no]->stats->dropped_toomany +
- *oct->droq[oq_no]->stats->dropped_nomem
+ /* sum of oct->droq[oq_no]->stats->rx_dropped +
+ * oct->droq[oq_no]->stats->dropped_nodispatch +
+ * oct->droq[oq_no]->stats->dropped_toomany +
+ * oct->droq[oq_no]->stats->dropped_nomem
*/
data[i++] = CVM_CAST64(netstats->rx_dropped);
- /*sum of oct->instr_queue[iq_no]->stats.tx_dropped */
+ /* sum of oct->instr_queue[iq_no]->stats.tx_dropped */
data[i++] = CVM_CAST64(netstats->tx_dropped);
/* firmware tx stats */
- /*per_core_stats[cvmx_get_core_num()].link_stats[mdata->from_ifidx].
- *fromhost.fw_total_sent
+ /* per_core_stats[cvmx_get_core_num()].link_stats[mdata->from_ifidx].
+ * fromhost.fw_total_sent
*/
- data[i++] = CVM_CAST64(oct_dev->link_stats.fromhost.fw_total_sent);
- /*per_core_stats[i].link_stats[port].fromwire.fw_total_fwd */
- data[i++] = CVM_CAST64(oct_dev->link_stats.fromhost.fw_total_fwd);
- /*per_core_stats[j].link_stats[i].fromhost.fw_err_pko */
- data[i++] = CVM_CAST64(oct_dev->link_stats.fromhost.fw_err_pko);
- /*per_core_stats[j].link_stats[i].fromhost.fw_err_link */
- data[i++] = CVM_CAST64(oct_dev->link_stats.fromhost.fw_err_link);
- /*per_core_stats[cvmx_get_core_num()].link_stats[idx].fromhost.
- *fw_err_drop
+ data[i++] = CVM_CAST64(tstats->fw_total_sent);
+ /* per_core_stats[i].link_stats[port].fromwire.fw_total_fwd */
+ data[i++] = CVM_CAST64(tstats->fw_total_fwd);
+ /* per_core_stats[j].link_stats[i].fromhost.fw_err_pko */
+ data[i++] = CVM_CAST64(tstats->fw_err_pko);
+ /* per_core_stats[j].link_stats[i].fromhost.fw_err_link */
+ data[i++] = CVM_CAST64(tstats->fw_err_link);
+ /* per_core_stats[cvmx_get_core_num()].link_stats[idx].fromhost.
+ * fw_err_drop
*/
- data[i++] = CVM_CAST64(oct_dev->link_stats.fromhost.fw_err_drop);
+ data[i++] = CVM_CAST64(tstats->fw_err_drop);
- /*per_core_stats[cvmx_get_core_num()].link_stats[idx].fromhost.fw_tso */
- data[i++] = CVM_CAST64(oct_dev->link_stats.fromhost.fw_tso);
- /*per_core_stats[cvmx_get_core_num()].link_stats[idx].fromhost.
- *fw_tso_fwd
+ /* per_core_stats[cvmx_get_core_num()].link_stats[idx].fromhost.fw_tso */
+ data[i++] = CVM_CAST64(tstats->fw_tso);
+ /* per_core_stats[cvmx_get_core_num()].link_stats[idx].fromhost.
+ * fw_tso_fwd
*/
- data[i++] = CVM_CAST64(oct_dev->link_stats.fromhost.fw_tso_fwd);
- /*per_core_stats[cvmx_get_core_num()].link_stats[idx].fromhost.
- *fw_err_tso
+ data[i++] = CVM_CAST64(tstats->fw_tso_fwd);
+ /* per_core_stats[cvmx_get_core_num()].link_stats[idx].fromhost.
+ * fw_err_tso
*/
- data[i++] = CVM_CAST64(oct_dev->link_stats.fromhost.fw_err_tso);
- /*per_core_stats[cvmx_get_core_num()].link_stats[idx].fromhost.
- *fw_tx_vxlan
+ data[i++] = CVM_CAST64(tstats->fw_err_tso);
+ /* per_core_stats[cvmx_get_core_num()].link_stats[idx].fromhost.
+ * fw_tx_vxlan
*/
- data[i++] = CVM_CAST64(oct_dev->link_stats.fromhost.fw_tx_vxlan);
+ data[i++] = CVM_CAST64(tstats->fw_tx_vxlan);
/* mac tx statistics */
- /*CVMX_BGXX_CMRX_TX_STAT5 */
- data[i++] = CVM_CAST64(oct_dev->link_stats.fromhost.total_pkts_sent);
- /*CVMX_BGXX_CMRX_TX_STAT4 */
- data[i++] = CVM_CAST64(oct_dev->link_stats.fromhost.total_bytes_sent);
- /*CVMX_BGXX_CMRX_TX_STAT15 */
- data[i++] = CVM_CAST64(oct_dev->link_stats.fromhost.mcast_pkts_sent);
- /*CVMX_BGXX_CMRX_TX_STAT14 */
- data[i++] = CVM_CAST64(oct_dev->link_stats.fromhost.bcast_pkts_sent);
- /*CVMX_BGXX_CMRX_TX_STAT17 */
- data[i++] = CVM_CAST64(oct_dev->link_stats.fromhost.ctl_sent);
- /*CVMX_BGXX_CMRX_TX_STAT0 */
- data[i++] = CVM_CAST64(oct_dev->link_stats.fromhost.total_collisions);
- /*CVMX_BGXX_CMRX_TX_STAT3 */
- data[i++] = CVM_CAST64(oct_dev->link_stats.fromhost.one_collision_sent);
- /*CVMX_BGXX_CMRX_TX_STAT2 */
- data[i++] =
- CVM_CAST64(oct_dev->link_stats.fromhost.multi_collision_sent);
- /*CVMX_BGXX_CMRX_TX_STAT0 */
- data[i++] = CVM_CAST64(oct_dev->link_stats.fromhost.max_collision_fail);
- /*CVMX_BGXX_CMRX_TX_STAT1 */
- data[i++] = CVM_CAST64(oct_dev->link_stats.fromhost.max_deferral_fail);
- /*CVMX_BGXX_CMRX_TX_STAT16 */
- data[i++] = CVM_CAST64(oct_dev->link_stats.fromhost.fifo_err);
- /*CVMX_BGXX_CMRX_TX_STAT6 */
- data[i++] = CVM_CAST64(oct_dev->link_stats.fromhost.runts);
+ /* CVMX_BGXX_CMRX_TX_STAT5 */
+ data[i++] = CVM_CAST64(tstats->total_pkts_sent);
+ /* CVMX_BGXX_CMRX_TX_STAT4 */
+ data[i++] = CVM_CAST64(tstats->total_bytes_sent);
+ /* CVMX_BGXX_CMRX_TX_STAT15 */
+ data[i++] = CVM_CAST64(tstats->mcast_pkts_sent);
+ /* CVMX_BGXX_CMRX_TX_STAT14 */
+ data[i++] = CVM_CAST64(tstats->bcast_pkts_sent);
+ /* CVMX_BGXX_CMRX_TX_STAT17 */
+ data[i++] = CVM_CAST64(tstats->ctl_sent);
+ /* CVMX_BGXX_CMRX_TX_STAT0 */
+ data[i++] = CVM_CAST64(tstats->total_collisions);
+ /* CVMX_BGXX_CMRX_TX_STAT3 */
+ data[i++] = CVM_CAST64(tstats->one_collision_sent);
+ /* CVMX_BGXX_CMRX_TX_STAT2 */
+ data[i++] = CVM_CAST64(tstats->multi_collision_sent);
+ /* CVMX_BGXX_CMRX_TX_STAT0 */
+ data[i++] = CVM_CAST64(tstats->max_collision_fail);
+ /* CVMX_BGXX_CMRX_TX_STAT1 */
+ data[i++] = CVM_CAST64(tstats->max_deferral_fail);
+ /* CVMX_BGXX_CMRX_TX_STAT16 */
+ data[i++] = CVM_CAST64(tstats->fifo_err);
+ /* CVMX_BGXX_CMRX_TX_STAT6 */
+ data[i++] = CVM_CAST64(tstats->runts);
/* RX firmware stats */
- /*per_core_stats[cvmx_get_core_num()].link_stats[ifidx].fromwire.
- *fw_total_rcvd
+ /* per_core_stats[cvmx_get_core_num()].link_stats[ifidx].fromwire.
+ * fw_total_rcvd
*/
- data[i++] = CVM_CAST64(oct_dev->link_stats.fromwire.fw_total_rcvd);
- /*per_core_stats[cvmx_get_core_num()].link_stats[ifidx].fromwire.
- *fw_total_fwd
+ data[i++] = CVM_CAST64(rstats->fw_total_rcvd);
+ /* per_core_stats[cvmx_get_core_num()].link_stats[ifidx].fromwire.
+ * fw_total_fwd
*/
- data[i++] = CVM_CAST64(oct_dev->link_stats.fromwire.fw_total_fwd);
- /*per_core_stats[core_id].link_stats[ifidx].fromwire.jabber_err */
- data[i++] = CVM_CAST64(oct_dev->link_stats.fromwire.jabber_err);
- /*per_core_stats[core_id].link_stats[ifidx].fromwire.l2_err */
- data[i++] = CVM_CAST64(oct_dev->link_stats.fromwire.l2_err);
- /*per_core_stats[core_id].link_stats[ifidx].fromwire.frame_err */
- data[i++] = CVM_CAST64(oct_dev->link_stats.fromwire.frame_err);
- /*per_core_stats[cvmx_get_core_num()].link_stats[ifidx].fromwire.
- *fw_err_pko
+ data[i++] = CVM_CAST64(rstats->fw_total_fwd);
+ /* per_core_stats[core_id].link_stats[ifidx].fromwire.jabber_err */
+ data[i++] = CVM_CAST64(rstats->jabber_err);
+ /* per_core_stats[core_id].link_stats[ifidx].fromwire.l2_err */
+ data[i++] = CVM_CAST64(rstats->l2_err);
+ /* per_core_stats[core_id].link_stats[ifidx].fromwire.frame_err */
+ data[i++] = CVM_CAST64(rstats->frame_err);
+ /* per_core_stats[cvmx_get_core_num()].link_stats[ifidx].fromwire.
+ * fw_err_pko
*/
- data[i++] = CVM_CAST64(oct_dev->link_stats.fromwire.fw_err_pko);
- /*per_core_stats[j].link_stats[i].fromwire.fw_err_link */
- data[i++] = CVM_CAST64(oct_dev->link_stats.fromwire.fw_err_link);
- /*per_core_stats[cvmx_get_core_num()].link_stats[lro_ctx->ifidx].
- *fromwire.fw_err_drop
+ data[i++] = CVM_CAST64(rstats->fw_err_pko);
+ /* per_core_stats[j].link_stats[i].fromwire.fw_err_link */
+ data[i++] = CVM_CAST64(rstats->fw_err_link);
+ /* per_core_stats[cvmx_get_core_num()].link_stats[lro_ctx->ifidx].
+ * fromwire.fw_err_drop
*/
- data[i++] = CVM_CAST64(oct_dev->link_stats.fromwire.fw_err_drop);
+ data[i++] = CVM_CAST64(rstats->fw_err_drop);
- /*per_core_stats[cvmx_get_core_num()].link_stats[lro_ctx->ifidx].
- *fromwire.fw_rx_vxlan
+ /* per_core_stats[cvmx_get_core_num()].link_stats[lro_ctx->ifidx].
+ * fromwire.fw_rx_vxlan
*/
- data[i++] = CVM_CAST64(oct_dev->link_stats.fromwire.fw_rx_vxlan);
- /*per_core_stats[cvmx_get_core_num()].link_stats[lro_ctx->ifidx].
- *fromwire.fw_rx_vxlan_err
+ data[i++] = CVM_CAST64(rstats->fw_rx_vxlan);
+ /* per_core_stats[cvmx_get_core_num()].link_stats[lro_ctx->ifidx].
+ * fromwire.fw_rx_vxlan_err
*/
- data[i++] = CVM_CAST64(oct_dev->link_stats.fromwire.fw_rx_vxlan_err);
+ data[i++] = CVM_CAST64(rstats->fw_rx_vxlan_err);
/* LRO */
- /*per_core_stats[cvmx_get_core_num()].link_stats[ifidx].fromwire.
- *fw_lro_pkts
+ /* per_core_stats[cvmx_get_core_num()].link_stats[ifidx].fromwire.
+ * fw_lro_pkts
*/
- data[i++] = CVM_CAST64(oct_dev->link_stats.fromwire.fw_lro_pkts);
- /*per_core_stats[cvmx_get_core_num()].link_stats[ifidx].fromwire.
- *fw_lro_octs
+ data[i++] = CVM_CAST64(rstats->fw_lro_pkts);
+ /* per_core_stats[cvmx_get_core_num()].link_stats[ifidx].fromwire.
+ * fw_lro_octs
*/
- data[i++] = CVM_CAST64(oct_dev->link_stats.fromwire.fw_lro_octs);
- /*per_core_stats[j].link_stats[i].fromwire.fw_total_lro */
- data[i++] = CVM_CAST64(oct_dev->link_stats.fromwire.fw_total_lro);
- /*per_core_stats[j].link_stats[i].fromwire.fw_lro_aborts */
- data[i++] = CVM_CAST64(oct_dev->link_stats.fromwire.fw_lro_aborts);
- /*per_core_stats[cvmx_get_core_num()].link_stats[ifidx].fromwire.
- *fw_lro_aborts_port
+ data[i++] = CVM_CAST64(rstats->fw_lro_octs);
+ /* per_core_stats[j].link_stats[i].fromwire.fw_total_lro */
+ data[i++] = CVM_CAST64(rstats->fw_total_lro);
+ /* per_core_stats[j].link_stats[i].fromwire.fw_lro_aborts */
+ data[i++] = CVM_CAST64(rstats->fw_lro_aborts);
+ /* per_core_stats[cvmx_get_core_num()].link_stats[ifidx].fromwire.
+ * fw_lro_aborts_port
*/
- data[i++] = CVM_CAST64(oct_dev->link_stats.fromwire.fw_lro_aborts_port);
- /*per_core_stats[cvmx_get_core_num()].link_stats[ifidx].fromwire.
- *fw_lro_aborts_seq
+ data[i++] = CVM_CAST64(rstats->fw_lro_aborts_port);
+ /* per_core_stats[cvmx_get_core_num()].link_stats[ifidx].fromwire.
+ * fw_lro_aborts_seq
*/
- data[i++] = CVM_CAST64(oct_dev->link_stats.fromwire.fw_lro_aborts_seq);
- /*per_core_stats[cvmx_get_core_num()].link_stats[ifidx].fromwire.
- *fw_lro_aborts_tsval
+ data[i++] = CVM_CAST64(rstats->fw_lro_aborts_seq);
+ /* per_core_stats[cvmx_get_core_num()].link_stats[ifidx].fromwire.
+ * fw_lro_aborts_tsval
*/
- data[i++] =
- CVM_CAST64(oct_dev->link_stats.fromwire.fw_lro_aborts_tsval);
- /*per_core_stats[cvmx_get_core_num()].link_stats[ifidx].fromwire.
- *fw_lro_aborts_timer
+ data[i++] = CVM_CAST64(rstats->fw_lro_aborts_tsval);
+ /* per_core_stats[cvmx_get_core_num()].link_stats[ifidx].fromwire.
+ * fw_lro_aborts_timer
*/
/* intrmod: packet forward rate */
- data[i++] =
- CVM_CAST64(oct_dev->link_stats.fromwire.fw_lro_aborts_timer);
- /*per_core_stats[j].link_stats[i].fromwire.fw_lro_aborts */
- data[i++] = CVM_CAST64(oct_dev->link_stats.fromwire.fwd_rate);
+ data[i++] = CVM_CAST64(rstats->fw_lro_aborts_timer);
+ /* per_core_stats[j].link_stats[i].fromwire.fw_lro_aborts */
+ data[i++] = CVM_CAST64(rstats->fwd_rate);
/* mac: link-level stats */
- /*CVMX_BGXX_CMRX_RX_STAT0 */
- data[i++] = CVM_CAST64(oct_dev->link_stats.fromwire.total_rcvd);
- /*CVMX_BGXX_CMRX_RX_STAT1 */
- data[i++] = CVM_CAST64(oct_dev->link_stats.fromwire.bytes_rcvd);
- /*CVMX_PKI_STATX_STAT5 */
- data[i++] = CVM_CAST64(oct_dev->link_stats.fromwire.total_bcst);
- /*CVMX_PKI_STATX_STAT5 */
- data[i++] = CVM_CAST64(oct_dev->link_stats.fromwire.total_mcst);
- /*wqe->word2.err_code or wqe->word2.err_level */
- data[i++] = CVM_CAST64(oct_dev->link_stats.fromwire.runts);
- /*CVMX_BGXX_CMRX_RX_STAT2 */
- data[i++] = CVM_CAST64(oct_dev->link_stats.fromwire.ctl_rcvd);
- /*CVMX_BGXX_CMRX_RX_STAT6 */
- data[i++] = CVM_CAST64(oct_dev->link_stats.fromwire.fifo_err);
- /*CVMX_BGXX_CMRX_RX_STAT4 */
- data[i++] = CVM_CAST64(oct_dev->link_stats.fromwire.dmac_drop);
- /*wqe->word2.err_code or wqe->word2.err_level */
- data[i++] = CVM_CAST64(oct_dev->link_stats.fromwire.fcs_err);
- /*lio->link_changes*/
+ /* CVMX_BGXX_CMRX_RX_STAT0 */
+ data[i++] = CVM_CAST64(rstats->total_rcvd);
+ /* CVMX_BGXX_CMRX_RX_STAT1 */
+ data[i++] = CVM_CAST64(rstats->bytes_rcvd);
+ /* CVMX_PKI_STATX_STAT5 */
+ data[i++] = CVM_CAST64(rstats->total_bcst);
+ /* CVMX_PKI_STATX_STAT5 */
+ data[i++] = CVM_CAST64(rstats->total_mcst);
+ /* wqe->word2.err_code or wqe->word2.err_level */
+ data[i++] = CVM_CAST64(rstats->runts);
+ /* CVMX_BGXX_CMRX_RX_STAT2 */
+ data[i++] = CVM_CAST64(rstats->ctl_rcvd);
+ /* CVMX_BGXX_CMRX_RX_STAT6 */
+ data[i++] = CVM_CAST64(rstats->fifo_err);
+ /* CVMX_BGXX_CMRX_RX_STAT4 */
+ data[i++] = CVM_CAST64(rstats->dmac_drop);
+ /* wqe->word2.err_code or wqe->word2.err_level */
+ data[i++] = CVM_CAST64(rstats->fcs_err);
+ /* lio->link_changes*/
data[i++] = CVM_CAST64(lio->link_changes);
for (j = 0; j < MAX_OCTEON_INSTR_QUEUES(oct_dev); j++) {
+ struct oct_iq_stats *stats;
+
if (!(oct_dev->io_qmask.iq & BIT_ULL(j)))
continue;
- /*packets to network port*/
- /*# of packets tx to network */
- data[i++] = CVM_CAST64(oct_dev->instr_queue[j]->stats.tx_done);
- /*# of bytes tx to network */
- data[i++] =
- CVM_CAST64(oct_dev->instr_queue[j]->stats.tx_tot_bytes);
- /*# of packets dropped */
- data[i++] =
- CVM_CAST64(oct_dev->instr_queue[j]->stats.tx_dropped);
- /*# of tx fails due to queue full */
- data[i++] =
- CVM_CAST64(oct_dev->instr_queue[j]->stats.tx_iq_busy);
- /*XXX gather entries sent */
- data[i++] =
- CVM_CAST64(oct_dev->instr_queue[j]->stats.sgentry_sent);
-
- /*instruction to firmware: data and control */
- /*# of instructions to the queue */
- data[i++] =
- CVM_CAST64(oct_dev->instr_queue[j]->stats.instr_posted);
- /*# of instructions processed */
- data[i++] = CVM_CAST64(oct_dev->instr_queue[j]->
- stats.instr_processed);
- /*# of instructions could not be processed */
- data[i++] = CVM_CAST64(oct_dev->instr_queue[j]->
- stats.instr_dropped);
- /*bytes sent through the queue */
- data[i++] =
- CVM_CAST64(oct_dev->instr_queue[j]->stats.bytes_sent);
-
- /*tso request*/
- data[i++] = CVM_CAST64(oct_dev->instr_queue[j]->stats.tx_gso);
- /*vxlan request*/
- data[i++] = CVM_CAST64(oct_dev->instr_queue[j]->stats.tx_vxlan);
- /*txq restart*/
- data[i++] =
- CVM_CAST64(oct_dev->instr_queue[j]->stats.tx_restart);
+
+ stats = &oct_dev->instr_queue[j]->stats;
+
+ /* packets to network port */
+ /* # of packets tx to network */
+ data[i++] = CVM_CAST64(stats->tx_done);
+ /* # of bytes tx to network */
+ data[i++] = CVM_CAST64(stats->tx_tot_bytes);
+ /* # of packets dropped */
+ data[i++] = CVM_CAST64(stats->tx_dropped);
+ /* # of tx fails due to queue full */
+ data[i++] = CVM_CAST64(stats->tx_iq_busy);
+ /* XXX gather entries sent */
+ data[i++] = CVM_CAST64(stats->sgentry_sent);
+
+ /* instruction to firmware: data and control */
+ /* # of instructions to the queue */
+ data[i++] = CVM_CAST64(stats->instr_posted);
+ /* # of instructions processed */
+ data[i++] = CVM_CAST64(stats->instr_processed);
+ /* # of instructions could not be processed */
+ data[i++] = CVM_CAST64(stats->instr_dropped);
+ /* bytes sent through the queue */
+ data[i++] = CVM_CAST64(stats->bytes_sent);
+
+ /* tso request */
+ data[i++] = CVM_CAST64(stats->tx_gso);
+ /* vxlan request */
+ data[i++] = CVM_CAST64(stats->tx_vxlan);
+ /* txq restart */
+ data[i++] = CVM_CAST64(stats->tx_restart);
}
/* RX */
for (j = 0; j < MAX_OCTEON_OUTPUT_QUEUES(oct_dev); j++) {
+ struct oct_droq_stats *stats;
+
if (!(oct_dev->io_qmask.oq & BIT_ULL(j)))
continue;
- /*packets send to TCP/IP network stack */
- /*# of packets to network stack */
- data[i++] =
- CVM_CAST64(oct_dev->droq[j]->stats.rx_pkts_received);
- /*# of bytes to network stack */
- data[i++] =
- CVM_CAST64(oct_dev->droq[j]->stats.rx_bytes_received);
- /*# of packets dropped */
- data[i++] = CVM_CAST64(oct_dev->droq[j]->stats.dropped_nomem +
- oct_dev->droq[j]->stats.dropped_toomany +
- oct_dev->droq[j]->stats.rx_dropped);
- data[i++] =
- CVM_CAST64(oct_dev->droq[j]->stats.dropped_nomem);
- data[i++] =
- CVM_CAST64(oct_dev->droq[j]->stats.dropped_toomany);
- data[i++] =
- CVM_CAST64(oct_dev->droq[j]->stats.rx_dropped);
-
- /*control and data path*/
- data[i++] =
- CVM_CAST64(oct_dev->droq[j]->stats.pkts_received);
- data[i++] =
- CVM_CAST64(oct_dev->droq[j]->stats.bytes_received);
- data[i++] =
- CVM_CAST64(oct_dev->droq[j]->stats.dropped_nodispatch);
-
- data[i++] =
- CVM_CAST64(oct_dev->droq[j]->stats.rx_vxlan);
- data[i++] =
- CVM_CAST64(oct_dev->droq[j]->stats.rx_alloc_failure);
+ stats = &oct_dev->droq[j]->stats;
+ /* packets send to TCP/IP network stack */
+ /* # of packets to network stack */
+ data[i++] = CVM_CAST64(stats->rx_pkts_received);
+ /* # of bytes to network stack */
+ data[i++] = CVM_CAST64(stats->rx_bytes_received);
+ /* # of packets dropped */
+ data[i++] = CVM_CAST64(stats->dropped_nomem +
+ stats->dropped_toomany +
+ stats->rx_dropped);
+ data[i++] = CVM_CAST64(stats->dropped_nomem);
+ data[i++] = CVM_CAST64(stats->dropped_toomany);
+ data[i++] = CVM_CAST64(stats->rx_dropped);
+
+ /* control and data path */
+ data[i++] = CVM_CAST64(stats->pkts_received);
+ data[i++] = CVM_CAST64(stats->bytes_received);
+ data[i++] = CVM_CAST64(stats->dropped_nodispatch);
+
+ data[i++] = CVM_CAST64(stats->rx_vxlan);
+ data[i++] = CVM_CAST64(stats->rx_alloc_failure);
}
}
@@ -1073,73 +1061,65 @@ static void lio_vf_get_ethtool_stats(struct net_device *netdev,
data[i++] = CVM_CAST64(lio->link_changes);
for (vj = 0; vj < lio->linfo.num_txpciq; vj++) {
+ struct oct_iq_stats *stats;
+
j = lio->linfo.txpciq[vj].s.q_no;
+ stats = &oct_dev->instr_queue[j]->stats;
/* packets to network port */
/* # of packets tx to network */
- data[i++] = CVM_CAST64(oct_dev->instr_queue[j]->stats.tx_done);
+ data[i++] = CVM_CAST64(stats->tx_done);
/* # of bytes tx to network */
- data[i++] = CVM_CAST64(
- oct_dev->instr_queue[j]->stats.tx_tot_bytes);
+ data[i++] = CVM_CAST64(stats->tx_tot_bytes);
/* # of packets dropped */
- data[i++] = CVM_CAST64(
- oct_dev->instr_queue[j]->stats.tx_dropped);
+ data[i++] = CVM_CAST64(stats->tx_dropped);
/* # of tx fails due to queue full */
- data[i++] = CVM_CAST64(
- oct_dev->instr_queue[j]->stats.tx_iq_busy);
+ data[i++] = CVM_CAST64(stats->tx_iq_busy);
/* XXX gather entries sent */
- data[i++] = CVM_CAST64(
- oct_dev->instr_queue[j]->stats.sgentry_sent);
+ data[i++] = CVM_CAST64(stats->sgentry_sent);
/* instruction to firmware: data and control */
/* # of instructions to the queue */
- data[i++] = CVM_CAST64(
- oct_dev->instr_queue[j]->stats.instr_posted);
+ data[i++] = CVM_CAST64(stats->instr_posted);
/* # of instructions processed */
- data[i++] =
- CVM_CAST64(oct_dev->instr_queue[j]->stats.instr_processed);
+ data[i++] = CVM_CAST64(stats->instr_processed);
/* # of instructions could not be processed */
- data[i++] =
- CVM_CAST64(oct_dev->instr_queue[j]->stats.instr_dropped);
+ data[i++] = CVM_CAST64(stats->instr_dropped);
/* bytes sent through the queue */
- data[i++] = CVM_CAST64(
- oct_dev->instr_queue[j]->stats.bytes_sent);
+ data[i++] = CVM_CAST64(stats->bytes_sent);
/* tso request */
- data[i++] = CVM_CAST64(oct_dev->instr_queue[j]->stats.tx_gso);
+ data[i++] = CVM_CAST64(stats->tx_gso);
/* vxlan request */
- data[i++] = CVM_CAST64(oct_dev->instr_queue[j]->stats.tx_vxlan);
+ data[i++] = CVM_CAST64(stats->tx_vxlan);
/* txq restart */
- data[i++] = CVM_CAST64(
- oct_dev->instr_queue[j]->stats.tx_restart);
+ data[i++] = CVM_CAST64(stats->tx_restart);
}
/* RX */
for (vj = 0; vj < lio->linfo.num_rxpciq; vj++) {
- j = lio->linfo.rxpciq[vj].s.q_no;
+ struct oct_droq_stats *stats;
+ j = lio->linfo.rxpciq[vj].s.q_no;
+ stats = &oct_dev->droq[j]->stats;
/* packets send to TCP/IP network stack */
/* # of packets to network stack */
- data[i++] = CVM_CAST64(
- oct_dev->droq[j]->stats.rx_pkts_received);
+ data[i++] = CVM_CAST64(stats->rx_pkts_received);
/* # of bytes to network stack */
- data[i++] = CVM_CAST64(
- oct_dev->droq[j]->stats.rx_bytes_received);
- data[i++] = CVM_CAST64(oct_dev->droq[j]->stats.dropped_nomem +
- oct_dev->droq[j]->stats.dropped_toomany +
- oct_dev->droq[j]->stats.rx_dropped);
- data[i++] = CVM_CAST64(oct_dev->droq[j]->stats.dropped_nomem);
- data[i++] = CVM_CAST64(oct_dev->droq[j]->stats.dropped_toomany);
- data[i++] = CVM_CAST64(oct_dev->droq[j]->stats.rx_dropped);
+ data[i++] = CVM_CAST64(stats->rx_bytes_received);
+ data[i++] = CVM_CAST64(stats->dropped_nomem +
+ stats->dropped_toomany +
+ stats->rx_dropped);
+ data[i++] = CVM_CAST64(stats->dropped_nomem);
+ data[i++] = CVM_CAST64(stats->dropped_toomany);
+ data[i++] = CVM_CAST64(stats->rx_dropped);
/* control and data path */
- data[i++] = CVM_CAST64(oct_dev->droq[j]->stats.pkts_received);
- data[i++] = CVM_CAST64(oct_dev->droq[j]->stats.bytes_received);
- data[i++] =
- CVM_CAST64(oct_dev->droq[j]->stats.dropped_nodispatch);
-
- data[i++] = CVM_CAST64(oct_dev->droq[j]->stats.rx_vxlan);
- data[i++] =
- CVM_CAST64(oct_dev->droq[j]->stats.rx_alloc_failure);
+ data[i++] = CVM_CAST64(stats->pkts_received);
+ data[i++] = CVM_CAST64(stats->bytes_received);
+ data[i++] = CVM_CAST64(stats->dropped_nodispatch);
+
+ data[i++] = CVM_CAST64(stats->rx_vxlan);
+ data[i++] = CVM_CAST64(stats->rx_alloc_failure);
}
}
^ permalink raw reply related
* RE: [PATCH net-next 2/2] bpf: Remove the capability check for cgroup skb eBPF program
From: YUAN Linyu @ 2017-06-01 0:17 UTC (permalink / raw)
To: Chenbo Feng, netdev@vger.kernel.org, David Miller
Cc: Lorenzo Colitti, Chenbo Feng
In-Reply-To: <1496254013-18719-2-git-send-email-chenbofeng.kernel@gmail.com>
hi
> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org]
> On Behalf Of Chenbo Feng
> Sent: Thursday, June 01, 2017 2:07 AM
> To: netdev@vger.kernel.org; David Miller
> Cc: Lorenzo Colitti; Chenbo Feng
> Subject: [PATCH net-next 2/2] bpf: Remove the capability check for cgroup skb
> eBPF program
> - if (type != BPF_PROG_TYPE_SOCKET_FILTER
> && !capable(CAP_SYS_ADMIN))
> + if (type != BPF_PROG_TYPE_SOCKET_FILTER
> + && type != BPF_PROG_TYPE_CGROUP_SKB
> + && !capable(CAP_SYS_ADMIN))
checkpatch.pl prefer put && at end of line
> return -EPERM;
>
> /* plain bpf_prog allocation */
> --
> 2.7.4
^ permalink raw reply
* Re: net/ipv4: use-after-free in add_grec
From: Eric Dumazet @ 2017-06-01 0:13 UTC (permalink / raw)
To: Eric Dumazet
Cc: Cong Wang, Andrey Konovalov, David S. Miller, Alexey Kuznetsov,
James Morris, Hideaki YOSHIFUJI, Patrick McHardy, netdev, LKML,
Dmitry Vyukov, Kostya Serebryany, syzkaller
In-Reply-To: <CANn89iKC1ACPNfazjYF_jDr_Ojz6ROn3Ny_m0SHK5G+MZctDmQ@mail.gmail.com>
On Wed, 2017-05-31 at 16:55 -0700, Eric Dumazet wrote:
> The issue here is the timer firing while ip_mc_clear_src() has been
> already called.
>
> My patch should fix the problem.
>
> Or another one using del_timer_sync() instead of del_timer() in
> igmp_stop_timer(), but such a change would be more invasive,
> since the del_timer_sync() would need to happen while im->lock
> spinlock is not held.
BTW, I guess that Andrey could try to add a delay to trigger the bug
more often.
diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
index 44fd86de2823dd17de16276a8ec01b190e69b8b4..84fff17daab0832c470a613b29b2aaade07cec0a 100644
--- a/net/ipv4/igmp.c
+++ b/net/ipv4/igmp.c
@@ -798,7 +798,7 @@ static void igmp_timer_expire(unsigned long data)
}
im->reporter = 1;
spin_unlock(&im->lock);
-
+ udelay(10000);
if (IGMP_V1_SEEN(in_dev))
igmp_send_report(in_dev, im, IGMP_HOST_MEMBERSHIP_REPORT);
else if (IGMP_V2_SEEN(in_dev))
^ permalink raw reply related
* Re: [PATCH] b43legacy: Fix a sleep-in-atomic bug in b43legacy_op_bss_info_changed
From: Larry Finger @ 2017-06-01 0:07 UTC (permalink / raw)
To: Michael Büsch, Kalle Valo
Cc: Jia-Ju Bai, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
b43-dev-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170531173107.25eeda48@wiggum>
On 05/31/2017 10:32 AM, Michael Büsch wrote:
> On Wed, 31 May 2017 13:26:43 +0300
> Kalle Valo <kvalo-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> wrote:
>
>> Jia-Ju Bai <baijiaju1990-9Onoh4P/yGk@public.gmane.org> writes:
>>
>>> The driver may sleep under a spin lock, and the function call path is:
>>> b43legacy_op_bss_info_changed (acquire the lock by spin_lock_irqsave)
>>> b43legacy_synchronize_irq
>>> synchronize_irq --> may sleep
>>>
>>> To fix it, the lock is released before b43legacy_synchronize_irq, and the
>>> lock is acquired again after this function.
>>>
>>> Signed-off-by: Jia-Ju Bai <baijiaju1990-9Onoh4P/yGk@public.gmane.org>
>>> ---
>>> drivers/net/wireless/broadcom/b43legacy/main.c | 2 ++
>>> 1 file changed, 2 insertions(+)
>>>
>>> diff --git a/drivers/net/wireless/broadcom/b43legacy/main.c b/drivers/net/wireless/broadcom/b43legacy/main.c
>>> index f1e3dad..31ead21 100644
>>> --- a/drivers/net/wireless/broadcom/b43legacy/main.c
>>> +++ b/drivers/net/wireless/broadcom/b43legacy/main.c
>>> @@ -2859,7 +2859,9 @@ static void b43legacy_op_bss_info_changed(struct ieee80211_hw *hw,
>>> b43legacy_write32(dev, B43legacy_MMIO_GEN_IRQ_MASK, 0);
>>>
>>> if (changed & BSS_CHANGED_BSSID) {
>>> + spin_unlock_irqrestore(&wl->irq_lock, flags);
>>> b43legacy_synchronize_irq(dev);
>>> + spin_lock_irqsave(&wl->irq_lock, flags);
>>
>> To me this looks like a fragile workaround and not a real fix. You can
>> easily add new race conditions with releasing the lock like this.
>>
>
>
> I think releasing the lock possibly is fine. It certainly is better than
> sleeping with a lock held.
> We disabled the device interrupts just before this line.
>
> However I think the synchronize_irq should be outside of the
> conditional right after the write to B43legacy_MMIO_GEN_IRQ_MASK. (So
> two lines above)
> I don't think it makes sense to only synchronize if BSS_CHANGED_BSSID
> is set.
>
>
> On the other hand b43 does not have this irq-disabling foobar anymore.
> So somebody must have removed it. Maybe you can find the commit that
> removed this stuff from b43 and port it to b43legacy?
>
>
> So I would vote for moving the synchronize_irq up outside of the
> conditional and put the unlock/lock sequence around it.
> And as a second patch on top of that try to remove this stuff
> altogether like b43 did.
The patch that removed it in b43 is
commit 36dbd9548e92268127b0c31b0e121e63e9207108
Author: Michael Buesch <mb-fseUSCV1ubazQB+pC5nmwQ@public.gmane.org>
Date: Fri Sep 4 22:51:29 2009 +0200
b43: Use a threaded IRQ handler
Use a threaded IRQ handler to allow locking the mutex and
sleeping while executing an interrupt.
This removes usage of the irq_lock spinlock, but introduces
a new hardirq_lock, which is _only_ used for the PCI/SSB lowlevel
hard-irq handler. Sleeping busses (SDIO) will use mutex instead.
Signed-off-by: Michael Buesch <mb-fseUSCV1ubazQB+pC5nmwQ@public.gmane.org>
Tested-by: Larry Finger <Larry.Finger-tQ5ms3gMjBLk1uMJSBkQmQ@public.gmane.org>
Signed-off-by: John W. Linville <linville-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
I vaguely remember this patch. Although it is roughly a 1000-line fix, I will
try to port it to b43legacy. I still have an old BCM4306 PCMCIA card that I can
test in a PowerBook G4.
I agree with Michael that this is the way to go. Both of Jia-Ju's patches should
be rejected.
Larry
^ permalink raw reply
* Re: net/ipv4: use-after-free in add_grec
From: Eric Dumazet @ 2017-05-31 23:55 UTC (permalink / raw)
To: Cong Wang
Cc: Eric Dumazet, Andrey Konovalov, David S. Miller, Alexey Kuznetsov,
James Morris, Hideaki YOSHIFUJI, Patrick McHardy, netdev, LKML,
Dmitry Vyukov, Kostya Serebryany, syzkaller
In-Reply-To: <CAM_iQpUM+vhYXkA=ZaEg7O-hEHROPxPorS-T9nQwL6H9CLtMpA@mail.gmail.com>
On Wed, May 31, 2017 at 4:49 PM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> On Wed, May 31, 2017 at 9:12 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>> On Wed, 2017-05-31 at 11:46 +0200, Andrey Konovalov wrote:
>>> Hi,
>>>
>>> I've got the following error report while fuzzing the kernel with syzkaller.
>>>
>>> On commit 5ed02dbb497422bf225783f46e6eadd237d23d6b (4.12-rc3).
>>>
>>> Unfortunately it's not reproducible.
>>>
>>> ==================================================================
>>> BUG: KASAN: use-after-free in add_grec+0x101e/0x1090 net/ipv4/igmp.c:473
>>> Read of size 8 at addr ffff88003053c1a0 by task swapper/0/0
>>>
>>> CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.12.0-rc3+ #370
>>> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
>>> Call Trace:
>>> <IRQ>
>>> __dump_stack lib/dump_stack.c:16 [inline]
>>> dump_stack+0x292/0x395 lib/dump_stack.c:52
>>> print_address_description+0x73/0x280 mm/kasan/report.c:252
>>> kasan_report_error mm/kasan/report.c:351 [inline]
>>> kasan_report+0x22b/0x340 mm/kasan/report.c:408
>>> __asan_report_load8_noabort+0x14/0x20 mm/kasan/report.c:429
>>> add_grec+0x101e/0x1090 net/ipv4/igmp.c:473
>>> igmpv3_send_cr net/ipv4/igmp.c:663 [inline]
>>> igmp_ifc_timer_expire+0x46d/0xa80 net/ipv4/igmp.c:768
>>> IPVS: length: 51 != 8
>>> call_timer_fn+0x23f/0x800 kernel/time/timer.c:1268
>>> expire_timers kernel/time/timer.c:1307 [inline]
>>> __run_timers+0x94e/0xcd0 kernel/time/timer.c:1601
>>> run_timer_softirq+0x21/0x80 kernel/time/timer.c:1614
>>> __do_softirq+0x2fb/0xb99 kernel/softirq.c:284
>>> invoke_softirq kernel/softirq.c:364 [inline]
>>> irq_exit+0x19e/0x1d0 kernel/softirq.c:405
>>> exiting_irq arch/x86/include/asm/apic.h:652 [inline]
>>> smp_apic_timer_interrupt+0x76/0xa0 arch/x86/kernel/apic/apic.c:966
>>> apic_timer_interrupt+0x93/0xa0 arch/x86/entry/entry_64.S:481
>>> RIP: 0010:native_safe_halt+0x6/0x10 arch/x86/include/asm/irqflags.h:53
>>> RSP: 0018:ffffffff85a079a8 EFLAGS: 00000282 ORIG_RAX: ffffffffffffff10
>>> RAX: dffffc0000000020 RBX: 1ffffffff0b40f38 RCX: 0000000000000000
>>> RDX: 0000000000000000 RSI: 0000000000000001 RDI: ffffffff85a2a9e4
>>> RBP: ffffffff85a079a8 R08: 0000000000000000 R09: 0000000000000000
>>> R10: 0000000000000000 R11: 0000000000000000 R12: dffffc0000000000
>>> R13: ffffffff85a07a60 R14: ffffffff86171338 R15: 1ffffffff0b40f5b
>>> </IRQ>
>>> arch_safe_halt arch/x86/include/asm/paravirt.h:98 [inline]
>>> default_idle+0x8f/0x440 arch/x86/kernel/process.c:341
>>> arch_cpu_idle+0xa/0x10 arch/x86/kernel/process.c:332
>>> default_idle_call+0x36/0x60 kernel/sched/idle.c:98
>>> cpuidle_idle_call kernel/sched/idle.c:156 [inline]
>>> do_idle+0x348/0x420 kernel/sched/idle.c:245
>>> cpu_startup_entry+0x18/0x20 kernel/sched/idle.c:350
>>> rest_init+0x18d/0x1a0 init/main.c:415
>>> start_kernel+0x747/0x779 init/main.c:679
>>> x86_64_start_reservations+0x2a/0x2c arch/x86/kernel/head64.c:196
>>> x86_64_start_kernel+0x132/0x141 arch/x86/kernel/head64.c:177
>>> secondary_startup_64+0x9f/0x9f arch/x86/kernel/head_64.S:304
>>>
>>> Allocated by task 30543:
>>> save_stack_trace+0x16/0x20 arch/x86/kernel/stacktrace.c:59
>>> save_stack+0x43/0xd0 mm/kasan/kasan.c:513
>>> set_track mm/kasan/kasan.c:525 [inline]
>>> kasan_kmalloc+0xad/0xe0 mm/kasan/kasan.c:617
>>> kmem_cache_alloc_trace+0x82/0x270 mm/slub.c:2745
>>> kmalloc include/linux/slab.h:492 [inline]
>>> kzalloc include/linux/slab.h:665 [inline]
>>> ip_mc_add1_src net/ipv4/igmp.c:1909 [inline]
>>> ip_mc_add_src+0x6cd/0x1020 net/ipv4/igmp.c:2033
>>> ip_mc_msfilter+0x5e5/0xcf0 net/ipv4/igmp.c:2403
>>> do_ip_setsockopt.isra.12+0x2d47/0x38c0 net/ipv4/ip_sockglue.c:959
>>> ip_setsockopt+0x3a/0xb0 net/ipv4/ip_sockglue.c:1256
>>> tcp_setsockopt+0x82/0xd0 net/ipv4/tcp.c:2740
>>> sock_common_setsockopt+0x95/0xd0 net/core/sock.c:2844
>>> SYSC_setsockopt net/socket.c:1798 [inline]
>>> SyS_setsockopt+0x270/0x3a0 net/socket.c:1777
>>> entry_SYSCALL_64_fastpath+0x1f/0xbe
>>>
>>> Freed by task 30543:
>>> save_stack_trace+0x16/0x20 arch/x86/kernel/stacktrace.c:59
>>> save_stack+0x43/0xd0 mm/kasan/kasan.c:513
>>> set_track mm/kasan/kasan.c:525 [inline]
>>> kasan_slab_free+0x72/0xc0 mm/kasan/kasan.c:590
>>> slab_free_hook mm/slub.c:1357 [inline]
>>> slab_free_freelist_hook mm/slub.c:1379 [inline]
>>> slab_free mm/slub.c:2961 [inline]
>>> kfree+0xe8/0x2b0 mm/slub.c:3882
>>> ip_mc_clear_src+0x69/0x1c0 net/ipv4/igmp.c:2078
>>> ip_mc_dec_group+0x19a/0x470 net/ipv4/igmp.c:1618
>>> ip_mc_drop_socket+0x145/0x230 net/ipv4/igmp.c:2609
>>> inet_release+0x4e/0x1c0 net/ipv4/af_inet.c:411
>>> sock_release+0x8d/0x1e0 net/socket.c:597
>>> sock_close+0x16/0x20 net/socket.c:1072
>>> __fput+0x332/0x7f0 fs/file_table.c:209
>>> ____fput+0x15/0x20 fs/file_table.c:245
>>> task_work_run+0x19b/0x270 kernel/task_work.c:116
>>> exit_task_work include/linux/task_work.h:21 [inline]
>>> do_exit+0x18a3/0x2820 kernel/exit.c:878
>>> do_group_exit+0x149/0x420 kernel/exit.c:982
>>> get_signal+0x76d/0x1790 kernel/signal.c:2318
>>> do_signal+0xd2/0x2130 arch/x86/kernel/signal.c:808
>>> exit_to_usermode_loop+0x17a/0x210 arch/x86/entry/common.c:157
>>> prepare_exit_to_usermode arch/x86/entry/common.c:194 [inline]
>>> syscall_return_slowpath+0x3ba/0x410 arch/x86/entry/common.c:263
>>> entry_SYSCALL_64_fastpath+0xbc/0xbe
>>>
>>> The buggy address belongs to the object at ffff88003053c1a0
>>> which belongs to the cache kmalloc-64 of size 64
>>> The buggy address is located 0 bytes inside of
>>> 64-byte region [ffff88003053c1a0, ffff88003053c1e0)
>>> The buggy address belongs to the page:
>>> page:ffffea0000c14f00 count:1 mapcount:0 mapping: (null)
>>> index:0x0 compound_mapcount: 0
>>> flags: 0x100000000008100(slab|head)
>>> raw: 0100000000008100 0000000000000000 0000000000000000 0000000100140014
>>> raw: ffffea0000c2f520 ffffea0000e20aa0 ffff88003e80f740 0000000000000000
>>> page dumped because: kasan: bad access detected
>>>
>>> Memory state around the buggy address:
>>> ffff88003053c080: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
>>> ffff88003053c100: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
>>> >ffff88003053c180: fc fc fc fc fb fb fb fb fb fb fb fb fc fc fc fc
>>> ^
>>> ffff88003053c200: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
>>> ffff88003053c280: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
>>> ==================================================================
>>
>> I have the feeling that ip_mc_clear_src() is called too soon.
>>
>> We should call it once all users have released their reference.
>
> In this case, ip_mc_clear_src() is called when the ->users hits
> zero, therefore I think we possibly miss the refcnt on ->users.
>
> diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
> index 44fd86d..c2f3347 100644
> --- a/net/ipv4/igmp.c
> +++ b/net/ipv4/igmp.c
> @@ -1886,6 +1886,7 @@ static int ip_mc_del_src(struct in_device
> *in_dev, __be32 *pmca, int sfmode,
> igmp_ifc_event(pmc->interface);
> #endif
> }
> + pmc->users--;
> out_unlock:
> spin_unlock_bh(&pmc->lock);
> return err;
> @@ -2025,6 +2026,7 @@ static int ip_mc_add_src(struct in_device
> *in_dev, __be32 *pmca, int sfmode,
> #ifdef CONFIG_IP_MULTICAST
> sf_markstate(pmc);
> #endif
> + pmc->users++;
> isexclude = pmc->sfmode == MCAST_EXCLUDE;
> if (!delta)
> pmc->sfcount[sfmode]++;
The issue here is the timer firing while ip_mc_clear_src() has been
already called.
My patch should fix the problem.
Or another one using del_timer_sync() instead of del_timer() in
igmp_stop_timer(), but such a change would be more invasive,
since the del_timer_sync() would need to happen while im->lock
spinlock is not held.
^ permalink raw reply
* Re: net/ipv4: use-after-free in add_grec
From: Cong Wang @ 2017-05-31 23:49 UTC (permalink / raw)
To: Eric Dumazet
Cc: Andrey Konovalov, David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, netdev, LKML, Eric Dumazet,
Dmitry Vyukov, Kostya Serebryany, syzkaller
In-Reply-To: <1496247154.27480.3.camel@edumazet-glaptop3.roam.corp.google.com>
On Wed, May 31, 2017 at 9:12 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Wed, 2017-05-31 at 11:46 +0200, Andrey Konovalov wrote:
>> Hi,
>>
>> I've got the following error report while fuzzing the kernel with syzkaller.
>>
>> On commit 5ed02dbb497422bf225783f46e6eadd237d23d6b (4.12-rc3).
>>
>> Unfortunately it's not reproducible.
>>
>> ==================================================================
>> BUG: KASAN: use-after-free in add_grec+0x101e/0x1090 net/ipv4/igmp.c:473
>> Read of size 8 at addr ffff88003053c1a0 by task swapper/0/0
>>
>> CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.12.0-rc3+ #370
>> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
>> Call Trace:
>> <IRQ>
>> __dump_stack lib/dump_stack.c:16 [inline]
>> dump_stack+0x292/0x395 lib/dump_stack.c:52
>> print_address_description+0x73/0x280 mm/kasan/report.c:252
>> kasan_report_error mm/kasan/report.c:351 [inline]
>> kasan_report+0x22b/0x340 mm/kasan/report.c:408
>> __asan_report_load8_noabort+0x14/0x20 mm/kasan/report.c:429
>> add_grec+0x101e/0x1090 net/ipv4/igmp.c:473
>> igmpv3_send_cr net/ipv4/igmp.c:663 [inline]
>> igmp_ifc_timer_expire+0x46d/0xa80 net/ipv4/igmp.c:768
>> IPVS: length: 51 != 8
>> call_timer_fn+0x23f/0x800 kernel/time/timer.c:1268
>> expire_timers kernel/time/timer.c:1307 [inline]
>> __run_timers+0x94e/0xcd0 kernel/time/timer.c:1601
>> run_timer_softirq+0x21/0x80 kernel/time/timer.c:1614
>> __do_softirq+0x2fb/0xb99 kernel/softirq.c:284
>> invoke_softirq kernel/softirq.c:364 [inline]
>> irq_exit+0x19e/0x1d0 kernel/softirq.c:405
>> exiting_irq arch/x86/include/asm/apic.h:652 [inline]
>> smp_apic_timer_interrupt+0x76/0xa0 arch/x86/kernel/apic/apic.c:966
>> apic_timer_interrupt+0x93/0xa0 arch/x86/entry/entry_64.S:481
>> RIP: 0010:native_safe_halt+0x6/0x10 arch/x86/include/asm/irqflags.h:53
>> RSP: 0018:ffffffff85a079a8 EFLAGS: 00000282 ORIG_RAX: ffffffffffffff10
>> RAX: dffffc0000000020 RBX: 1ffffffff0b40f38 RCX: 0000000000000000
>> RDX: 0000000000000000 RSI: 0000000000000001 RDI: ffffffff85a2a9e4
>> RBP: ffffffff85a079a8 R08: 0000000000000000 R09: 0000000000000000
>> R10: 0000000000000000 R11: 0000000000000000 R12: dffffc0000000000
>> R13: ffffffff85a07a60 R14: ffffffff86171338 R15: 1ffffffff0b40f5b
>> </IRQ>
>> arch_safe_halt arch/x86/include/asm/paravirt.h:98 [inline]
>> default_idle+0x8f/0x440 arch/x86/kernel/process.c:341
>> arch_cpu_idle+0xa/0x10 arch/x86/kernel/process.c:332
>> default_idle_call+0x36/0x60 kernel/sched/idle.c:98
>> cpuidle_idle_call kernel/sched/idle.c:156 [inline]
>> do_idle+0x348/0x420 kernel/sched/idle.c:245
>> cpu_startup_entry+0x18/0x20 kernel/sched/idle.c:350
>> rest_init+0x18d/0x1a0 init/main.c:415
>> start_kernel+0x747/0x779 init/main.c:679
>> x86_64_start_reservations+0x2a/0x2c arch/x86/kernel/head64.c:196
>> x86_64_start_kernel+0x132/0x141 arch/x86/kernel/head64.c:177
>> secondary_startup_64+0x9f/0x9f arch/x86/kernel/head_64.S:304
>>
>> Allocated by task 30543:
>> save_stack_trace+0x16/0x20 arch/x86/kernel/stacktrace.c:59
>> save_stack+0x43/0xd0 mm/kasan/kasan.c:513
>> set_track mm/kasan/kasan.c:525 [inline]
>> kasan_kmalloc+0xad/0xe0 mm/kasan/kasan.c:617
>> kmem_cache_alloc_trace+0x82/0x270 mm/slub.c:2745
>> kmalloc include/linux/slab.h:492 [inline]
>> kzalloc include/linux/slab.h:665 [inline]
>> ip_mc_add1_src net/ipv4/igmp.c:1909 [inline]
>> ip_mc_add_src+0x6cd/0x1020 net/ipv4/igmp.c:2033
>> ip_mc_msfilter+0x5e5/0xcf0 net/ipv4/igmp.c:2403
>> do_ip_setsockopt.isra.12+0x2d47/0x38c0 net/ipv4/ip_sockglue.c:959
>> ip_setsockopt+0x3a/0xb0 net/ipv4/ip_sockglue.c:1256
>> tcp_setsockopt+0x82/0xd0 net/ipv4/tcp.c:2740
>> sock_common_setsockopt+0x95/0xd0 net/core/sock.c:2844
>> SYSC_setsockopt net/socket.c:1798 [inline]
>> SyS_setsockopt+0x270/0x3a0 net/socket.c:1777
>> entry_SYSCALL_64_fastpath+0x1f/0xbe
>>
>> Freed by task 30543:
>> save_stack_trace+0x16/0x20 arch/x86/kernel/stacktrace.c:59
>> save_stack+0x43/0xd0 mm/kasan/kasan.c:513
>> set_track mm/kasan/kasan.c:525 [inline]
>> kasan_slab_free+0x72/0xc0 mm/kasan/kasan.c:590
>> slab_free_hook mm/slub.c:1357 [inline]
>> slab_free_freelist_hook mm/slub.c:1379 [inline]
>> slab_free mm/slub.c:2961 [inline]
>> kfree+0xe8/0x2b0 mm/slub.c:3882
>> ip_mc_clear_src+0x69/0x1c0 net/ipv4/igmp.c:2078
>> ip_mc_dec_group+0x19a/0x470 net/ipv4/igmp.c:1618
>> ip_mc_drop_socket+0x145/0x230 net/ipv4/igmp.c:2609
>> inet_release+0x4e/0x1c0 net/ipv4/af_inet.c:411
>> sock_release+0x8d/0x1e0 net/socket.c:597
>> sock_close+0x16/0x20 net/socket.c:1072
>> __fput+0x332/0x7f0 fs/file_table.c:209
>> ____fput+0x15/0x20 fs/file_table.c:245
>> task_work_run+0x19b/0x270 kernel/task_work.c:116
>> exit_task_work include/linux/task_work.h:21 [inline]
>> do_exit+0x18a3/0x2820 kernel/exit.c:878
>> do_group_exit+0x149/0x420 kernel/exit.c:982
>> get_signal+0x76d/0x1790 kernel/signal.c:2318
>> do_signal+0xd2/0x2130 arch/x86/kernel/signal.c:808
>> exit_to_usermode_loop+0x17a/0x210 arch/x86/entry/common.c:157
>> prepare_exit_to_usermode arch/x86/entry/common.c:194 [inline]
>> syscall_return_slowpath+0x3ba/0x410 arch/x86/entry/common.c:263
>> entry_SYSCALL_64_fastpath+0xbc/0xbe
>>
>> The buggy address belongs to the object at ffff88003053c1a0
>> which belongs to the cache kmalloc-64 of size 64
>> The buggy address is located 0 bytes inside of
>> 64-byte region [ffff88003053c1a0, ffff88003053c1e0)
>> The buggy address belongs to the page:
>> page:ffffea0000c14f00 count:1 mapcount:0 mapping: (null)
>> index:0x0 compound_mapcount: 0
>> flags: 0x100000000008100(slab|head)
>> raw: 0100000000008100 0000000000000000 0000000000000000 0000000100140014
>> raw: ffffea0000c2f520 ffffea0000e20aa0 ffff88003e80f740 0000000000000000
>> page dumped because: kasan: bad access detected
>>
>> Memory state around the buggy address:
>> ffff88003053c080: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
>> ffff88003053c100: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
>> >ffff88003053c180: fc fc fc fc fb fb fb fb fb fb fb fb fc fc fc fc
>> ^
>> ffff88003053c200: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
>> ffff88003053c280: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
>> ==================================================================
>
> I have the feeling that ip_mc_clear_src() is called too soon.
>
> We should call it once all users have released their reference.
In this case, ip_mc_clear_src() is called when the ->users hits
zero, therefore I think we possibly miss the refcnt on ->users.
diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
index 44fd86d..c2f3347 100644
--- a/net/ipv4/igmp.c
+++ b/net/ipv4/igmp.c
@@ -1886,6 +1886,7 @@ static int ip_mc_del_src(struct in_device
*in_dev, __be32 *pmca, int sfmode,
igmp_ifc_event(pmc->interface);
#endif
}
+ pmc->users--;
out_unlock:
spin_unlock_bh(&pmc->lock);
return err;
@@ -2025,6 +2026,7 @@ static int ip_mc_add_src(struct in_device
*in_dev, __be32 *pmca, int sfmode,
#ifdef CONFIG_IP_MULTICAST
sf_markstate(pmc);
#endif
+ pmc->users++;
isexclude = pmc->sfmode == MCAST_EXCLUDE;
if (!delta)
pmc->sfcount[sfmode]++;
^ permalink raw reply related
* Re: [PATCH v2 net-next 0/8] Introduce bpf ID
From: David Miller @ 2017-05-31 23:35 UTC (permalink / raw)
To: kafai; +Cc: netdev, ast, daniel, kernel-team
In-Reply-To: <cover.1496256722.git.kafai@fb.com>
From: Martin KaFai Lau <kafai@fb.com>
Date: Wed, 31 May 2017 11:58:54 -0700
> This patch series:
> 1) Introduce ID for both bpf_prog and bpf_map.
> 2) Add bpf commands to iterate the prog IDs and map
> IDs of the system.
> 3) Add bpf commands to get a prog/map fd from an ID
> 4) Add bpf command to get prog/map info from a fd.
> The prog/map info is a jump start in this patchset
> and it is not meant to be a complete list. They can
> be extended in the future patches.
>
> v2:
> Compiler warning fixes:
> - Remove lockdep_is_held() usage. Add comment
> to explain the lock situation instead.
> - Add static for idr related variables
> - Add __user to the uattr param in bpf_prog_get_info_by_fd()
> and bpf_map_get_info_by_fd().
Series applied, thanks Martin.
^ permalink raw reply
* Re: [PATCH net-next 6/9] bpf: fix stack_depth usage by test_bpf.ko
From: David Miller @ 2017-05-31 23:30 UTC (permalink / raw)
To: ast; +Cc: daniel, netdev, kernel-team
In-Reply-To: <0da22ba6-bb34-23da-2722-3e45fd195e17@fb.com>
From: Alexei Starovoitov <ast@fb.com>
Date: Wed, 31 May 2017 15:41:09 -0700
> On 5/31/17 11:45 AM, Alexei Starovoitov wrote:
>> On 5/31/17 11:43 AM, David Miller wrote:
>>> From: Alexei Starovoitov <ast@fb.com>
>>> Date: Wed, 31 May 2017 11:39:37 -0700
>>>
>>>> On 5/31/17 11:15 AM, David Miller wrote:
>>>>> From: Alexei Starovoitov <ast@fb.com>
>>>>> Date: Tue, 30 May 2017 13:31:32 -0700
>>>>>
>>>>>> test_bpf.ko doesn't call verifier before selecting interpreter or
>>>>>> JITing,
>>>>>> hence the tests need to manually specify the amount of stack they
>>>>>> consume.
>>>>>>
>>>>>> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
>>>>>> Acked-by: Daniel Borkmann <daniel@iogearbox.net>
>>>>>
>>>>> I do not like this and the previous patch, it seems so error prone.
>>>>
>>>> in what sense 'error prone' ?
>>>
>>> In the sense that a human computes these numbers, and nothing checks
>>> if it is correct or not until program perhaps crashes if the value is
>>> wrong.
>>
>> right. that's how all these tests are.
>> See bpf_fill_ld_abs_vlan_push_pop() for example.
>> If that codegen has a bug, it will crash the kernel.
>> That's why it's done from kernel module to do things
>> that user space cannot do.
>
> btw, when added a bunch of these '.stack_depth = 40'
> I was thinking to randomize these values in [40, 512]
> range to stress test interpreter and JITs more,
> but then decided not to do that to avoid questions
> why these numbers don't match the instructions.
> Now I'm thinking we should actually do it for two reasons:
> - to stress test stuff
> - and to demonstrate more clearly that test_bpf.ko can
> really do things that user space cannot and that's the
> purpose of this .ko
> Thoughts?
Ok I've applied the base series for now.
Feel free to build that kind of stuff on top.
Thanks for explaining.
^ permalink raw reply
* Re: [PATCH net-next 0/9] bpf: stack depth tracking
From: David Miller @ 2017-05-31 23:30 UTC (permalink / raw)
To: ast; +Cc: daniel, netdev, kernel-team
In-Reply-To: <20170530203135.3642768-1-ast@fb.com>
From: Alexei Starovoitov <ast@fb.com>
Date: Tue, 30 May 2017 13:31:26 -0700
> Introduce tracking of bpf program stack depth in the verifier and use that
> info to reduce bpf program stack consumption in the interpreter and x64 JIT.
> Other JITs can take advantage of it as well in the future.
> Most of the programs consume very little stack, so it's good optimization
> in general and it's the first step toward bpf to bpf function calls.
>
> Also use internal opcode for bpf_tail_call() marking to make clear
> that jmp|call|x opcode is not uapi and may be used for actual
> indirect call opcode in the future.
Series applied, thanks Alexei.
^ permalink raw reply
* Re: [PATCH V3] neigh: Really delete an arp/neigh entry on "ip neigh delete" or "arp -d"
From: David Miller @ 2017-05-31 23:26 UTC (permalink / raw)
To: sowmini.varadhan; +Cc: netdev, stephen
In-Reply-To: <1496188346-83229-1-git-send-email-sowmini.varadhan@oracle.com>
From: Sowmini Varadhan <sowmini.varadhan@oracle.com>
Date: Tue, 30 May 2017 16:52:26 -0700
Just as a side note in the future, if you could fix the date
and time on your computer, that would be awesome :-)
^ 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