Netdev List
 help / color / mirror / Atom feed
* [PATCH net 2/3] net/af_iucv: fix skb handling on HiperTransport xmit error
From: Julian Wiedmann @ 2018-09-05 14:55 UTC (permalink / raw)
  To: David Miller
  Cc: netdev, linux-s390, Martin Schwidefsky, Heiko Carstens,
	Stefan Raspl, Ursula Braun, Julian Wiedmann
In-Reply-To: <20180905145512.30165-1-jwi@linux.ibm.com>

When sending an skb, afiucv_hs_send() bails out on various error
conditions. But currently the caller has no way of telling whether the
skb was freed or not - resulting in potentially either
a) leaked skbs from iucv_send_ctrl(), or
b) double-free's from iucv_sock_sendmsg().

As dev_queue_xmit() will always consume the skb (even on error), be
consistent and also free the skb from all other error paths. This way
callers no longer need to care about managing the skb.

Signed-off-by: Julian Wiedmann <jwi@linux.ibm.com>
Reviewed-by: Ursula Braun <ubraun@linux.ibm.com>
---
 net/iucv/af_iucv.c | 34 +++++++++++++++++++++++-----------
 1 file changed, 23 insertions(+), 11 deletions(-)

diff --git a/net/iucv/af_iucv.c b/net/iucv/af_iucv.c
index 01000c14417f..e2f16a0173a9 100644
--- a/net/iucv/af_iucv.c
+++ b/net/iucv/af_iucv.c
@@ -351,20 +351,28 @@ static int afiucv_hs_send(struct iucv_message *imsg, struct sock *sock,
 		memcpy(&phs_hdr->iucv_hdr, imsg, sizeof(struct iucv_message));
 
 	skb->dev = iucv->hs_dev;
-	if (!skb->dev)
-		return -ENODEV;
-	if (!(skb->dev->flags & IFF_UP) || !netif_carrier_ok(skb->dev))
-		return -ENETDOWN;
+	if (!skb->dev) {
+		err = -ENODEV;
+		goto err_free;
+	}
+	if (!(skb->dev->flags & IFF_UP) || !netif_carrier_ok(skb->dev)) {
+		err = -ENETDOWN;
+		goto err_free;
+	}
 	if (skb->len > skb->dev->mtu) {
-		if (sock->sk_type == SOCK_SEQPACKET)
-			return -EMSGSIZE;
-		else
-			skb_trim(skb, skb->dev->mtu);
+		if (sock->sk_type == SOCK_SEQPACKET) {
+			err = -EMSGSIZE;
+			goto err_free;
+		}
+		skb_trim(skb, skb->dev->mtu);
 	}
 	skb->protocol = cpu_to_be16(ETH_P_AF_IUCV);
 	nskb = skb_clone(skb, GFP_ATOMIC);
-	if (!nskb)
-		return -ENOMEM;
+	if (!nskb) {
+		err = -ENOMEM;
+		goto err_free;
+	}
+
 	skb_queue_tail(&iucv->send_skb_q, nskb);
 	err = dev_queue_xmit(skb);
 	if (net_xmit_eval(err)) {
@@ -375,6 +383,10 @@ static int afiucv_hs_send(struct iucv_message *imsg, struct sock *sock,
 		WARN_ON(atomic_read(&iucv->msg_recv) < 0);
 	}
 	return net_xmit_eval(err);
+
+err_free:
+	kfree_skb(skb);
+	return err;
 }
 
 static struct sock *__iucv_get_sock_by_name(char *nm)
@@ -1167,7 +1179,7 @@ static int iucv_sock_sendmsg(struct socket *sock, struct msghdr *msg,
 		err = afiucv_hs_send(&txmsg, sk, skb, 0);
 		if (err) {
 			atomic_dec(&iucv->msg_sent);
-			goto fail;
+			goto out;
 		}
 	} else { /* Classic VM IUCV transport */
 		skb_queue_tail(&iucv->send_skb_q, skb);
-- 
2.16.4

^ permalink raw reply related

* [PATCH net 1/3] net/af_iucv: drop inbound packets with invalid flags
From: Julian Wiedmann @ 2018-09-05 14:55 UTC (permalink / raw)
  To: David Miller
  Cc: netdev, linux-s390, Martin Schwidefsky, Heiko Carstens,
	Stefan Raspl, Ursula Braun, Julian Wiedmann
In-Reply-To: <20180905145512.30165-1-jwi@linux.ibm.com>

Inbound packets may have any combination of flag bits set in their iucv
header. If we don't know how to handle a specific combination, drop the
skb instead of leaking it.

To clarify what error is returned in this case, replace the hard-coded
0 with the corresponding macro.

Signed-off-by: Julian Wiedmann <jwi@linux.ibm.com>
---
 net/iucv/af_iucv.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/iucv/af_iucv.c b/net/iucv/af_iucv.c
index a21d8ed0a325..01000c14417f 100644
--- a/net/iucv/af_iucv.c
+++ b/net/iucv/af_iucv.c
@@ -2155,8 +2155,8 @@ static int afiucv_hs_rcv(struct sk_buff *skb, struct net_device *dev,
 	struct sock *sk;
 	struct iucv_sock *iucv;
 	struct af_iucv_trans_hdr *trans_hdr;
+	int err = NET_RX_SUCCESS;
 	char nullstring[8];
-	int err = 0;
 
 	if (skb->len < (ETH_HLEN + sizeof(struct af_iucv_trans_hdr))) {
 		WARN_ONCE(1, "AF_IUCV too short skb, len=%d, min=%d",
@@ -2254,7 +2254,7 @@ static int afiucv_hs_rcv(struct sk_buff *skb, struct net_device *dev,
 		err = afiucv_hs_callback_rx(sk, skb);
 		break;
 	default:
-		;
+		kfree_skb(skb);
 	}
 
 	return err;
-- 
2.16.4

^ permalink raw reply related

* [PATCH net 0/3] net/iucv: fixes 2018-09-05
From: Julian Wiedmann @ 2018-09-05 14:55 UTC (permalink / raw)
  To: David Miller
  Cc: netdev, linux-s390, Martin Schwidefsky, Heiko Carstens,
	Stefan Raspl, Ursula Braun, Julian Wiedmann

Hi Dave,

please apply three straight-forward fixes for iucv. One that prevents
leaking the skb on malformed inbound packets, one to fix the error
handling on transmit error, and one to get rid of a compile warning.

Thanks,
Julian


Julian Wiedmann (3):
  net/af_iucv: drop inbound packets with invalid flags
  net/af_iucv: fix skb handling on HiperTransport xmit error
  net/iucv: declare iucv_path_table_empty() as static

 net/iucv/af_iucv.c | 38 +++++++++++++++++++++++++-------------
 net/iucv/iucv.c    |  2 +-
 2 files changed, 26 insertions(+), 14 deletions(-)

-- 
2.16.4

^ permalink raw reply

* Re: pull-request: mac80211-next 2018-09-05
From: David Miller @ 2018-09-05 14:49 UTC (permalink / raw)
  To: johannes; +Cc: netdev, linux-wireless
In-Reply-To: <20180905090542.21790-1-johannes@sipsolutions.net>

From: Johannes Berg <johannes@sipsolutions.net>
Date: Wed,  5 Sep 2018 11:05:41 +0200

> Here's a pull request for net-next. Thanks for including
> net, that way I could include Stanislaw's patch to check
> the regulatory WMM data.
> 
> Please pull and let me know if there's any problem.

Pulled, thanks!

^ permalink raw reply

* Re: [PATCH net 2/3] tls: clear key material from kernel memory when do_tls_setsockopt_conf fails
From: Boris Pismenny @ 2018-09-05 13:53 UTC (permalink / raw)
  To: Sabrina Dubroca, netdev; +Cc: Ilya Lesokhin, Aviad Yehezkel, Dave Watson
In-Reply-To: <695fc3449c2ba927e97abd48b452a31ec73d6c61.1536152698.git.sd@queasysnail.net>

Hi Sabrina,

On 9/5/2018 4:21 PM, Sabrina Dubroca wrote:
> Fixes: 3c4d7559159b ("tls: kernel TLS support")
> Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
> ---
>   net/tls/tls_main.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c
> index 180b6640e531..0d432d025471 100644
> --- a/net/tls/tls_main.c
> +++ b/net/tls/tls_main.c
> @@ -499,7 +499,7 @@ static int do_tls_setsockopt_conf(struct sock *sk, char __user *optval,
>   	goto out;
>   
>   err_crypto_info:
> -	memset(crypto_info, 0, sizeof(*crypto_info));
> +	memzero_explicit(crypto_info, sizeof(struct tls12_crypto_info_aes_gcm_128));

Besides the key, there are other (not secret) information in 
tls12_crypto_info_aes_gcm_128. I'd prefer you do not delete it to enable 
users to obtain it (using getsockopt) in case we decide to implement a 
fallback to userspace in the future. Such a fallback must obtain the 
kernel's iv, and record sequence number.

Thanks,
Boris.

^ permalink raw reply

* Re: [PATCH v3 3/3] IB/ipoib: Log sysfs 'dev_id' accesses from userspace
From: Leon Romanovsky @ 2018-09-05 13:50 UTC (permalink / raw)
  To: Arseny Maslennikov; +Cc: linux-rdma, Doug Ledford, Jason Gunthorpe, netdev
In-Reply-To: <20180903161316.25121-4-ar@cs.msu.ru>

[-- Attachment #1: Type: text/plain, Size: 2324 bytes --]

On Mon, Sep 03, 2018 at 07:13:16PM +0300, Arseny Maslennikov wrote:
> Signed-off-by: Arseny Maslennikov <ar@cs.msu.ru>
> ---
>  drivers/infiniband/ulp/ipoib/ipoib_main.c | 38 +++++++++++++++++++++++
>  1 file changed, 38 insertions(+)
>
> diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c b/drivers/infiniband/ulp/ipoib/ipoib_main.c
> index 30f840f874b3..7386e5bde3d3 100644
> --- a/drivers/infiniband/ulp/ipoib/ipoib_main.c
> +++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
> @@ -2386,6 +2386,42 @@ int ipoib_add_pkey_attr(struct net_device *dev)
>  	return device_create_file(&dev->dev, &dev_attr_pkey);
>  }
>
> +/*
> + * We erroneously exposed the iface's port number in the dev_id
> + * sysfs field long after dev_port was introduced for that purpose[1],
> + * and we need to stop everyone from relying on that.
> + * Let's overload the shower routine for the dev_id file here
> + * to gently bring the issue up.
> + *
> + * [1] https://www.spinics.net/lists/netdev/msg272123.html
> + */
> +static ssize_t dev_id_show(struct device *dev,
> +			   struct device_attribute *attr, char *buf)
> +{
> +	struct net_device *ndev = to_net_dev(dev);
> +	ssize_t ret = -EINVAL;
> +
> +	if (ndev->dev_id == ndev->dev_port) {
> +		netdev_info_once(ndev,
> +			"\"%s\" wants to know my dev_id. "
> +			"Should it look at dev_port instead?\n",
> +			current->comm);
> +		netdev_info_once(ndev,
> +			"See Documentation/ABI/testing/sysfs-class-net for more info.\n");
> +	}
> +
> +	ret = sprintf(buf, "%#x\n", ndev->dev_id);
> +
> +	return ret;
> +}
> +static DEVICE_ATTR_RO(dev_id);
> +

I don't see this field among exposed by IPoIB, why should we expose it now?

> +int ipoib_intercept_dev_id_attr(struct net_device *dev)
> +{
> +	device_remove_file(&dev->dev, &dev_attr_dev_id);
> +	return device_create_file(&dev->dev, &dev_attr_dev_id);

Why isn't enough to rely on netdev code?

> +}
> +
>  static struct net_device *ipoib_add_port(const char *format,
>  					 struct ib_device *hca, u8 port)
>  {
> @@ -2427,6 +2463,8 @@ static struct net_device *ipoib_add_port(const char *format,
>  	 */
>  	ndev->priv_destructor = ipoib_intf_free;
>
> +	if (ipoib_intercept_dev_id_attr(ndev))
> +		goto sysfs_failed;
>  	if (ipoib_cm_add_mode_attr(ndev))
>  		goto sysfs_failed;
>  	if (ipoib_add_pkey_attr(ndev))
> --
> 2.19.0.rc1
>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply

* RE: [PATCH net 3/3] tls: zero the crypto information from tls_context before freeing
From: Vakul Garg @ 2018-09-05 13:48 UTC (permalink / raw)
  To: Sabrina Dubroca, netdev@vger.kernel.org
  Cc: Boris Pismenny, Ilya Lesokhin, Aviad Yehezkel, Dave Watson
In-Reply-To: <c2b629a06486cd9c17a42cf2efe11ad26d8f9ef6.1536152698.git.sd@queasysnail.net>



> -----Original Message-----
> From: netdev-owner@vger.kernel.org <netdev-owner@vger.kernel.org> On
> Behalf Of Sabrina Dubroca
> Sent: Wednesday, September 5, 2018 6:52 PM
> To: netdev@vger.kernel.org
> Cc: Sabrina Dubroca <sd@queasysnail.net>; Boris Pismenny
> <borisp@mellanox.com>; Ilya Lesokhin <ilyal@mellanox.com>; Aviad
> Yehezkel <aviadye@mellanox.com>; Dave Watson <davejwatson@fb.com>
> Subject: [PATCH net 3/3] tls: zero the crypto information from tls_context
> before freeing
> 
> This contains key material in crypto_send_aes_gcm_128 and
> crypto_recv_aes_gcm_128.
> 
> Fixes: 3c4d7559159b ("tls: kernel TLS support")
> Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
> ---
>  include/net/tls.h  |  1 +
>  net/tls/tls_main.c | 14 ++++++++++++--
>  2 files changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/include/net/tls.h b/include/net/tls.h index
> d5c683e8bb22..2010d23112f9 100644
> --- a/include/net/tls.h
> +++ b/include/net/tls.h
> @@ -180,6 +180,7 @@ struct tls_context {
>  		struct tls_crypto_info crypto_recv;
>  		struct tls12_crypto_info_aes_gcm_128
> crypto_recv_aes_gcm_128;
>  	};
> +	char tls_crypto_ctx_end[0];

This makes the key zeroization dependent upon the position of unions in structure.
Can you zeroise the crypto_send, crypto_recv separately using two memzero_explicit calls? 

> 
>  	struct list_head list;
>  	struct net_device *netdev;
> diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c index
> 0d432d025471..d3a57c0b2182 100644
> --- a/net/tls/tls_main.c
> +++ b/net/tls/tls_main.c
> @@ -241,6 +241,16 @@ static void tls_write_space(struct sock *sk)
>  	ctx->sk_write_space(sk);
>  }
> 
> +static void tls_ctx_free(struct tls_context *ctx) {
> +	if (!ctx)
> +		return;
> +
> +	memzero_explicit(&ctx->crypto_send,
> +			 offsetof(struct tls_context, tls_crypto_ctx_end));
> +	kfree(ctx);
> +}
> +
>  static void tls_sk_proto_close(struct sock *sk, long timeout)  {
>  	struct tls_context *ctx = tls_get_ctx(sk); @@ -294,7 +304,7 @@
> static void tls_sk_proto_close(struct sock *sk, long timeout)  #else
>  	{
>  #endif
> -		kfree(ctx);
> +		tls_ctx_free(ctx);
>  		ctx = NULL;
>  	}
> 
> @@ -305,7 +315,7 @@ static void tls_sk_proto_close(struct sock *sk, long
> timeout)
>  	 * for sk->sk_prot->unhash [tls_hw_unhash]
>  	 */
>  	if (free_ctx)
> -		kfree(ctx);
> +		tls_ctx_free(ctx);
>  }
> 
>  static int do_tls_getsockopt_tx(struct sock *sk, char __user *optval,
> --
> 2.18.0

^ permalink raw reply

* Re: [PATCH bpf-next] bpf/verifier: properly clear union members after a ctx read
From: Edward Cree @ 2018-09-05 13:47 UTC (permalink / raw)
  To: Alexei Starovoitov; +Cc: daniel, ast, netdev
In-Reply-To: <20180905022323.6lkmq2kmv5ejwy3c@ast-mbp.dhcp.thefacebook.com>

On 05/09/18 03:23, Alexei Starovoitov wrote:
> So would you agree it's fair to add
> Fixes: f1174f77b50c ("bpf/verifier: rework value tracking")
> ?
Sure.  Though I don't think it needs backporting, as it's a conservative
 bug (i.e. it merely prevents pruning, but that's safe security-wise).
> How about patch like the following:
> ------------
> From 422fd975ed78645ab67d2eb50ff6e1ff6fb3de32 Mon Sep 17 00:00:00 2001
> From: Alexei Starovoitov <ast@kernel.org>
> Date: Tue, 4 Sep 2018 19:13:44 -0700
> Subject: [PATCH] bpf/verifier: fix verifier instability
>
> Fixes: f1174f77b50c ("bpf/verifier: rework value tracking")
> Debugged-by: Edward Cree <ecree@solarflare.com> 
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Acked-by: Edward Cree <ecree@solarflare.com>

^ permalink raw reply

* Re: [PATCH v3 2/3] IB/ipoib: Use dev_port to expose network interface port numbers
From: Leon Romanovsky @ 2018-09-05 13:41 UTC (permalink / raw)
  To: Arseny Maslennikov; +Cc: linux-rdma, Doug Ledford, Jason Gunthorpe, netdev
In-Reply-To: <20180903161316.25121-3-ar@cs.msu.ru>

[-- Attachment #1: Type: text/plain, Size: 1113 bytes --]

On Mon, Sep 03, 2018 at 07:13:15PM +0300, Arseny Maslennikov wrote:
> Some InfiniBand network devices have multiple ports on the same PCI
> function. This initializes the `dev_port' sysfs field of those
> network interfaces with their port number.
>
> Prior to this the kernel erroneously used the `dev_id' sysfs
> field of those network interfaces to convey the port number to userspace.
>
> The use of `dev_id' was considered correct until Linux 3.15,
> when another field, `dev_port', was defined for this particular
> purpose and `dev_id' was reserved for distinguishing stacked ifaces
> (e.g: VLANs) with the same hardware address as their parent device.
>
> Similar fixes to net/mlx4_en and many other drivers, which started
> exporting this information through `dev_id' before 3.15, were accepted
> into the kernel 4 years ago.
> See 76a066f2a2a0 (`net/mlx4_en: Expose port number through sysfs').
>
> Signed-off-by: Arseny Maslennikov <ar@cs.msu.ru>
> ---
>  drivers/infiniband/ulp/ipoib/ipoib_main.c | 2 ++
>  1 file changed, 2 insertions(+)
>

Thanks,
Reviewed-by: Leon Romanovsky <leonro@mellanox.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply

* Re: [oss-drivers] Re: [RFC bpf-next PATCH] samples/bpf: xdp1 add XDP hardware offload option
From: Jesper Dangaard Brouer @ 2018-09-05 13:41 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Nick Viljoen, oss-drivers, netdev, John W. Linville, jhsiao,
	Quentin Monnet, brouer
In-Reply-To: <20180904185957.0d344534@cakuba>

On Tue, 4 Sep 2018 18:59:57 +0200
Jakub Kicinski <jakub.kicinski@netronome.com> wrote:

> On Tue, 4 Sep 2018 18:49:33 +0200, Jesper Dangaard Brouer wrote:
> > On Tue, 4 Sep 2018 17:09:12 +0200
> > Jakub Kicinski <jakub.kicinski@netronome.com> wrote:
> >   
> > > On Tue, 04 Sep 2018 16:59:19 +0200, Jesper Dangaard Brouer wrote:    
> > > > Trying to use XDP hardware offloading via XDP_FLAGS_HW_MODE
> > > > and setting the ifindex in prog_load_attr.ifindex before
> > > > loading the BPF code via bpf_prog_load_xattr().
> > > > 
> > > > This unfortunately does not seem to work...
> > > > - Am I doing something wrong?
> > > > 
> > > > Notice, I also disable the map BPF_MAP_TYPE_PERCPU_ARRAY
> > > > to make sure it was not related to the map (not supporting
> > > > offloading).
> > > > 
> > > > Failed with:
> > > >  # ./xdp1 -O $(</sys/class/net/enp130s0np1/ifindex)
> > > >  libbpf: load bpf program failed: Invalid argument
> > > >  libbpf: failed to load program 'xdp1'
> > > >  libbpf: failed to load object './xdp1_kern.o'
> > > > 
> > > > Tested on kernel 4.18.0-2.el8.x86_64 with driver nfp
> > > >  Ethernet controller: Netronome Systems, Inc. Device 4000      
> > > 
> > > Are you running the BPF capable FW?
> > > 
> > > https://help.netronome.com/support/solutions/articles/36000050009-agilio-ebpf-2-0-6-extended-berkeley-packet-filter    
> > 
> > I'm likely not running the correct firmware...
> > 
> > Can you tell me, with the ethtool -i output, if I'm running the
> > appropriate firmware?
> > 
> > # ethtool -i enp129s0np1
> > driver: nfp
> > version: 4.18.0-2.el8.x86_64 SMP mod_unl
> > firmware-version: 0.0.3.5 0.21 nic-2.0.7 nic
> > expansion-rom-version: 
> > bus-info: 0000:81:00.0
> > supports-statistics: yes
> > supports-test: no
> > supports-eeprom-access: no
> > supports-register-dump: yes
> > supports-priv-flags: no  
> 
> Yup, the BPF firmware says bpf in firmware version.

Downloaded: agilio-bpf-firmware-2.0.6.121-1.noarch.rpm

RPM install it:
 rpm -hiv  agilio-bpf-firmware-2.0.6.121-1.noarch.rpm

# rpm -ql agilio-bpf-firmware
/opt/netronome/firmware/agilio-bpf
/opt/netronome/firmware/agilio-bpf/nic_AMDA0058-0011_2x40.nffw
/opt/netronome/firmware/agilio-bpf/nic_AMDA0058-0012_2x40.nffw
/opt/netronome/firmware/agilio-bpf/nic_AMDA0078-0011_1x100.nffw
/opt/netronome/firmware/agilio-bpf/nic_AMDA0081-0001_1x40.nffw
/opt/netronome/firmware/agilio-bpf/nic_AMDA0081-0001_4x10.nffw
/opt/netronome/firmware/agilio-bpf/nic_AMDA0096-0001_2x10.nffw
/opt/netronome/firmware/agilio-bpf/nic_AMDA0097-0001_2x40.nffw
/opt/netronome/firmware/agilio-bpf/nic_AMDA0097-0001_4x10_1x40.nffw
/opt/netronome/firmware/agilio-bpf/nic_AMDA0097-0001_8x10.nffw
/opt/netronome/firmware/agilio-bpf/nic_AMDA0099-0001_1x10_1x25.nffw
/opt/netronome/firmware/agilio-bpf/nic_AMDA0099-0001_2x10.nffw
/opt/netronome/firmware/agilio-bpf/nic_AMDA0099-0001_2x25.nffw

Netronome: Basic Firmware User Guide
 https://help.netronome.com/support/solutions/articles/36000049975-basic-firmware-user-guide

A section says after installing the firmware, unload and reload the
driver kernel module will upgrade the firmware.

 ## reload driver to load new firmware
 rmmod nfp; modprobe nfp

Firmware upgrade worked! :-) This is by-far the easiest firmware
upgrade I've experienced.  And I don't have to reboot the machine :-)

 # ethtool -i enp130s0np1  | grep firmware-version
 firmware-version: 0.0.3.5 0.21 bpf-2.0.6.121 ebpf

Notice the bpf/ebpf strings in firmware-version.

 
> > If this is a firmware version case, then we should really improve the
> > errors we are giving the user, the -EINVAL can be anything.
> > 
> >  "libbpf: load bpf program failed: Invalid argument"  
> 
> That is true.

Same goes for improving the error message, when loading a map type that
offload cannot handle.

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer

^ permalink raw reply

* [PATCH net 3/3] tls: zero the crypto information from tls_context before freeing
From: Sabrina Dubroca @ 2018-09-05 13:21 UTC (permalink / raw)
  To: netdev
  Cc: Sabrina Dubroca, Boris Pismenny, Ilya Lesokhin, Aviad Yehezkel,
	Dave Watson
In-Reply-To: <cover.1536152698.git.sd@queasysnail.net>

This contains key material in crypto_send_aes_gcm_128 and
crypto_recv_aes_gcm_128.

Fixes: 3c4d7559159b ("tls: kernel TLS support")
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
---
 include/net/tls.h  |  1 +
 net/tls/tls_main.c | 14 ++++++++++++--
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/include/net/tls.h b/include/net/tls.h
index d5c683e8bb22..2010d23112f9 100644
--- a/include/net/tls.h
+++ b/include/net/tls.h
@@ -180,6 +180,7 @@ struct tls_context {
 		struct tls_crypto_info crypto_recv;
 		struct tls12_crypto_info_aes_gcm_128 crypto_recv_aes_gcm_128;
 	};
+	char tls_crypto_ctx_end[0];
 
 	struct list_head list;
 	struct net_device *netdev;
diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c
index 0d432d025471..d3a57c0b2182 100644
--- a/net/tls/tls_main.c
+++ b/net/tls/tls_main.c
@@ -241,6 +241,16 @@ static void tls_write_space(struct sock *sk)
 	ctx->sk_write_space(sk);
 }
 
+static void tls_ctx_free(struct tls_context *ctx)
+{
+	if (!ctx)
+		return;
+
+	memzero_explicit(&ctx->crypto_send,
+			 offsetof(struct tls_context, tls_crypto_ctx_end));
+	kfree(ctx);
+}
+
 static void tls_sk_proto_close(struct sock *sk, long timeout)
 {
 	struct tls_context *ctx = tls_get_ctx(sk);
@@ -294,7 +304,7 @@ static void tls_sk_proto_close(struct sock *sk, long timeout)
 #else
 	{
 #endif
-		kfree(ctx);
+		tls_ctx_free(ctx);
 		ctx = NULL;
 	}
 
@@ -305,7 +315,7 @@ static void tls_sk_proto_close(struct sock *sk, long timeout)
 	 * for sk->sk_prot->unhash [tls_hw_unhash]
 	 */
 	if (free_ctx)
-		kfree(ctx);
+		tls_ctx_free(ctx);
 }
 
 static int do_tls_getsockopt_tx(struct sock *sk, char __user *optval,
-- 
2.18.0

^ permalink raw reply related

* [PATCH net 2/3] tls: clear key material from kernel memory when do_tls_setsockopt_conf fails
From: Sabrina Dubroca @ 2018-09-05 13:21 UTC (permalink / raw)
  To: netdev
  Cc: Sabrina Dubroca, Boris Pismenny, Ilya Lesokhin, Aviad Yehezkel,
	Dave Watson
In-Reply-To: <cover.1536152698.git.sd@queasysnail.net>

Fixes: 3c4d7559159b ("tls: kernel TLS support")
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
---
 net/tls/tls_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c
index 180b6640e531..0d432d025471 100644
--- a/net/tls/tls_main.c
+++ b/net/tls/tls_main.c
@@ -499,7 +499,7 @@ static int do_tls_setsockopt_conf(struct sock *sk, char __user *optval,
 	goto out;
 
 err_crypto_info:
-	memset(crypto_info, 0, sizeof(*crypto_info));
+	memzero_explicit(crypto_info, sizeof(struct tls12_crypto_info_aes_gcm_128));
 out:
 	return rc;
 }
-- 
2.18.0

^ permalink raw reply related

* [PATCH net 1/3] tls: don't copy the key out of tls12_crypto_info_aes_gcm_128
From: Sabrina Dubroca @ 2018-09-05 13:21 UTC (permalink / raw)
  To: netdev
  Cc: Sabrina Dubroca, Boris Pismenny, Ilya Lesokhin, Aviad Yehezkel,
	Dave Watson
In-Reply-To: <cover.1536152698.git.sd@queasysnail.net>

There's no need to copy the key to an on-stack buffer before calling
crypto_aead_setkey().

Fixes: 3c4d7559159b ("tls: kernel TLS support")
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
---
 net/tls/tls_sw.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index 52fbe727d7c1..268053b04563 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -1130,7 +1130,6 @@ void tls_sw_free_resources_rx(struct sock *sk)
 
 int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx, int tx)
 {
-	char keyval[TLS_CIPHER_AES_GCM_128_KEY_SIZE];
 	struct tls_crypto_info *crypto_info;
 	struct tls12_crypto_info_aes_gcm_128 *gcm_128_info;
 	struct tls_sw_context_tx *sw_ctx_tx = NULL;
@@ -1259,9 +1258,7 @@ int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx, int tx)
 
 	ctx->push_pending_record = tls_sw_push_pending_record;
 
-	memcpy(keyval, gcm_128_info->key, TLS_CIPHER_AES_GCM_128_KEY_SIZE);
-
-	rc = crypto_aead_setkey(*aead, keyval,
+	rc = crypto_aead_setkey(*aead, gcm_128_info->key,
 				TLS_CIPHER_AES_GCM_128_KEY_SIZE);
 	if (rc)
 		goto free_aead;
-- 
2.18.0

^ permalink raw reply related

* [PATCH net 0/3] tls: don't leave keys in kernel memory
From: Sabrina Dubroca @ 2018-09-05 13:21 UTC (permalink / raw)
  To: netdev
  Cc: Sabrina Dubroca, Aviad Yehezkel, Boris Pismenny, Dave Watson,
	Ilya Lesokhin

There are a few places where the RX/TX key for a TLS socket is copied
to kernel memory. This series clears those memory areas when they're no
longer needed.

Sabrina Dubroca (3):
  tls: don't copy the key out of tls12_crypto_info_aes_gcm_128
  tls: clear key material from kernel memory when do_tls_setsockopt_conf
    fails
  tls: zero the crypto information from tls_context before freeing

 include/net/tls.h  |  1 +
 net/tls/tls_main.c | 16 +++++++++++++---
 net/tls/tls_sw.c   |  5 +----
 3 files changed, 15 insertions(+), 7 deletions(-)

-- 
2.18.0

^ permalink raw reply

* Re: [RFC PATCH net-next V2 0/6] XDP rx handler
From: David Ahern @ 2018-09-05 17:20 UTC (permalink / raw)
  To: Jason Wang, Jesper Dangaard Brouer
  Cc: Alexei Starovoitov, netdev, linux-kernel, ast, daniel, mst
In-Reply-To: <9a1d9340-8fd0-4e27-0938-adf361fe6939@redhat.com>

[ sorry for the delay; focused on the nexthop RFC ]

On 8/20/18 12:34 AM, Jason Wang wrote:
> 
> 
> On 2018年08月18日 05:15, David Ahern wrote:
>> On 8/15/18 9:34 PM, Jason Wang wrote:
>>> I may miss something but BPF forbids loop. Without a loop how can we
>>> make sure all stacked devices is enumerated correctly without knowing
>>> the topology in advance?
>> netdev_for_each_upper_dev_rcu
>>
>> BPF helpers allow programs to do lookups in kernel tables, in this case
>> the ability to find an upper device that would receive the packet.
> 
> So if I understand correctly, you mean using
> netdev_for_each_upper_dev_rcu() inside a BPF helper? If yes, I think we
> may still need device specific logic. E.g for macvlan,
> netdev_for_each_upper_dev_rcu() enumerates all macvlan devices on top a
> lower device. But what we need is one of the macvlan that matches the
> dst mac address which is similar to what XDP rx handler did. And it
> would become more complicated if we have multiple layers of device.

My device lookup helper takes the base port index (starting device),
vlan protocol, vlan tag and dest mac. So, yes, the mac address is used
to uniquely identify the stacked device.

> 
> So let's consider a simple case, consider we have 5 macvlan devices:
> 
> macvlan0: doing some packet filtering before passing packets to TCP/IP
> stack
> macvlan1: modify packets and redirect to another interface
> macvlan2: modify packets and transmit packet back through XDP_TX
> macvlan3: deliver packets to AF_XDP
> macvtap0: deliver packets raw XDP to VM
> 
> So, with XDP rx handler, what we need to just to attach five different
> XDP programs to each macvlan device. Your idea is to do all things in
> the root device XDP program. This looks complicated and not flexible
> since it needs to care a lot of things, e.g adding/removing
> actions/policies. And XDP program needs to call BPF helper that use
> netdev_for_each_upper_dev_rcu() to work correctly with stacked device.
> 

Stacking on top of a nic port can have all kinds of combinations of
vlans, bonds, bridges, vlans on bonds and bridges, macvlans, etc. I
suspect trying to install a program for layer 3 forwarding on each one
and iteratively running the programs would kill the performance gained
from forwarding with xdp.

^ permalink raw reply

* RE: [PATCH v2 2/2] net: ethernet: i40evf: fix potential build error
From: Keller, Jacob E @ 2018-09-05 16:52 UTC (permalink / raw)
  To: Wang Dongsheng, Kirsher, Jeffrey T,
	sergei.shtylyov@cogentembedded.com
  Cc: davem@davemloft.net, intel-wired-lan@lists.osuosl.org,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <1536114430-21356-2-git-send-email-dongsheng.wang@hxt-semitech.com>



> -----Original Message-----
> From: Wang Dongsheng [mailto:dongsheng.wang@hxt-semitech.com]
> Sent: Tuesday, September 04, 2018 7:27 PM
> To: Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com>;
> sergei.shtylyov@cogentembedded.com
> Cc: Keller, Jacob E <jacob.e.keller@intel.com>; davem@davemloft.net; intel-
> wired-lan@lists.osuosl.org; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org; Wang Dongsheng <dongsheng.wang@hxt-
> semitech.com>
> Subject: [PATCH v2 2/2] net: ethernet: i40evf: fix potential build error
> 
> Can't have non-inline function in a header file.
> There is a risk of "Multiple definition" from cross-including.
> 
> Signed-off-by: Wang Dongsheng <dongsheng.wang@hxt-semitech.com>

Acked-by: Jacob Keller <jacob.e.keller@intel.com>

> ---
>  .../intel/i40evf/i40e_ethtool_stats.h         | 25 ++-----------------
>  .../ethernet/intel/i40evf/i40evf_ethtool.c    | 24 ++++++++++++++++++
>  2 files changed, 26 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/i40evf/i40e_ethtool_stats.h
> b/drivers/net/ethernet/intel/i40evf/i40e_ethtool_stats.h
> index 60b595dd8c39..d70a071f065f 100644
> --- a/drivers/net/ethernet/intel/i40evf/i40e_ethtool_stats.h
> +++ b/drivers/net/ethernet/intel/i40evf/i40e_ethtool_stats.h
> @@ -181,29 +181,8 @@ i40evf_add_queue_stats(u64 **data, struct i40e_ring
> *ring)
>  	*data += size;
>  }
> 
> -/**
> - * __i40e_add_stat_strings - copy stat strings into ethtool buffer
> - * @p: ethtool supplied buffer
> - * @stats: stat definitions array
> - * @size: size of the stats array
> - *
> - * Format and copy the strings described by stats into the buffer pointed at
> - * by p.
> - **/
> -static void __i40e_add_stat_strings(u8 **p, const struct i40e_stats stats[],
> -				    const unsigned int size, ...)
> -{
> -	unsigned int i;
> -
> -	for (i = 0; i < size; i++) {
> -		va_list args;
> -
> -		va_start(args, size);
> -		vsnprintf(*p, ETH_GSTRING_LEN, stats[i].stat_string, args);
> -		*p += ETH_GSTRING_LEN;
> -		va_end(args);
> -	}
> -}
> +void __i40e_add_stat_strings(u8 **p, const struct i40e_stats stats[],
> +			     const unsigned int size, ...);
> 
>  /**
>   * 40e_add_stat_strings - copy stat strings into ethtool buffer
> diff --git a/drivers/net/ethernet/intel/i40evf/i40evf_ethtool.c
> b/drivers/net/ethernet/intel/i40evf/i40evf_ethtool.c
> index 9319971c5c92..c9a54f6de61e 100644
> --- a/drivers/net/ethernet/intel/i40evf/i40evf_ethtool.c
> +++ b/drivers/net/ethernet/intel/i40evf/i40evf_ethtool.c
> @@ -171,6 +171,30 @@ static void i40evf_get_priv_flag_strings(struct
> net_device *netdev, u8 *data)
>  	}
>  }
> 
> +/**
> + * __i40e_add_stat_strings - copy stat strings into ethtool buffer
> + * @p: ethtool supplied buffer
> + * @stats: stat definitions array
> + * @size: size of the stats array
> + *
> + * Format and copy the strings described by stats into the buffer pointed at
> + * by p.
> + **/
> +void __i40e_add_stat_strings(u8 **p, const struct i40e_stats stats[],
> +			     const unsigned int size, ...)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < size; i++) {
> +		va_list args;
> +
> +		va_start(args, size);
> +		vsnprintf(*p, ETH_GSTRING_LEN, stats[i].stat_string, args);
> +		*p += ETH_GSTRING_LEN;
> +		va_end(args);
> +	}
> +}
> +
>  /**
>   * i40evf_get_stat_strings - Get stat strings
>   * @netdev: network interface device structure
> --
> 2.18.0

^ permalink raw reply

* RE: [PATCH v2 1/2] net: ethernet: i40e: fix build error
From: Keller, Jacob E @ 2018-09-05 16:52 UTC (permalink / raw)
  To: Wang Dongsheng, Kirsher, Jeffrey T,
	sergei.shtylyov@cogentembedded.com
  Cc: davem@davemloft.net, intel-wired-lan@lists.osuosl.org,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <1536114430-21356-1-git-send-email-dongsheng.wang@hxt-semitech.com>



> -----Original Message-----
> From: Wang Dongsheng [mailto:dongsheng.wang@hxt-semitech.com]
> Sent: Tuesday, September 04, 2018 7:27 PM
> To: Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com>;
> sergei.shtylyov@cogentembedded.com
> Cc: Keller, Jacob E <jacob.e.keller@intel.com>; davem@davemloft.net; intel-
> wired-lan@lists.osuosl.org; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org; Wang Dongsheng <dongsheng.wang@hxt-
> semitech.com>
> Subject: [PATCH v2 1/2] net: ethernet: i40e: fix build error
> 
> Remove "inline" from __i40e_add_stat_strings.
> 
> In file included from
> drivers/net/ethernet/intel/i40e/i40e_ethtool.c:9:0:
> drivers/net/ethernet/intel/i40e/i40e_ethtool.c: In function
> ‘__i40e_add_stat_strings’:
> drivers/net/ethernet/intel/i40e/i40e_ethtool_stats.h:193:20: error:
> function ‘__i40e_add_stat_strings’ can never be inlined because it uses
> variable argument lists
>  static inline void __i40e_add_stat_strings(u8 **p, const struct
> 					    i40e_stats stats[],
> 
> Signed-off-by: Wang Dongsheng <dongsheng.wang@hxt-semitech.com>

Thanks for the fix.

A bit off topic, but these two files in the i40e and i40evf share some code. Is there a good mechanism for sharing these between the two drivers that would allow the modules to be independent? That would be ideal.

Acked-by: Jacob Keller <jacob.e.keller@intel.com>

> ---
> v2:
> 1. Move function.
> 2. Include a new patch at [2/2].
> 
> ---
>  .../net/ethernet/intel/i40e/i40e_ethtool.c    | 24 ++++++++++++++++++
>  .../ethernet/intel/i40e/i40e_ethtool_stats.h  | 25 ++-----------------
>  2 files changed, 26 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> index d7d3974beca2..f4a70d67a80a 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> @@ -1821,6 +1821,30 @@ static void i40e_get_ethtool_stats(struct net_device
> *netdev,
>  		  "ethtool stats count mismatch!");
>  }
> 
> +/**
> + * __i40e_add_stat_strings - copy stat strings into ethtool buffer
> + * @p: ethtool supplied buffer
> + * @stats: stat definitions array
> + * @size: size of the stats array
> + *
> + * Format and copy the strings described by stats into the buffer pointed at
> + * by p.
> + **/
> +void __i40e_add_stat_strings(u8 **p, const struct i40e_stats stats[],
> +			     const unsigned int size, ...)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < size; i++) {
> +		va_list args;
> +
> +		va_start(args, size);
> +		vsnprintf(*p, ETH_GSTRING_LEN, stats[i].stat_string, args);
> +		*p += ETH_GSTRING_LEN;
> +		va_end(args);
> +	}
> +}
> +
>  /**
>   * i40e_get_stat_strings - copy stat strings into supplied buffer
>   * @netdev: the netdev to collect strings for
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool_stats.h
> b/drivers/net/ethernet/intel/i40e/i40e_ethtool_stats.h
> index bba1cb0b658f..0874c352136a 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_ethtool_stats.h
> +++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool_stats.h
> @@ -181,29 +181,8 @@ i40e_add_queue_stats(u64 **data, struct i40e_ring
> *ring)
>  	*data += size;
>  }
> 
> -/**
> - * __i40e_add_stat_strings - copy stat strings into ethtool buffer
> - * @p: ethtool supplied buffer
> - * @stats: stat definitions array
> - * @size: size of the stats array
> - *
> - * Format and copy the strings described by stats into the buffer pointed at
> - * by p.
> - **/
> -static inline void __i40e_add_stat_strings(u8 **p, const struct i40e_stats stats[],
> -				    const unsigned int size, ...)
> -{
> -	unsigned int i;
> -
> -	for (i = 0; i < size; i++) {
> -		va_list args;
> -
> -		va_start(args, size);
> -		vsnprintf(*p, ETH_GSTRING_LEN, stats[i].stat_string, args);
> -		*p += ETH_GSTRING_LEN;
> -		va_end(args);
> -	}
> -}
> +void __i40e_add_stat_strings(u8 **p, const struct i40e_stats stats[],
> +			     const unsigned int size, ...);
> 
>  /**
>   * 40e_add_stat_strings - copy stat strings into ethtool buffer
> --
> 2.18.0


^ permalink raw reply

* [PATCH net-next] bnxt_en: remove set but not used variable 'addr_type'
From: YueHaibing @ 2018-09-05 11:44 UTC (permalink / raw)
  To: Michael Chan, David S. Miller; +Cc: YueHaibing, netdev, kernel-janitors

Fixes gcc '-Wunused-but-set-variable' warning:

drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c: In function 'bnxt_tc_parse_flow':
drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c:186:6: warning:
 variable 'addr_type' set but not used [-Wunused-but-set-variable]

Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c | 15 ---------------
 1 file changed, 15 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c
index 092c817..f4ba9b3 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c
@@ -181,7 +181,6 @@ static int bnxt_tc_parse_flow(struct bnxt *bp,
 			      struct bnxt_tc_flow *flow)
 {
 	struct flow_dissector *dissector = tc_flow_cmd->dissector;
-	u16 addr_type = 0;
 
 	/* KEY_CONTROL and KEY_BASIC are needed for forming a meaningful key */
 	if ((dissector->used_keys & BIT(FLOW_DISSECTOR_KEY_CONTROL)) == 0 ||
@@ -191,13 +190,6 @@ static int bnxt_tc_parse_flow(struct bnxt *bp,
 		return -EOPNOTSUPP;
 	}
 
-	if (dissector_uses_key(dissector, FLOW_DISSECTOR_KEY_CONTROL)) {
-		struct flow_dissector_key_control *key =
-			GET_KEY(tc_flow_cmd, FLOW_DISSECTOR_KEY_CONTROL);
-
-		addr_type = key->addr_type;
-	}
-
 	if (dissector_uses_key(dissector, FLOW_DISSECTOR_KEY_BASIC)) {
 		struct flow_dissector_key_basic *key =
 			GET_KEY(tc_flow_cmd, FLOW_DISSECTOR_KEY_BASIC);
@@ -293,13 +285,6 @@ static int bnxt_tc_parse_flow(struct bnxt *bp,
 		flow->l4_mask.icmp.code = mask->code;
 	}
 
-	if (dissector_uses_key(dissector, FLOW_DISSECTOR_KEY_ENC_CONTROL)) {
-		struct flow_dissector_key_control *key =
-			GET_KEY(tc_flow_cmd, FLOW_DISSECTOR_KEY_ENC_CONTROL);
-
-		addr_type = key->addr_type;
-	}
-
 	if (dissector_uses_key(dissector, FLOW_DISSECTOR_KEY_ENC_IPV4_ADDRS)) {
 		struct flow_dissector_key_ipv4_addrs *key =
 			GET_KEY(tc_flow_cmd, FLOW_DISSECTOR_KEY_ENC_IPV4_ADDRS);

^ permalink raw reply related

* Re: [PATCH net-next] net/mlx5e: Make function mlx5i_grp_sw_update_stats() static
From: Leon Romanovsky @ 2018-09-05 11:25 UTC (permalink / raw)
  To: Wei Yongjun
  Cc: Saeed Mahameed, Jason Gunthorpe, Or Gerlitz, Tariq Toukan,
	Feras Daoud, Alex Vesker, Erez Shitrit, netdev, linux-rdma,
	kernel-janitors
In-Reply-To: <1536146162-132732-1-git-send-email-weiyongjun1@huawei.com>

[-- Attachment #1: Type: text/plain, Size: 487 bytes --]

On Wed, Sep 05, 2018 at 11:16:02AM +0000, Wei Yongjun wrote:
> Fixes the following sparse warning:
>
> drivers/net/ethernet/mellanox/mlx5/core/ipoib/ipoib.c:119:6: warning:
>  symbol 'mlx5i_grp_sw_update_stats' was not declared. Should it be static?
>
> Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
> ---
>  drivers/net/ethernet/mellanox/mlx5/core/ipoib/ipoib.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>

Thanks,
Reviewed-by: Leon Romanovsky <leonro@mellanox.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply

* [PATCH net-next v1] net/tls: Set count of SG entries if sk_alloc_sg returns -ENOSPC
From: Vakul Garg @ 2018-09-05 16:27 UTC (permalink / raw)
  To: netdev; +Cc: borisp, aviadye, davejwatson, davem, doronrk, Vakul Garg

tls_sw_sendmsg() allocates plaintext and encrypted SG entries using
function sk_alloc_sg(). In case the number of SG entries hit
MAX_SKB_FRAGS, sk_alloc_sg() returns -ENOSPC and sets the variable for
current SG index to '0'. This leads to calling of function
tls_push_record() with 'sg_encrypted_num_elem = 0' and later causes
kernel crash. To fix this, set the number of SG elements to the number
of elements in plaintext/encrypted SG arrays in case sk_alloc_sg()
returns -ENOSPC.

Signed-off-by: Vakul Garg <vakul.garg@nxp.com>
---
 net/tls/tls_sw.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index be4f2e990f9f..2dad3dc7be60 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -263,6 +263,9 @@ static int alloc_encrypted_sg(struct sock *sk, int len)
 			 &ctx->sg_encrypted_num_elem,
 			 &ctx->sg_encrypted_size, 0);
 
+	if (rc == -ENOSPC)
+		ctx->sg_encrypted_num_elem = ARRAY_SIZE(ctx->sg_encrypted_data);
+
 	return rc;
 }
 
@@ -276,6 +279,9 @@ static int alloc_plaintext_sg(struct sock *sk, int len)
 			 &ctx->sg_plaintext_num_elem, &ctx->sg_plaintext_size,
 			 tls_ctx->pending_open_record_frags);
 
+	if (rc == -ENOSPC)
+		ctx->sg_plaintext_num_elem = ARRAY_SIZE(ctx->sg_plaintext_data);
+
 	return rc;
 }
 
-- 
2.13.6

^ permalink raw reply related

* [PATCH net-next] net/mlx5e: Make function mlx5i_grp_sw_update_stats() static
From: Wei Yongjun @ 2018-09-05 11:16 UTC (permalink / raw)
  To: Saeed Mahameed, Leon Romanovsky, Jason Gunthorpe, Or Gerlitz,
	Tariq Toukan, Feras Daoud, Alex Vesker, Erez Shitrit
  Cc: Wei Yongjun, netdev, linux-rdma, kernel-janitors

Fixes the following sparse warning:

drivers/net/ethernet/mellanox/mlx5/core/ipoib/ipoib.c:119:6: warning:
 symbol 'mlx5i_grp_sw_update_stats' was not declared. Should it be static?

Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/ipoib/ipoib.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/ipoib/ipoib.c b/drivers/net/ethernet/mellanox/mlx5/core/ipoib/ipoib.c
index 48886b3..3dd9f88 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/ipoib/ipoib.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/ipoib/ipoib.c
@@ -116,7 +116,7 @@ static void mlx5i_cleanup(struct mlx5e_priv *priv)
 	/* Do nothing .. */
 }
 
-void mlx5i_grp_sw_update_stats(struct mlx5e_priv *priv)
+static void mlx5i_grp_sw_update_stats(struct mlx5e_priv *priv)
 {
 	struct mlx5e_sw_stats s = { 0 };
 	int i, j;

^ permalink raw reply related

* 4.18.6 dl_seq_start [xt_hashlimit]  unable to handle kernel NULL pointer dereference at 0000000000000050
From: Sami Farin @ 2018-09-05 10:55 UTC (permalink / raw)
  To: Linux Networking Mailing List; +Cc: Christoph Hellwig

4.17 worked ok, this with 32 GB Ryzen system.

BUG: unable to handle kernel NULL pointer dereference at 0000000000000050
PGD 0 P4D 0 
Oops: 0000 [#1] PREEMPT SMP NOPTI
CPU: 0 PID: 6303 Comm: grep Tainted: G                T 4.18.6+ #16
Hardware name: To Be Filled By O.E.M. To Be Filled By O.E.M./X370 Taichi, BIOS P4.60 03/02/2018
RIP: 0010:dl_seq_start+0x11/0x60 [xt_hashlimit]
Code: ff 5d 41 5c 41 5d 41 5e 41 5f c3 0f 1f 40 00 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 55 48 89 f5 53 48 8b 87 d8 00 00 00 <48> 8b 78 50 e8 36 3b 6f de 48 89 c3 48 8d 78 48 e8 ca e6 0a df 8b 
RSP: 0018:ffffa79e88befde0 EFLAGS: 00010246
RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000
RDX: 0000000000000000 RSI: ffffa79e88befe18 RDI: ffff9a64733417a0
RBP: ffffa79e88befe18 R08: 0000000000000000 R09: 00000000657547bf
R10: ffffffff9f2bf98a R11: ffff9a6470f5a6c0 R12: ffffa79e88befeb0
R13: ffff9a6471879200 R14: ffff9a6471879200 R15: ffff9a64733417a0
FS:  00007f6798784740(0000) GS:ffff9a649ea00000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000050 CR3: 00000007f335c000 CR4: 00000000003406f0
Call Trace:
 seq_read+0xc0/0x470
 proc_reg_read+0x49/0x70
 vfs_read+0x8a/0x140
 ksys_read+0x52/0xc0
 do_syscall_64+0x6f/0x353
 ? trace_hardirqs_off_thunk+0x1a/0x1c
 entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x7f67980eee21
Code: fe ff ff 50 48 8d 3d 46 b6 09 00 e8 f9 04 02 00 66 0f 1f 84 00 00 00 00 00 48 8d 05 c1 3b 2d 00 8b 00 85 c0 75 13 31 c0 0f 05 <48> 3d 00 f0 ff ff 77 57 c3 66 0f 1f 44 00 00 41 54 49 89 d4 55 48 
RSP: 002b:00007ffc314f7a68 EFLAGS: 00000246 ORIG_RAX: 0000000000000000
RAX: ffffffffffffffda RBX: 0000000000008000 RCX: 00007f67980eee21
RDX: 0000000000008000 RSI: 000055f0d317f000 RDI: 0000000000000003
RBP: 0000000000008000 R08: 0000000000000000 R09: 0000000000009008
R10: 0000000000000000 R11: 0000000000000246 R12: 000055f0d317f000
R13: 0000000000000003 R14: 000055f0d317e830 R15: 0000000000000003
Modules linked in: arptable_filter arp_tables nfnetlink_acct ip6table_mangle nf_log_ipv6 xt_hl ip6t_REJECT nf_reject_ipv6 ip6t_rt ip6table_filter ip6_tables ipt_MASQUERADE iptable_nat nf_nat_ipv4 nf_nat iptable_raw xt_mark xt_connmark iptable_mangle nf_log_ipv4 nf_log_common xt_LOG xt_length xt_limit ipt_REJECT nf_reject_ipv4 nf_conntrack_ipv4 nf_defrag_ipv4 xt_connlimit nf_conncount xt_multiport xt_hashlimit xt_owner xt_set xt_conntrack iptable_filter ip_set_bitmap_port ip_set_hash_mac ip_set_hash_net ip_set nf_conntrack_netlink nfnetlink bnep hwmon_vid iwlmvm snd_usb_audio snd_usbmidi_lib snd_hwdep snd_rawmidi mac80211 iwlwifi btusb btrtl kvm_amd btbcm btintel bluetooth kvm cfg80211 ecdh_generic irqbypass sp5100_tco wmi_bmof k10temp i2c_piix4 snd_hda_codec_realtek rfkill snd_hda_codec_ge
 neric
 snd_hda_codec_hdmi snd_hda_intel snd_hda_codec snd_hda_core rtc_cmos acpi_cpufreq binfmt_misc snd_pcm_oss snd_mixer_oss snd_seq snd_seq_device snd_pcm tcp_cubic tcp_westwood br_netfilter bridge stp llc ip_tables scsi_transport_iscsi algif_skcipher af_alg uas usb_storage usbhid mxm_wmi ccp igb xhci_pci xhci_hcd usbcore usb_common wmi button 8021q mrp sunrpc snd_timer snd soundcore fuse tun xt_tcpudp x_tables tcp_bbr nf_conntrack_ipv6 nf_defrag_ipv6 nf_conntrack sch_fq_codel sch_htb sch_pie analog gameport joydev i2c_dev ecryptfs autofs4 amdkfd amd_iommu_v2 [last unloaded: pcspkr]
CR2: 0000000000000050
---[ end trace 0e097a943554aa36 ]---


-- 
Do what you love because life is too short for anything else.
https://samifar.in/

^ permalink raw reply

* Re: [PATCH V3] net: qca_spi: Fix race condition in spi transfers
From: David Miller @ 2018-09-05 15:10 UTC (permalink / raw)
  To: stefan.wahren; +Cc: netdev, linux-kernel
In-Reply-To: <1536153798-16494-1-git-send-email-stefan.wahren@i2se.com>

From: Stefan Wahren <stefan.wahren@i2se.com>
Date: Wed,  5 Sep 2018 15:23:18 +0200

> With performance optimization the spi transfer and messages of basic
> register operations like qcaspi_read_register moved into the private
> driver structure. But they weren't protected against mutual access
> (e.g. between driver kthread and ethtool). So dumping the QCA7000
> registers via ethtool during network traffic could make spi_sync
> hang forever, because the completion in spi_message is overwritten.
> 
> So revert the optimization completely.
> 
> Fixes: 291ab06ecf676 ("net: qualcomm: new Ethernet over SPI driver for QCA700")
> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>

Applied and queued up for -stable, thanks.

^ permalink raw reply

* Re: [PATCH net] be2net: Fix memory leak in be_cmd_get_profile_config()
From: David Miller @ 2018-09-05 15:08 UTC (permalink / raw)
  To: poros
  Cc: netdev, ivecera, sathya.perla, ajit.khaparde,
	sriharsha.basavapatna, somnath.kotur, linux-kernel
In-Reply-To: <20180905123746.7399-1-poros@redhat.com>

From: Petr Oros <poros@redhat.com>
Date: Wed,  5 Sep 2018 14:37:45 +0200

> DMA allocated memory is lost in be_cmd_get_profile_config() when we
> call it with non-NULL port_res parameter.
> 
> Signed-off-by: Petr Oros <poros@redhat.com>

Applied, thank you.

^ permalink raw reply

* Re: [PATCH net-next] net: lan743x_ptp: make function lan743x_ptp_set_sync_ts_insert() static
From: David Miller @ 2018-09-05 15:07 UTC (permalink / raw)
  To: yuehaibing; +Cc: bryan.whitehead, UNGLinuxDriver, linux-kernel, netdev
In-Reply-To: <20180905122437.19756-1-yuehaibing@huawei.com>

From: YueHaibing <yuehaibing@huawei.com>
Date: Wed, 5 Sep 2018 20:24:37 +0800

> Fixes the following sparse warning:
> 
> drivers/net/ethernet/microchip/lan743x_ptp.c:980:6: warning:
>  symbol 'lan743x_ptp_set_sync_ts_insert' was not declared. Should it be static?
> 
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>

Applied.

^ 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