Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH] mac80211: Remove attribute packed from struct 'action'
From: Johannes Berg @ 2019-01-24 18:19 UTC (permalink / raw)
  To: Mathieu Malaterre; +Cc: David S. Miller, linux-wireless, netdev, LKML
In-Reply-To: <CA+7wUszXbRt=j1C1PF11EtnO2ojLpvL8Zs60c7UcEDrqjRMxOw@mail.gmail.com>

On Thu, 2019-01-24 at 19:14 +0100, Mathieu Malaterre wrote:

> > I'm not sure this will work on all platforms, didn't something like
> > alpha pad out u8's to u32 when not requiring packing?
> 
> I was not aware of that.

TBH, I'm not actually sure about that. Pure hearsay. If anyone knows,
I'd like to know too :)

johannes


^ permalink raw reply

* Re: [PATCH] mac80211: Remove attribute packed from struct 'action'
From: Mathieu Malaterre @ 2019-01-24 18:14 UTC (permalink / raw)
  To: Johannes Berg; +Cc: David S. Miller, linux-wireless, netdev, LKML
In-Reply-To: <2b8b377a84883db1a45afb42204d8fb5a7d5ccf9.camel@sipsolutions.net>

On Thu, Jan 24, 2019 at 7:08 PM Johannes Berg <johannes@sipsolutions.net> wrote:
>
> On Thu, 2019-01-24 at 19:05 +0100, Mathieu Malaterre wrote:
> > During refactor in commit 9e478066eae4 ("mac80211: fix MU-MIMO
> > follow-MAC mode") a new struct 'action' was declared with packed
> > attribute as:
> >
> >   struct {
> >           struct ieee80211_hdr_3addr hdr;
> >           u8 category;
> >           u8 action_code;
> >   } __packed action;
> >
> > But since struct 'ieee80211_hdr_3addr' is declared with an aligned
> > keyword as:
> >
> >   struct ieee80211_hdr {
> >       __le16 frame_control;
> >       __le16 duration_id;
> >       u8 addr1[ETH_ALEN];
> >       u8 addr2[ETH_ALEN];
> >       u8 addr3[ETH_ALEN];
> >       __le16 seq_ctrl;
> >       u8 addr4[ETH_ALEN];
> >   } __packed __aligned(2);
> >
> > Solve the ambiguity of placing aligned structure in a packed one by
> > removing the packed attribute from struct. This seems to be the behavior
> > of gcc anyway, since the following is still compiling:
> >
> >   BUILD_BUG_ON(sizeof(action) != IEEE80211_MIN_ACTION_SIZE + 1);
>
> I'm not sure this will work on all platforms, didn't something like
> alpha pad out u8's to u32 when not requiring packing?

I was not aware of that.

> I guess I'd feel better about using __packed __aligned(2) here as well,
> which should solve the warning too?

Indeed, I will re-spin a v2 then.

^ permalink raw reply

* Re: [PATCH bpf-next v2] selftests/bpf: suppress readelf stderr when probing for BTF support
From: Song Liu @ 2019-01-24 18:14 UTC (permalink / raw)
  To: Stanislav Fomichev
  Cc: Networking, David S . Miller, Alexei Starovoitov, Daniel Borkmann
In-Reply-To: <20190124165429.36378-1-sdf@google.com>

On Thu, Jan 24, 2019 at 8:57 AM Stanislav Fomichev <sdf@google.com> wrote:
>
> Before:
> $ make -s -C tools/testing/selftests/bpf
> readelf: Error: Missing knowledge of 32-bit reloc types used in DWARF
> sections of machine number 247
> readelf: Warning: unable to apply unsupported reloc type 10 to section
> .debug_info
> readelf: Warning: unable to apply unsupported reloc type 1 to section
> .debug_info
> readelf: Warning: unable to apply unsupported reloc type 10 to section
> .debug_info
>
> After:
> $ make -s -C tools/testing/selftests/bpf
>
> v2:
> * use llvm-readelf instead of redirecting binutils' readelf stderr to
>   /dev/null
>
> Signed-off-by: Stanislav Fomichev <sdf@google.com>

This looks good.

Acked-by: Song Liu <songliubraving@fb.com>

Thanks!

> ---
>  tools/testing/selftests/bpf/Makefile | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index 70229de510f5..1cca5f2567c2 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -95,6 +95,7 @@ $(BPFOBJ): force
>  CLANG ?= clang
>  LLC   ?= llc
>  LLVM_OBJCOPY ?= llvm-objcopy
> +LLVM_READELF ?= llvm-readelf
>  BTF_PAHOLE ?= pahole
>
>  PROBE := $(shell $(LLC) -march=bpf -mcpu=probe -filetype=null /dev/null 2>&1)
> @@ -131,7 +132,7 @@ BTF_PAHOLE_PROBE := $(shell $(BTF_PAHOLE) --help 2>&1 | grep BTF)
>  BTF_OBJCOPY_PROBE := $(shell $(LLVM_OBJCOPY) --help 2>&1 | grep -i 'usage.*llvm')
>  BTF_LLVM_PROBE := $(shell echo "int main() { return 0; }" | \
>                           $(CLANG) -target bpf -O2 -g -c -x c - -o ./llvm_btf_verify.o; \
> -                         readelf -S ./llvm_btf_verify.o | grep BTF; \
> +                         $(LLVM_READELF) -S ./llvm_btf_verify.o | grep BTF; \
>                           /bin/rm -f ./llvm_btf_verify.o)
>
>  ifneq ($(BTF_LLVM_PROBE),)
> --
> 2.20.1.321.g9e740568ce-goog
>

^ permalink raw reply

* Re: [PATCH net-next v1 1/2] net: dsa: mv88e6xxx: Default CMODE to 1000BaseX only on 6390X
From: Andrew Lunn @ 2019-01-24 18:11 UTC (permalink / raw)
  To: Marek Behun; +Cc: netdev, Florian Fainelli, David Miller
In-Reply-To: <20190124190451.40af3127@nic.cz>

On Thu, Jan 24, 2019 at 07:04:51PM +0100, Marek Behun wrote:
> What properties does the cpu port node need to contain to force it?
> phy-mode = "2500base-x"; is not enough.

Hi Marek

For DSA ports we have:

                                                phy-mode = "rgmii-txid";
                                                fixed-link {
                                                        speed = <1000>;
                                                        full-duplex;
                                                };

See dsa_port_fixed_link_register_of()

    Andrew

^ permalink raw reply

* Re: [PATCH] mac80211: Remove attribute packed from struct 'action'
From: Johannes Berg @ 2019-01-24 18:08 UTC (permalink / raw)
  To: Mathieu Malaterre; +Cc: David S. Miller, linux-wireless, netdev, linux-kernel
In-Reply-To: <20190124180535.20216-1-malat@debian.org>

On Thu, 2019-01-24 at 19:05 +0100, Mathieu Malaterre wrote:
> During refactor in commit 9e478066eae4 ("mac80211: fix MU-MIMO
> follow-MAC mode") a new struct 'action' was declared with packed
> attribute as:
> 
>   struct {
>           struct ieee80211_hdr_3addr hdr;
>           u8 category;
>           u8 action_code;
>   } __packed action;
> 
> But since struct 'ieee80211_hdr_3addr' is declared with an aligned
> keyword as:
> 
>   struct ieee80211_hdr {
>   	__le16 frame_control;
>   	__le16 duration_id;
>   	u8 addr1[ETH_ALEN];
>   	u8 addr2[ETH_ALEN];
>   	u8 addr3[ETH_ALEN];
>   	__le16 seq_ctrl;
>   	u8 addr4[ETH_ALEN];
>   } __packed __aligned(2);
> 
> Solve the ambiguity of placing aligned structure in a packed one by
> removing the packed attribute from struct. This seems to be the behavior
> of gcc anyway, since the following is still compiling:
> 
>   BUILD_BUG_ON(sizeof(action) != IEEE80211_MIN_ACTION_SIZE + 1);

I'm not sure this will work on all platforms, didn't something like
alpha pad out u8's to u32 when not requiring packing?

I guess I'd feel better about using __packed __aligned(2) here as well,
which should solve the warning too?

johannes



^ permalink raw reply

* [PATCH 1/2] Bluetooth: Annotate implicit fall through in l2cap_config_rsp
From: Mathieu Malaterre @ 2019-01-24 18:07 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Mathieu Malaterre, Gustavo Padovan, Johan Hedberg,
	David S. Miller, linux-bluetooth, netdev, linux-kernel

There is a plan to build the kernel with -Wimplicit-fallthrough and
this place in the code produced a warning (W=1).

This commit removes the following warning:

  net/bluetooth/l2cap_core.c:4223:6: warning: this statement may fall through [-Wimplicit-fallthrough=]

Cc: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
Signed-off-by: Mathieu Malaterre <malat@debian.org>
---
 net/bluetooth/l2cap_core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 2a7fb517d460..a5b76e24940a 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -4244,6 +4244,7 @@ static inline int l2cap_config_rsp(struct l2cap_conn *conn,
 				goto done;
 			break;
 		}
+		/* fall through */
 
 	default:
 		l2cap_chan_set_err(chan, ECONNRESET);
-- 
2.19.2


^ permalink raw reply related

* [PATCH 2/2] Bluetooth: Correctly annotate implicit fall through in __rfcomm_dlc_close
From: Mathieu Malaterre @ 2019-01-24 18:07 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Mathieu Malaterre, Peter Hurley, Johan Hedberg, David S. Miller,
	linux-bluetooth, netdev, linux-kernel
In-Reply-To: <20190124180724.20910-1-malat@debian.org>

There is a plan to build the kernel with -Wimplicit-fallthrough and
this place in the code produced a warning (W=1).

In this particular case put the fall through comment on a separate line so
as to match the regular expression expected by GCC.

This commit removes the following warning:

  net/bluetooth/rfcomm/core.c:479:6: warning: this statement may fall through [-Wimplicit-fallthrough=]

Cc: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Mathieu Malaterre <malat@debian.org>
---
 net/bluetooth/rfcomm/core.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
index 1a635df80643..caaae6accafa 100644
--- a/net/bluetooth/rfcomm/core.c
+++ b/net/bluetooth/rfcomm/core.c
@@ -480,9 +480,8 @@ static int __rfcomm_dlc_close(struct rfcomm_dlc *d, int err)
 			__rfcomm_dlc_disconn(d);
 			break;
 		}
-		/* if closing a dlc in a session that hasn't been started,
-		 * just close and unlink the dlc
-		 */
+		/* fall through - if closing a dlc in a session that hasn't */
+		/* been started, just close and unlink the dlc */
 
 	default:
 		rfcomm_dlc_clear_timer(d);
-- 
2.19.2


^ permalink raw reply related

* [PATCH] mac80211: Remove attribute packed from struct 'action'
From: Mathieu Malaterre @ 2019-01-24 18:05 UTC (permalink / raw)
  To: Johannes Berg
  Cc: Mathieu Malaterre, Johannes Berg, David S. Miller, linux-wireless,
	netdev, linux-kernel

During refactor in commit 9e478066eae4 ("mac80211: fix MU-MIMO
follow-MAC mode") a new struct 'action' was declared with packed
attribute as:

  struct {
          struct ieee80211_hdr_3addr hdr;
          u8 category;
          u8 action_code;
  } __packed action;

But since struct 'ieee80211_hdr_3addr' is declared with an aligned
keyword as:

  struct ieee80211_hdr {
  	__le16 frame_control;
  	__le16 duration_id;
  	u8 addr1[ETH_ALEN];
  	u8 addr2[ETH_ALEN];
  	u8 addr3[ETH_ALEN];
  	__le16 seq_ctrl;
  	u8 addr4[ETH_ALEN];
  } __packed __aligned(2);

Solve the ambiguity of placing aligned structure in a packed one by
removing the packed attribute from struct. This seems to be the behavior
of gcc anyway, since the following is still compiling:

  BUILD_BUG_ON(sizeof(action) != IEEE80211_MIN_ACTION_SIZE + 1);

This removes the following warning (W=1):

  net/mac80211/rx.c:234:2: warning: alignment 1 of 'struct <anonymous>' is less than 2 [-Wpacked-not-aligned]

Cc: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Mathieu Malaterre <malat@debian.org>
---
 net/mac80211/rx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 45aad3d3108c..709359650149 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -231,7 +231,7 @@ static void ieee80211_handle_mu_mimo_mon(struct ieee80211_sub_if_data *sdata,
 		struct ieee80211_hdr_3addr hdr;
 		u8 category;
 		u8 action_code;
-	} __packed action;
+	} action;
 
 	if (!sdata)
 		return;
-- 
2.19.2


^ permalink raw reply related

* Re: [PATCH net-next v1 1/2] net: dsa: mv88e6xxx: Default CMODE to 1000BaseX only on 6390X
From: Marek Behun @ 2019-01-24 18:04 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: netdev, Florian Fainelli, David Miller
In-Reply-To: <20190124164805.GJ482@lunn.ch>

What properties does the cpu port node need to contain to force it?
phy-mode = "2500base-x"; is not enough.
Marek

On Thu, 24 Jan 2019 17:48:05 +0100
Andrew Lunn <andrew@lunn.ch> wrote:

> > yes, I wanted to set the cpu port mode in DTS at first, but if I
> > understood the code correctly (maybe I made a mistake), the CPU port
> > mode is not determined from DTS (since there is no phy). Am I
> > wrong?  
> 
> It should work. I have boards which use it for DSA ports. I'm not sure
> i have a board with it for a CPU port though.
> 
>    Andrew


^ permalink raw reply

* Re: [PATCH v4 bpf-next 1/9] bpf: introduce bpf_spin_lock
From: Peter Zijlstra @ 2019-01-24 18:01 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: davem, daniel, jakub.kicinski, netdev, kernel-team, mingo,
	will.deacon, Paul McKenney, jannh
In-Reply-To: <20190124041403.2100609-2-ast@kernel.org>


Thanks for having kernel/locking people on Cc...

On Wed, Jan 23, 2019 at 08:13:55PM -0800, Alexei Starovoitov wrote:

> Implementation details:
> - on !SMP bpf_spin_lock() becomes nop

Because no BPF program is preemptible? I don't see any assertions or
even a comment that says this code is non-preemptible.

AFAICT some of the BPF_RUN_PROG things are under rcu_read_lock() only,
which is not sufficient.

> - on architectures that don't support queued_spin_lock trivial lock is used.
>   Note that arch_spin_lock cannot be used, since not all archs agree that
>   zero == unlocked and sizeof(arch_spinlock_t) != sizeof(__u32).

I really don't much like direct usage of qspinlock; esp. not as a
surprise.

Why does it matter if 0 means unlocked; that's what
__ARCH_SPIN_LOCK_UNLOCKED is for.

I get the sizeof(__u32) thing, but why not key off of that?

> Next steps:
> - allow bpf_spin_lock in other map types (like cgroup local storage)
> - introduce BPF_F_LOCK flag for bpf_map_update() syscall and helper
>   to request kernel to grab bpf_spin_lock before rewriting the value.
>   That will serialize access to map elements.

So clearly this map stuff is shared between bpf proglets, otherwise
there would not be a need for locking. But what happens if one is from
task context and another from IRQ context?

I don't see a local_irq_save()/restore() anywhere. What avoids the
trivial lock inversion?


> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index a74972b07e74..2e98e4caf5aa 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -221,6 +221,63 @@ const struct bpf_func_proto bpf_get_current_comm_proto = {
>  	.arg2_type	= ARG_CONST_SIZE,
>  };
>  
> +#ifndef CONFIG_QUEUED_SPINLOCKS
> +struct dumb_spin_lock {
> +	atomic_t val;
> +};
> +#endif
> +
> +notrace BPF_CALL_1(bpf_spin_lock, struct bpf_spin_lock *, lock)
> +{
> +#if defined(CONFIG_SMP)
> +#ifdef CONFIG_QUEUED_SPINLOCKS
> +	struct qspinlock *qlock = (void *)lock;
> +
> +	BUILD_BUG_ON(sizeof(*qlock) != sizeof(*lock));
> +	queued_spin_lock(qlock);
> +#else
> +	struct dumb_spin_lock *qlock = (void *)lock;
> +
> +	BUILD_BUG_ON(sizeof(*qlock) != sizeof(*lock));
> +	do {
> +		while (atomic_read(&qlock->val) != 0)
> +			cpu_relax();
> +	} while (atomic_cmpxchg(&qlock->val, 0, 1) != 0);
> +#endif
> +#endif
> +	return 0;
> +}
> +
> +const struct bpf_func_proto bpf_spin_lock_proto = {
> +	.func		= bpf_spin_lock,
> +	.gpl_only	= false,
> +	.ret_type	= RET_VOID,
> +	.arg1_type	= ARG_PTR_TO_SPIN_LOCK,
> +};
> +
> +notrace BPF_CALL_1(bpf_spin_unlock, struct bpf_spin_lock *, lock)
> +{
> +#if defined(CONFIG_SMP)
> +#ifdef CONFIG_QUEUED_SPINLOCKS
> +	struct qspinlock *qlock = (void *)lock;
> +
> +	queued_spin_unlock(qlock);
> +#else
> +	struct dumb_spin_lock *qlock = (void *)lock;
> +
> +	atomic_set(&qlock->val, 0);

And this is broken... That should've been atomic_set_release() at the
very least.

And this would again be the moment where I go pester you about the BPF
memory model :-)

> +#endif
> +#endif
> +	return 0;
> +}
> +
> +const struct bpf_func_proto bpf_spin_unlock_proto = {
> +	.func		= bpf_spin_unlock,
> +	.gpl_only	= false,
> +	.ret_type	= RET_VOID,
> +	.arg1_type	= ARG_PTR_TO_SPIN_LOCK,
> +};

^ permalink raw reply

* Re: [PATCH stable 4.4 05/11] ip: use rb trees for IP frag queue.
From: Greg KH @ 2019-01-24 17:58 UTC (permalink / raw)
  To: Mao Wenan; +Cc: netdev, eric.dumazet, davem, stable, edumazet
In-Reply-To: <1548209986-83527-6-git-send-email-maowenan@huawei.com>

On Wed, Jan 23, 2019 at 10:19:40AM +0800, Mao Wenan wrote:
> From: Peter Oskolkov <posk@google.com>
> 
> [ Upstream commit fa0f527358bd900ef92f925878ed6bfbd51305cc ]

This commit is not in the 4.14.y tree, any specific reason why not?

thanks,

greg k-h

^ permalink raw reply

* Re: [PATCH] NET:AX25:ROSE NULL ax25_cb kernel panic
From: Bernard Pidoux @ 2019-01-24 17:30 UTC (permalink / raw)
  To: linux-hams
  Cc: Ralf Baechle, Dmitry Vyukov, David Miller, Linux Netdev List,
	n1uro, f6bvp, Dmitry Vyukov
In-Reply-To: <19a0a879-8a8d-4c84-5de9-5ce222377fcb@free.fr>

> [PATCH] [ROSE] NULL ax25_cb kernel panic
>
> When an internally generated frame is handled by rose_xmit(),
> rose_route_frame() is called:
>
>          if (!rose_route_frame(skb, NULL)) {
>                  dev_kfree_skb(skb);
>                  stats->tx_errors++;
>                  return NETDEV_TX_OK;
>          }
>
> We have the same code sequence in Net/Rom where an internally generated
> frame is handled by nr_xmit() calling nr_route_frame(skb, NULL).
> However, in this function NULL argument is tested while it is not in
> rose_route_frame().
> Then kernel panic occurs later on when calling ax25cmp() with a NULL
> ax25_cb argument as reported many times and recently with syzbot.
>
> We need to test if ax25 is NULL before using it.
>
> Here is the patch:
>
> diff --git a/net/rose/rose_route.c b/net/rose/rose_route.c
> index 77e9f85a2c92..7f075255a372 100644
> --- a/net/rose/rose_route.c
> +++ b/net/rose/rose_route.c
> @@ -850,6 +850,7 @@ void rose_link_device_down(struct net_device *dev)
>
>   /*
>    *	Route a frame to an appropriate AX.25 connection.
> + *	a NULL ax25_cb indicates an internally generated frame.
>    */
>   int rose_route_frame(struct sk_buff *skb, ax25_cb *ax25)
>   {
> @@ -867,6 +868,10 @@ int rose_route_frame(struct sk_buff *skb, ax25_cb
> *ax25)
>
>   	if (skb->len < ROSE_MIN_LEN)
>   		return res;
> +
> +	if (!ax25)
> +		return rose_loopback_queue(skb, NULL);
> +
>   	frametype = skb->data[2];
>   	lci = ((skb->data[0] << 8) & 0xF00) + ((skb->data[1] << 0) & 0x0FF);
>   	if (frametype == ROSE_CALL_REQUEST &&
> Reported-by:syzbot+1a2c456a1ea08fa5b5f7@syzkaller.appspotmail.com
> Signed-off-by: Bernard Pidoux, f6bvp <f6bvp@free.fr>


^ permalink raw reply

* [PATCH] Bluetooth: make hw_err static, reduces object code size
From: Colin King @ 2019-01-24 17:22 UTC (permalink / raw)
  To: Marcel Holtmann, Johan Hedberg, David S . Miller, linux-bluetooth,
	netdev
  Cc: kernel-janitors, linux-kernel

From: Colin Ian King <colin.king@canonical.com>

Don't populate the const array hw_err on the stack but instead make
it static. Makes the object code smaller by 45 bytes:

Before:
   text	   data	    bss	    dec	    hex	filename
 100880	  21090	   1088	 123058	  1e0b2	linux/net/bluetooth/hci_core.o

After:
   text	   data	    bss	    dec	    hex	filename
 100739	  21186	   1088	 123013	  1e085	linux/net/bluetooth/hci_core.o

(gcc version 8.2.0 x86_64)

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 net/bluetooth/hci_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 7352fe85674b..40cd6b48473a 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -3401,7 +3401,7 @@ EXPORT_SYMBOL(hci_resume_dev);
 /* Reset HCI device */
 int hci_reset_dev(struct hci_dev *hdev)
 {
-	const u8 hw_err[] = { HCI_EV_HARDWARE_ERROR, 0x01, 0x00 };
+	static const u8 hw_err[] = { HCI_EV_HARDWARE_ERROR, 0x01, 0x00 };
 	struct sk_buff *skb;
 
 	skb = bt_skb_alloc(3, GFP_ATOMIC);
-- 
2.19.1


^ permalink raw reply related

* [PATCH net-next 7/7] net: tls: Add tests for TLS 1.3
From: Dave Watson @ 2019-01-24 17:21 UTC (permalink / raw)
  To: netdev@vger.kernel.org, Dave Miller
  Cc: Vakul Garg, Boris Pismenny, Aviad Yehezkel, John Fastabend,
	Daniel Borkmann

Change most tests to TLS 1.3, while adding tests for previous TLS 1.2
behavior.

Signed-off-by: Dave Watson <davejwatson@fb.com>
---
 tools/testing/selftests/net/tls.c | 76 ++++++++++++++++++++++++++++++-
 1 file changed, 75 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/net/tls.c b/tools/testing/selftests/net/tls.c
index c356f481de79..4ac50ccb3272 100644
--- a/tools/testing/selftests/net/tls.c
+++ b/tools/testing/selftests/net/tls.c
@@ -42,7 +42,7 @@ FIXTURE_SETUP(tls)
 	len = sizeof(addr);
 
 	memset(&tls12, 0, sizeof(tls12));
-	tls12.info.version = TLS_1_2_VERSION;
+	tls12.info.version = TLS_1_3_VERSION;
 	tls12.info.cipher_type = TLS_CIPHER_AES_GCM_128;
 
 	addr.sin_family = AF_INET;
@@ -825,4 +825,78 @@ TEST(keysizes) {
 	close(cfd);
 }
 
+TEST(tls12) {
+	int fd, cfd;
+	bool notls;
+
+	struct tls12_crypto_info_aes_gcm_128 tls12;
+	struct sockaddr_in addr;
+	socklen_t len;
+	int sfd, ret;
+
+	notls = false;
+	len = sizeof(addr);
+
+	memset(&tls12, 0, sizeof(tls12));
+	tls12.info.version = TLS_1_2_VERSION;
+	tls12.info.cipher_type = TLS_CIPHER_AES_GCM_128;
+
+	addr.sin_family = AF_INET;
+	addr.sin_addr.s_addr = htonl(INADDR_ANY);
+	addr.sin_port = 0;
+
+	fd = socket(AF_INET, SOCK_STREAM, 0);
+	sfd = socket(AF_INET, SOCK_STREAM, 0);
+
+	ret = bind(sfd, &addr, sizeof(addr));
+	ASSERT_EQ(ret, 0);
+	ret = listen(sfd, 10);
+	ASSERT_EQ(ret, 0);
+
+	ret = getsockname(sfd, &addr, &len);
+	ASSERT_EQ(ret, 0);
+
+	ret = connect(fd, &addr, sizeof(addr));
+	ASSERT_EQ(ret, 0);
+
+	ret = setsockopt(fd, IPPROTO_TCP, TCP_ULP, "tls", sizeof("tls"));
+	if (ret != 0) {
+		notls = true;
+		printf("Failure setting TCP_ULP, testing without tls\n");
+	}
+
+	if (!notls) {
+		ret = setsockopt(fd, SOL_TLS, TLS_TX, &tls12,
+				 sizeof(tls12));
+		ASSERT_EQ(ret, 0);
+	}
+
+	cfd = accept(sfd, &addr, &len);
+	ASSERT_GE(cfd, 0);
+
+	if (!notls) {
+		ret = setsockopt(cfd, IPPROTO_TCP, TCP_ULP, "tls",
+				 sizeof("tls"));
+		ASSERT_EQ(ret, 0);
+
+		ret = setsockopt(cfd, SOL_TLS, TLS_RX, &tls12,
+				 sizeof(tls12));
+		ASSERT_EQ(ret, 0);
+	}
+
+	close(sfd);
+
+	char const *test_str = "test_read";
+	int send_len = 10;
+	char buf[10];
+
+	send_len = strlen(test_str) + 1;
+	EXPECT_EQ(send(fd, test_str, send_len, 0), send_len);
+	EXPECT_NE(recv(cfd, buf, send_len, 0), -1);
+	EXPECT_EQ(memcmp(buf, test_str, send_len), 0);
+
+	close(fd);
+	close(cfd);
+}
+
 TEST_HARNESS_MAIN
-- 
2.17.1


^ permalink raw reply related

* [PATCH net-next 5/7] net: tls: Refactor control message handling on recv
From: Dave Watson @ 2019-01-24 17:21 UTC (permalink / raw)
  To: netdev@vger.kernel.org, Dave Miller
  Cc: Vakul Garg, Boris Pismenny, Aviad Yehezkel, John Fastabend,
	Daniel Borkmann

For TLS 1.3, the control message is encrypted.  Handle control
message checks after decryption.

Signed-off-by: Dave Watson <davejwatson@fb.com>
---
 net/tls/tls_sw.c | 88 ++++++++++++++++++++++++------------------------
 1 file changed, 44 insertions(+), 44 deletions(-)

diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index 1003494d0563..367f5b0128f2 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -1421,16 +1421,15 @@ static int decrypt_skb_update(struct sock *sk, struct sk_buff *skb,
 
 			return err;
 		}
+		rxm->offset += tls_ctx->rx.prepend_size;
+		rxm->full_len -= tls_ctx->rx.overhead_size;
+		tls_advance_record_sn(sk, &tls_ctx->rx);
+		ctx->decrypted = true;
+		ctx->saved_data_ready(sk);
 	} else {
 		*zc = false;
 	}
 
-	rxm->offset += tls_ctx->rx.prepend_size;
-	rxm->full_len -= tls_ctx->rx.overhead_size;
-	tls_advance_record_sn(sk, &tls_ctx->rx);
-	ctx->decrypted = true;
-	ctx->saved_data_ready(sk);
-
 	return err;
 }
 
@@ -1609,6 +1608,25 @@ int tls_sw_recvmsg(struct sock *sk,
 
 		rxm = strp_msg(skb);
 
+		to_decrypt = rxm->full_len - tls_ctx->rx.overhead_size;
+
+		if (to_decrypt <= len && !is_kvec && !is_peek &&
+		    ctx->control == TLS_RECORD_TYPE_DATA)
+			zc = true;
+
+		err = decrypt_skb_update(sk, skb, &msg->msg_iter,
+					 &chunk, &zc, ctx->async_capable);
+		if (err < 0 && err != -EINPROGRESS) {
+			tls_err_abort(sk, EBADMSG);
+			goto recv_end;
+		}
+
+		if (err == -EINPROGRESS) {
+			async = true;
+			num_async++;
+			goto pick_next_record;
+		}
+
 		if (!cmsg) {
 			int cerr;
 
@@ -1626,40 +1644,22 @@ int tls_sw_recvmsg(struct sock *sk,
 			goto recv_end;
 		}
 
-		to_decrypt = rxm->full_len - tls_ctx->rx.overhead_size;
-
-		if (to_decrypt <= len && !is_kvec && !is_peek)
-			zc = true;
-
-		err = decrypt_skb_update(sk, skb, &msg->msg_iter,
-					 &chunk, &zc, ctx->async_capable);
-		if (err < 0 && err != -EINPROGRESS) {
-			tls_err_abort(sk, EBADMSG);
-			goto recv_end;
-		}
-
-		if (err == -EINPROGRESS) {
-			async = true;
-			num_async++;
-			goto pick_next_record;
-		} else {
-			if (!zc) {
-				if (rxm->full_len > len) {
-					retain_skb = true;
-					chunk = len;
-				} else {
-					chunk = rxm->full_len;
-				}
+		if (!zc) {
+			if (rxm->full_len > len) {
+				retain_skb = true;
+				chunk = len;
+			} else {
+				chunk = rxm->full_len;
+			}
 
-				err = skb_copy_datagram_msg(skb, rxm->offset,
-							    msg, chunk);
-				if (err < 0)
-					goto recv_end;
+			err = skb_copy_datagram_msg(skb, rxm->offset,
+						    msg, chunk);
+			if (err < 0)
+				goto recv_end;
 
-				if (!is_peek) {
-					rxm->offset = rxm->offset + chunk;
-					rxm->full_len = rxm->full_len - chunk;
-				}
+			if (!is_peek) {
+				rxm->offset = rxm->offset + chunk;
+				rxm->full_len = rxm->full_len - chunk;
 			}
 		}
 
@@ -1759,15 +1759,15 @@ ssize_t tls_sw_splice_read(struct socket *sock,  loff_t *ppos,
 	if (!skb)
 		goto splice_read_end;
 
-	/* splice does not support reading control messages */
-	if (ctx->control != TLS_RECORD_TYPE_DATA) {
-		err = -ENOTSUPP;
-		goto splice_read_end;
-	}
-
 	if (!ctx->decrypted) {
 		err = decrypt_skb_update(sk, skb, NULL, &chunk, &zc, false);
 
+		/* splice does not support reading control messages */
+		if (ctx->control != TLS_RECORD_TYPE_DATA) {
+			err = -ENOTSUPP;
+			goto splice_read_end;
+		}
+
 		if (err < 0) {
 			tls_err_abort(sk, EBADMSG);
 			goto splice_read_end;
-- 
2.17.1


^ permalink raw reply related

* [PATCH net-next 6/7] net: tls: Add tls 1.3 support
From: Dave Watson @ 2019-01-24 17:21 UTC (permalink / raw)
  To: netdev@vger.kernel.org, Dave Miller
  Cc: Vakul Garg, Boris Pismenny, Aviad Yehezkel, John Fastabend,
	Daniel Borkmann

TLS 1.3 has minor changes from TLS 1.2 at the record layer.

* Header now hardcodes the same version and application content type in
  the header.
* The real content type is appended after the data, before encryption (or
  after decryption).
* The IV is xored with the sequence number, instead of concatinating four
  bytes of IV with the explicit IV.
* Zero-padding:  No exlicit length is given, we search backwards from the
  end of the decrypted data for the first non-zero byte, which is the
  content type.  Currently recv supports reading zero-padding, but there
  is no way for send to add zero padding.

Signed-off-by: Dave Watson <davejwatson@fb.com>
---
 include/net/tls.h             |  66 ++++++++++++++-----
 include/uapi/linux/tls.h      |   4 ++
 net/tls/tls_device.c          |   5 +-
 net/tls/tls_device_fallback.c |   3 +-
 net/tls/tls_main.c            |   3 +-
 net/tls/tls_sw.c              | 116 +++++++++++++++++++++++++++-------
 6 files changed, 154 insertions(+), 43 deletions(-)

diff --git a/include/net/tls.h b/include/net/tls.h
index 9edb2cc7023a..a93a8ed8f716 100644
--- a/include/net/tls.h
+++ b/include/net/tls.h
@@ -119,6 +119,9 @@ struct tls_rec {
 	/* AAD | msg_encrypted.sg.data (data contains overhead for hdr & iv & tag) */
 	struct scatterlist sg_aead_out[2];
 
+	char content_type;
+	struct scatterlist sg_content_type;
+
 	char aad_space[TLS_AAD_SPACE_SIZE];
 	u8 iv_data[TLS_CIPHER_AES_GCM_128_IV_SIZE +
 		   TLS_CIPHER_AES_GCM_128_SALT_SIZE];
@@ -204,6 +207,7 @@ struct cipher_context {
 	u16 rec_seq_size;
 	char *rec_seq;
 	u16 aad_size;
+	u16 tail_size;
 };
 
 union tls_crypto_context {
@@ -398,49 +402,77 @@ static inline bool tls_bigint_increment(unsigned char *seq, int len)
 }
 
 static inline void tls_advance_record_sn(struct sock *sk,
-					 struct cipher_context *ctx)
+					 struct cipher_context *ctx,
+					 int version)
 {
 	if (tls_bigint_increment(ctx->rec_seq, ctx->rec_seq_size))
 		tls_err_abort(sk, EBADMSG);
-	tls_bigint_increment(ctx->iv + TLS_CIPHER_AES_GCM_128_SALT_SIZE,
-			     ctx->iv_size);
+
+	if (version != TLS_1_3_VERSION) {
+		tls_bigint_increment(ctx->iv + TLS_CIPHER_AES_GCM_128_SALT_SIZE,
+				     ctx->iv_size);
+	}
 }
 
 static inline void tls_fill_prepend(struct tls_context *ctx,
 			     char *buf,
 			     size_t plaintext_len,
-			     unsigned char record_type)
+			     unsigned char record_type,
+			     int version)
 {
 	size_t pkt_len, iv_size = ctx->tx.iv_size;
 
-	pkt_len = plaintext_len + iv_size + ctx->tx.tag_size;
+	pkt_len = plaintext_len + ctx->tx.tag_size;
+	if (version != TLS_1_3_VERSION) {
+		pkt_len += iv_size;
+
+		memcpy(buf + TLS_NONCE_OFFSET,
+		       ctx->tx.iv + TLS_CIPHER_AES_GCM_128_SALT_SIZE, iv_size);
+	}
 
 	/* we cover nonce explicit here as well, so buf should be of
 	 * size KTLS_DTLS_HEADER_SIZE + KTLS_DTLS_NONCE_EXPLICIT_SIZE
 	 */
-	buf[0] = record_type;
-	buf[1] = TLS_VERSION_MINOR(ctx->crypto_send.info.version);
-	buf[2] = TLS_VERSION_MAJOR(ctx->crypto_send.info.version);
+	buf[0] = version == TLS_1_3_VERSION ?
+		   TLS_RECORD_TYPE_DATA : record_type;
+	/* Note that VERSION must be TLS_1_2 for both TLS1.2 and TLS1.3 */
+	buf[1] = TLS_1_2_VERSION_MINOR;
+	buf[2] = TLS_1_2_VERSION_MAJOR;
 	/* we can use IV for nonce explicit according to spec */
 	buf[3] = pkt_len >> 8;
 	buf[4] = pkt_len & 0xFF;
-	memcpy(buf + TLS_NONCE_OFFSET,
-	       ctx->tx.iv + TLS_CIPHER_AES_GCM_128_SALT_SIZE, iv_size);
 }
 
 static inline void tls_make_aad(char *buf,
 				size_t size,
 				char *record_sequence,
 				int record_sequence_size,
-				unsigned char record_type)
+				unsigned char record_type,
+				int version)
+{
+	if (version != TLS_1_3_VERSION) {
+		memcpy(buf, record_sequence, record_sequence_size);
+		buf += 8;
+	} else {
+		size += TLS_CIPHER_AES_GCM_128_TAG_SIZE;
+	}
+
+	buf[0] = version == TLS_1_3_VERSION ?
+		  TLS_RECORD_TYPE_DATA : record_type;
+	buf[1] = TLS_1_2_VERSION_MAJOR;
+	buf[2] = TLS_1_2_VERSION_MINOR;
+	buf[3] = size >> 8;
+	buf[4] = size & 0xFF;
+}
+
+static inline void xor_iv_with_seq(int version, char *iv, char *seq)
 {
-	memcpy(buf, record_sequence, record_sequence_size);
+	int i;
 
-	buf[8] = record_type;
-	buf[9] = TLS_1_2_VERSION_MAJOR;
-	buf[10] = TLS_1_2_VERSION_MINOR;
-	buf[11] = size >> 8;
-	buf[12] = size & 0xFF;
+	if (version == TLS_1_3_VERSION) {
+		for (i = 0; i < 8; i++)
+			iv[i + 4] ^= seq[i];
+	}
 }
 
 static inline struct tls_context *tls_get_ctx(const struct sock *sk)
diff --git a/include/uapi/linux/tls.h b/include/uapi/linux/tls.h
index 9affceaa3db4..401d6f01de6a 100644
--- a/include/uapi/linux/tls.h
+++ b/include/uapi/linux/tls.h
@@ -51,6 +51,10 @@
 #define TLS_1_2_VERSION_MINOR	0x3
 #define TLS_1_2_VERSION		TLS_VERSION_NUMBER(TLS_1_2)
 
+#define TLS_1_3_VERSION_MAJOR	0x3
+#define TLS_1_3_VERSION_MINOR	0x4
+#define TLS_1_3_VERSION		TLS_VERSION_NUMBER(TLS_1_3)
+
 /* Supported ciphers */
 #define TLS_CIPHER_AES_GCM_128				51
 #define TLS_CIPHER_AES_GCM_128_IV_SIZE			8
diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c
index d753e362d2d9..7ee9008b2187 100644
--- a/net/tls/tls_device.c
+++ b/net/tls/tls_device.c
@@ -257,7 +257,8 @@ static int tls_push_record(struct sock *sk,
 	tls_fill_prepend(ctx,
 			 skb_frag_address(frag),
 			 record->len - ctx->tx.prepend_size,
-			 record_type);
+			 record_type,
+			 ctx->crypto_send.info.version);
 
 	/* HW doesn't care about the data in the tag, because it fills it. */
 	dummy_tag_frag.page = skb_frag_page(frag);
@@ -270,7 +271,7 @@ static int tls_push_record(struct sock *sk,
 	spin_unlock_irq(&offload_ctx->lock);
 	offload_ctx->open_record = NULL;
 	set_bit(TLS_PENDING_CLOSED_RECORD, &ctx->flags);
-	tls_advance_record_sn(sk, &ctx->tx);
+	tls_advance_record_sn(sk, &ctx->tx, ctx->crypto_send.info.version);
 
 	for (i = 0; i < record->num_frags; i++) {
 		frag = &record->frags[i];
diff --git a/net/tls/tls_device_fallback.c b/net/tls/tls_device_fallback.c
index 450a6dbc5a88..54c3a758f2a7 100644
--- a/net/tls/tls_device_fallback.c
+++ b/net/tls/tls_device_fallback.c
@@ -73,7 +73,8 @@ static int tls_enc_record(struct aead_request *aead_req,
 	len -= TLS_CIPHER_AES_GCM_128_IV_SIZE;
 
 	tls_make_aad(aad, len - TLS_CIPHER_AES_GCM_128_TAG_SIZE,
-		     (char *)&rcd_sn, sizeof(rcd_sn), buf[0]);
+		(char *)&rcd_sn, sizeof(rcd_sn), buf[0],
+		TLS_1_2_VERSION);
 
 	memcpy(iv + TLS_CIPHER_AES_GCM_128_SALT_SIZE, buf + TLS_HEADER_SIZE,
 	       TLS_CIPHER_AES_GCM_128_IV_SIZE);
diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c
index 0f028cfdf835..d1c2fd9a3f63 100644
--- a/net/tls/tls_main.c
+++ b/net/tls/tls_main.c
@@ -463,7 +463,8 @@ static int do_tls_setsockopt_conf(struct sock *sk, char __user *optval,
 	}
 
 	/* check version */
-	if (crypto_info->version != TLS_1_2_VERSION) {
+	if (crypto_info->version != TLS_1_2_VERSION &&
+	    crypto_info->version != TLS_1_3_VERSION) {
 		rc = -ENOTSUPP;
 		goto err_crypto_info;
 	}
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index 367f5b0128f2..5e46112c9efd 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -120,6 +120,34 @@ static int skb_nsg(struct sk_buff *skb, int offset, int len)
         return __skb_nsg(skb, offset, len, 0);
 }
 
+static int padding_length(struct tls_sw_context_rx *ctx,
+			  struct tls_context *tls_ctx, struct sk_buff *skb)
+{
+	struct strp_msg *rxm = strp_msg(skb);
+	int sub = 0;
+
+	/* Determine zero-padding length */
+	if (tls_ctx->crypto_recv.info.version == TLS_1_3_VERSION) {
+		char content_type = 0;
+		int err;
+		int back = 17;
+
+		while (content_type == 0) {
+			if (back > rxm->full_len)
+				return -EBADMSG;
+			err = skb_copy_bits(skb,
+					    rxm->offset + rxm->full_len - back,
+					    &content_type, 1);
+			if (content_type)
+				break;
+			sub++;
+			back++;
+		}
+		ctx->control = content_type;
+	}
+	return sub;
+}
+
 static void tls_decrypt_done(struct crypto_async_request *req, int err)
 {
 	struct aead_request *aead_req = (struct aead_request *)req;
@@ -142,7 +170,7 @@ static void tls_decrypt_done(struct crypto_async_request *req, int err)
 		tls_err_abort(skb->sk, err);
 	} else {
 		struct strp_msg *rxm = strp_msg(skb);
-
+		rxm->full_len -= padding_length(ctx, tls_ctx, skb);
 		rxm->offset += tls_ctx->rx.prepend_size;
 		rxm->full_len -= tls_ctx->rx.overhead_size;
 	}
@@ -448,6 +476,8 @@ static int tls_do_encryption(struct sock *sk,
 	int rc;
 
 	memcpy(rec->iv_data, tls_ctx->tx.iv, sizeof(rec->iv_data));
+	xor_iv_with_seq(tls_ctx->crypto_send.info.version, rec->iv_data,
+			tls_ctx->tx.rec_seq);
 
 	sge->offset += tls_ctx->tx.prepend_size;
 	sge->length -= tls_ctx->tx.prepend_size;
@@ -483,7 +513,8 @@ static int tls_do_encryption(struct sock *sk,
 
 	/* Unhook the record from context if encryption is not failure */
 	ctx->open_rec = NULL;
-	tls_advance_record_sn(sk, &tls_ctx->tx);
+	tls_advance_record_sn(sk, &tls_ctx->tx,
+			      tls_ctx->crypto_send.info.version);
 	return rc;
 }
 
@@ -640,7 +671,17 @@ static int tls_push_record(struct sock *sk, int flags,
 
 	i = msg_pl->sg.end;
 	sk_msg_iter_var_prev(i);
-	sg_mark_end(sk_msg_elem(msg_pl, i));
+
+	rec->content_type = record_type;
+	if (tls_ctx->crypto_send.info.version == TLS_1_3_VERSION) {
+		/* Add content type to end of message.  No padding added */
+		sg_set_buf(&rec->sg_content_type, &rec->content_type, 1);
+		sg_mark_end(&rec->sg_content_type);
+		sg_chain(msg_pl->sg.data, msg_pl->sg.end + 1,
+			 &rec->sg_content_type);
+	} else {
+		sg_mark_end(sk_msg_elem(msg_pl, i));
+	}
 
 	i = msg_pl->sg.start;
 	sg_chain(rec->sg_aead_in, 2, rec->inplace_crypto ?
@@ -653,18 +694,22 @@ static int tls_push_record(struct sock *sk, int flags,
 	i = msg_en->sg.start;
 	sg_chain(rec->sg_aead_out, 2, &msg_en->sg.data[i]);
 
-	tls_make_aad(rec->aad_space, msg_pl->sg.size,
+	tls_make_aad(rec->aad_space, msg_pl->sg.size + tls_ctx->tx.tail_size,
 		     tls_ctx->tx.rec_seq, tls_ctx->tx.rec_seq_size,
-		     record_type);
+		     record_type,
+		     tls_ctx->crypto_send.info.version);
 
 	tls_fill_prepend(tls_ctx,
 			 page_address(sg_page(&msg_en->sg.data[i])) +
-			 msg_en->sg.data[i].offset, msg_pl->sg.size,
-			 record_type);
+			 msg_en->sg.data[i].offset,
+			 msg_pl->sg.size + tls_ctx->tx.tail_size,
+			 record_type,
+			 tls_ctx->crypto_send.info.version);
 
 	tls_ctx->pending_open_record_frags = false;
 
-	rc = tls_do_encryption(sk, tls_ctx, ctx, req, msg_pl->sg.size, i);
+	rc = tls_do_encryption(sk, tls_ctx, ctx, req,
+			       msg_pl->sg.size + tls_ctx->tx.tail_size, i);
 	if (rc < 0) {
 		if (rc != -EINPROGRESS) {
 			tls_err_abort(sk, EBADMSG);
@@ -1292,7 +1337,8 @@ static int decrypt_internal(struct sock *sk, struct sk_buff *skb,
 	u8 *aad, *iv, *mem = NULL;
 	struct scatterlist *sgin = NULL;
 	struct scatterlist *sgout = NULL;
-	const int data_len = rxm->full_len - tls_ctx->rx.overhead_size;
+	const int data_len = rxm->full_len - tls_ctx->rx.overhead_size +
+		tls_ctx->rx.tail_size;
 
 	if (*zc && (out_iov || out_sg)) {
 		if (out_iov)
@@ -1343,12 +1389,20 @@ static int decrypt_internal(struct sock *sk, struct sk_buff *skb,
 		kfree(mem);
 		return err;
 	}
-	memcpy(iv, tls_ctx->rx.iv, TLS_CIPHER_AES_GCM_128_SALT_SIZE);
+	if (tls_ctx->crypto_recv.info.version == TLS_1_3_VERSION)
+		memcpy(iv, tls_ctx->rx.iv, crypto_aead_ivsize(ctx->aead_recv));
+	else
+		memcpy(iv, tls_ctx->rx.iv, TLS_CIPHER_AES_GCM_128_SALT_SIZE);
+
+	xor_iv_with_seq(tls_ctx->crypto_recv.info.version, iv,
+			tls_ctx->rx.rec_seq);
 
 	/* Prepare AAD */
-	tls_make_aad(aad, rxm->full_len - tls_ctx->rx.overhead_size,
+	tls_make_aad(aad, rxm->full_len - tls_ctx->rx.overhead_size +
+		     tls_ctx->rx.tail_size,
 		     tls_ctx->rx.rec_seq, tls_ctx->rx.rec_seq_size,
-		     ctx->control);
+		     ctx->control,
+		     tls_ctx->crypto_recv.info.version);
 
 	/* Prepare sgin */
 	sg_init_table(sgin, n_sgin);
@@ -1405,6 +1459,7 @@ static int decrypt_skb_update(struct sock *sk, struct sk_buff *skb,
 {
 	struct tls_context *tls_ctx = tls_get_ctx(sk);
 	struct tls_sw_context_rx *ctx = tls_sw_ctx_rx(tls_ctx);
+	int version = tls_ctx->crypto_recv.info.version;
 	struct strp_msg *rxm = strp_msg(skb);
 	int err = 0;
 
@@ -1417,13 +1472,17 @@ static int decrypt_skb_update(struct sock *sk, struct sk_buff *skb,
 		err = decrypt_internal(sk, skb, dest, NULL, chunk, zc, async);
 		if (err < 0) {
 			if (err == -EINPROGRESS)
-				tls_advance_record_sn(sk, &tls_ctx->rx);
+				tls_advance_record_sn(sk, &tls_ctx->rx,
+						      version);
 
 			return err;
 		}
+
+		rxm->full_len -= padding_length(ctx, tls_ctx, skb);
+
 		rxm->offset += tls_ctx->rx.prepend_size;
 		rxm->full_len -= tls_ctx->rx.overhead_size;
-		tls_advance_record_sn(sk, &tls_ctx->rx);
+		tls_advance_record_sn(sk, &tls_ctx->rx, version);
 		ctx->decrypted = true;
 		ctx->saved_data_ready(sk);
 	} else {
@@ -1611,7 +1670,8 @@ int tls_sw_recvmsg(struct sock *sk,
 		to_decrypt = rxm->full_len - tls_ctx->rx.overhead_size;
 
 		if (to_decrypt <= len && !is_kvec && !is_peek &&
-		    ctx->control == TLS_RECORD_TYPE_DATA)
+		    ctx->control == TLS_RECORD_TYPE_DATA &&
+		    tls_ctx->crypto_recv.info.version != TLS_1_3_VERSION)
 			zc = true;
 
 		err = decrypt_skb_update(sk, skb, &msg->msg_iter,
@@ -1835,9 +1895,12 @@ static int tls_read_size(struct strparser *strp, struct sk_buff *skb)
 
 	data_len = ((header[4] & 0xFF) | (header[3] << 8));
 
-	cipher_overhead = tls_ctx->rx.tag_size + tls_ctx->rx.iv_size;
+	cipher_overhead = tls_ctx->rx.tag_size;
+	if (tls_ctx->crypto_recv.info.version != TLS_1_3_VERSION)
+		cipher_overhead += tls_ctx->rx.iv_size;
 
-	if (data_len > TLS_MAX_PAYLOAD_SIZE + cipher_overhead) {
+	if (data_len > TLS_MAX_PAYLOAD_SIZE + cipher_overhead +
+	    tls_ctx->rx.tail_size) {
 		ret = -EMSGSIZE;
 		goto read_failure;
 	}
@@ -1846,12 +1909,12 @@ static int tls_read_size(struct strparser *strp, struct sk_buff *skb)
 		goto read_failure;
 	}
 
-	if (header[1] != TLS_VERSION_MINOR(tls_ctx->crypto_recv.info.version) ||
-	    header[2] != TLS_VERSION_MAJOR(tls_ctx->crypto_recv.info.version)) {
+	/* Note that both TLS1.3 and TLS1.2 use TLS_1_2 version here */
+	if (header[1] != TLS_1_2_VERSION_MINOR ||
+	    header[2] != TLS_1_2_VERSION_MAJOR) {
 		ret = -EINVAL;
 		goto read_failure;
 	}
-
 #ifdef CONFIG_TLS_DEVICE
 	handle_device_resync(strp->sk, TCP_SKB_CB(skb)->seq + rxm->offset,
 			     *(u64*)tls_ctx->rx.rec_seq);
@@ -2098,10 +2161,19 @@ int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx, int tx)
 		goto free_priv;
 	}
 
-	cctx->aad_size = TLS_AAD_SPACE_SIZE;
+	if (crypto_info->version == TLS_1_3_VERSION) {
+		nonce_size = 0;
+		cctx->aad_size = TLS_HEADER_SIZE;
+		cctx->tail_size = 1;
+	} else {
+		cctx->aad_size = TLS_AAD_SPACE_SIZE;
+		cctx->tail_size = 0;
+	}
+
 	cctx->prepend_size = TLS_HEADER_SIZE + nonce_size;
 	cctx->tag_size = tag_size;
-	cctx->overhead_size = cctx->prepend_size + cctx->tag_size;
+	cctx->overhead_size = cctx->prepend_size + cctx->tag_size +
+		cctx->tail_size;
 	cctx->iv_size = iv_size;
 	cctx->iv = kmalloc(iv_size + TLS_CIPHER_AES_GCM_128_SALT_SIZE,
 			   GFP_KERNEL);
-- 
2.17.1


^ permalink raw reply related

* Re: [PATCH net-next v3 2/2] net/mlx5e: Don't overwrite pedit action when multiple pedit used
From: Or Gerlitz @ 2019-01-24 17:21 UTC (permalink / raw)
  To: Tonghao Zhang; +Cc: Saeed Mahameed, Linux Netdev List
In-Reply-To: <1548156126-8021-2-git-send-email-xiangxia.m.yue@gmail.com>

On Tue, Jan 22, 2019 at 4:38 PM <xiangxia.m.yue@gmail.com> wrote:
>
> From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>
> In some case, we may use multiple pedit actions to modify packets.
> The command shown as below: the last pedit action is effective.
>
> $ tc filter add dev netdev_rep parent ffff: protocol ip prio 1  \
>         flower skip_sw ip_proto icmp dst_ip 3.3.3.3                     \
>         action pedit ex munge ip dst set 192.168.1.100 pipe             \
>         action pedit ex munge eth src set 00:00:00:00:00:01 pipe        \
>         action pedit ex munge eth dst set 00:00:00:00:00:02 pipe        \
>         action csum ip pipe                                             \
>         action tunnel_key set src_ip 1.1.1.100 dst_ip 1.1.1.200 dst_port 4789 id 100 \
>         action mirred egress redirect dev vxlan0
>
> To fix it, we add max_mod_hdr_actions to mlx5e_tc_flow_parse_attr struction,
> max_mod_hdr_actions will store the max pedit action number we support and
> num_mod_hdr_actions indicates how many pedit action we used, and store all
> pedit action to mod_hdr_actions.

There's a comment just above offload_pedit_fields saying:

/* On input attr->num_mod_hdr_actions tells how many HW actions can be parsed at
 * max from the SW pedit action. On success, it says how many HW actions were
 * actually parsed.
 */

I guess it will have to change now, right?

^ permalink raw reply

* [PATCH net-next 4/7] net: tls: Refactor tls aad space size calculation
From: Dave Watson @ 2019-01-24 17:20 UTC (permalink / raw)
  To: netdev@vger.kernel.org, Dave Miller
  Cc: Vakul Garg, Boris Pismenny, Aviad Yehezkel, John Fastabend,
	Daniel Borkmann

TLS 1.3 has a different AAD size, use a variable in the code to
make TLS 1.3 support easy.

Signed-off-by: Dave Watson <davejwatson@fb.com>
---
 include/net/tls.h |  1 +
 net/tls/tls_sw.c  | 17 +++++++++--------
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/include/net/tls.h b/include/net/tls.h
index 675161ed37eb..9edb2cc7023a 100644
--- a/include/net/tls.h
+++ b/include/net/tls.h
@@ -203,6 +203,7 @@ struct cipher_context {
 	char *iv;
 	u16 rec_seq_size;
 	char *rec_seq;
+	u16 aad_size;
 };
 
 union tls_crypto_context {
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index 706ff02dc379..1003494d0563 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -185,7 +185,7 @@ static int tls_do_decryption(struct sock *sk,
 	int ret;
 
 	aead_request_set_tfm(aead_req, ctx->aead_recv);
-	aead_request_set_ad(aead_req, TLS_AAD_SPACE_SIZE);
+	aead_request_set_ad(aead_req, tls_ctx->rx.aad_size);
 	aead_request_set_crypt(aead_req, sgin, sgout,
 			       data_len + tls_ctx->rx.tag_size,
 			       (u8 *)iv_recv);
@@ -289,12 +289,12 @@ static struct tls_rec *tls_get_rec(struct sock *sk)
 
 	sg_init_table(rec->sg_aead_in, 2);
 	sg_set_buf(&rec->sg_aead_in[0], rec->aad_space,
-		   sizeof(rec->aad_space));
+		   tls_ctx->tx.aad_size);
 	sg_unmark_end(&rec->sg_aead_in[1]);
 
 	sg_init_table(rec->sg_aead_out, 2);
 	sg_set_buf(&rec->sg_aead_out[0], rec->aad_space,
-		   sizeof(rec->aad_space));
+		   tls_ctx->tx.aad_size);
 	sg_unmark_end(&rec->sg_aead_out[1]);
 
 	return rec;
@@ -455,7 +455,7 @@ static int tls_do_encryption(struct sock *sk,
 	msg_en->sg.curr = start;
 
 	aead_request_set_tfm(aead_req, ctx->aead_send);
-	aead_request_set_ad(aead_req, TLS_AAD_SPACE_SIZE);
+	aead_request_set_ad(aead_req, tls_ctx->tx.aad_size);
 	aead_request_set_crypt(aead_req, rec->sg_aead_in,
 			       rec->sg_aead_out,
 			       data_len, rec->iv_data);
@@ -1317,7 +1317,7 @@ static int decrypt_internal(struct sock *sk, struct sk_buff *skb,
 
 	aead_size = sizeof(*aead_req) + crypto_aead_reqsize(ctx->aead_recv);
 	mem_size = aead_size + (nsg * sizeof(struct scatterlist));
-	mem_size = mem_size + TLS_AAD_SPACE_SIZE;
+	mem_size = mem_size + tls_ctx->rx.aad_size;
 	mem_size = mem_size + crypto_aead_ivsize(ctx->aead_recv);
 
 	/* Allocate a single block of memory which contains
@@ -1333,7 +1333,7 @@ static int decrypt_internal(struct sock *sk, struct sk_buff *skb,
 	sgin = (struct scatterlist *)(mem + aead_size);
 	sgout = sgin + n_sgin;
 	aad = (u8 *)(sgout + n_sgout);
-	iv = aad + TLS_AAD_SPACE_SIZE;
+	iv = aad + tls_ctx->rx.aad_size;
 
 	/* Prepare IV */
 	err = skb_copy_bits(skb, rxm->offset + TLS_HEADER_SIZE,
@@ -1352,7 +1352,7 @@ static int decrypt_internal(struct sock *sk, struct sk_buff *skb,
 
 	/* Prepare sgin */
 	sg_init_table(sgin, n_sgin);
-	sg_set_buf(&sgin[0], aad, TLS_AAD_SPACE_SIZE);
+	sg_set_buf(&sgin[0], aad, tls_ctx->rx.aad_size);
 	err = skb_to_sgvec(skb, &sgin[1],
 			   rxm->offset + tls_ctx->rx.prepend_size,
 			   rxm->full_len - tls_ctx->rx.prepend_size);
@@ -1364,7 +1364,7 @@ static int decrypt_internal(struct sock *sk, struct sk_buff *skb,
 	if (n_sgout) {
 		if (out_iov) {
 			sg_init_table(sgout, n_sgout);
-			sg_set_buf(&sgout[0], aad, TLS_AAD_SPACE_SIZE);
+			sg_set_buf(&sgout[0], aad, tls_ctx->rx.aad_size);
 
 			*chunk = 0;
 			err = tls_setup_from_iter(sk, out_iov, data_len,
@@ -2098,6 +2098,7 @@ int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx, int tx)
 		goto free_priv;
 	}
 
+	cctx->aad_size = TLS_AAD_SPACE_SIZE;
 	cctx->prepend_size = TLS_HEADER_SIZE + nonce_size;
 	cctx->tag_size = tag_size;
 	cctx->overhead_size = cctx->prepend_size + cctx->tag_size;
-- 
2.17.1


^ permalink raw reply related

* Re: tc filter insertion rate degradation
From: Dennis Zhou @ 2019-01-24 17:21 UTC (permalink / raw)
  To: Eric Dumazet, Vlad Buslov
  Cc: Tejun Heo, Linux Kernel Network Developers, Yevgeny Kliteynik,
	Yossef Efraim, Maor Gottlieb
In-Reply-To: <CANn89iKb_vW+LA-91RV=zuAqbNycPFUYW54w_S=KZ3HdcWPw6Q@mail.gmail.com>

Hi Vlad and Eric,

On Tue, Jan 22, 2019 at 09:33:10AM -0800, Eric Dumazet wrote:
> On Mon, Jan 21, 2019 at 3:24 AM Vlad Buslov <vladbu@mellanox.com> wrote:
> >
> > Hi Eric,
> >
> > I've been investigating significant tc filter insertion rate degradation
> > and it seems it is caused by your commit 001c96db0181 ("net: align
> > gnet_stats_basic_cpu struct"). With this commit insertion rate is
> > reduced from ~65k rules/sec to ~43k rules/sec when inserting 1m rules
> > from file in tc batch mode on my machine.
> >
> > Tc perf profile indicates that pcpu allocator now consumes 2x CPU:
> >
> > 1) Before:
> >
> > Samples: 63K of event 'cycles:ppp', Event count (approx.): 48796480071
> >   Children      Self  Co  Shared Object     Symbol
> > +   21.19%     3.38%  tc  [kernel.vmlinux]  [k] pcpu_alloc
> > +    3.45%     0.25%  tc  [kernel.vmlinux]  [k] pcpu_alloc_area
> >
> > 2) After:
> >
> > Samples1: 92K of event 'cycles:ppp', Event count (approx.): 71446806550
> >   Children      Self  Co  Shared Object     Symbol
> > +   44.67%     3.99%  tc  [kernel.vmlinux]  [k] pcpu_alloc
> > +   19.25%     0.22%  tc  [kernel.vmlinux]  [k] pcpu_alloc_area
> >
> > It seems that it takes much more work for pcpu allocator to perform
> > allocation with new stricter alignment requirements. Not sure if it is
> > expected behavior or not in this case.
> >
> > Regards,
> > Vlad

Would you mind sharing a little more information with me:
1) output before and after a run of /sys/kernel/debug/percpu_stats
2) a full perf output
3) a reproducer

I'm a little surprised we're spending time in pcpu_alloc_area(), but it
might be due to constantly breaking the hint as an immediate guess.

> 
> Hi Vlad
> 
> I guess this is more a question for per-cpu allocator experts / maintainers ?
> 
> 16-bytes alignment for 16-bytes objects sound quite reasonable [1]
> 

The alignment request seems reasonable. But as Tejun mentioned in a
reply to this, the overhead of forced alignment would be both in percpu
memory itself and in allocation time due to the stricter requirement.

Thanks,
Dennis

^ permalink raw reply

* [PATCH net-next 3/7] net: tls: Support 256 bit keys
From: Dave Watson @ 2019-01-24 17:20 UTC (permalink / raw)
  To: netdev@vger.kernel.org, Dave Miller
  Cc: Vakul Garg, Boris Pismenny, Aviad Yehezkel, John Fastabend,
	Daniel Borkmann

Wire up support for 256 bit keys from the setsockopt to the crypto framework

Signed-off-by: Dave Watson <davejwatson@fb.com>
---
 include/net/tls.h                 |  5 ++-
 include/uapi/linux/tls.h          | 15 ++++++++
 net/tls/tls_main.c                | 33 +++++++++++++++-
 net/tls/tls_sw.c                  | 29 +++++++++++++--
 tools/testing/selftests/net/tls.c | 62 +++++++++++++++++++++++++++++++
 5 files changed, 137 insertions(+), 7 deletions(-)

diff --git a/include/net/tls.h b/include/net/tls.h
index eb73e62ac8c9..675161ed37eb 100644
--- a/include/net/tls.h
+++ b/include/net/tls.h
@@ -207,7 +207,10 @@ struct cipher_context {
 
 union tls_crypto_context {
 	struct tls_crypto_info info;
-	struct tls12_crypto_info_aes_gcm_128 aes_gcm_128;
+	union {
+		struct tls12_crypto_info_aes_gcm_128 aes_gcm_128;
+		struct tls12_crypto_info_aes_gcm_256 aes_gcm_256;
+	};
 };
 
 struct tls_context {
diff --git a/include/uapi/linux/tls.h b/include/uapi/linux/tls.h
index ff02287495ac..9affceaa3db4 100644
--- a/include/uapi/linux/tls.h
+++ b/include/uapi/linux/tls.h
@@ -59,6 +59,13 @@
 #define TLS_CIPHER_AES_GCM_128_TAG_SIZE		16
 #define TLS_CIPHER_AES_GCM_128_REC_SEQ_SIZE		8
 
+#define TLS_CIPHER_AES_GCM_256				52
+#define TLS_CIPHER_AES_GCM_256_IV_SIZE			8
+#define TLS_CIPHER_AES_GCM_256_KEY_SIZE		32
+#define TLS_CIPHER_AES_GCM_256_SALT_SIZE		4
+#define TLS_CIPHER_AES_GCM_256_TAG_SIZE		16
+#define TLS_CIPHER_AES_GCM_256_REC_SEQ_SIZE		8
+
 #define TLS_SET_RECORD_TYPE	1
 #define TLS_GET_RECORD_TYPE	2
 
@@ -75,4 +82,12 @@ struct tls12_crypto_info_aes_gcm_128 {
 	unsigned char rec_seq[TLS_CIPHER_AES_GCM_128_REC_SEQ_SIZE];
 };
 
+struct tls12_crypto_info_aes_gcm_256 {
+	struct tls_crypto_info info;
+	unsigned char iv[TLS_CIPHER_AES_GCM_256_IV_SIZE];
+	unsigned char key[TLS_CIPHER_AES_GCM_256_KEY_SIZE];
+	unsigned char salt[TLS_CIPHER_AES_GCM_256_SALT_SIZE];
+	unsigned char rec_seq[TLS_CIPHER_AES_GCM_256_REC_SEQ_SIZE];
+};
+
 #endif /* _UAPI_LINUX_TLS_H */
diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c
index d36d095cbcf0..0f028cfdf835 100644
--- a/net/tls/tls_main.c
+++ b/net/tls/tls_main.c
@@ -372,6 +372,30 @@ static int do_tls_getsockopt_tx(struct sock *sk, char __user *optval,
 			rc = -EFAULT;
 		break;
 	}
+	case TLS_CIPHER_AES_GCM_256: {
+		struct tls12_crypto_info_aes_gcm_256 *
+		  crypto_info_aes_gcm_256 =
+		  container_of(crypto_info,
+			       struct tls12_crypto_info_aes_gcm_256,
+			       info);
+
+		if (len != sizeof(*crypto_info_aes_gcm_256)) {
+			rc = -EINVAL;
+			goto out;
+		}
+		lock_sock(sk);
+		memcpy(crypto_info_aes_gcm_256->iv,
+		       ctx->tx.iv + TLS_CIPHER_AES_GCM_256_SALT_SIZE,
+		       TLS_CIPHER_AES_GCM_256_IV_SIZE);
+		memcpy(crypto_info_aes_gcm_256->rec_seq, ctx->tx.rec_seq,
+		       TLS_CIPHER_AES_GCM_256_REC_SEQ_SIZE);
+		release_sock(sk);
+		if (copy_to_user(optval,
+				 crypto_info_aes_gcm_256,
+				 sizeof(*crypto_info_aes_gcm_256)))
+			rc = -EFAULT;
+		break;
+	}
 	default:
 		rc = -EINVAL;
 	}
@@ -412,6 +436,7 @@ static int do_tls_setsockopt_conf(struct sock *sk, char __user *optval,
 {
 	struct tls_crypto_info *crypto_info;
 	struct tls_context *ctx = tls_get_ctx(sk);
+	size_t optsize;
 	int rc = 0;
 	int conf;
 
@@ -444,8 +469,12 @@ static int do_tls_setsockopt_conf(struct sock *sk, char __user *optval,
 	}
 
 	switch (crypto_info->cipher_type) {
-	case TLS_CIPHER_AES_GCM_128: {
-		if (optlen != sizeof(struct tls12_crypto_info_aes_gcm_128)) {
+	case TLS_CIPHER_AES_GCM_128:
+	case TLS_CIPHER_AES_GCM_256: {
+		optsize = crypto_info->cipher_type == TLS_CIPHER_AES_GCM_128 ?
+			sizeof(struct tls12_crypto_info_aes_gcm_128) :
+			sizeof(struct tls12_crypto_info_aes_gcm_256);
+		if (optlen != optsize) {
 			rc = -EINVAL;
 			goto err_crypto_info;
 		}
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index acfafe8190a3..706ff02dc379 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -1997,6 +1997,7 @@ int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx, int tx)
 {
 	struct tls_crypto_info *crypto_info;
 	struct tls12_crypto_info_aes_gcm_128 *gcm_128_info;
+	struct tls12_crypto_info_aes_gcm_256 *gcm_256_info;
 	struct tls_sw_context_tx *sw_ctx_tx = NULL;
 	struct tls_sw_context_rx *sw_ctx_rx = NULL;
 	struct cipher_context *cctx;
@@ -2004,7 +2005,8 @@ int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx, int tx)
 	struct strp_callbacks cb;
 	u16 nonce_size, tag_size, iv_size, rec_seq_size;
 	struct crypto_tfm *tfm;
-	char *iv, *rec_seq;
+	char *iv, *rec_seq, *key, *salt;
+	size_t keysize;
 	int rc = 0;
 
 	if (!ctx) {
@@ -2065,6 +2067,24 @@ int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx, int tx)
 		 ((struct tls12_crypto_info_aes_gcm_128 *)crypto_info)->rec_seq;
 		gcm_128_info =
 			(struct tls12_crypto_info_aes_gcm_128 *)crypto_info;
+		keysize = TLS_CIPHER_AES_GCM_128_KEY_SIZE;
+		key = gcm_128_info->key;
+		salt = gcm_128_info->salt;
+		break;
+	}
+	case TLS_CIPHER_AES_GCM_256: {
+		nonce_size = TLS_CIPHER_AES_GCM_256_IV_SIZE;
+		tag_size = TLS_CIPHER_AES_GCM_256_TAG_SIZE;
+		iv_size = TLS_CIPHER_AES_GCM_256_IV_SIZE;
+		iv = ((struct tls12_crypto_info_aes_gcm_256 *)crypto_info)->iv;
+		rec_seq_size = TLS_CIPHER_AES_GCM_256_REC_SEQ_SIZE;
+		rec_seq =
+		 ((struct tls12_crypto_info_aes_gcm_256 *)crypto_info)->rec_seq;
+		gcm_256_info =
+			(struct tls12_crypto_info_aes_gcm_256 *)crypto_info;
+		keysize = TLS_CIPHER_AES_GCM_256_KEY_SIZE;
+		key = gcm_256_info->key;
+		salt = gcm_256_info->salt;
 		break;
 	}
 	default:
@@ -2088,7 +2108,8 @@ int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx, int tx)
 		rc = -ENOMEM;
 		goto free_priv;
 	}
-	memcpy(cctx->iv, gcm_128_info->salt, TLS_CIPHER_AES_GCM_128_SALT_SIZE);
+	/* Note: 128 & 256 bit salt are the same size */
+	memcpy(cctx->iv, salt, TLS_CIPHER_AES_GCM_128_SALT_SIZE);
 	memcpy(cctx->iv + TLS_CIPHER_AES_GCM_128_SALT_SIZE, iv, iv_size);
 	cctx->rec_seq_size = rec_seq_size;
 	cctx->rec_seq = kmemdup(rec_seq, rec_seq_size, GFP_KERNEL);
@@ -2108,8 +2129,8 @@ int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx, int tx)
 
 	ctx->push_pending_record = tls_sw_push_pending_record;
 
-	rc = crypto_aead_setkey(*aead, gcm_128_info->key,
-				TLS_CIPHER_AES_GCM_128_KEY_SIZE);
+	rc = crypto_aead_setkey(*aead, key, keysize);
+
 	if (rc)
 		goto free_aead;
 
diff --git a/tools/testing/selftests/net/tls.c b/tools/testing/selftests/net/tls.c
index ff68ed19c0ef..c356f481de79 100644
--- a/tools/testing/selftests/net/tls.c
+++ b/tools/testing/selftests/net/tls.c
@@ -763,4 +763,66 @@ TEST_F(tls, control_msg)
 	EXPECT_EQ(memcmp(buf, test_str, send_len), 0);
 }
 
+TEST(keysizes) {
+	struct tls12_crypto_info_aes_gcm_256 tls12;
+	struct sockaddr_in addr;
+	int sfd, ret, fd, cfd;
+	socklen_t len;
+	bool notls;
+
+	notls = false;
+	len = sizeof(addr);
+
+	memset(&tls12, 0, sizeof(tls12));
+	tls12.info.version = TLS_1_2_VERSION;
+	tls12.info.cipher_type = TLS_CIPHER_AES_GCM_256;
+
+	addr.sin_family = AF_INET;
+	addr.sin_addr.s_addr = htonl(INADDR_ANY);
+	addr.sin_port = 0;
+
+	fd = socket(AF_INET, SOCK_STREAM, 0);
+	sfd = socket(AF_INET, SOCK_STREAM, 0);
+
+	ret = bind(sfd, &addr, sizeof(addr));
+	ASSERT_EQ(ret, 0);
+	ret = listen(sfd, 10);
+	ASSERT_EQ(ret, 0);
+
+	ret = getsockname(sfd, &addr, &len);
+	ASSERT_EQ(ret, 0);
+
+	ret = connect(fd, &addr, sizeof(addr));
+	ASSERT_EQ(ret, 0);
+
+	ret = setsockopt(fd, IPPROTO_TCP, TCP_ULP, "tls", sizeof("tls"));
+	if (ret != 0) {
+		notls = true;
+		printf("Failure setting TCP_ULP, testing without tls\n");
+	}
+
+	if (!notls) {
+		ret = setsockopt(fd, SOL_TLS, TLS_TX, &tls12,
+				 sizeof(tls12));
+		EXPECT_EQ(ret, 0);
+	}
+
+	cfd = accept(sfd, &addr, &len);
+	ASSERT_GE(cfd, 0);
+
+	if (!notls) {
+		ret = setsockopt(cfd, IPPROTO_TCP, TCP_ULP, "tls",
+				 sizeof("tls"));
+		EXPECT_EQ(ret, 0);
+
+		ret = setsockopt(cfd, SOL_TLS, TLS_RX, &tls12,
+				 sizeof(tls12));
+		EXPECT_EQ(ret, 0);
+	}
+
+	close(sfd);
+	close(fd);
+	close(cfd);
+}
+
 TEST_HARNESS_MAIN
-- 
2.17.1


^ permalink raw reply related

* [PATCH net-next 1/7] net: tls: Save iv in tls_rec for async crypto requests
From: Dave Watson @ 2019-01-24 17:20 UTC (permalink / raw)
  To: netdev@vger.kernel.org, Dave Miller
  Cc: Vakul Garg, Boris Pismenny, Aviad Yehezkel, John Fastabend,
	Daniel Borkmann

aead_request_set_crypt takes an iv pointer, and we change the iv
soon after setting it.  Some async crypto algorithms don't save the iv,
so we need to save it in the tls_rec for async requests.

Found by hardcoding x64 aesni to use async crypto manager (to test the async
codepath), however I don't think this combination can happen in the wild.
Presumably other hardware offloads will need this fix, but there have been
no user reports.

Fixes: a42055e8d2c30 ("Add support for async encryption of records...")
Signed-off-by: Dave Watson <davejwatson@fb.com>
---
 include/net/tls.h | 2 ++
 net/tls/tls_sw.c  | 4 +++-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/include/net/tls.h b/include/net/tls.h
index 90bf52db573e..4592606e136a 100644
--- a/include/net/tls.h
+++ b/include/net/tls.h
@@ -120,6 +120,8 @@ struct tls_rec {
 	struct scatterlist sg_aead_out[2];
 
 	char aad_space[TLS_AAD_SPACE_SIZE];
+	u8 iv_data[TLS_CIPHER_AES_GCM_128_IV_SIZE +
+		   TLS_CIPHER_AES_GCM_128_SALT_SIZE];
 	struct aead_request aead_req;
 	u8 aead_req_ctx[];
 };
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index 86b9527c4826..0382a4674bb3 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -447,6 +447,8 @@ static int tls_do_encryption(struct sock *sk,
 	struct scatterlist *sge = sk_msg_elem(msg_en, start);
 	int rc;
 
+	memcpy(rec->iv_data, tls_ctx->tx.iv, sizeof(rec->iv_data));
+
 	sge->offset += tls_ctx->tx.prepend_size;
 	sge->length -= tls_ctx->tx.prepend_size;
 
@@ -456,7 +458,7 @@ static int tls_do_encryption(struct sock *sk,
 	aead_request_set_ad(aead_req, TLS_AAD_SPACE_SIZE);
 	aead_request_set_crypt(aead_req, rec->sg_aead_in,
 			       rec->sg_aead_out,
-			       data_len, tls_ctx->tx.iv);
+			       data_len, rec->iv_data);
 
 	aead_request_set_callback(aead_req, CRYPTO_TFM_REQ_MAY_BACKLOG,
 				  tls_encrypt_done, sk);
-- 
2.17.1


^ permalink raw reply related

* [PATCH net-next 2/7] net: tls: Set async_capable for tls zerocopy only if we see EINPROGRESS
From: Dave Watson @ 2019-01-24 17:20 UTC (permalink / raw)
  To: netdev@vger.kernel.org, Dave Miller
  Cc: Vakul Garg, Boris Pismenny, Aviad Yehezkel, John Fastabend,
	Daniel Borkmann

Currently we don't zerocopy if the crypto framework async bit is set.
However some crypto algorithms (such as x86 AESNI) support async,
but in the context of sendmsg, will never run asynchronously.  Instead,
check for actual EINPROGRESS return code before assuming algorithm is
async.

Signed-off-by: Dave Watson <davejwatson@fb.com>
---
 include/net/tls.h | 1 +
 net/tls/tls_sw.c  | 4 ++--
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/include/net/tls.h b/include/net/tls.h
index 4592606e136a..eb73e62ac8c9 100644
--- a/include/net/tls.h
+++ b/include/net/tls.h
@@ -139,6 +139,7 @@ struct tls_sw_context_tx {
 	struct list_head tx_list;
 	atomic_t encrypt_pending;
 	int async_notify;
+	int async_capable;
 
 #define BIT_TX_SCHEDULED	0
 	unsigned long tx_bitmask;
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index 0382a4674bb3..acfafe8190a3 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -673,6 +673,7 @@ static int tls_push_record(struct sock *sk, int flags,
 				tls_merge_open_record(sk, rec, tmp, orig_end);
 			}
 		}
+		ctx->async_capable = 1;
 		return rc;
 	} else if (split) {
 		msg_pl = &tmp->msg_plaintext;
@@ -814,8 +815,7 @@ int tls_sw_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
 	long timeo = sock_sndtimeo(sk, msg->msg_flags & MSG_DONTWAIT);
 	struct tls_context *tls_ctx = tls_get_ctx(sk);
 	struct tls_sw_context_tx *ctx = tls_sw_ctx_tx(tls_ctx);
-	struct crypto_tfm *tfm = crypto_aead_tfm(ctx->aead_send);
-	bool async_capable = tfm->__crt_alg->cra_flags & CRYPTO_ALG_ASYNC;
+	bool async_capable = ctx->async_capable;
 	unsigned char record_type = TLS_RECORD_TYPE_DATA;
 	bool is_kvec = iov_iter_is_kvec(&msg->msg_iter);
 	bool eor = !(msg->msg_flags & MSG_MORE);
-- 
2.17.1


^ permalink raw reply related

* [PATCH net] ibmveth: Do not process frames after calling napi_reschedule
From: Thomas Falcon @ 2019-01-24 17:17 UTC (permalink / raw)
  To: netdev; +Cc: Thomas Falcon

The IBM virtual ethernet driver's polling function continues
to process frames after rescheduling NAPI, resulting in a warning
if it exhausted its budget. Do not restart polling after calling
napi_reschedule. Instead let frames be processed in the following
instance.

Signed-off-by: Thomas Falcon <tlfalcon@linux.ibm.com>
---
 drivers/net/ethernet/ibm/ibmveth.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmveth.c b/drivers/net/ethernet/ibm/ibmveth.c
index 098d876..dd71d5d 100644
--- a/drivers/net/ethernet/ibm/ibmveth.c
+++ b/drivers/net/ethernet/ibm/ibmveth.c
@@ -1313,7 +1313,6 @@ static int ibmveth_poll(struct napi_struct *napi, int budget)
 	unsigned long lpar_rc;
 	u16 mss = 0;
 
-restart_poll:
 	while (frames_processed < budget) {
 		if (!ibmveth_rxq_pending_buffer(adapter))
 			break;
@@ -1401,7 +1400,6 @@ static int ibmveth_poll(struct napi_struct *napi, int budget)
 		    napi_reschedule(napi)) {
 			lpar_rc = h_vio_signal(adapter->vdev->unit_address,
 					       VIO_IRQ_DISABLE);
-			goto restart_poll;
 		}
 	}
 
-- 
1.8.3.1


^ permalink raw reply related

* Re: [PATCH] net: mlx5: allow default ip_proto to offload
From: Or Gerlitz @ 2019-01-24 17:14 UTC (permalink / raw)
  To: Tonghao Zhang
  Cc: Or Gerlitz, Saeed Mahameed, Leon Romanovsky, Linux Netdev List
In-Reply-To: <CAMDZJNXqC-eW0aaNUP5OGvCUQiYT_C=19H9C2jp9TW=+Z5KgWw@mail.gmail.com>

On Sat, Jan 19, 2019 at 10:50 AM Tonghao Zhang <xiangxia.m.yue@gmail.com> wrote:
> On Fri, Jan 18, 2019 at 10:19 PM Or Gerlitz <gerlitz.or@gmail.com> wrote:
> > On Thu, Jan 17, 2019 at 3:34 PM Tonghao Zhang <xiangxia.m.yue@gmail.com> wrote:
> > > On Thu, Jan 17, 2019 at 8:58 PM Or Gerlitz <gerlitz.or@gmail.com> wrote:
> > > > On Thu, Jan 17, 2019 at 11:28 AM <xiangxia.m.yue@gmail.com> wrote:
> > > > > From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> >
> > > with this patch, run the command [2], we will not get err log,
> > > and the filter work in hw.
> >
> > This whole thing is done for a reason which is the inability of the current HW
> > to adjust checksum/crc for few L3 protocols. Such adjustment is needed if
> > you modify some fields of L3 headers, e.g re-write src/dst IP address.
> I got it, thanks
> > > We should consider ip_proto == 0, in some case, we only
> > > modify dest ip or src ip.
> >
> > we can't let it go without clear matching on the ip protocol, as I explained
> > above. With my proposed patch you will be able to NAT much more protocols
> > (all of them expect for three, and we're working to reduce that), but
> > you still need
> > a tc rule per ip proto
>
>
> > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> > b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> > index 608025ca5c04..affb523e0e35 100644
> > --- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> > +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> > @@ -2167,11 +2167,11 @@ static bool
> > modify_header_match_supported(struct mlx5_flow_spec *spec,
> >         }
> >
> >         ip_proto = MLX5_GET(fte_match_set_lyr_2_4, headers_v, ip_protocol);
> > -       if (modify_ip_header && ip_proto != IPPROTO_TCP &&
> > -           ip_proto != IPPROTO_UDP && ip_proto != IPPROTO_ICMP) {
> > +       if (modify_ip_header && (ip_proto == IPPROTO_ICMPV6 ||
> > +           ip_proto == IPPROTO_SCTP || ip_proto == IPPROTO_UDPLITE)) {
> >                 NL_SET_ERR_MSG_MOD(extack,
> > -                                  "can't offload re-write of non TCP/UDP");
> > -               pr_info("can't offload re-write of ip proto %d\n", ip_proto);
> > +                                  "can't offload this re-write of IP
> > addresses");
> > +               pr_info("can't offload re-write of IP addrs for ip
> > proto %d\n", ip_proto);
> >                 return false;
> >         }

> This patch work for me too, because ip_proto == 0 will not return err(
> and my patch allow ip_proto == 0 and not return err) and will you send
> it to net-next ? because i can't find it in net-next.

basically, I was planning to upstream it on this cycle, but your
comment below is
something I need to look at

> and one question, In your patch, should we check ip_proto is valid ?
> for example, ip_proto == 18,  is not valid protocol.

yeah, this becomes a bit ugly, I need to see how to address that

> flower ip_proto 18

^ permalink raw reply

* Re: [PATCH 6/8] power: reset: at91-reset: add support for sam9x60 SoC
From: Sebastian Reichel @ 2019-01-24 17:11 UTC (permalink / raw)
  To: Nicolas.Ferre
  Cc: alexandre.belloni, Ludovic.Desroches, linux-kernel,
	linux-arm-kernel, linux-pm, netdev, davem, linux-usb, stern,
	gregkh, robh+dt, devicetree
In-Reply-To: <a74956fc-6085-eec2-c1b8-ed116b2297d8@microchip.com>

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

Hi,

On Thu, Jan 24, 2019 at 10:34:50AM +0000, Nicolas.Ferre@microchip.com wrote:
> Hi Sebastian,
> 
> On 23/01/2019 at 19:34, Sebastian Reichel wrote:
> > Hi,
> > 
> > On Wed, Jan 16, 2019 at 10:57:42AM +0100, Nicolas Ferre wrote:
> >> Add support for additional reset causes and the proper compatibility
> >> string for sam9x60 SoC. The restart function is the same as the samx7.
> >>
> >> Signed-off-by: Nicolas Ferre <nicolas.ferre@microchip.com>
> >> ---
> >>   drivers/power/reset/at91-reset.c | 13 +++++++++++++
> >>   1 file changed, 13 insertions(+)
> >>
> >> diff --git a/drivers/power/reset/at91-reset.c b/drivers/power/reset/at91-reset.c
> >> index f44a9ffcc2ab..44ca983a49a1 100644
> >> --- a/drivers/power/reset/at91-reset.c
> >> +++ b/drivers/power/reset/at91-reset.c
> >> @@ -44,6 +44,9 @@ enum reset_type {
> >>   	RESET_TYPE_WATCHDOG	= 2,
> >>   	RESET_TYPE_SOFTWARE	= 3,
> >>   	RESET_TYPE_USER		= 4,
> >> +	RESET_TYPE_CPU_FAIL	= 6,
> >> +	RESET_TYPE_XTAL_FAIL	= 7,
> >> +	RESET_TYPE_ULP2		= 8,
> > 
> > what happened to 5? :)
> 
> That a good question ;-)
> 
> It's marked as "Reserved"... which opens up a whole new field of 
> speculation :-)

Ok :)

> [..]
> 
> >>   	{ .compatible = "atmel,samx7-rstc", .data = samx7_restart },
> >> +	{ .compatible = "microchip,sam9x60-rstc", .data = samx7_restart },
> >>   	{ /* sentinel */ }
> >>   };
> >>   MODULE_DEVICE_TABLE(of, at91_reset_of_match);
> > 
> > Patch looks fine to me. But I will wait a bit with merging, so that
> > Alexandre or Ludovic have a chance to provide feedback.
> 
> What about merging this patch with the whole series through the at91 
> then arm-soc trees?

It seems to be possible to merge this standalone, but merging
through at91/arm-soc is also fine with me.

Acked-by: Sebastian Reichel <sebastian.reichel@collabora.com>

-- Sebastian

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

^ 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