* [PATCH 1/2] net: team: fix memory leak in team_nl_send_port_list_get
From: Pan Bian @ 2017-04-24 7:04 UTC (permalink / raw)
To: Jiri Pirko, netdev; +Cc: linux-kernel, Pan Bian
In function team_nl_send_port_list_get(), pointer skb keeps the return
value of nlmsg_new(). When the call to genlmsg_put() fails, the memory
is not freed. This will result in a memory leak bug. This patch fixes
it.
Fixes: fbd69cda90e7 ("team: fix memory leak")
Signed-off-by: Pan Bian <bianpan2016@163.com>
---
drivers/net/team/team.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
index f8c81f1..dd3a2e9 100644
--- a/drivers/net/team/team.c
+++ b/drivers/net/team/team.c
@@ -2634,8 +2634,10 @@ static int team_nl_send_port_list_get(struct team *team, u32 portid, u32 seq,
hdr = genlmsg_put(skb, portid, seq, &team_nl_family, flags | NLM_F_MULTI,
TEAM_CMD_PORT_LIST_GET);
- if (!hdr)
+ if (!hdr) {
+ nlmsg_free(skb);
return -EMSGSIZE;
+ }
if (nla_put_u32(skb, TEAM_ATTR_TEAM_IFINDEX, team->dev->ifindex))
goto nla_put_failure;
--
1.9.1
^ permalink raw reply related
* [PATCH 2/2] net: team: fix memory leak in team_nl_send_options_get
From: Pan Bian @ 2017-04-24 7:04 UTC (permalink / raw)
To: Jiri Pirko, netdev; +Cc: linux-kernel, Pan Bian
In function team_nl_send_options_get(), pointer skb keeps the return
value of function nlmsg_new(). When the call to genlmsg_put() fails, the
control flow directly returns and does not free skb. This will result in
a memory leak bug. This patch fixes it.
Fixes: 8ea7fd0d8792 ("team: fix memory leak")
Signed-off-by: Pan Bian <bianpan2016@163.com>
---
drivers/net/team/team.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
index dd3a2e9..85c0124 100644
--- a/drivers/net/team/team.c
+++ b/drivers/net/team/team.c
@@ -2361,8 +2361,10 @@ static int team_nl_send_options_get(struct team *team, u32 portid, u32 seq,
hdr = genlmsg_put(skb, portid, seq, &team_nl_family, flags | NLM_F_MULTI,
TEAM_CMD_OPTIONS_GET);
- if (!hdr)
+ if (!hdr) {
+ nlmsg_free(skb);
return -EMSGSIZE;
+ }
if (nla_put_u32(skb, TEAM_ATTR_TEAM_IFINDEX, team->dev->ifindex))
goto nla_put_failure;
--
1.9.1
^ permalink raw reply related
* Re: [PATCH net-next 1/2] flow_dissector: add mpls support (v2)
From: Jiri Pirko @ 2017-04-24 7:05 UTC (permalink / raw)
To: Benjamin LaHaise
Cc: netdev, Benjamin LaHaise, David S. Miller, Simon Horman,
Jamal Hadi Salim, Cong Wang, Jiri Pirko, Hadar Hen Zion, Gao Feng
In-Reply-To: <1492894367-11637-2-git-send-email-benjamin.lahaise@netronome.com>
Sat, Apr 22, 2017 at 10:52:46PM CEST, benjamin.lahaise@netronome.com wrote:
>Add support for parsing MPLS flows to the flow dissector in preparation for
>adding MPLS match support to cls_flower.
>
>Signed-off-by: Benjamin LaHaise <benjamin.lahaise@netronome.com>
>Signed-off-by: Benjamin LaHaise <bcrl@kvack.org>
This looks odd :)
>Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
^ permalink raw reply
* Re: [PATCH net-next 2/2] cls_flower: add support for matching MPLS fields (v2)
From: Jiri Pirko @ 2017-04-24 7:07 UTC (permalink / raw)
To: Benjamin LaHaise
Cc: netdev, Benjamin LaHaise, David S. Miller, Simon Horman,
Jamal Hadi Salim, Cong Wang, Jiri Pirko, Eric Dumazet,
Hadar Hen Zion, Gao Feng
In-Reply-To: <1492894367-11637-3-git-send-email-benjamin.lahaise@netronome.com>
Sat, Apr 22, 2017 at 10:52:47PM CEST, benjamin.lahaise@netronome.com wrote:
>Add support to the tc flower classifier to match based on fields in MPLS
>labels (TTL, Bottom of Stack, TC field, Label).
>
>Signed-off-by: Benjamin LaHaise <benjamin.lahaise@netronome.com>
>Signed-off-by: Benjamin LaHaise <bcrl@kvack.org>
>Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
^ permalink raw reply
* Re: [PATCH 2/2] net: team: fix memory leak in team_nl_send_options_get
From: Jiri Pirko @ 2017-04-24 7:11 UTC (permalink / raw)
To: Pan Bian; +Cc: netdev, linux-kernel
In-Reply-To: <1493017495-3578-1-git-send-email-bianpan2016@163.com>
Mon, Apr 24, 2017 at 09:04:55AM CEST, bianpan2016@163.com wrote:
>In function team_nl_send_options_get(), pointer skb keeps the return
>value of function nlmsg_new(). When the call to genlmsg_put() fails, the
>control flow directly returns and does not free skb. This will result in
>a memory leak bug. This patch fixes it.
>
>Fixes: 8ea7fd0d8792 ("team: fix memory leak")
test1:~/net-next$ git log 8ea7fd0d8792
fatal: ambiguous argument '8ea7fd0d8792': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions, like this:
'git <command> [<revision>...] -- [<file>...]'
Please look up the commit that introduces the issue. Also no newline
in between "fixes" and "signed off".
Also. The subject should be:
[PATCH net 2/2] team: fix memory leak in team_nl_send_options_get
You can see this right away if you look in the mailing list archive...
>
>Signed-off-by: Pan Bian <bianpan2016@163.com>
>---
> drivers/net/team/team.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
>index dd3a2e9..85c0124 100644
>--- a/drivers/net/team/team.c
>+++ b/drivers/net/team/team.c
>@@ -2361,8 +2361,10 @@ static int team_nl_send_options_get(struct team *team, u32 portid, u32 seq,
>
> hdr = genlmsg_put(skb, portid, seq, &team_nl_family, flags | NLM_F_MULTI,
> TEAM_CMD_OPTIONS_GET);
>- if (!hdr)
>+ if (!hdr) {
>+ nlmsg_free(skb);
> return -EMSGSIZE;
>+ }
>
> if (nla_put_u32(skb, TEAM_ATTR_TEAM_IFINDEX, team->dev->ifindex))
> goto nla_put_failure;
>--
>1.9.1
>
>
^ permalink raw reply
* Re: [PATCH] ipvs: explicitly forbid ipv6 service/dest creation if ipv6 mod is disabled
From: Julian Anastasov @ 2017-04-24 7:21 UTC (permalink / raw)
To: Paolo Abeni
Cc: netfilter-devel, lvs-devel, Wensong Zhang, Simon Horman, netdev
In-Reply-To: <1493016651.5744.1.camel@redhat.com>
Hello,
On Mon, 24 Apr 2017, Paolo Abeni wrote:
> Hi,
>
> The problem with the patched code is that it tries to resolve ipv6
> addresses that are not created/validated by the kernel.
OK. Simon, please apply to ipvs tree.
Acked-by: Julian Anastasov <ja@ssi.bg>
Regards
--
Julian Anastasov <ja@ssi.bg>
^ permalink raw reply
* [PATCH net] bridge: shutdown bridge device before removing it
From: Xin Long @ 2017-04-24 7:25 UTC (permalink / raw)
To: network dev; +Cc: davem, nikolay
During removing a bridge device, if the bridge is still up, a new mdb entry
still can be added in br_multicast_add_group() after all mdb entries are
removed in br_multicast_dev_del(). Like the path:
mld_ifc_timer_expire ->
mld_sendpack -> ...
br_multicast_rcv ->
br_multicast_add_group
The new mp's timer will be set up. If the timer expires after the bridge
is freed, it may cause use-after-free panic in br_multicast_group_expired.
This can happen when ip link remove a bridge or destroy a netns with a
bridge device inside.
As we can see in br_del_bridge, brctl is also supposed to remove a bridge
device after it's shutdown.
This patch is to call dev_close at the beginning of br_dev_delete so that
netif_running check in br_multicast_add_group can avoid this issue. But
to keep consistent with before, it will not remove the IFF_UP check in
br_del_bridge for brctl.
Reported-by: Jianwen Ji <jiji@redhat.com>
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
net/bridge/br_if.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index 56a2a72..8175f13 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -305,6 +305,8 @@ void br_dev_delete(struct net_device *dev, struct list_head *head)
struct net_bridge *br = netdev_priv(dev);
struct net_bridge_port *p, *n;
+ dev_close(br->dev);
+
list_for_each_entry_safe(p, n, &br->port_list, list) {
del_nbp(p);
}
--
2.1.0
^ permalink raw reply related
* [PATCH net] xfrm: do the garbage collection after flushing policy
From: Xin Long @ 2017-04-24 7:33 UTC (permalink / raw)
To: network dev; +Cc: davem, Herbert Xu, fw
Now xfrm garbage collection can be triggered by 'ip xfrm policy del'.
These is no reason not to do it after flushing policies, especially
considering that 'garbage collection deferred' is only triggered
when it reaches gc_thresh.
It's no good that the policy is gone but the xdst still hold there.
The worse thing is that xdst->route/orig_dst is also hold and can
not be released even if the orig_dst is already expired.
This patch is to do the garbage collection if there is any policy
removed in xfrm_policy_flush.
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
net/xfrm/xfrm_policy.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 236cbbc..dfc77b9 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -1006,6 +1006,10 @@ int xfrm_policy_flush(struct net *net, u8 type, bool task_valid)
err = -ESRCH;
out:
spin_unlock_bh(&net->xfrm.xfrm_policy_lock);
+
+ if (cnt)
+ xfrm_garbage_collect(net);
+
return err;
}
EXPORT_SYMBOL(xfrm_policy_flush);
--
2.1.0
^ permalink raw reply related
* Re: [PATCH iproute2 net 3/8] tc/pedit: Introduce 'add' operation
From: Amir Vadai @ 2017-04-24 7:52 UTC (permalink / raw)
To: Jamal Hadi Salim; +Cc: Stephen Hemminger, netdev, Or Gerlitz
In-Reply-To: <f358025b-2618-3f22-b593-2e4e70fb7a79@mojatatu.com>
On Sun, Apr 23, 2017 at 01:44:51PM -0400, Jamal Hadi Salim wrote:
>
> Thanks for the excellent work.
>
> On 17-04-23 08:53 AM, Amir Vadai wrote:
> > This command could be useful to increase/decrease fields value.
> >
>
> Does this contradict the "retain" feature? Example rule to
> retain the second nibble but set the first to 0xA
> (essentially it X-ORs):
> tc filter add dev lo parent ffff: protocol ip prio 10 \
> u32 match ip dst 127.0.0.1/32 flowid 1:1 \
> action pedit munge offset 0 u8 set 0x0A retain 0xF0
You mean, for example increasing a single nible in an ipv4 address?
If so, then it should work:
# tc filter add dev veth0 parent ffff: u32 \
match ip dport 23 0xffff \
action pedit ex \
munge ip src add 1.0.0.0 retain 0xff000000
# tc -s filter show dev veth0 parent ffff:
filter protocol all pref 49151 u32
filter protocol all pref 49151 u32 fh 801: ht divisor 1
filter protocol all pref 49151 u32 fh 801::800 order 2048 key ht 801 bkt 0 terminal flowid ??? (rule hit 0 success 0)
match 00000017/0000ffff at 20 (success 0 )
action order 1: pedit action pass keys 1
index 48 ref 1 bind 1 installed 1 sec used 1 sec
key #0 at ipv4+12: add 01000000 mask 00ffffff
Action statistics:
Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
backlog 0b 0p requeues 0
>
> Mostly curious about hardware handling.
As to hardware handling, Or did the implementation so he will answer
better. AFAIK, current implementation doesn't allow partial field
setting/adding, so such a rule can't be offloaded. I think it is only to
make things simple and because there was no use case for that.
To my knowledge, hardware should support such thing if needed.
Amir
>
> cheers,
> jamal
>
^ permalink raw reply
* Re: macvlan: Fix device ref leak when purging bc_queue
From: Herbert Xu @ 2017-04-24 7:56 UTC (permalink / raw)
To: Joe.Ghalam; +Cc: davem, Clifford.Wichmann, netdev
In-Reply-To: <1492785652578.53801@Dell.com>
On Fri, Apr 21, 2017 at 02:40:50PM +0000, Joe.Ghalam@dell.com wrote:
> That's not true. macvlan_dellink() unregisters the queue, and macvlan_process_broadcast() will never get called. Please note that I'm not speculating. I have traced enabled on the dev_put and dev_hold, and I'm reporting a real, reproducible issue.
The only thing that can stop macvlan_process_broadcast from getting
called is macvlan_port_destroy. Nothing else can stop the work
queue, unless of course the work queue mechanism itself is broken.
So if you're sure macvlan_port_destroy is never even called in
your case, then you'll need to start debugging the kernel work
queue mechanism to see why macvlan_process_broadcast is not getting
called.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH net] xfrm: fix stack access out of bounds with CONFIG_XFRM_SUB_POLICY
From: Herbert Xu @ 2017-04-24 7:59 UTC (permalink / raw)
To: Sabrina Dubroca; +Cc: netdev, Steffen Klassert
In-Reply-To: <20170421120531.GA12502@bistromath.localdomain>
On Fri, Apr 21, 2017 at 02:05:31PM +0200, Sabrina Dubroca wrote:
>
> You're right. I had a note about that but it got lost. The bug
> (ignoring what flavor of flowi is passed) is older than that (from the
> introduction of subpolicies, I suspect, but I would have to dig more
> into the history), but this commit removed the last uses of
> origin/partner.
>
> Looking into raw_sendmsg(), this code may have been safe before
> 9d6ec938019c ("ipv4: Use flowi4 in public route lookup interfaces."),
> since full flowi were used.
>
> If we want a fix for kernels that don't have ca116922afa8, we would
> probably need to take the size of the flowi* struct depending on the
> address family passed to xfrm_resolve_and_create_bundle().
>
>
> I'm not sure how to proceed here, any advice?
I think the Fixes header should simply refer to the commit that
introduced the bug. You're instead using it as a hint for how
to backport the patch. That is beyond the scope of the Fixes
header.
So if the bug started with 9d6ec938019c then just use that in
your Fixes header. If you want to help the backporter with more
information then you can put ca116922afa8 in the body of the commit
description.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* [PATCH net-next] net: dsa: LAN9303: add i2c dependency
From: Tobias Regnery @ 2017-04-24 8:07 UTC (permalink / raw)
To: andrew, vivien.didelot, f.fainelli, netdev, linux-kernel, jbe
Cc: Tobias Regnery
The Kconfig symbol for the I2C mode of the LAN9303 driver selects
REGMAP_I2C. This symbol depends on I2C but the driver doesen't has a
dependency on I2C.
With CONFIG_I2C=n kconfig fails to select REGMAP_I2C and we get the
following warning:
warning: (NET_DSA_SMSC_LAN9303_I2C) selects REGMAP_I2C which has unmet direct dependencies (I2C)
This results in a build failure because the regmap-i2c functions aren't
available:
drivers/base/regmap/regmap-i2c.c: In function 'regmap_smbus_byte_reg_read':
drivers/base/regmap/regmap-i2c.c:29:8: error: implicit declaration of function 'i2c_smbus_read_byte_data' [-Werror=implicit-function-declaration]
drivers/base/regmap/regmap-i2c.c: In function 'regmap_smbus_byte_reg_write':
drivers/base/regmap/regmap-i2c.c:47:9: error: implicit declaration of function 'i2c_smbus_write_byte_data' [-Werror=implicit-function-declaration]
...
Fix this by adding a kconfig dependency on the I2C subsystem.
Fixes: be4e119f9914 ("net: dsa: LAN9303: add I2C managed mode support")
Signed-off-by: Tobias Regnery <tobias.regnery@gmail.com>
---
drivers/net/dsa/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/dsa/Kconfig b/drivers/net/dsa/Kconfig
index 131a5b1cbfc8..862ee22303c2 100644
--- a/drivers/net/dsa/Kconfig
+++ b/drivers/net/dsa/Kconfig
@@ -59,7 +59,7 @@ config NET_DSA_SMSC_LAN9303
config NET_DSA_SMSC_LAN9303_I2C
tristate "SMSC/Microchip LAN9303 3-ports 10/100 ethernet switch in I2C managed mode"
- depends on NET_DSA
+ depends on NET_DSA && I2C
select NET_DSA_SMSC_LAN9303
select REGMAP_I2C
---help---
--
2.11.0
^ permalink raw reply related
* Re: [Intel-wired-lan] [PATCH 1/2] e1000e: Don't return uninitialized stats
From: Neftin, Sasha @ 2017-04-24 8:17 UTC (permalink / raw)
To: netdev@vger.kernel.org; +Cc: intel-wired-lan, Ruinskiy, Dima
In-Reply-To: <630A6B92B7EDEB45A87E20D3D286660171182EED@hasmsx109.ger.corp.intel.com>
On 4/23/2017 15:53, Neftin, Sasha wrote:
> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@lists.osuosl.org] On Behalf Of Benjamin Poirier
> Sent: Saturday, April 22, 2017 00:20
> To: Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com>
> Cc: netdev@vger.kernel.org; intel-wired-lan@lists.osuosl.org; Stefan Priebe <s.priebe@profihost.ag>
> Subject: [Intel-wired-lan] [PATCH 1/2] e1000e: Don't return uninitialized stats
>
> Some statistics passed to ethtool are garbage because e1000e_get_stats64() doesn't write them, for example: tx_heartbeat_errors. This leaks kernel memory to userspace and confuses users.
>
> Do like ixgbe and use dev_get_stats() which first zeroes out rtnl_link_stats64.
>
> Reported-by: Stefan Priebe <s.priebe@profihost.ag>
> Signed-off-by: Benjamin Poirier <bpoirier@suse.com>
> ---
> drivers/net/ethernet/intel/e1000e/ethtool.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/intel/e1000e/ethtool.c b/drivers/net/ethernet/intel/e1000e/ethtool.c
> index 7aff68a4a4df..f117b90cdc2f 100644
> --- a/drivers/net/ethernet/intel/e1000e/ethtool.c
> +++ b/drivers/net/ethernet/intel/e1000e/ethtool.c
> @@ -2063,7 +2063,7 @@ static void e1000_get_ethtool_stats(struct net_device *netdev,
>
> pm_runtime_get_sync(netdev->dev.parent);
>
> - e1000e_get_stats64(netdev, &net_stats);
> + dev_get_stats(netdev, &net_stats);
>
> pm_runtime_put_sync(netdev->dev.parent);
>
> --
> 2.12.2
>
> _______________________________________________
> Intel-wired-lan mailing list
> Intel-wired-lan@lists.osuosl.org
> http://lists.osuosl.org/mailman/listinfo/intel-wired-lan
Hello,
We would like to not accept this patch. Suggested generic method
'*dev_get_stats' (net/core/dev.c) calls 'ops->ndo_get_stats64' method
which eventually calls e1000e_get_stats64 (netdev.c) - so there is same
functionality. Also, see that 'e1000e_get_stats64' method in netdev.c
(line 5928) calls 'memset' with 0's before update statistics. Local
sanity check in our lab shows 'tx_heartbeat_errors' counter reported as 0.
Thanks,
Sasha
^ permalink raw reply
* Re: [PATCH net-next] net: dsa: LAN9303: add i2c dependency
From: Juergen Borleis @ 2017-04-24 8:22 UTC (permalink / raw)
To: Tobias Regnery; +Cc: andrew, vivien.didelot, f.fainelli, netdev, linux-kernel
In-Reply-To: <20170424080737.24897-1-tobias.regnery@gmail.com>
Hi Tobias,
On Monday 24 April 2017 10:07:37 Tobias Regnery wrote:
> The Kconfig symbol for the I2C mode of the LAN9303 driver selects
> REGMAP_I2C. This symbol depends on I2C but the driver doesen't has a
> dependency on I2C.
>
> With CONFIG_I2C=n kconfig fails to select REGMAP_I2C and we get the
> following warning:
>
> warning: (NET_DSA_SMSC_LAN9303_I2C) selects REGMAP_I2C which has unmet
> direct dependencies (I2C)
>
> This results in a build failure because the regmap-i2c functions aren't
> available:
>
> drivers/base/regmap/regmap-i2c.c: In function 'regmap_smbus_byte_reg_read':
> drivers/base/regmap/regmap-i2c.c:29:8: error: implicit declaration of function 'i2c_smbus_read_byte_data' [-Werror=implicit-function-declaration]
>
> drivers/base/regmap/regmap-i2c.c: In function 'regmap_smbus_byte_reg_write':
> drivers/base/regmap/regmap-i2c.c:47:9: error: implicit declaration of function 'i2c_smbus_write_byte_data' [-Werror=implicit-function-declaration]
> ...
>
> Fix this by adding a kconfig dependency on the I2C subsystem.
>
> Fixes: be4e119f9914 ("net: dsa: LAN9303: add I2C managed mode support")
> Signed-off-by: Tobias Regnery <tobias.regnery@gmail.com>
> ---
> drivers/net/dsa/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/dsa/Kconfig b/drivers/net/dsa/Kconfig
> index 131a5b1cbfc8..862ee22303c2 100644
> --- a/drivers/net/dsa/Kconfig
> +++ b/drivers/net/dsa/Kconfig
> @@ -59,7 +59,7 @@ config NET_DSA_SMSC_LAN9303
>
> config NET_DSA_SMSC_LAN9303_I2C
> tristate "SMSC/Microchip LAN9303 3-ports 10/100 ethernet switch in I2C managed mode"
> - depends on NET_DSA
> + depends on NET_DSA && I2C
> select NET_DSA_SMSC_LAN9303
> select REGMAP_I2C
> ---help---
Thanks, but already found and fixed by Arnd Bergmann.
jb
--
Pengutronix e.K. | Juergen Borleis |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* Re: [Intel-wired-lan] [PATCH 1/2] e1000e: Don't return uninitialized stats
From: Paul Menzel @ 2017-04-24 8:23 UTC (permalink / raw)
To: Benjamin Poirier; +Cc: Jeff Kirsher, netdev, intel-wired-lan, Stefan Priebe
In-Reply-To: <20170421212012.25950-1-bpoirier@suse.com>
Dear Benjamin,
Thank you for your fix.
On 04/21/17 23:20, Benjamin Poirier wrote:
> Some statistics passed to ethtool are garbage because e1000e_get_stats64()
> doesn't write them, for example: tx_heartbeat_errors. This leaks kernel
> memory to userspace and confuses users.
Could you please give specific examples to reproduce the issue? That way
your fix can also be tested.
[…]
Kind regards,
Paul
^ permalink raw reply
* Re: [PATCH v2 2/2] iproute2: add support for invisible qdisc dumping
From: Jiri Pirko @ 2017-04-24 8:43 UTC (permalink / raw)
To: Jiri Kosina
Cc: David S. Miller, Stephen Hemminger, Eric Dumazet,
Jamal Hadi Salim, Phil Sutter, Cong Wang, Daniel Borkmann, netdev,
linux-kernel
In-Reply-To: <alpine.LSU.2.20.1703081303490.31814@cbobk.fhfr.pm>
Wed, Mar 08, 2017 at 01:04:42PM CET, jikos@kernel.org wrote:
>From: Jiri Kosina <jkosina@suse.cz>
>
>Support the new TCA_DUMP_INVISIBLE netlink attribute that allows asking
>kernel to perform 'full qdisc dump', as for historical reasons some of the
>default qdiscs are being hidden by the kernel.
>
>The command syntax is being extended by voluntary 'invisible' argument to
>'tc qdisc show'.
>
>Signed-off-by: Jiri Kosina <jkosina@suse.cz>
>---
[...]
>@@ -325,7 +328,25 @@ static int tc_qdisc_list(int argc, char **argv)
> filter_ifindex = t.tcm_ifindex;
> }
>
>- if (rtnl_dump_request(&rth, RTM_GETQDISC, &t, sizeof(t)) < 0) {
>+ if (dump_invisible) {
>+ struct {
>+ struct nlmsghdr n;
>+ struct tcmsg t;
>+ char buf[256];
>+ } req = {
>+ .n.nlmsg_type = RTM_GETQDISC,
>+ .n.nlmsg_len = NLMSG_LENGTH(sizeof(struct tcmsg)),
>+ };
>+
>+ req.t.tcm_family = AF_UNSPEC;
>+
>+ addattr(&req.n, 256, TCA_DUMP_INVISIBLE);
>+ if (rtnl_dump_request_n(&rth, &req.n) < 0) {
Jirko, you ignore the values that were previously set in "t". I think
that you can change this to do rtnl_dump_request_n always and simplify
the code a bit.
>+ perror("Cannot send dump request");
>+ return 1;
>+ }
>+
>+ } else if (rtnl_dump_request(&rth, RTM_GETQDISC, &t, sizeof(t)) < 0) {
> perror("Cannot send dump request");
> return 1;
> }
>--
>Jiri Kosina
>SUSE Labs
>
^ permalink raw reply
* Re: [PATCH] brcm80211: brcmfmac: Ensure that incoming skb's are writable
From: James Hughes @ 2017-04-24 8:45 UTC (permalink / raw)
To: Arend Van Spriel
Cc: netdev, Franky Lin, Hante Meuleman, Kalle Valo, linux-wireless
In-Reply-To: <e2e5d950-d186-1a27-4b30-4ae2e9838103@broadcom.com>
On 23 April 2017 at 20:34, Arend Van Spriel
<arend.vanspriel@broadcom.com> wrote:
> On 21-4-2017 11:22, James Hughes wrote:
>> On 20 April 2017 at 20:48, Arend van Spriel
>> <arend.vanspriel@broadcom.com> wrote:
>>> + linux-wireless
>>>
>>> On 4/20/2017 1:16 PM, James Hughes wrote:
>>>>
>>>> The driver was adding header information to incoming skb
>>>> without ensuring the head was uncloned and hence writable.
>>>>
>>>> skb_cow_head has been used to ensure they are writable, however,
>>>> this required some changes to error handling to ensure that
>>>> if skb_cow_head failed it was not ignored.
>>>>
>>>> This really needs to be reviewed by someone who is more familiar
>>>> with this code base to ensure any deallocation of skb's is
>>>> still correct.
>>>>
>>>> Signed-off-by: James Hughes <james.hughes@raspberrypi.org>
>>>> ---
>>>> .../wireless/broadcom/brcm80211/brcmfmac/bcdc.c | 15 ++++++++--
>>>> .../wireless/broadcom/brcm80211/brcmfmac/core.c | 23 +++++-----------
>>>> .../broadcom/brcm80211/brcmfmac/fwsignal.c | 32
>>>> +++++++++++++++++-----
>>>> .../wireless/broadcom/brcm80211/brcmfmac/sdio.c | 7 ++++-
>>>> 4 files changed, 51 insertions(+), 26 deletions(-)
>>>>
>>>
>>> [...]
>>>
>>>> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
>>>> b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
>>>> index 5eaac13..08272e8 100644
>>>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
>>>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
>>>> @@ -198,7 +198,7 @@ static netdev_tx_t brcmf_netdev_start_xmit(struct
>>>> sk_buff *skb,
>>>> int ret;
>>>> struct brcmf_if *ifp = netdev_priv(ndev);
>>>> struct brcmf_pub *drvr = ifp->drvr;
>>>> - struct ethhdr *eh = (struct ethhdr *)(skb->data);
>>>> + struct ethhdr *eh;
>>>> brcmf_dbg(DATA, "Enter, bsscfgidx=%d\n", ifp->bsscfgidx);
>>>> @@ -212,23 +212,14 @@ static netdev_tx_t brcmf_netdev_start_xmit(struct
>>>> sk_buff *skb,
>>>> }
>>>> /* Make sure there's enough room for any header */
>>>> - if (skb_headroom(skb) < drvr->hdrlen) {
>>>> - struct sk_buff *skb2;
>>>> -
>>>> - brcmf_dbg(INFO, "%s: insufficient headroom\n",
>>>> - brcmf_ifname(ifp));
>>>> - drvr->bus_if->tx_realloc++;
>>>> - skb2 = skb_realloc_headroom(skb, drvr->hdrlen);
>>>> - dev_kfree_skb(skb);
>>>> - skb = skb2;
>>>> - if (skb == NULL) {
>>>> - brcmf_err("%s: skb_realloc_headroom failed\n",
>>>> - brcmf_ifname(ifp));
>>>> - ret = -ENOMEM;
>>>> - goto done;
>>>> - }
>>>
>>>
>>> What you are throwing away here is code that assures there is sufficient
>>> headroom for protocol and bus layer in the tx path, because that is
>>> determined by drvr->hdrlen. This is where the skb is handed to the driver so
>>> if you could leave the functionality above *and* assure it is writeable that
>>> would be the best solution as there is no need for all the other changes
>>> down the tx path.
>>
>> The skb_cow_head function takes the required headroom as a parameter
>> and will ensure that there is enough space, so I don't think this code
>> segment is
>> required or have I misunderstood what you mean here?
>
> I looked more into what skb_cow_head() and skb_realloc_headroom()
> actually do and it seems you are right.
>
>> Is it safe to rely on the _cow_ being done here and not further down
>> in the stack?
>> Or at least checked further down in the stack. Previous comments from
>> another patch
>> requested that the _cow_ be done close to the actual addition of the
>> header. I presume
>> it is unlikely/impossible that the functions that add header
>> information down the stack
>> will be called without the above being done first?
>
> It is safe. During probe sequence each layer in the driver stack
> increments drvr->hdrlen with headroom it needs. So drvr->hdrlen will
> indicate the total headroom needed when .start_xmit() is called. And
> yes, the .start_xmit() is the single point of entry in the transmit
> path. So the other locations where skb_push() is done are safe.
>
OK, I will redo the patch (making it v3 + changelogs), taking this in
to account.
>>>
>>>> + ret = skb_cow_head(skb, drvr->hdrlen);
>>>> + if (ret) {
>>>
>>>
>>> So move the realloc code above here instead of simply freeing the skb.
>>>
>>>> + dev_kfree_skb_any(skb);
>>>> + goto done;
>>>> }
>>>> + eh = (struct ethhdr *)(skb->data);
>>>
>>>
>>> Now this is actually a separate fix so I would like a separate patch for it.
>>>
>>
>> No problem, but see final paragraph below.
>
> Let me see...
I'll split this fix out, but should I issue it in the same patch set
or as a separate patch?
And for a different patch, should I base it on the _cow_ version patch
or the original version?
>
>>> I have a RPi3 sitting on my desk so how can I replicate the issue. It was
>>> something about broadcast/multicast traffic when using AP mode and a bridge,
>>> right?
>>>
>>> Regards,
>>> Arend
>>
>> See this issue for details on replication.
>> https://github.com/raspberrypi/firmware/issues/673
>>
>> The bridge I use is setup using a similar procedure to that described
>> here. https://www.raspberrypi.org/documentation/configuration/wireless/access-point.md
>>
>> I'm happy to pass over this work to you guys if you think it is
>> appropriate, you are the
>> experts on the codebase.
>
> Sure. However, I must say you did an excellent job digging into the
> issue and root causing it. We always were under the impression that we
> could treat the skb as our own when handed to us in .start_xmit() callback.
>
> Regards,
> Arend
Thanks! I suspect that a few drivers are making the same mistake, Eric
found 6 over and above the smsc95xx and this one that I originally
found.
Regards
James
^ permalink raw reply
* Re: [RFC PATCH 3/7] net: add option to get information about timestamped packets
From: Miroslav Lichvar @ 2017-04-24 9:00 UTC (permalink / raw)
To: Willem de Bruijn
Cc: Network Development, Richard Cochran, Willem de Bruijn,
Soheil Hassas Yeganeh, Keller, Jacob E, Denny Page, Jiri Benc
In-Reply-To: <CAF=yD-+tJpY7XJrqxga=0_kn-FjS0M=yuu59m_ZRRrrcpJCbZw@mail.gmail.com>
On Thu, Apr 13, 2017 at 12:16:09PM -0400, Willem de Bruijn wrote:
> On Thu, Apr 13, 2017 at 11:18 AM, Miroslav Lichvar <mlichvar@redhat.com> wrote:
> > On Thu, Apr 13, 2017 at 10:37:07AM -0400, Willem de Bruijn wrote:
> >> Why is this L2 length needed?
> >
> > It's needed for incoming packets to allow converting of preamble
> > timestamps to trailer timestamps.
>
> Receiving the mac length of a packet sounds like a feature independent
> from timestamping.
I agree, but so far nobody suggested another use for this information.
Do you have any suggestions?
The idea was that if it is useful only with HW timestamping, it would
be better to save it only with the timestamp, so there is no
performance impact in the more common case when HW timestamping is
disabled. Am I overly cautious here?
> Either an ioctl similar to SIOCGIFMTU or, if it may
> vary due to existince of vlan headers, a new independent cmsg at the
> SOL_SOCKET layer.
It's not just the VLAN headers. The length of the IP header may vary
with IP options, so the offset of the UDP data in the packet cannot be
assumed to be constant.
Now I'm wondering if it's actually necessary to save the original
value of skb->mac_len + skb->len. Would "skb->data - skb->head -
skb->mac_header + skb->len" always work as the L2 length for received
packets at the time when the cmsg is prepared?
As for the original ifindex, it seems to me it does need to be saved
to a new field since __netif_receive_skb_core() intentionally
overwrites skb->skb_iif. What would be the best place for it, sk_buff
or skb_shared_info?
And would it really be acceptable to save it for all packets in
__netif_receive_skb_core(), even when HW timestamping is disabled?
Seeing how the code and the data structures were optimized over time,
I have a feeling it would not be accepted.
--
Miroslav Lichvar
^ permalink raw reply
* Re: [PATCH v4 net-next] mdio_bus: Issue GPIO RESET to PHYs.
From: Roger Quadros @ 2017-04-24 9:04 UTC (permalink / raw)
To: Andrew Lunn, Lars-Peter Clausen
Cc: davem, Florian Fainelli, tony, nsekhar, jsarha, netdev,
linux-omap, linux-kernel
In-Reply-To: <20170423233553.GA23498@lunn.ch>
On 24/04/17 02:35, Andrew Lunn wrote:
> On Fri, Apr 21, 2017 at 03:31:09PM +0200, Lars-Peter Clausen wrote:
>> On 04/21/2017 03:15 PM, Roger Quadros wrote:
>>> diff --git a/Documentation/devicetree/bindings/net/mdio.txt b/Documentation/devicetree/bindings/net/mdio.txt
>>> new file mode 100644
>>> index 0000000..4ffbbac
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/net/mdio.txt
>>> @@ -0,0 +1,33 @@
>>> +Common MDIO bus properties.
>>> +
>>> +These are generic properties that can apply to any MDIO bus.
>>> +
>>> +Optional properties:
>>> +- reset-gpios: List of one or more GPIOs that control the RESET lines
>>> + of the PHYs on that MDIO bus.
>>> +- reset-delay-us: RESET pulse width in microseconds as per PHY datasheet.
>>> +
>>> +A list of child nodes, one per device on the bus is expected. These
>>> +should follow the generic phy.txt, or a device specific binding document.
>>> +
>>> +Example :
>>> +This example shows these optional properties, plus other properties
>>> +required for the TI Davinci MDIO driver.
>>> +
>>> + davinci_mdio: ethernet@0x5c030000 {
>>> + compatible = "ti,davinci_mdio";
>>> + reg = <0x5c030000 0x1000>;
>>> + #address-cells = <1>;
>>> + #size-cells = <0>;
>>> +
>>> + reset-gpios = <&gpio2 5 GPIO_ACTIVE_LOW>;
>>> + reset-delay-us = <2>; /* PHY datasheet states 1us min */
>>
>> If this is the reset line of the PHY shouldn't it be a property of the PHY
>> node rather than of the MDIO controller node (which might have a reset on
>> its own)?
>>> +
>>> + ethphy0: ethernet-phy@1 {
>>> + reg = <1>;
>>> + };
>>> +
>>> + ethphy1: ethernet-phy@3 {
>>> + reg = <3>;
>>> + };
>
> Hi Lars-Peter
>
> We discussed this when the first proposal was made. There are two
> cases, to consider.
>
> 1) Here, one GPIO line resets all PHYs on the same MDIO bus. In this
> example, two PHYs.
>
> 2) There is one GPIO line per PHY. That is a separate case, and as you
> say, the reset line should probably be considered a PHY property, not
> an MDIO property. However, it can be messy, since in order to probe
> the MDIO bus, you probably need to take the PHY out of reset.
>
> Anyway, this patch addresses the first case, so should be accepted. If
> anybody wants to address the second case, they are free to do so.
Thanks for the explanation Andrew.
For the second case, even if the RESET GPIO property is specified
in the PHY node, the RESET *will* have to be done by the MDIO bus driver
else the PHY might not be probed at all.
Whether we need additional code to just to make the DT look prettier is
questionable and if required can come as a separate patch.
cheers,
-roger
^ permalink raw reply
* Re: [PATCH net-next v5 1/2] net sched actions: dump more than TCA_ACT_MAX_PRIO actions per batch
From: Simon Horman @ 2017-04-24 9:14 UTC (permalink / raw)
To: Jiri Pirko; +Cc: Jamal Hadi Salim, davem, xiyou.wangcong, eric.dumazet, netdev
In-Reply-To: <20170420142453.GF1886@nanopsycho.orion>
On Thu, Apr 20, 2017 at 04:24:53PM +0200, Jiri Pirko wrote:
> Thu, Apr 20, 2017 at 04:18:50PM CEST, jhs@mojatatu.com wrote:
> >On 17-04-20 09:59 AM, Jiri Pirko wrote:
> >> Thu, Apr 20, 2017 at 03:06:21PM CEST, jhs@mojatatu.com wrote:
> >> > From: Jamal Hadi Salim <jhs@mojatatu.com>
...
> >> > + if (tcaa[TCAA_ACT_FLAGS])
> >> > + act_flags = nla_get_u32(tcaa[TCAA_ACT_FLAGS]);
> >>
> >> I still believe this is wrong. Should be a separate attr per flag.
> >> For user experience breakage reasons:
> >> 2 kernels should not behave differently on the exact same value passed
> >> from userspace:
> >> User passes 0x2. Now the kernel will ignore the set bit, the next kernel
> >> will recognize it as a valid flag and do something.
> >> Please let the discussion reach a consensus before pushing this again.
> >>
> >>
> >
> >Jiri - I dont agree. There is no such breakage. Refer to my previous
> >email. Lets just move on.
>
> Anyone else has opinion on this?
At the risk of jumping into a hornets nest, yes, I have an opinion:
* A agree with Jiri that a separate attribute per flag seems to be
the cleanest option, however;
* I think it would be reasonable from a UABI PoV to permit currently unused
bits of TCAA_ACT_FLAGS to be re-uses so long as the kernel checks that
they are zero until they are designated to have some use. I believe this
implies that the default value for any future uses of these bits would be
zero.
Jamal, I am confused about why are you so concerned about the space
consumed by this attribute, it's per-message, right? Is it the bigger
picture you are worried about - a similar per-entry flag at some point in
the future?
^ permalink raw reply
* Re: [PATCH net-next v4 1/2] net sched actions: dump more than TCA_ACT_MAX_PRIO actions per batch
From: Simon Horman @ 2017-04-24 9:27 UTC (permalink / raw)
To: Jamal Hadi Salim; +Cc: David Miller, eric.dumazet, jiri, netdev, xiyou.wangcong
In-Reply-To: <c6333422-ae6d-149c-09ec-c5dfb43d68b3@mojatatu.com>
On Fri, Apr 21, 2017 at 02:11:00PM -0400, Jamal Hadi Salim wrote:
> On 17-04-21 12:12 PM, David Miller wrote:
>
> >Yes for existing attributes we are stuck in the mud because of how
> >we've handled things in the past. I'm not saying we should change
> >behavior for existing attributes.
> >
> >I'm talking about any newly added attribute from here on out, and
> >that we need to require checks for them.
> >
>
> Please bear with me. I want to make sure to get this right.
>
> Lets say I updated the kernel today to reject transactions with
> bits it didnt understand. Lets call this "old kernel". A tc that
> understands/sets these bits and nothing else. Call it "old tc".
> 3 months later:
> I add one more bit setting to introduce a new feature in a new
> kernel version. Lets call this new "kernel". I update to
> understand new bits. Call it "new tc".
>
> The possibilities:
> a) old tc + old kernel combo. No problem
> b) new tc + new kernel combo. No problem.
> c) old tc + new kernel combo. No problem.
> d) new tc + old kernel. Rejection.
>
> For #d if i have a smart tc it would retry with a new combination
> which restores its behavior to old tc level. Of course this means
> apps would have to be rewritten going forward to understand these
> mechanics.
> Alternative is to request for capabilities first then doing a
> lowest common denominator request.
> But even that is a lot more code and crossing user/kernel twice.
>From my PoV, for #d user-space has to either get smart or fail.
Creating new tc involved work in order to support the new feature.
Part of that work would be a decision weather or not to provide
compatibility for old kernel or to bail out gracefully.
> There is a simpler approach that would work going forward.
> How about letting the user choose their fate? Set something maybe
> in the netlink header to tell the kernel "if you dont understand
> something I am asking for - please ignore it and do what you can".
> This would maintain current behavior but would force the user to
> explicitly state so.
>
> cheers,
> jamal
>
^ permalink raw reply
* [net 1/1] team: fix memory leaks
From: Pan Bian @ 2017-04-24 9:36 UTC (permalink / raw)
To: Jiri Pirko, netdev; +Cc: linux-kernel, Pan Bian
In functions team_nl_send_port_list_get() and
team_nl_send_options_get(), pointer skb keeps the return value of
nlmsg_new(). When the call to genlmsg_put() fails, the memory is not
freed(). This will result in memory leak bugs.
Fixes: 9b00cf2d1024 ("team: implement multipart netlink messages for
options transfers")
Signed-off-by: Pan Bian <bianpan2016@163.com>
---
drivers/net/team/team.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
index f8c81f1..85c0124 100644
--- a/drivers/net/team/team.c
+++ b/drivers/net/team/team.c
@@ -2361,8 +2361,10 @@ static int team_nl_send_options_get(struct team *team, u32 portid, u32 seq,
hdr = genlmsg_put(skb, portid, seq, &team_nl_family, flags | NLM_F_MULTI,
TEAM_CMD_OPTIONS_GET);
- if (!hdr)
+ if (!hdr) {
+ nlmsg_free(skb);
return -EMSGSIZE;
+ }
if (nla_put_u32(skb, TEAM_ATTR_TEAM_IFINDEX, team->dev->ifindex))
goto nla_put_failure;
@@ -2634,8 +2636,10 @@ static int team_nl_send_port_list_get(struct team *team, u32 portid, u32 seq,
hdr = genlmsg_put(skb, portid, seq, &team_nl_family, flags | NLM_F_MULTI,
TEAM_CMD_PORT_LIST_GET);
- if (!hdr)
+ if (!hdr) {
+ nlmsg_free(skb);
return -EMSGSIZE;
+ }
if (nla_put_u32(skb, TEAM_ATTR_TEAM_IFINDEX, team->dev->ifindex))
goto nla_put_failure;
--
1.9.1
^ permalink raw reply related
* Re: [net 1/1] team: fix memory leaks
From: Jiri Pirko @ 2017-04-24 9:42 UTC (permalink / raw)
To: Pan Bian; +Cc: netdev, linux-kernel
In-Reply-To: <1493026612-12383-1-git-send-email-bianpan2016@163.com>
Interesting. In last reply, I put the "[]" prefix example exactly as it
should be, yet you managed to have it wrong...
Mon, Apr 24, 2017 at 11:36:52AM CEST, bianpan2016@163.com wrote:
>In functions team_nl_send_port_list_get() and
>team_nl_send_options_get(), pointer skb keeps the return value of
>nlmsg_new(). When the call to genlmsg_put() fails, the memory is not
>freed(). This will result in memory leak bugs.
>
>Fixes: 9b00cf2d1024 ("team: implement multipart netlink messages for
>options transfers")
No linewraps here, please.
>Signed-off-by: Pan Bian <bianpan2016@163.com>
>---
> drivers/net/team/team.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
>index f8c81f1..85c0124 100644
>--- a/drivers/net/team/team.c
>+++ b/drivers/net/team/team.c
>@@ -2361,8 +2361,10 @@ static int team_nl_send_options_get(struct team *team, u32 portid, u32 seq,
>
> hdr = genlmsg_put(skb, portid, seq, &team_nl_family, flags | NLM_F_MULTI,
> TEAM_CMD_OPTIONS_GET);
>- if (!hdr)
>+ if (!hdr) {
>+ nlmsg_free(skb);
> return -EMSGSIZE;
>+ }
>
> if (nla_put_u32(skb, TEAM_ATTR_TEAM_IFINDEX, team->dev->ifindex))
> goto nla_put_failure;
>@@ -2634,8 +2636,10 @@ static int team_nl_send_port_list_get(struct team *team, u32 portid, u32 seq,
>
> hdr = genlmsg_put(skb, portid, seq, &team_nl_family, flags | NLM_F_MULTI,
> TEAM_CMD_PORT_LIST_GET);
>- if (!hdr)
>+ if (!hdr) {
>+ nlmsg_free(skb);
> return -EMSGSIZE;
>+ }
>
> if (nla_put_u32(skb, TEAM_ATTR_TEAM_IFINDEX, team->dev->ifindex))
> goto nla_put_failure;
>--
>1.9.1
>
>
^ permalink raw reply
* Re: [PATCH v2 net-next] net: ipv6: send unsolicited NA if enabled for all interfaces
From: Simon Horman @ 2017-04-24 9:42 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, hannes
In-Reply-To: <1492877413-21906-1-git-send-email-dsa@cumulusnetworks.com>
On Sat, Apr 22, 2017 at 09:10:13AM -0700, David Ahern wrote:
> When arp_notify is set to 1 for either a specific interface or for 'all'
> interfaces, gratuitous arp requests are sent. Since ndisc_notify is the
> ipv6 equivalent to arp_notify, it should follow the same semantics.
> Commit 4a6e3c5def13 ("net: ipv6: send unsolicited NA on admin up") sends
> the NA on admin up. The final piece is checking devconf_all->ndisc_notify
> in addition to the per device setting. Add it.
>
> Fixes: 5cb04436eef6 ("ipv6: add knob to send unsolicited ND on link-layer address change")
> Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
> ---
> v2
> - update commit message with subject of commit 4a6e3c5def13 per comment
> from Sergei
>
> net/ipv6/ndisc.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
> index b23822e64228..d310dc41209a 100644
> --- a/net/ipv6/ndisc.c
> +++ b/net/ipv6/ndisc.c
> @@ -1753,7 +1753,8 @@ static int ndisc_netdev_event(struct notifier_block *this, unsigned long event,
> idev = in6_dev_get(dev);
> if (!idev)
> break;
> - if (idev->cnf.ndisc_notify)
> + if (idev->cnf.ndisc_notify ||
> + net->ipv6.devconf_all->ndisc_notify)
> ndisc_send_unsol_na(dev);
> in6_dev_put(idev);
> break;
> --
> 2.1.4
>
^ permalink raw reply
* Re: [net 1/1] team: fix memory leaks
From: Jiri Pirko @ 2017-04-24 9:43 UTC (permalink / raw)
To: Pan Bian; +Cc: netdev, linux-kernel
In-Reply-To: <1493026612-12383-1-git-send-email-bianpan2016@163.com>
Plus since you have only one patch, please do not do "1/1" in the
email subject. Thanks.
Mon, Apr 24, 2017 at 11:36:52AM CEST, bianpan2016@163.com wrote:
>In functions team_nl_send_port_list_get() and
>team_nl_send_options_get(), pointer skb keeps the return value of
>nlmsg_new(). When the call to genlmsg_put() fails, the memory is not
>freed(). This will result in memory leak bugs.
>
>Fixes: 9b00cf2d1024 ("team: implement multipart netlink messages for
>options transfers")
>Signed-off-by: Pan Bian <bianpan2016@163.com>
>---
> drivers/net/team/team.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
>index f8c81f1..85c0124 100644
>--- a/drivers/net/team/team.c
>+++ b/drivers/net/team/team.c
>@@ -2361,8 +2361,10 @@ static int team_nl_send_options_get(struct team *team, u32 portid, u32 seq,
>
> hdr = genlmsg_put(skb, portid, seq, &team_nl_family, flags | NLM_F_MULTI,
> TEAM_CMD_OPTIONS_GET);
>- if (!hdr)
>+ if (!hdr) {
>+ nlmsg_free(skb);
> return -EMSGSIZE;
>+ }
>
> if (nla_put_u32(skb, TEAM_ATTR_TEAM_IFINDEX, team->dev->ifindex))
> goto nla_put_failure;
>@@ -2634,8 +2636,10 @@ static int team_nl_send_port_list_get(struct team *team, u32 portid, u32 seq,
>
> hdr = genlmsg_put(skb, portid, seq, &team_nl_family, flags | NLM_F_MULTI,
> TEAM_CMD_PORT_LIST_GET);
>- if (!hdr)
>+ if (!hdr) {
>+ nlmsg_free(skb);
> return -EMSGSIZE;
>+ }
>
> if (nla_put_u32(skb, TEAM_ATTR_TEAM_IFINDEX, team->dev->ifindex))
> goto nla_put_failure;
>--
>1.9.1
>
>
^ 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