Netdev List
 help / color / mirror / Atom feed
* Re: [net PATCH] net: tls, fix sk_write_space NULL write when tx disabled
From: David Miller @ 2019-08-15 19:41 UTC (permalink / raw)
  To: john.fastabend; +Cc: jakub.kicinski, ying.xue, netdev, andreyknvl
In-Reply-To: <156576071416.1402.5907777786031481705.stgit@ubuntu3-kvm1>

From: John Fastabend <john.fastabend@gmail.com>
Date: Wed, 14 Aug 2019 05:31:54 +0000

> The ctx->sk_write_space pointer is only set when TLS tx mode is enabled.
> When running without TX mode its a null pointer but we still set the
> sk sk_write_space pointer on close().
> 
> Fix the close path to only overwrite sk->sk_write_space when the current
> pointer is to the tls_write_space function indicating the tls module should
> clean it up properly as well.
> 
> Reported-by: Hillf Danton <hdanton@sina.com>
> Cc: Ying Xue <ying.xue@windriver.com>
> Cc: Andrey Konovalov <andreyknvl@google.com>
> Fixes: 57c722e932cfb ("net/tls: swap sk_write_space on close")
> Signed-off-by: John Fastabend <john.fastabend@gmail.com>

Applied, thanks John.

That Fixes tag takes one through an interesting chain of fixes to fixes.
I'll queue this up for -stable and make sure it all ends up in the
proper place.

Thanks.

^ permalink raw reply

* Re: pull-request: can-next 2019-08-14,pull-request: can-next 2019-08-14
From: David Miller @ 2019-08-15 19:43 UTC (permalink / raw)
  To: mkl; +Cc: netdev, kernel, linux-can
In-Reply-To: <f0658ccd-389f-fc60-7538-c512112b9978@pengutronix.de>

From: Marc Kleine-Budde <mkl@pengutronix.de>
Date: Wed, 14 Aug 2019 08:16:24 +0200

> this is a pull request for net-next/master consisting of 41 patches.
 ...

Pulled, thanks Marc.

^ permalink raw reply

* Re: [PATCH] net: myri10ge: fix memory leaks
From: David Miller @ 2019-08-15 19:45 UTC (permalink / raw)
  To: wenwen; +Cc: christopher.lee, netdev, linux-kernel
In-Reply-To: <1565764719-6488-1-git-send-email-wenwen@cs.uga.edu>

From: Wenwen Wang <wenwen@cs.uga.edu>
Date: Wed, 14 Aug 2019 01:38:39 -0500

> In myri10ge_probe(), myri10ge_alloc_slices() is invoked to allocate slices
> related structures. Later on, myri10ge_request_irq() is used to get an irq.
> However, if this process fails, the allocated slices related structures are
> not deallocated, leading to memory leaks. To fix this issue, revise the
> target label of the goto statement to 'abort_with_slices'.
> 
> Signed-off-by: Wenwen Wang <wenwen@cs.uga.edu>

Applied, thanks.

^ permalink raw reply

* [PATCH mlx5-next 0/5] Mellanox, Updates for mlx5-next branch 2019-08-15
From: Saeed Mahameed @ 2019-08-15 19:46 UTC (permalink / raw)
  To: Saeed Mahameed, Leon Romanovsky
  Cc: netdev@vger.kernel.org, linux-rdma@vger.kernel.org

Hi All,

This series includes misc updates for mlx5-next shared branch.

mlx5 HW spec and bits updates:
1) Aya exposes IP-in-IP capability in mlx5_core.
2) Maxim exposes lag tx port affinity capabilities.
3) Moshe adds VNIC_ENV internal rq counter bits.

Misc updates:
4) Saeed, two compiler warnings cleanups

In case of no objection this series will be applied to mlx5-next branch
and sent later as pull request to both rdma-next and net-next branches.

Thanks,
Saeed.

---

Aya Levin (1):
  net/mlx5: Expose IP-in-IP capability bit

Maxim Mikityanskiy (1):
  net/mlx5: Add lag_tx_port_affinity capability bit

Moshe Shemesh (1):
  net/mlx5: Add support for VNIC_ENV internal rq counter

Saeed Mahameed (2):
  net/mlx5: Add missing include file to lib/crypto.c
  net/mlx5: Improve functions documentation

 drivers/net/ethernet/mellanox/mlx5/core/eq.c  | 22 +++++++++++--------
 .../ethernet/mellanox/mlx5/core/lib/crypto.c  |  1 +
 include/linux/mlx5/mlx5_ifc.h                 | 18 +++++++++++----
 3 files changed, 28 insertions(+), 13 deletions(-)

-- 
2.21.0


^ permalink raw reply

* [PATCH mlx5-next 1/5] net/mlx5: Add missing include file to lib/crypto.c
From: Saeed Mahameed @ 2019-08-15 19:46 UTC (permalink / raw)
  To: Saeed Mahameed, Leon Romanovsky
  Cc: netdev@vger.kernel.org, linux-rdma@vger.kernel.org
In-Reply-To: <20190815194543.14369-1-saeedm@mellanox.com>

Add missing include file to avoid compiler warnings:
drivers/net/ethernet/mellanox/mlx5/core//lib/crypto.c:6:5:
warning: no previous prototype for ‘mlx5_create_encryption_key’
    6 | int mlx5_create_encryption_key(struct mlx5_core_dev *mdev,
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/ethernet/mellanox/mlx5/core//lib/crypto.c:60:6:
 warning: no previous prototype for ‘mlx5_destroy_encryption_key’
   60 | void mlx5_destroy_encryption_key(struct mlx5_core_dev *mdev, ...

Fixes: 45d3b55dc665 ("net/mlx5: Add crypto library to support create/destroy encryption key")
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/lib/crypto.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/crypto.c b/drivers/net/ethernet/mellanox/mlx5/core/lib/crypto.c
index ea9ee88491e5..22bc45c831d2 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/lib/crypto.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/crypto.c
@@ -2,6 +2,7 @@
 // Copyright (c) 2019 Mellanox Technologies.
 
 #include "mlx5_core.h"
+#include "lib/mlx5.h"
 
 int mlx5_create_encryption_key(struct mlx5_core_dev *mdev,
 			       void *key, u32 sz_bytes,
-- 
2.21.0


^ permalink raw reply related

* [PATCH mlx5-next 4/5] net/mlx5: Expose IP-in-IP capability bit
From: Saeed Mahameed @ 2019-08-15 19:46 UTC (permalink / raw)
  To: Saeed Mahameed, Leon Romanovsky
  Cc: netdev@vger.kernel.org, linux-rdma@vger.kernel.org, Aya Levin,
	Tariq Toukan
In-Reply-To: <20190815194543.14369-1-saeedm@mellanox.com>

From: Aya Levin <ayal@mellanox.com>

Expose Fw indication that it supports Stateless Offloads for IP over IP
tunneled packets. The following offloads are supported for the inner
packets: RSS, RX & TX Checksum Offloads, LSO and Flow Steering.

Signed-off-by: Aya Levin <ayal@mellanox.com>
Reviewed-by: Tariq Toukan <tariqt@mellanox.com>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
 include/linux/mlx5/mlx5_ifc.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/linux/mlx5/mlx5_ifc.h b/include/linux/mlx5/mlx5_ifc.h
index c788f895b350..2837fe4d8901 100644
--- a/include/linux/mlx5/mlx5_ifc.h
+++ b/include/linux/mlx5/mlx5_ifc.h
@@ -808,7 +808,9 @@ struct mlx5_ifc_per_protocol_networking_offload_caps_bits {
 	u8         swp_csum[0x1];
 	u8         swp_lso[0x1];
 	u8         cqe_checksum_full[0x1];
-	u8         reserved_at_24[0xc];
+	u8         reserved_at_24[0x5];
+	u8         tunnel_stateless_ip_over_ip[0x1];
+	u8         reserved_at_2a[0x6];
 	u8         max_vxlan_udp_ports[0x8];
 	u8         reserved_at_38[0x6];
 	u8         max_geneve_opt_len[0x1];
-- 
2.21.0


^ permalink raw reply related

* [PATCH mlx5-next 5/5] net/mlx5: Add lag_tx_port_affinity capability bit
From: Saeed Mahameed @ 2019-08-15 19:46 UTC (permalink / raw)
  To: Saeed Mahameed, Leon Romanovsky
  Cc: netdev@vger.kernel.org, linux-rdma@vger.kernel.org,
	Maxim Mikityanskiy, Tariq Toukan
In-Reply-To: <20190815194543.14369-1-saeedm@mellanox.com>

From: Maxim Mikityanskiy <maximmi@mellanox.com>

Add the lag_tx_port_affinity HCA capability bit that indicates that
setting port affinity of TISes is supported.

Signed-off-by: Maxim Mikityanskiy <maximmi@mellanox.com>
Reviewed-by: Tariq Toukan <tariqt@mellanox.com>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
 include/linux/mlx5/mlx5_ifc.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/linux/mlx5/mlx5_ifc.h b/include/linux/mlx5/mlx5_ifc.h
index 2837fe4d8901..1e55cf73e88c 100644
--- a/include/linux/mlx5/mlx5_ifc.h
+++ b/include/linux/mlx5/mlx5_ifc.h
@@ -1249,7 +1249,9 @@ struct mlx5_ifc_cmd_hca_cap_bits {
 	u8         reserved_at_263[0x8];
 	u8         log_bf_reg_size[0x5];
 
-	u8         reserved_at_270[0xb];
+	u8         reserved_at_270[0x8];
+	u8         lag_tx_port_affinity[0x1];
+	u8         reserved_at_279[0x2];
 	u8         lag_master[0x1];
 	u8         num_lag_ports[0x4];
 
-- 
2.21.0


^ permalink raw reply related

* [PATCH mlx5-next 2/5] net/mlx5: Improve functions documentation
From: Saeed Mahameed @ 2019-08-15 19:46 UTC (permalink / raw)
  To: Saeed Mahameed, Leon Romanovsky
  Cc: netdev@vger.kernel.org, linux-rdma@vger.kernel.org
In-Reply-To: <20190815194543.14369-1-saeedm@mellanox.com>

Fix documentation of mlx5_eq_enable/disable to cleanup compiler warnings.

drivers/net/ethernet/mellanox/mlx5/core//eq.c:334:
warning: Function parameter or member 'dev' not described in 'mlx5_eq_enable'
warning: Function parameter or member 'eq' not described in 'mlx5_eq_enable'
warning: Function parameter or member 'nb' not described in 'mlx5_eq_enable'

drivers/net/ethernet/mellanox/mlx5/core//eq.c:355:
warning: Function parameter or member 'dev' not described in 'mlx5_eq_disable'
warning: Function parameter or member 'eq' not described in 'mlx5_eq_disable'
warning: Function parameter or member 'nb' not described in 'mlx5_eq_disable'

Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/eq.c | 22 ++++++++++++--------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eq.c b/drivers/net/ethernet/mellanox/mlx5/core/eq.c
index 2df9aaa421c6..a0e78ab64618 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/eq.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/eq.c
@@ -324,10 +324,13 @@ create_map_eq(struct mlx5_core_dev *dev, struct mlx5_eq *eq,
 
 /**
  * mlx5_eq_enable - Enable EQ for receiving EQEs
- * @dev - Device which owns the eq
- * @eq - EQ to enable
- * @nb - notifier call block
- * mlx5_eq_enable - must be called after EQ is created in device.
+ * @dev : Device which owns the eq
+ * @eq  : EQ to enable
+ * @nb  : Notifier call block
+ *
+ * Must be called after EQ is created in device.
+ *
+ * @return: 0 if no error
  */
 int mlx5_eq_enable(struct mlx5_core_dev *dev, struct mlx5_eq *eq,
 		   struct notifier_block *nb)
@@ -344,11 +347,12 @@ int mlx5_eq_enable(struct mlx5_core_dev *dev, struct mlx5_eq *eq,
 EXPORT_SYMBOL(mlx5_eq_enable);
 
 /**
- * mlx5_eq_disable - Enable EQ for receiving EQEs
- * @dev - Device which owns the eq
- * @eq - EQ to disable
- * @nb - notifier call block
- * mlx5_eq_disable - must be called before EQ is destroyed.
+ * mlx5_eq_disable - Disable EQ for receiving EQEs
+ * @dev : Device which owns the eq
+ * @eq  : EQ to disable
+ * @nb  : Notifier call block
+ *
+ * Must be called before EQ is destroyed.
  */
 void mlx5_eq_disable(struct mlx5_core_dev *dev, struct mlx5_eq *eq,
 		     struct notifier_block *nb)
-- 
2.21.0


^ permalink raw reply related

* [PATCH mlx5-next 3/5] net/mlx5: Add support for VNIC_ENV internal rq counter
From: Saeed Mahameed @ 2019-08-15 19:46 UTC (permalink / raw)
  To: Saeed Mahameed, Leon Romanovsky
  Cc: netdev@vger.kernel.org, linux-rdma@vger.kernel.org, Moshe Shemesh
In-Reply-To: <20190815194543.14369-1-saeedm@mellanox.com>

From: Moshe Shemesh <moshe@mellanox.com>

Add mlx5 interface support for reading internal rq out of buffer counter
as part of QUERY_VNIC_ENV command. The command is used by the driver to
query vnic diagnostic statistics from FW.

Signed-off-by: Moshe Shemesh <moshe@mellanox.com>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
 include/linux/mlx5/mlx5_ifc.h | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/include/linux/mlx5/mlx5_ifc.h b/include/linux/mlx5/mlx5_ifc.h
index ab6ae723aae6..c788f895b350 100644
--- a/include/linux/mlx5/mlx5_ifc.h
+++ b/include/linux/mlx5/mlx5_ifc.h
@@ -1116,7 +1116,9 @@ struct mlx5_ifc_cmd_hca_cap_bits {
 	u8         cache_line_128byte[0x1];
 	u8         reserved_at_165[0x4];
 	u8         rts2rts_qp_counters_set_id[0x1];
-	u8         reserved_at_16a[0x5];
+	u8         reserved_at_16a[0x2];
+	u8         vnic_env_int_rq_oob[0x1];
+	u8         reserved_at_16d[0x2];
 	u8         qcam_reg[0x1];
 	u8         gid_table_size[0x10];
 
@@ -2772,7 +2774,11 @@ struct mlx5_ifc_vnic_diagnostic_statistics_bits {
 
 	u8         transmit_discard_vport_down[0x40];
 
-	u8         reserved_at_140[0xec0];
+	u8         reserved_at_140[0xa0];
+
+	u8         internal_rq_out_of_buffer[0x20];
+
+	u8         reserved_at_200[0xe00];
 };
 
 struct mlx5_ifc_traffic_counter_bits {
-- 
2.21.0


^ permalink raw reply related

* Re: [PATCH v2 bpf-next 1/4] bpf: unprivileged BPF access via /dev/bpf
From: Kees Cook @ 2019-08-15 19:46 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Andy Lutomirski, Song Liu, Networking, bpf, Alexei Starovoitov,
	Daniel Borkmann, Kernel Team, Lorenz Bauer, Jann Horn, Greg KH,
	Linux API, LSM List
In-Reply-To: <20190813215823.3sfbakzzjjykyng2@ast-mbp>

On Tue, Aug 13, 2019 at 02:58:25PM -0700, Alexei Starovoitov wrote:
> agree that containers (namespaces) reduce amount of trust necessary
> for apps to run, but the end goal is not security though.

Unsurprisingly, I totally disagree: this is the very definition of
improved "security": reduced attack surface, confined trust, etc.

> Linux has become a single user system.

I hope this is just hyperbole, because it's not true in reality. I agree
that the vast majority of Linux devices are single-user-at-a-time
systems now (rather than the "shell servers" of yore), but the system
still has to be expected to confine users from each other, root, and the
hardware. Switching users on Chrome OS or a distro laptop, etc is still
very much expected to _mean_ something.

> If user can ssh into the host they can become root.
> If arbitrary code can run on the host it will be break out of any sandbox.
> Containers are not providing the level of security that is enough
> to run arbitrary code. VMs can do it better, but cpu bugs don't make it easy.

I'm not sure why you draw the line for VMs -- they're just as buggy
as anything else. Regardless, I reject this line of thinking: yes,
all software is buggy, but that isn't a reason to give up. In fact,
we should be trying very hard to create safe code (*insert arguments
for sane languages and toolchains here*).

If you look at software safety as a binary, you will always be
disappointed. If you look at it as it manifests in the real world,
then there is some perspective to be had. Reachability of flaws becomes
a major factor; exploit chain length becomes a factor. There are very
real impacts to be had from security hardening, sandboxing, etc. Of
course nothing is perfect, but the current state of the world isn't
as you describe. (And I say this with the knowledge of how long
the lifetime of bugs are in the kernel.)

> Containers are used to make production systems safer.

Yes.

> Some people call it more 'secure', but it's clearly not secure for
> arbitrary code

Perhaps it's just a language issue. "More secure" and "safer" mean
mostly the same thing to me. I tend to think "safer" is actually
a superset that includes things that wreck the user experience but
aren't actually in the privilege manipulation realm. In the traditional
"security" triad of confidentiality, integrity, and availability, I tend
to weigh availability less highly, but a bug that stops someone from
doing their work but doesn't wreck data, let them switch users, etc,
is still considered a "security" issue by many folks. The fewer bugs
someone is exposed to improves their security, safety, whatever. The
easiest way to do that is confinement and its associated attack surface
reduction. tl;dr: security and safety are very use-case-specific
continuum, not a binary state.

> When we say 'unprivileged bpf' we really mean arbitrary malicious bpf program.
> It's been a constant source of pain. The constant blinding, randomization,
> verifier speculative analysis, all spectre v1, v2, v4 mitigations
> are simply not worth it. It's a lot of complex kernel code without users.
> There is not a single use case to allow arbitrary malicious bpf
> program to be loaded and executed.

The world isn't binary (safe code/malicious code), and we need to build
systems that can be used safely even when things go wrong. Yes, probably
no one has a system that _intentionally_ feeds eBPF into the kernel from
a web form. But there is probably someone who does it unintentionally,
or has a user login exposed on a system where unpriv BPF is enabled. The
point is to create primitives as safely as possible so when things DO
go wrong, they fail safe instead of making things worse.

I'm all for a "less privileged than root" API for eBPF, but I get worried
when I see "security" being treated as a binary state. Especially when
it is considered an always-failed state. :)

-- 
Kees Cook

^ permalink raw reply

* [PATCH] net: cavium: fix driver name
From: Stephen Hemminger @ 2019-08-15 19:49 UTC (permalink / raw)
  To: yuehaibing; +Cc: netdev, Stephen Hemminger

The driver name gets exposed in sysfs under /sys/bus/pci/drivers
so it should look like other devices. Change it to be common
format (instead of "Cavium PTP").

This is a trivial fix that was observed by accident because
Debian kernels were building this driver into kernel (bug).

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/ethernet/cavium/common/cavium_ptp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/cavium/common/cavium_ptp.c b/drivers/net/ethernet/cavium/common/cavium_ptp.c
index 73632b843749..b821c9e1604c 100644
--- a/drivers/net/ethernet/cavium/common/cavium_ptp.c
+++ b/drivers/net/ethernet/cavium/common/cavium_ptp.c
@@ -10,7 +10,7 @@
 
 #include "cavium_ptp.h"
 
-#define DRV_NAME	"Cavium PTP Driver"
+#define DRV_NAME "cavium_ptp"
 
 #define PCI_DEVICE_ID_CAVIUM_PTP	0xA00C
 #define PCI_DEVICE_ID_CAVIUM_RST	0xA00E
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH net-next v4 0/2] qed*: Support for NVM config attributes.
From: David Miller @ 2019-08-15 19:54 UTC (permalink / raw)
  To: skalluru; +Cc: netdev, mkalderon, aelior
In-Reply-To: <20190814081153.18889-1-skalluru@marvell.com>

From: Sudarsana Reddy Kalluru <skalluru@marvell.com>
Date: Wed, 14 Aug 2019 01:11:51 -0700

> The patch series adds support for managing the NVM config attributes.
> Patch (1) adds functionality to update config attributes via MFW.
> Patch (2) adds driver interface for updating the config attributes.
> 
> Changes from previous versions:
> -------------------------------
> v4: Added more details on the functionality and its usage.
> v3: Removed unused variable.
> v2: Removed unused API.
> 
> Please consider applying this series to "net-next".

Series applied.

^ permalink raw reply

* Re: [PATCH] net: pch_gbe: Fix memory leaks
From: Wenwen Wang @ 2019-08-15 20:03 UTC (permalink / raw)
  To: David Miller
  Cc: Richard Fontana, Allison Randal, Alexios Zavras,
	Greg Kroah-Hartman, Thomas Gleixner,
	open list:NETWORKING [GENERAL], open list, Wenwen Wang
In-Reply-To: <20190815.123430.831231953098536795.davem@davemloft.net>

On Thu, Aug 15, 2019 at 3:34 PM David Miller <davem@davemloft.net> wrote:
>
> From: Wenwen Wang <wenwen@cs.uga.edu>
> Date: Tue, 13 Aug 2019 20:33:45 -0500
>
> > In pch_gbe_set_ringparam(), if netif_running() returns false, 'tx_old' and
> > 'rx_old' are not deallocated, leading to memory leaks. To fix this issue,
> > move the free statements after the if branch.
> >
> > Signed-off-by: Wenwen Wang <wenwen@cs.uga.edu>
>
> Why would they be "deallocated"?  They are still assigned to
> adapter->tx_ring and adapter->rx_ring.

'adapter->tx_ring' and 'adapter->rx_ring' has been covered by newly
allocated 'txdr' and 'rxdr' respectively before this if statement.

Wenwen

^ permalink raw reply

* Re: [PATCH] wimax/i2400m: fix a memory leak bug
From: Wenwen Wang @ 2019-08-15 20:05 UTC (permalink / raw)
  To: Wenwen Wang, Inaky Perez-Gonzalez,
	supporter:INTEL WIRELESS WIMAX CONNECTION 2400, David S. Miller,
	open list:NETWORKING DRIVERS, open list
In-Reply-To: <20190815184505.o7o2ojt7ag4shh7u@oracle.com>

On Thu, Aug 15, 2019 at 2:45 PM Liam R. Howlett <Liam.Howlett@oracle.com> wrote:
>
> * Wenwen Wang <wenwen@cs.uga.edu> [190815 14:05]:
> > In i2400m_barker_db_init(), 'options_orig' is allocated through kstrdup()
> > to hold the original command line options. Then, the options are parsed.
> > However, if an error occurs during the parsing process, 'options_orig' is
> > not deallocated, leading to a memory leak bug. To fix this issue, free
> > 'options_orig' before returning the error.
> >
> > Signed-off-by: Wenwen Wang <wenwen@cs.uga.edu>
> > ---
> >  drivers/net/wimax/i2400m/fw.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/wimax/i2400m/fw.c b/drivers/net/wimax/i2400m/fw.c
> > index e9fc168..6b36f6d 100644
> > --- a/drivers/net/wimax/i2400m/fw.c
> > +++ b/drivers/net/wimax/i2400m/fw.c
> > @@ -342,6 +342,7 @@ int i2400m_barker_db_init(const char *_options)
> >                                      "a 32-bit number\n",
> >                                      __func__, token);
> >                               result = -EINVAL;
> > +                             kfree(options_orig);
> >                               goto error_parse;
> >                       }
> >                       if (barker == 0) {
> > @@ -350,8 +351,10 @@ int i2400m_barker_db_init(const char *_options)
> >                               continue;
> >                       }
> >                       result = i2400m_barker_db_add(barker);
> > -                     if (result < 0)
> > +                     if (result < 0) {
> > +                             kfree(options_orig);
> >                               goto error_add;
>
> I know that you didn't add this error_add label, but it seems like the
> incorrect goto label.  Although looking at the caller indicates an add
> failed, this label is used prior to and after the memory leak you are
> trying to fix.  It might be better to change this label to something
> like error_parse_add and move the kfree to the unwinding.  If a new
> label is used, it becomes more clear as to what is being undone and
> there aren't two jumps into an unwind from two very different stages of
> the function.  Adding a new label also has the benefit of moving the
> kfree to the unwind of error_parse.

Thanks for your suggestion! I will rework the patch.

Wenwen

^ permalink raw reply

* Re: [PATCH bpf] tools: bpftool: close prog FD before exit on showing a single program
From: Andrii Nakryiko @ 2019-08-15 20:16 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Quentin Monnet, Alexei Starovoitov, Daniel Borkmann, bpf,
	Networking, oss-drivers
In-Reply-To: <20190815110917.657de4e3@cakuba.netronome.com>

On Thu, Aug 15, 2019 at 11:09 AM Jakub Kicinski
<jakub.kicinski@netronome.com> wrote:
>
> On Thu, 15 Aug 2019 11:05:16 -0700, Andrii Nakryiko wrote:
> > > > Would it be better to make show_prog(fd) close provided fd instead or
> > > > is it used in some other context where FD should live longer (I
> > > > haven't checked, sorry)?
> > >
> > > I think it used to close that's how the bug crept in. Other than the bug
> > > it's fine the way it is.
> >
> > So are you saying that show_prog() should or should not close FD?
>
> Yup, it we'd have to rename it to indicate it closes the fd, and it's
> only called in two places. Not worth the churn.

OK, I'm fine with that.

Acked-by: Andrii Nakryiko <andriin@fb.com>

^ permalink raw reply

* Re: [PATCH v3 1/2] rtw88: pci: Rearrange the memory usage for skb in RX ISR
From: Brian Norris @ 2019-08-15 20:25 UTC (permalink / raw)
  To: Jian-Hong Pan
  Cc: Yan-Hsuan Chuang, Kalle Valo, David S . Miller, Larry Finger,
	David Laight, Christoph Hellwig, linux-wireless,
	<netdev@vger.kernel.org>, Linux Kernel, linux, Daniel Drake,
	stable
In-Reply-To: <20190710083825.7115-1-jian-hong@endlessm.com>

Hi all,

I realize this already is merged, and it had some previous review
comments that led to the decisions in this patch, but I'd still like
to ask here, where I think I'm reaching the relevant parties:

On Wed, Jul 10, 2019 at 1:43 AM Jian-Hong Pan <jian-hong@endlessm.com> wrote:
...
> This patch allocates a new, data-sized skb first in RX ISR. After
> copying the data in, we pass it to the upper layers. However, if skb
> allocation fails, we effectively drop the frame. In both cases, the
> original, full size ring skb is reused.
>
> In addition, by fixing the kernel crash, the RX routine should now
> generally behave better under low memory conditions.
>
> Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=204053
> Signed-off-by: Jian-Hong Pan <jian-hong@endlessm.com>
> Cc: <stable@vger.kernel.org>
> ---
> v2:
>  - Allocate new data-sized skb and put data into it, then pass it to
>    mac80211. Reuse the original skb in RX ring by DMA sync.

Is it really wise to force an extra memcpy() for *every* delivery?
Isn't there some other strategy that could be used to properly handle
low-memory scenarios while still passing the original buffer up to
higher layers most of the time? Or is it really so bad to keep
re-allocating RTK_PCI_RX_BUF_SIZE (>8KB) of contiguous memory, to
re-fill the RX ring? And if that is so bad, can we reduce the
requirement for contiguous memory instead? (e.g., keep with smaller
buffers, and perform aggregation / scatter-gather only for frames that
are really larger?)

Anyway, that's mostly a long-term thought, as this patch is good for
fixing the important memory errors, even if it's not necessarily the
ideal solution.

Regards,
Brian

^ permalink raw reply

* [PATCH v2] wimax/i2400m: fix a memory leak bug
From: Wenwen Wang @ 2019-08-15 20:29 UTC (permalink / raw)
  To: Wenwen Wang
  Cc: Inaky Perez-Gonzalez,
	supporter:INTEL WIRELESS WIMAX CONNECTION 2400, David S. Miller,
	open list:NETWORKING DRIVERS, open list

In i2400m_barker_db_init(), 'options_orig' is allocated through kstrdup()
to hold the original command line options. Then, the options are parsed.
However, if an error occurs during the parsing process, 'options_orig' is
not deallocated, leading to a memory leak bug. To fix this issue, free
'options_orig' before returning the error.

Signed-off-by: Wenwen Wang <wenwen@cs.uga.edu>
---
 drivers/net/wimax/i2400m/fw.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wimax/i2400m/fw.c b/drivers/net/wimax/i2400m/fw.c
index e9fc168..489cba9 100644
--- a/drivers/net/wimax/i2400m/fw.c
+++ b/drivers/net/wimax/i2400m/fw.c
@@ -351,13 +351,15 @@ int i2400m_barker_db_init(const char *_options)
 			}
 			result = i2400m_barker_db_add(barker);
 			if (result < 0)
-				goto error_add;
+				goto error_parse_add;
 		}
 		kfree(options_orig);
 	}
 	return 0;
 
+error_parse_add:
 error_parse:
+	kfree(options_orig);
 error_add:
 	kfree(i2400m_barker_db);
 	return result;
-- 
2.7.4


^ permalink raw reply related

* Re: [PATCH] net: pch_gbe: Fix memory leaks
From: David Miller @ 2019-08-15 20:42 UTC (permalink / raw)
  To: wenwen
  Cc: rfontana, allison, alexios.zavras, gregkh, tglx, netdev,
	linux-kernel
In-Reply-To: <CAAa=b7ft-crBJm+H9U7Bn2dcgfjQsE8o53p2ryBWK3seQoF3Cg@mail.gmail.com>

From: Wenwen Wang <wenwen@cs.uga.edu>
Date: Thu, 15 Aug 2019 16:03:39 -0400

> On Thu, Aug 15, 2019 at 3:34 PM David Miller <davem@davemloft.net> wrote:
>>
>> From: Wenwen Wang <wenwen@cs.uga.edu>
>> Date: Tue, 13 Aug 2019 20:33:45 -0500
>>
>> > In pch_gbe_set_ringparam(), if netif_running() returns false, 'tx_old' and
>> > 'rx_old' are not deallocated, leading to memory leaks. To fix this issue,
>> > move the free statements after the if branch.
>> >
>> > Signed-off-by: Wenwen Wang <wenwen@cs.uga.edu>
>>
>> Why would they be "deallocated"?  They are still assigned to
>> adapter->tx_ring and adapter->rx_ring.
> 
> 'adapter->tx_ring' and 'adapter->rx_ring' has been covered by newly
> allocated 'txdr' and 'rxdr' respectively before this if statement.

That only happens inside of the if() statement, that's why rx_old and
tx_old are only freed in that code path.

^ permalink raw reply

* Re: [PATCH] net: pch_gbe: Fix memory leaks
From: Wenwen Wang @ 2019-08-15 20:46 UTC (permalink / raw)
  To: David Miller
  Cc: Richard Fontana, Allison Randal, Alexios Zavras,
	Greg Kroah-Hartman, Thomas Gleixner,
	open list:NETWORKING [GENERAL], open list, Wenwen Wang
In-Reply-To: <20190815.134230.1028411309377288636.davem@davemloft.net>

On Thu, Aug 15, 2019 at 4:42 PM David Miller <davem@davemloft.net> wrote:
>
> From: Wenwen Wang <wenwen@cs.uga.edu>
> Date: Thu, 15 Aug 2019 16:03:39 -0400
>
> > On Thu, Aug 15, 2019 at 3:34 PM David Miller <davem@davemloft.net> wrote:
> >>
> >> From: Wenwen Wang <wenwen@cs.uga.edu>
> >> Date: Tue, 13 Aug 2019 20:33:45 -0500
> >>
> >> > In pch_gbe_set_ringparam(), if netif_running() returns false, 'tx_old' and
> >> > 'rx_old' are not deallocated, leading to memory leaks. To fix this issue,
> >> > move the free statements after the if branch.
> >> >
> >> > Signed-off-by: Wenwen Wang <wenwen@cs.uga.edu>
> >>
> >> Why would they be "deallocated"?  They are still assigned to
> >> adapter->tx_ring and adapter->rx_ring.
> >
> > 'adapter->tx_ring' and 'adapter->rx_ring' has been covered by newly
> > allocated 'txdr' and 'rxdr' respectively before this if statement.
>
> That only happens inside of the if() statement, that's why rx_old and
> tx_old are only freed in that code path.

That happens not only inside of the if statement, but also before the
if statement, just after 'txdr' and 'rxdr' are allocated.

Wenwen

^ permalink raw reply

* Re: [PATCH] net: pch_gbe: Fix memory leaks
From: David Miller @ 2019-08-15 20:51 UTC (permalink / raw)
  To: wenwen
  Cc: rfontana, allison, alexios.zavras, gregkh, tglx, netdev,
	linux-kernel
In-Reply-To: <CAAa=b7duRXsiVBfzbvHhoU000gGh53Mme3ZKCO5SoiTdgRaXtg@mail.gmail.com>

From: Wenwen Wang <wenwen@cs.uga.edu>
Date: Thu, 15 Aug 2019 16:46:05 -0400

> On Thu, Aug 15, 2019 at 4:42 PM David Miller <davem@davemloft.net> wrote:
>>
>> From: Wenwen Wang <wenwen@cs.uga.edu>
>> Date: Thu, 15 Aug 2019 16:03:39 -0400
>>
>> > On Thu, Aug 15, 2019 at 3:34 PM David Miller <davem@davemloft.net> wrote:
>> >>
>> >> From: Wenwen Wang <wenwen@cs.uga.edu>
>> >> Date: Tue, 13 Aug 2019 20:33:45 -0500
>> >>
>> >> > In pch_gbe_set_ringparam(), if netif_running() returns false, 'tx_old' and
>> >> > 'rx_old' are not deallocated, leading to memory leaks. To fix this issue,
>> >> > move the free statements after the if branch.
>> >> >
>> >> > Signed-off-by: Wenwen Wang <wenwen@cs.uga.edu>
>> >>
>> >> Why would they be "deallocated"?  They are still assigned to
>> >> adapter->tx_ring and adapter->rx_ring.
>> >
>> > 'adapter->tx_ring' and 'adapter->rx_ring' has been covered by newly
>> > allocated 'txdr' and 'rxdr' respectively before this if statement.
>>
>> That only happens inside of the if() statement, that's why rx_old and
>> tx_old are only freed in that code path.
> 
> That happens not only inside of the if statement, but also before the
> if statement, just after 'txdr' and 'rxdr' are allocated.

Then the assignments inside of the if() statement are redundant.

Something doesn't add up here, please make the code consistent.

^ permalink raw reply

* [PATCH bpf 1/1] xdp: unpin xdp umem pages in error path
From: Ivan Khoronzhuk @ 2019-08-15 20:56 UTC (permalink / raw)
  To: bjorn.topel
  Cc: magnus.karlsson, jonathan.lemon, davem, ast, daniel,
	jakub.kicinski, hawk, john.fastabend, netdev, bpf, linux-kernel,
	Ivan Khoronzhuk

Fix mem leak caused by missed unpin routine for umem pages.
Fixes: 8aef7340ae9695 ("commit xsk: introduce xdp_umem_page")

Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
---

Based on bpf/master

 net/xdp/xdp_umem.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/xdp/xdp_umem.c b/net/xdp/xdp_umem.c
index 83de74ca729a..688aac7a6943 100644
--- a/net/xdp/xdp_umem.c
+++ b/net/xdp/xdp_umem.c
@@ -365,7 +365,7 @@ static int xdp_umem_reg(struct xdp_umem *umem, struct xdp_umem_reg *mr)
 	umem->pages = kcalloc(umem->npgs, sizeof(*umem->pages), GFP_KERNEL);
 	if (!umem->pages) {
 		err = -ENOMEM;
-		goto out_account;
+		goto out_pin;
 	}
 
 	for (i = 0; i < umem->npgs; i++)
@@ -373,6 +373,8 @@ static int xdp_umem_reg(struct xdp_umem *umem, struct xdp_umem_reg *mr)
 
 	return 0;
 
+out_pin:
+	xdp_umem_unpin_pages(umem);
 out_account:
 	xdp_umem_unaccount_pages(umem);
 	return err;
-- 
2.17.1


^ permalink raw reply related

* Re: [PATCH net-next] r8152: divide the tx and rx bottom functions
From: David Miller @ 2019-08-15 20:58 UTC (permalink / raw)
  To: hayeswang; +Cc: netdev, nic_swsd, linux-kernel
In-Reply-To: <1394712342-15778-301-Taiwan-albertk@realtek.com>

From: Hayes Wang <hayeswang@realtek.com>
Date: Wed, 14 Aug 2019 16:30:17 +0800

> Move the tx bottom function from NAPI to a new tasklet. Then, for
> multi-cores, the bottom functions of tx and rx may be run at same
> time with different cores. This is used to improve performance.
> 
> Signed-off-by: Hayes Wang <hayeswang@realtek.com>

Theoretically, yes.

But do you have actual performance numbers showing this to be worth
the change?

Always provide performance numbers with changes that are supposed to
improve performance.

^ permalink raw reply

* Re: [PATCH net] net/packet: fix race in tpacket_snd()
From: David Miller @ 2019-08-15 21:00 UTC (permalink / raw)
  To: edumazet; +Cc: netdev, eric.dumazet, syzkaller
In-Reply-To: <20190814091157.215108-1-edumazet@google.com>

From: Eric Dumazet <edumazet@google.com>
Date: Wed, 14 Aug 2019 02:11:57 -0700

> packet_sendmsg() checks tx_ring.pg_vec to decide
> if it must call tpacket_snd().
> 
> Problem is that the check is lockless, meaning another thread
> can issue a concurrent setsockopt(PACKET_TX_RING ) to flip
> tx_ring.pg_vec back to NULL.
> 
> Given that tpacket_snd() grabs pg_vec_lock mutex, we can
> perform the check again to solve the race.
> 
> syzbot reported :
 ...
> Fixes: 69e3c75f4d54 ("net: TX_RING and packet mmap")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: syzbot <syzkaller@googlegroups.com>

Applied and queued up for -stable.

^ permalink raw reply

* Re: [PATCH bpf 1/1] xdp: unpin xdp umem pages in error path
From: Jonathan Lemon @ 2019-08-15 21:01 UTC (permalink / raw)
  To: Ivan Khoronzhuk
  Cc: bjorn.topel, magnus.karlsson, davem, ast, daniel, jakub.kicinski,
	hawk, john.fastabend, netdev, bpf, linux-kernel
In-Reply-To: <20190815205635.6536-1-ivan.khoronzhuk@linaro.org>



On 15 Aug 2019, at 13:56, Ivan Khoronzhuk wrote:

> Fix mem leak caused by missed unpin routine for umem pages.
> Fixes: 8aef7340ae9695 ("commit xsk: introduce xdp_umem_page")
>
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>

Acked-by: Jonathan Lemon <jonathan.lemon@gmail.com>

^ permalink raw reply

* Re: [PATCH 0/7] Netfilter fixes for net
From: David Miller @ 2019-08-15 21:02 UTC (permalink / raw)
  To: pablo; +Cc: netfilter-devel, netdev
In-Reply-To: <20190814092440.20087-1-pablo@netfilter.org>

From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Wed, 14 Aug 2019 11:24:33 +0200

> This patchset contains Netfilter fixes for net:
> 
> 1) Extend selftest to cover flowtable with ipsec, from Florian Westphal.
> 
> 2) Fix interaction of ipsec with flowtable, also from Florian.
> 
> 3) User-after-free with bound set to rule that fails to load.
> 
> 4) Adjust state and timeout for flows that expire.
> 
> 5) Timeout update race with flows in teardown state.
> 
> 6) Ensure conntrack id hash calculation use invariants as input,
>    from Dirk Morris.
> 
> 7) Do not push flows into flowtable for TCP fin/rst packets.
> 
> You can pull these changes from:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf.git

Pulled, thanks.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox