* [patch net-next] net: sched: cls_matchall: allow to delete filter
From: Jiri Pirko @ 2019-06-17 16:02 UTC (permalink / raw)
To: netdev; +Cc: davem, mlxsw, eli, jhs, xiyou.wangcong
From: Jiri Pirko <jiri@mellanox.com>
Currently user is unable to delete the filter. See following example:
$ tc filter add dev ens16np1 ingress pref 1 handle 1 matchall action drop
$ tc filter show dev ens16np1 ingress
filter protocol all pref 1 matchall chain 0
filter protocol all pref 1 matchall chain 0 handle 0x1
in_hw
action order 1: gact action drop
random type none pass val 0
index 1 ref 1 bind 1
$ tc filter del dev ens16np1 ingress pref 1 handle 1 matchall action drop
RTNETLINK answers: Operation not supported
Implement tcf_proto_ops->delete() op and allow user to delete the filter.
Reported-by: Eli Cohen <eli@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
net/sched/cls_matchall.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/net/sched/cls_matchall.c b/net/sched/cls_matchall.c
index 38c0a9f0f296..a30d2f8feb32 100644
--- a/net/sched/cls_matchall.c
+++ b/net/sched/cls_matchall.c
@@ -21,6 +21,7 @@ struct cls_mall_head {
unsigned int in_hw_count;
struct tc_matchall_pcnt __percpu *pf;
struct rcu_work rwork;
+ bool deleting;
};
static int mall_classify(struct sk_buff *skb, const struct tcf_proto *tp,
@@ -258,7 +259,11 @@ static int mall_change(struct net *net, struct sk_buff *in_skb,
static int mall_delete(struct tcf_proto *tp, void *arg, bool *last,
bool rtnl_held, struct netlink_ext_ack *extack)
{
- return -EOPNOTSUPP;
+ struct cls_mall_head *head = rtnl_dereference(tp->root);
+
+ head->deleting = true;
+ *last = true;
+ return 0;
}
static void mall_walk(struct tcf_proto *tp, struct tcf_walker *arg,
@@ -269,7 +274,7 @@ static void mall_walk(struct tcf_proto *tp, struct tcf_walker *arg,
if (arg->count < arg->skip)
goto skip;
- if (!head)
+ if (!head || head->deleting)
return;
if (arg->fn(tp, head, arg) < 0)
arg->stop = 1;
--
2.20.1
^ permalink raw reply related
* Re: [patch net-next internal] net: sched: cls_matchall: allow to delete filter
From: Jiri Pirko @ 2019-06-17 16:03 UTC (permalink / raw)
To: netdev; +Cc: davem, mlxsw, eli, jhs, xiyou.wangcong
In-Reply-To: <20190617160208.7548-1-jiri@resnulli.us>
Sorry, wrong prefix, please ignore.
^ permalink raw reply
* Re: [RFC PATCH net-next 2/2] net: tls: export protocol version and cipher to socket diag
From: Davide Caratti @ 2019-06-17 16:04 UTC (permalink / raw)
To: Jakub Kicinski
Cc: David S. Miller, Dave Watson, Boris Pismenny, Aviad Yehezkel,
John Fastabend, Daniel Borkmann, netdev
In-Reply-To: <20190605162555.59b4fb3e@cakuba.netronome.com>
On Wed, 2019-06-05 at 16:25 -0700, Jakub Kicinski wrote:
> On Wed, 5 Jun 2019 17:39:23 +0200, Davide Caratti wrote:
> > When an application configures kernel TLS on top of a TCP socket, it's
> > now possible for inet_diag_handler to collect information regarding the
> > protocol version and the cipher, in case INET_DIAG_INFO is requested.
> >
> > Signed-off-by: Davide Caratti <dcaratti@redhat.com>
> >
> > +enum {
>
> USPEC
>
> > + TLS_INFO_VERSION,
> > + TLS_INFO_CIPHER,
>
Ok,
> We need some indication of the directions in which kTLS is active
> (none, rx, tx, rx/tx).
>
> Also perhaps could you add TLS_SW vs TLS_HW etc. ? :)
I can add a couple of u16 (or larger?) bitmasks to dump txconf and rxconf.
do you think this is sufficient?
> > + __TLS_INFO_MAX,
> > +};
> > +
> Traditionally we put no new line between enum and the max define.
Ok, will fix that in v1.
> > +#define TLS_INFO_MAX (__TLS_INFO_MAX - 1)
> > +
> > #endif /* _UAPI_LINUX_TLS_H */
> > diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c
> > index fc81ae18cc44..14597526981c 100644
> > --- a/net/tls/tls_main.c
> > +++ b/net/tls/tls_main.c
> > @@ -39,6 +39,7 @@
> > #include <linux/netdevice.h>
> > #include <linux/sched/signal.h>
> > #include <linux/inetdevice.h>
> > +#include <linux/inet_diag.h>
> >
> > #include <net/tls.h>
> >
> > @@ -798,6 +799,46 @@ static int tls_init(struct sock *sk)
> > return rc;
> > }
> >
> > +static int tls_get_info(struct sock *sk, struct sk_buff *skb)
> > +{
> > + struct tls_context *ctx = tls_get_ctx(sk);
> > + struct nlattr *start = 0;
>
> Hm.. NULL? Does this not give you a warning?
I didn't notice it, but sure. will fix in v1.
> > + int err = 0;
>
> There should be no need to init this.
>
> > + if (sk->sk_state != TCP_ESTABLISHED)
>
> Hmm.. why this check? We never clean up the state once installed until
> the socket dies completely (currently, pending John's unhash work).
the goal was to ensure that we don't read ctx anymore after
tls_sk_proto_close() has freed ctx, and I thought that a test on the value
of sk_state was sufficient.
If it's not, then we might invent something else. For example, we might
defer freeing kTLS ctx, so that it's called as the very last thing with
tcp_cleanup_ulp().
> > + goto end;
>
> Please don't do this, just return 0; here.
>
> > + start = nla_nest_start_noflag(skb, ULP_INFO_TLS);
> > + if (!start) {
> > + err = -EMSGSIZE;
> > + goto nla_failure;
>
> return -EMSGSIZE;
>
> > + }
> > + err = nla_put_u16(skb, TLS_INFO_VERSION, ctx->prot_info.version);
> > + if (err < 0)
> > + goto nla_failure;
> > + err = nla_put_u16(skb, TLS_INFO_CIPHER, ctx->prot_info.cipher_type);
> > + if (err < 0)
> > + goto nla_failure;
> > + nla_nest_end(skb, start);
> > +end:
> > + return err;
>
> return 0;
>
> > +nla_failure:
> > + nla_nest_cancel(skb, start);
> > + goto end;
>
> return err;
Ok, i can remove that 'goto end'.
> > +}
> > +
> > +static size_t tls_get_info_size(struct sock *sk)
> > +{
> > + size_t size = 0;
> > +
> > + if (sk->sk_state != TCP_ESTABLISHED)
> > + return size;
> > +
> > + size += nla_total_size(0) /* ULP_INFO_TLS */
> > + + nla_total_size(sizeof(__u16)) /* TLS_INFO_VERSION */
> > + + nla_total_size(sizeof(__u16)); /* TLS_INFO_CIPHER */
> > + return size;
> > +}
>
> Same comments as on patch 1 and above.
sure, ok.
> > void tls_register_device(struct tls_device *device)
> > {
> > spin_lock_bh(&device_spinlock);
>
> Thanks for working on this, it was on my todo list! :)
thanks for the review!
--
davide
^ permalink raw reply
* Re: [PATCH] net: lio_core: fix potential sign-extension overflow on large shift
From: Dan Carpenter @ 2019-06-17 16:06 UTC (permalink / raw)
To: Colin King
Cc: Derek Chickles, Satanand Burla, Felix Manlunas, David S . Miller,
netdev, kernel-janitors, linux-kernel
In-Reply-To: <20190617155325.27017-1-colin.king@canonical.com>
On Mon, Jun 17, 2019 at 04:53:25PM +0100, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> Left shifting the signed int value 1 by 31 bits has undefined behaviour
> and the shift amount oq_no can be as much as 63. Fix this by widening
> the int 1 to 1ULL.
>
> Addresses-Coverity: ("Bad shift operation")
> Fixes: f21fb3ed364b ("Add support of Cavium Liquidio ethernet adapters")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
> drivers/net/ethernet/cavium/liquidio/lio_core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/cavium/liquidio/lio_core.c b/drivers/net/ethernet/cavium/liquidio/lio_core.c
> index 1c50c10b5a16..e78bdcee200f 100644
> --- a/drivers/net/ethernet/cavium/liquidio/lio_core.c
> +++ b/drivers/net/ethernet/cavium/liquidio/lio_core.c
> @@ -964,7 +964,7 @@ static void liquidio_schedule_droq_pkt_handlers(struct octeon_device *oct)
>
> if (droq->ops.poll_mode) {
> droq->ops.napi_fn(droq);
> - oct_priv->napi_mask |= (1 << oq_no);
> + oct_priv->napi_mask |= (1ULL << oq_no);
The function uses BIT_ULL(oq_no) earlier, so we should probably do the
same here.
regards,
dan carpenter
^ permalink raw reply
* [PATCH][V2] net: lio_core: fix potential sign-extension overflow on large shift
From: Colin King @ 2019-06-17 16:12 UTC (permalink / raw)
To: Derek Chickles, Satanand Burla, Felix Manlunas, David S . Miller,
netdev
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
Left shifting the signed int value 1 by 31 bits has undefined behaviour
and the shift amount oq_no can be as much as 63. Fix this by using
BIT_ULL(oq_no) instead.
Addresses-Coverity: ("Bad shift operation")
Fixes: f21fb3ed364b ("Add support of Cavium Liquidio ethernet adapters")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
V2: Use BIT_ULL(oq_no) instead of 1ULL << oq_no. Thanks to Dan Carpenter for
noting this is more appropriate.
---
drivers/net/ethernet/cavium/liquidio/lio_core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/cavium/liquidio/lio_core.c b/drivers/net/ethernet/cavium/liquidio/lio_core.c
index 1c50c10b5a16..d7e805749a5b 100644
--- a/drivers/net/ethernet/cavium/liquidio/lio_core.c
+++ b/drivers/net/ethernet/cavium/liquidio/lio_core.c
@@ -964,7 +964,7 @@ static void liquidio_schedule_droq_pkt_handlers(struct octeon_device *oct)
if (droq->ops.poll_mode) {
droq->ops.napi_fn(droq);
- oct_priv->napi_mask |= (1 << oq_no);
+ oct_priv->napi_mask |= BIT_ULL(oq_no);
} else {
tasklet_schedule(&oct_priv->droq_tasklet);
}
--
2.20.1
^ permalink raw reply related
* Re: [PATCH net-next 0/2] net: ipv4: remove erroneous advancement of list pointer
From: Tariq Toukan @ 2019-06-17 16:16 UTC (permalink / raw)
To: Florian Westphal, netdev@vger.kernel.org
Cc: Ran Rozenstein, Maor Gottlieb, edumazet@google.com
In-Reply-To: <20190617140228.12523-1-fw@strlen.de>
On 6/17/2019 5:02 PM, Florian Westphal wrote:
> Tariq reported a soft lockup on net-next that Mellanox was able to
> bisect to 2638eb8b50cf ("net: ipv4: provide __rcu annotation for ifa_list").
>
> While reviewing above patch I found a regression when addresses have a
> lifetime specified.
>
> Second patch extends rtnetlink.sh to trigger crash
> (without first patch applied).
>
Thanks Florian.
Ran, can you please test?
^ permalink raw reply
* Re: [PATCH v3 bpf-next 0/9] bpf: bounded loops and other features
From: Andrii Nakryiko @ 2019-06-17 16:39 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: David S. Miller, Daniel Borkmann, Networking, bpf, Kernel Team
In-Reply-To: <20190615191225.2409862-1-ast@kernel.org>
On Sat, Jun 15, 2019 at 12:12 PM Alexei Starovoitov <ast@kernel.org> wrote:
>
> v2->v3: fixed issues in backtracking pointed out by Andrii.
> The next step is to add a lot more tests for backtracking.
>
Tests would be great, verifier complexity is at the level, where it's
very easy to miss issues.
Was fuzzying approach ever discussed for BPF verifier? I.e., have a
fuzzer to generate both legal and illegal random small programs. Then
re-implement verifier as user-level program with straightforward
recursive exhaustive verification (so no state pruning logic, no
precise/coarse, etc, just register/stack state tracking) of all
possible branches. If kernel verifier's verdict differs from
user-level verifier's verdict - flag that as a test case and figure
out why they differ. Obviously that would work well only for small
programs, but that should be a good first step already.
In addition, if this is done, that user-land verifier can be a HUGE
help to BPF application developers, as libbpf would (potentially) be
able to generate better error messages using it as well.
> v1->v2: addressed Andrii's feedback.
>
> this patch set introduces verifier support for bounded loops and
> adds several other improvements.
> Ideally they would be introduced one at a time,
> but to support bounded loop the verifier needs to 'step back'
> in the patch 1. That patch introduces tracking of spill/fill
> of constants through the stack. Though it's a useful feature
> it hurts cilium tests.
> Patch 3 introduces another feature by extending is_branch_taken
> logic to 'if rX op rY' conditions. This feature is also
> necessary to support bounded loops.
> Then patch 4 adds support for the loops while adding
> key heuristics with jmp_processed.
> Introduction of parentage chain of verifier states in patch 4
> allows patch 9 to add backtracking of precise scalar registers
> which finally resolves degradation from patch 1.
>
> The end result is much faster verifier for existing programs
> and new support for loops.
> See patch 8 for many kinds of loops that are now validated.
> Patch 9 is the most tricky one and could be rewritten with
> a different algorithm in the future.
>
> Alexei Starovoitov (9):
> bpf: track spill/fill of constants
> selftests/bpf: fix tests due to const spill/fill
> bpf: extend is_branch_taken to registers
> bpf: introduce bounded loops
> bpf: fix callees pruning callers
> selftests/bpf: fix tests
> selftests/bpf: add basic verifier tests for loops
> selftests/bpf: add realistic loop tests
> bpf: precise scalar_value tracking
>
> include/linux/bpf_verifier.h | 69 +-
> kernel/bpf/verifier.c | 767 ++++++++++++++++--
> .../bpf/prog_tests/bpf_verif_scale.c | 67 +-
> tools/testing/selftests/bpf/progs/loop1.c | 28 +
> tools/testing/selftests/bpf/progs/loop2.c | 28 +
> tools/testing/selftests/bpf/progs/loop3.c | 22 +
> tools/testing/selftests/bpf/progs/pyperf.h | 6 +-
> tools/testing/selftests/bpf/progs/pyperf600.c | 9 +
> .../selftests/bpf/progs/pyperf600_nounroll.c | 8 +
> .../testing/selftests/bpf/progs/strobemeta.c | 10 +
> .../testing/selftests/bpf/progs/strobemeta.h | 528 ++++++++++++
> .../bpf/progs/strobemeta_nounroll1.c | 9 +
> .../bpf/progs/strobemeta_nounroll2.c | 9 +
> .../selftests/bpf/progs/test_seg6_loop.c | 261 ++++++
> .../selftests/bpf/progs/test_sysctl_loop1.c | 71 ++
> .../selftests/bpf/progs/test_sysctl_loop2.c | 72 ++
> .../selftests/bpf/progs/test_xdp_loop.c | 231 ++++++
> tools/testing/selftests/bpf/test_verifier.c | 11 +-
> tools/testing/selftests/bpf/verifier/calls.c | 22 +-
> tools/testing/selftests/bpf/verifier/cfg.c | 11 +-
> .../bpf/verifier/direct_packet_access.c | 3 +-
> .../bpf/verifier/helper_access_var_len.c | 28 +-
> tools/testing/selftests/bpf/verifier/loops1.c | 161 ++++
> 23 files changed, 2317 insertions(+), 114 deletions(-)
> create mode 100644 tools/testing/selftests/bpf/progs/loop1.c
> create mode 100644 tools/testing/selftests/bpf/progs/loop2.c
> create mode 100644 tools/testing/selftests/bpf/progs/loop3.c
> create mode 100644 tools/testing/selftests/bpf/progs/pyperf600.c
> create mode 100644 tools/testing/selftests/bpf/progs/pyperf600_nounroll.c
> create mode 100644 tools/testing/selftests/bpf/progs/strobemeta.c
> create mode 100644 tools/testing/selftests/bpf/progs/strobemeta.h
> create mode 100644 tools/testing/selftests/bpf/progs/strobemeta_nounroll1.c
> create mode 100644 tools/testing/selftests/bpf/progs/strobemeta_nounroll2.c
> create mode 100644 tools/testing/selftests/bpf/progs/test_seg6_loop.c
> create mode 100644 tools/testing/selftests/bpf/progs/test_sysctl_loop1.c
> create mode 100644 tools/testing/selftests/bpf/progs/test_sysctl_loop2.c
> create mode 100644 tools/testing/selftests/bpf/progs/test_xdp_loop.c
> create mode 100644 tools/testing/selftests/bpf/verifier/loops1.c
>
> --
> 2.20.0
>
^ permalink raw reply
* RE: [PATCH net-next 01/16] qlge: Remove irq_cnt
From: Manish Chopra @ 2019-06-17 16:49 UTC (permalink / raw)
To: Benjamin Poirier, GR-Linux-NIC-Dev, netdev@vger.kernel.org
In-Reply-To: <20190617074858.32467-1-bpoirier@suse.com>
> -----Original Message-----
> From: Benjamin Poirier <bpoirier@suse.com>
> Sent: Monday, June 17, 2019 1:19 PM
> To: Manish Chopra <manishc@marvell.com>; GR-Linux-NIC-Dev <GR-Linux-
> NIC-Dev@marvell.com>; netdev@vger.kernel.org
> Subject: [PATCH net-next 01/16] qlge: Remove irq_cnt
>
> qlge uses an irq enable/disable refcounting scheme that is:
> * poorly implemented
> Uses a spin_lock to protect accesses to the irq_cnt atomic variable
> * buggy
> Breaks when there is not a 1:1 sequence of irq - napi_poll, such as
> when using SO_BUSY_POLL.
> * unnecessary
> The purpose or irq_cnt is to reduce irq control writes when
> multiple work items result from one irq: the irq is re-enabled
> after all work is done.
> Analysis of the irq handler shows that there is only one case where
> there might be two workers scheduled at once, and those have
> separate irq masking bits.
>
> Therefore, remove irq_cnt.
>
> Additionally, we get a performance improvement:
> perf stat -e cycles -a -r5 super_netperf 100 -H 192.168.33.1 -t TCP_RR
>
> Before:
> 628560
> 628056
> 622103
> 622744
> 627202
> [...]
> 268,803,947,669 cycles ( +- 0.09% )
>
> After:
> 636300
> 634106
> 634984
> 638555
> 634188
> [...]
> 259,237,291,449 cycles ( +- 0.19% )
>
> Signed-off-by: Benjamin Poirier <bpoirier@suse.com>
> ---
> drivers/net/ethernet/qlogic/qlge/qlge.h | 7 --
> drivers/net/ethernet/qlogic/qlge/qlge_main.c | 98 ++++++--------------
> drivers/net/ethernet/qlogic/qlge/qlge_mpi.c | 1 -
> 3 files changed, 27 insertions(+), 79 deletions(-)
>
> diff --git a/drivers/net/ethernet/qlogic/qlge/qlge.h
> b/drivers/net/ethernet/qlogic/qlge/qlge.h
> index ad7c5eb8a3b6..5d9a36deda08 100644
> --- a/drivers/net/ethernet/qlogic/qlge/qlge.h
> +++ b/drivers/net/ethernet/qlogic/qlge/qlge.h
> @@ -1982,11 +1982,6 @@ struct intr_context {
> u32 intr_dis_mask; /* value/mask used to disable this intr */
> u32 intr_read_mask; /* value/mask used to read this intr */
> char name[IFNAMSIZ * 2];
> - atomic_t irq_cnt; /* irq_cnt is used in single vector
> - * environment. It's incremented for each
> - * irq handler that is scheduled. When each
> - * handler finishes it decrements irq_cnt and
> - * enables interrupts if it's zero. */
> irq_handler_t handler;
> };
>
> @@ -2074,7 +2069,6 @@ struct ql_adapter {
> u32 port; /* Port number this adapter */
>
> spinlock_t adapter_lock;
> - spinlock_t hw_lock;
> spinlock_t stats_lock;
>
> /* PCI Bus Relative Register Addresses */ @@ -2235,7 +2229,6 @@
> void ql_mpi_reset_work(struct work_struct *work); void
> ql_mpi_core_to_log(struct work_struct *work); int ql_wait_reg_rdy(struct
> ql_adapter *qdev, u32 reg, u32 bit, u32 ebit); void
> ql_queue_asic_error(struct ql_adapter *qdev);
> -u32 ql_enable_completion_interrupt(struct ql_adapter *qdev, u32 intr);
> void ql_set_ethtool_ops(struct net_device *ndev); int
> ql_read_xgmac_reg64(struct ql_adapter *qdev, u32 reg, u64 *data); void
> ql_mpi_idc_work(struct work_struct *work); diff --git
> a/drivers/net/ethernet/qlogic/qlge/qlge_main.c
> b/drivers/net/ethernet/qlogic/qlge/qlge_main.c
> index 6cae33072496..0bfbe11db795 100644
> --- a/drivers/net/ethernet/qlogic/qlge/qlge_main.c
> +++ b/drivers/net/ethernet/qlogic/qlge/qlge_main.c
> @@ -625,75 +625,26 @@ static void ql_disable_interrupts(struct ql_adapter
> *qdev)
> ql_write32(qdev, INTR_EN, (INTR_EN_EI << 16)); }
>
> -/* If we're running with multiple MSI-X vectors then we enable on the fly.
> - * Otherwise, we may have multiple outstanding workers and don't want to
> - * enable until the last one finishes. In this case, the irq_cnt gets
> - * incremented every time we queue a worker and decremented every time
> - * a worker finishes. Once it hits zero we enable the interrupt.
> - */
> -u32 ql_enable_completion_interrupt(struct ql_adapter *qdev, u32 intr)
> +static void ql_enable_completion_interrupt(struct ql_adapter *qdev, u32
> +intr)
> {
> - u32 var = 0;
> - unsigned long hw_flags = 0;
> - struct intr_context *ctx = qdev->intr_context + intr;
> -
> - if (likely(test_bit(QL_MSIX_ENABLED, &qdev->flags) && intr)) {
> - /* Always enable if we're MSIX multi interrupts and
> - * it's not the default (zeroeth) interrupt.
> - */
> - ql_write32(qdev, INTR_EN,
> - ctx->intr_en_mask);
> - var = ql_read32(qdev, STS);
> - return var;
> - }
> + struct intr_context *ctx = &qdev->intr_context[intr];
>
> - spin_lock_irqsave(&qdev->hw_lock, hw_flags);
> - if (atomic_dec_and_test(&ctx->irq_cnt)) {
> - ql_write32(qdev, INTR_EN,
> - ctx->intr_en_mask);
> - var = ql_read32(qdev, STS);
> - }
> - spin_unlock_irqrestore(&qdev->hw_lock, hw_flags);
> - return var;
> + ql_write32(qdev, INTR_EN, ctx->intr_en_mask);
> }
>
> -static u32 ql_disable_completion_interrupt(struct ql_adapter *qdev, u32
> intr)
> +static void ql_disable_completion_interrupt(struct ql_adapter *qdev,
> +u32 intr)
> {
> - u32 var = 0;
> - struct intr_context *ctx;
> + struct intr_context *ctx = &qdev->intr_context[intr];
>
> - /* HW disables for us if we're MSIX multi interrupts and
> - * it's not the default (zeroeth) interrupt.
> - */
> - if (likely(test_bit(QL_MSIX_ENABLED, &qdev->flags) && intr))
> - return 0;
> -
> - ctx = qdev->intr_context + intr;
> - spin_lock(&qdev->hw_lock);
> - if (!atomic_read(&ctx->irq_cnt)) {
> - ql_write32(qdev, INTR_EN,
> - ctx->intr_dis_mask);
> - var = ql_read32(qdev, STS);
> - }
> - atomic_inc(&ctx->irq_cnt);
> - spin_unlock(&qdev->hw_lock);
> - return var;
> + ql_write32(qdev, INTR_EN, ctx->intr_dis_mask);
> }
>
> static void ql_enable_all_completion_interrupts(struct ql_adapter *qdev) {
> int i;
> - for (i = 0; i < qdev->intr_count; i++) {
> - /* The enable call does a atomic_dec_and_test
> - * and enables only if the result is zero.
> - * So we precharge it here.
> - */
> - if (unlikely(!test_bit(QL_MSIX_ENABLED, &qdev->flags) ||
> - i == 0))
> - atomic_set(&qdev->intr_context[i].irq_cnt, 1);
> - ql_enable_completion_interrupt(qdev, i);
> - }
>
> + for (i = 0; i < qdev->intr_count; i++)
> + ql_enable_completion_interrupt(qdev, i);
> }
>
> static int ql_validate_flash(struct ql_adapter *qdev, u32 size, const char
> *str) @@ -2500,21 +2451,22 @@ static irqreturn_t qlge_isr(int irq, void
> *dev_id)
> u32 var;
> int work_done = 0;
>
> - spin_lock(&qdev->hw_lock);
> - if (atomic_read(&qdev->intr_context[0].irq_cnt)) {
> - netif_printk(qdev, intr, KERN_DEBUG, qdev->ndev,
> - "Shared Interrupt, Not ours!\n");
> - spin_unlock(&qdev->hw_lock);
> - return IRQ_NONE;
> - }
> - spin_unlock(&qdev->hw_lock);
> + /* Experience shows that when using INTx interrupts, the device
> does
> + * not always auto-mask the interrupt.
> + * When using MSI mode, the interrupt must be explicitly disabled
> + * (even though it is auto-masked), otherwise a later command to
> + * enable it is not effective.
> + */
> + if (!test_bit(QL_MSIX_ENABLED, &qdev->flags))
> + ql_disable_completion_interrupt(qdev, 0);
>
> - var = ql_disable_completion_interrupt(qdev, intr_context->intr);
> + var = ql_read32(qdev, STS);
>
> /*
> * Check for fatal error.
> */
> if (var & STS_FE) {
> + ql_disable_completion_interrupt(qdev, 0);
> ql_queue_asic_error(qdev);
> netdev_err(qdev->ndev, "Got fatal error, STS = %x.\n", var);
> var = ql_read32(qdev, ERR_STS);
> @@ -2534,7 +2486,6 @@ static irqreturn_t qlge_isr(int irq, void *dev_id)
> */
> netif_err(qdev, intr, qdev->ndev,
> "Got MPI processor interrupt.\n");
> - ql_disable_completion_interrupt(qdev, intr_context->intr);
> ql_write32(qdev, INTR_MASK, (INTR_MASK_PI << 16));
> queue_delayed_work_on(smp_processor_id(),
> qdev->workqueue, &qdev->mpi_work, 0);
> @@ -2550,11 +2501,18 @@ static irqreturn_t qlge_isr(int irq, void *dev_id)
> if (var & intr_context->irq_mask) {
> netif_info(qdev, intr, qdev->ndev,
> "Waking handler for rx_ring[0].\n");
> - ql_disable_completion_interrupt(qdev, intr_context->intr);
> napi_schedule(&rx_ring->napi);
> work_done++;
> + } else {
> + /* Experience shows that the device sometimes signals an
> + * interrupt but no work is scheduled from this function.
> + * Nevertheless, the interrupt is auto-masked. Therefore, we
> + * systematically re-enable the interrupt if we didn't
> + * schedule napi.
> + */
> + ql_enable_completion_interrupt(qdev, 0);
> }
> - ql_enable_completion_interrupt(qdev, intr_context->intr);
> +
> return work_done ? IRQ_HANDLED : IRQ_NONE; }
>
> @@ -3557,7 +3515,6 @@ static int ql_request_irq(struct ql_adapter *qdev)
> ql_resolve_queues_to_irqs(qdev);
>
> for (i = 0; i < qdev->intr_count; i++, intr_context++) {
> - atomic_set(&intr_context->irq_cnt, 0);
> if (test_bit(QL_MSIX_ENABLED, &qdev->flags)) {
> status = request_irq(qdev->msi_x_entry[i].vector,
> intr_context->handler,
> @@ -4642,7 +4599,6 @@ static int ql_init_device(struct pci_dev *pdev,
> struct net_device *ndev,
> goto err_out2;
> }
> qdev->msg_enable = netif_msg_init(debug, default_msg);
> - spin_lock_init(&qdev->hw_lock);
> spin_lock_init(&qdev->stats_lock);
>
> if (qlge_mpi_coredump) {
> diff --git a/drivers/net/ethernet/qlogic/qlge/qlge_mpi.c
> b/drivers/net/ethernet/qlogic/qlge/qlge_mpi.c
> index 957c72985a06..9e422bbbb6ab 100644
> --- a/drivers/net/ethernet/qlogic/qlge/qlge_mpi.c
> +++ b/drivers/net/ethernet/qlogic/qlge/qlge_mpi.c
> @@ -1257,7 +1257,6 @@ void ql_mpi_work(struct work_struct *work)
> /* End polled mode for MPI */
> ql_write32(qdev, INTR_MASK, (INTR_MASK_PI << 16) |
> INTR_MASK_PI);
> mutex_unlock(&qdev->mpi_mutex);
> - ql_enable_completion_interrupt(qdev, 0);
> }
>
> void ql_mpi_reset_work(struct work_struct *work)
> --
> 2.21.0
Hello Benjamin,
Just FYI. I am OOO for a week, so reviewing and testing these patches will take time.
Thanks,
Manish
^ permalink raw reply
* [PATCH] net: stmmac: add sanity check to device_property_read_u32_array call
From: Colin King @ 2019-06-17 16:58 UTC (permalink / raw)
To: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
David S . Miller, Maxime Coquelin, netdev, linux-stm32,
linux-arm-kernel
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
Currently the call to device_property_read_u32_array is not error checked
leading to potential garbage values in the delays array that are then used
in msleep delays. Add a sanity check to the property fetching.
Addresses-Coverity: ("Uninitialized scalar variable")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
index da310de06bf6..5b7923c0698c 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
@@ -242,6 +242,7 @@ int stmmac_mdio_reset(struct mii_bus *bus)
if (priv->device->of_node) {
struct gpio_desc *reset_gpio;
u32 delays[3];
+ int ret;
reset_gpio = devm_gpiod_get_optional(priv->device,
"snps,reset",
@@ -249,9 +250,15 @@ int stmmac_mdio_reset(struct mii_bus *bus)
if (IS_ERR(reset_gpio))
return PTR_ERR(reset_gpio);
- device_property_read_u32_array(priv->device,
- "snps,reset-delays-us",
- delays, ARRAY_SIZE(delays));
+ ret = device_property_read_u32_array(priv->device,
+ "snps,reset-delays-us",
+ delays,
+ ARRAY_SIZE(delays));
+ if (ret) {
+ dev_err(ndev->dev.parent,
+ "invalid property snps,reset-delays-us\n");
+ return -EINVAL;
+ }
if (delays[0])
msleep(DIV_ROUND_UP(delays[0], 1000));
--
2.20.1
^ permalink raw reply related
* Re: [PATCH v3] net: ipv4: move tcp_fastopen server side code to SipHash library
From: Eric Dumazet @ 2019-06-17 17:00 UTC (permalink / raw)
To: Ard Biesheuvel, netdev
Cc: linux-crypto, herbert, ebiggers, edumazet, davem, kuznet,
yoshfuji, jbaron, cpaasch, David.Laight, ycheng
In-Reply-To: <20190617080933.32152-1-ard.biesheuvel@linaro.org>
On 6/17/19 1:09 AM, Ard Biesheuvel wrote:
> Using a bare block cipher in non-crypto code is almost always a bad idea,
> not only for security reasons (and we've seen some examples of this in
> the kernel in the past), but also for performance reasons.
>
> In the TCP fastopen case, we call into the bare AES block cipher one or
> two times (depending on whether the connection is IPv4 or IPv6). On most
> systems, this results in a call chain such as
>
> crypto_cipher_encrypt_one(ctx, dst, src)
> crypto_cipher_crt(tfm)->cit_encrypt_one(crypto_cipher_tfm(tfm), ...);
> aesni_encrypt
> kernel_fpu_begin();
> aesni_enc(ctx, dst, src); // asm routine
> kernel_fpu_end();
>
> It is highly unlikely that the use of special AES instructions has a
> benefit in this case, especially since we are doing the above twice
> for IPv6 connections, instead of using a transform which can process
> the entire input in one go.
>
> We could switch to the cbcmac(aes) shash, which would at least get
> rid of the duplicated overhead in *some* cases (i.e., today, only
> arm64 has an accelerated implementation of cbcmac(aes), while x86 will
> end up using the generic cbcmac template wrapping the AES-NI cipher,
> which basically ends up doing exactly the above). However, in the given
> context, it makes more sense to use a light-weight MAC algorithm that
> is more suitable for the purpose at hand, such as SipHash.
>
> Since the output size of SipHash already matches our chosen value for
> TCP_FASTOPEN_COOKIE_SIZE, and given that it accepts arbitrary input
> sizes, this greatly simplifies the code as well.
>
> NOTE: Server farms backing a single server IP for load balancing purposes
> and sharing a single fastopen key will be adversely affected by
> this change unless all systems in the pool receive their kernel
> upgrades at the same time.
>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
All our fastopen packetdrill tests pass (after I changed all the cookie values in them)
Signed-off-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply
* [PATCH net 0/4] tcp: make sack processing more robust
From: Eric Dumazet @ 2019-06-17 17:03 UTC (permalink / raw)
To: David S . Miller
Cc: netdev, Eric Dumazet, Eric Dumazet, Greg Kroah-Hartman,
Jonathan Looney, Neal Cardwell, Tyler Hicks, Yuchung Cheng,
Bruce Curtis, Jonathan Lemon
Jonathan Looney brought to our attention multiple problems
in TCP stack at the sender side.
SACK processing can be abused by malicious peers to either
cause overflows, or increase of memory usage.
First two patches fix the immediate problems.
Since the malicious peers abuse senders by advertizing a very
small MSS in their SYN or SYNACK packet, the last two
patches add a new sysctl so that admins can chose a higher
limit for MSS clamping.
Eric Dumazet (4):
tcp: limit payload size of sacked skbs
tcp: tcp_fragment() should apply sane memory limits
tcp: add tcp_min_snd_mss sysctl
tcp: enforce tcp_min_snd_mss in tcp_mtu_probing()
Documentation/networking/ip-sysctl.txt | 8 ++++++++
include/linux/tcp.h | 4 ++++
include/net/netns/ipv4.h | 1 +
include/net/tcp.h | 2 ++
include/uapi/linux/snmp.h | 1 +
net/ipv4/proc.c | 1 +
net/ipv4/sysctl_net_ipv4.c | 11 +++++++++++
net/ipv4/tcp.c | 1 +
net/ipv4/tcp_input.c | 26 ++++++++++++++++++++------
net/ipv4/tcp_ipv4.c | 1 +
net/ipv4/tcp_output.c | 10 +++++++---
net/ipv4/tcp_timer.c | 1 +
12 files changed, 58 insertions(+), 9 deletions(-)
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply
* [PATCH net 1/4] tcp: limit payload size of sacked skbs
From: Eric Dumazet @ 2019-06-17 17:03 UTC (permalink / raw)
To: David S . Miller
Cc: netdev, Eric Dumazet, Eric Dumazet, Greg Kroah-Hartman,
Jonathan Looney, Neal Cardwell, Tyler Hicks, Yuchung Cheng,
Bruce Curtis, Jonathan Lemon
In-Reply-To: <20190617170354.37770-1-edumazet@google.com>
Jonathan Looney reported that TCP can trigger the following crash
in tcp_shifted_skb() :
BUG_ON(tcp_skb_pcount(skb) < pcount);
This can happen if the remote peer has advertized the smallest
MSS that linux TCP accepts : 48
An skb can hold 17 fragments, and each fragment can hold 32KB
on x86, or 64KB on PowerPC.
This means that the 16bit witdh of TCP_SKB_CB(skb)->tcp_gso_segs
can overflow.
Note that tcp_sendmsg() builds skbs with less than 64KB
of payload, so this problem needs SACK to be enabled.
SACK blocks allow TCP to coalesce multiple skbs in the retransmit
queue, thus filling the 17 fragments to maximal capacity.
CVE-2019-11477 -- u16 overflow of TCP_SKB_CB(skb)->tcp_gso_segs
Fixes: 832d11c5cd07 ("tcp: Try to restore large SKBs while SACK processing")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: Jonathan Looney <jtl@netflix.com>
Acked-by: Neal Cardwell <ncardwell@google.com>
Reviewed-by: Tyler Hicks <tyhicks@canonical.com>
Cc: Yuchung Cheng <ycheng@google.com>
Cc: Bruce Curtis <brucec@netflix.com>
Cc: Jonathan Lemon <jonathan.lemon@gmail.com>
---
include/linux/tcp.h | 4 ++++
include/net/tcp.h | 2 ++
net/ipv4/tcp.c | 1 +
net/ipv4/tcp_input.c | 26 ++++++++++++++++++++------
net/ipv4/tcp_output.c | 6 +++---
5 files changed, 30 insertions(+), 9 deletions(-)
diff --git a/include/linux/tcp.h b/include/linux/tcp.h
index 711361af9ce019f08c8b6accc33220b673b34d56..9a478a0cd3a20b40ed344f178e35228a0b8ee203 100644
--- a/include/linux/tcp.h
+++ b/include/linux/tcp.h
@@ -484,4 +484,8 @@ static inline u16 tcp_mss_clamp(const struct tcp_sock *tp, u16 mss)
return (user_mss && user_mss < mss) ? user_mss : mss;
}
+
+int tcp_skb_shift(struct sk_buff *to, struct sk_buff *from, int pcount,
+ int shiftlen);
+
#endif /* _LINUX_TCP_H */
diff --git a/include/net/tcp.h b/include/net/tcp.h
index ac2f53fbfa6b4cbf1fc615c952a5e1cac1124300..582c0caa98116740b5bde8c5dbb5d94fc69d1caa 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -51,6 +51,8 @@ void tcp_time_wait(struct sock *sk, int state, int timeo);
#define MAX_TCP_HEADER (128 + MAX_HEADER)
#define MAX_TCP_OPTION_SPACE 40
+#define TCP_MIN_SND_MSS 48
+#define TCP_MIN_GSO_SIZE (TCP_MIN_SND_MSS - MAX_TCP_OPTION_SPACE)
/*
* Never offer a window over 32767 without using window scaling. Some
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index f448a288d158c4baa6d8d5ed82c4b129404233a1..7dc9ab84bb69aa90953e98f9763287fcee3a1659 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -3873,6 +3873,7 @@ void __init tcp_init(void)
unsigned long limit;
unsigned int i;
+ BUILD_BUG_ON(TCP_MIN_SND_MSS <= MAX_TCP_OPTION_SPACE);
BUILD_BUG_ON(sizeof(struct tcp_skb_cb) >
FIELD_SIZEOF(struct sk_buff, cb));
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 38dfc308c0fb9832facadb0aeec8f3e4931901f4..d95ee40df6c2b020d590018bc41833b8a6aefa4a 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -1302,7 +1302,7 @@ static bool tcp_shifted_skb(struct sock *sk, struct sk_buff *prev,
TCP_SKB_CB(skb)->seq += shifted;
tcp_skb_pcount_add(prev, pcount);
- BUG_ON(tcp_skb_pcount(skb) < pcount);
+ WARN_ON_ONCE(tcp_skb_pcount(skb) < pcount);
tcp_skb_pcount_add(skb, -pcount);
/* When we're adding to gso_segs == 1, gso_size will be zero,
@@ -1368,6 +1368,21 @@ static int skb_can_shift(const struct sk_buff *skb)
return !skb_headlen(skb) && skb_is_nonlinear(skb);
}
+int tcp_skb_shift(struct sk_buff *to, struct sk_buff *from,
+ int pcount, int shiftlen)
+{
+ /* TCP min gso_size is 8 bytes (TCP_MIN_GSO_SIZE)
+ * Since TCP_SKB_CB(skb)->tcp_gso_segs is 16 bits, we need
+ * to make sure not storing more than 65535 * 8 bytes per skb,
+ * even if current MSS is bigger.
+ */
+ if (unlikely(to->len + shiftlen >= 65535 * TCP_MIN_GSO_SIZE))
+ return 0;
+ if (unlikely(tcp_skb_pcount(to) + pcount > 65535))
+ return 0;
+ return skb_shift(to, from, shiftlen);
+}
+
/* Try collapsing SACK blocks spanning across multiple skbs to a single
* skb.
*/
@@ -1473,7 +1488,7 @@ static struct sk_buff *tcp_shift_skb_data(struct sock *sk, struct sk_buff *skb,
if (!after(TCP_SKB_CB(skb)->seq + len, tp->snd_una))
goto fallback;
- if (!skb_shift(prev, skb, len))
+ if (!tcp_skb_shift(prev, skb, pcount, len))
goto fallback;
if (!tcp_shifted_skb(sk, prev, skb, state, pcount, len, mss, dup_sack))
goto out;
@@ -1491,11 +1506,10 @@ static struct sk_buff *tcp_shift_skb_data(struct sock *sk, struct sk_buff *skb,
goto out;
len = skb->len;
- if (skb_shift(prev, skb, len)) {
- pcount += tcp_skb_pcount(skb);
- tcp_shifted_skb(sk, prev, skb, state, tcp_skb_pcount(skb),
+ pcount = tcp_skb_pcount(skb);
+ if (tcp_skb_shift(prev, skb, pcount, len))
+ tcp_shifted_skb(sk, prev, skb, state, pcount,
len, mss, 0);
- }
out:
return prev;
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index f429e856e2631a9e6de1d2e060406742f97e538e..b8e3bbb852117459d131fbb41d69ae63bd251a3e 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -1454,8 +1454,8 @@ static inline int __tcp_mtu_to_mss(struct sock *sk, int pmtu)
mss_now -= icsk->icsk_ext_hdr_len;
/* Then reserve room for full set of TCP options and 8 bytes of data */
- if (mss_now < 48)
- mss_now = 48;
+ if (mss_now < TCP_MIN_SND_MSS)
+ mss_now = TCP_MIN_SND_MSS;
return mss_now;
}
@@ -2747,7 +2747,7 @@ static bool tcp_collapse_retrans(struct sock *sk, struct sk_buff *skb)
if (next_skb_size <= skb_availroom(skb))
skb_copy_bits(next_skb, 0, skb_put(skb, next_skb_size),
next_skb_size);
- else if (!skb_shift(skb, next_skb, next_skb_size))
+ else if (!tcp_skb_shift(skb, next_skb, 1, next_skb_size))
return false;
}
tcp_highest_sack_replace(sk, next_skb, skb);
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH net 2/4] tcp: tcp_fragment() should apply sane memory limits
From: Eric Dumazet @ 2019-06-17 17:03 UTC (permalink / raw)
To: David S . Miller
Cc: netdev, Eric Dumazet, Eric Dumazet, Greg Kroah-Hartman,
Jonathan Looney, Neal Cardwell, Tyler Hicks, Yuchung Cheng,
Bruce Curtis, Jonathan Lemon
In-Reply-To: <20190617170354.37770-1-edumazet@google.com>
Jonathan Looney reported that a malicious peer can force a sender
to fragment its retransmit queue into tiny skbs, inflating memory
usage and/or overflow 32bit counters.
TCP allows an application to queue up to sk_sndbuf bytes,
so we need to give some allowance for non malicious splitting
of retransmit queue.
A new SNMP counter is added to monitor how many times TCP
did not allow to split an skb if the allowance was exceeded.
Note that this counter might increase in the case applications
use SO_SNDBUF socket option to lower sk_sndbuf.
CVE-2019-11478 : tcp_fragment, prevent fragmenting a packet when the
socket is already using more than half the allowed space
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: Jonathan Looney <jtl@netflix.com>
Acked-by: Neal Cardwell <ncardwell@google.com>
Acked-by: Yuchung Cheng <ycheng@google.com>
Reviewed-by: Tyler Hicks <tyhicks@canonical.com>
Cc: Bruce Curtis <brucec@netflix.com>
Cc: Jonathan Lemon <jonathan.lemon@gmail.com>
---
include/uapi/linux/snmp.h | 1 +
net/ipv4/proc.c | 1 +
net/ipv4/tcp_output.c | 5 +++++
3 files changed, 7 insertions(+)
diff --git a/include/uapi/linux/snmp.h b/include/uapi/linux/snmp.h
index 86dc24a96c90ab047d5173d625450facd6c6dd79..fd42c1316d3d112ecd8a00d2b499d6f6901c5e81 100644
--- a/include/uapi/linux/snmp.h
+++ b/include/uapi/linux/snmp.h
@@ -283,6 +283,7 @@ enum
LINUX_MIB_TCPACKCOMPRESSED, /* TCPAckCompressed */
LINUX_MIB_TCPZEROWINDOWDROP, /* TCPZeroWindowDrop */
LINUX_MIB_TCPRCVQDROP, /* TCPRcvQDrop */
+ LINUX_MIB_TCPWQUEUETOOBIG, /* TCPWqueueTooBig */
__LINUX_MIB_MAX
};
diff --git a/net/ipv4/proc.c b/net/ipv4/proc.c
index 4370f4246e86dfe06a9e07cace848baeaf6cc4da..073273b751f8fcda1c9c79cd1ab566f2939b2517 100644
--- a/net/ipv4/proc.c
+++ b/net/ipv4/proc.c
@@ -287,6 +287,7 @@ static const struct snmp_mib snmp4_net_list[] = {
SNMP_MIB_ITEM("TCPAckCompressed", LINUX_MIB_TCPACKCOMPRESSED),
SNMP_MIB_ITEM("TCPZeroWindowDrop", LINUX_MIB_TCPZEROWINDOWDROP),
SNMP_MIB_ITEM("TCPRcvQDrop", LINUX_MIB_TCPRCVQDROP),
+ SNMP_MIB_ITEM("TCPWqueueTooBig", LINUX_MIB_TCPWQUEUETOOBIG),
SNMP_MIB_SENTINEL
};
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index b8e3bbb852117459d131fbb41d69ae63bd251a3e..1bb1c46b4abad100622d3f101a0a3ca0a6c8e881 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -1296,6 +1296,11 @@ int tcp_fragment(struct sock *sk, enum tcp_queue tcp_queue,
if (nsize < 0)
nsize = 0;
+ if (unlikely((sk->sk_wmem_queued >> 1) > sk->sk_sndbuf)) {
+ NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPWQUEUETOOBIG);
+ return -ENOMEM;
+ }
+
if (skb_unclone(skb, gfp))
return -ENOMEM;
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH net 3/4] tcp: add tcp_min_snd_mss sysctl
From: Eric Dumazet @ 2019-06-17 17:03 UTC (permalink / raw)
To: David S . Miller
Cc: netdev, Eric Dumazet, Eric Dumazet, Greg Kroah-Hartman,
Jonathan Looney, Neal Cardwell, Tyler Hicks, Yuchung Cheng,
Bruce Curtis, Jonathan Lemon
In-Reply-To: <20190617170354.37770-1-edumazet@google.com>
Some TCP peers announce a very small MSS option in their SYN and/or
SYN/ACK messages.
This forces the stack to send packets with a very high network/cpu
overhead.
Linux has enforced a minimal value of 48. Since this value includes
the size of TCP options, and that the options can consume up to 40
bytes, this means that each segment can include only 8 bytes of payload.
In some cases, it can be useful to increase the minimal value
to a saner value.
We still let the default to 48 (TCP_MIN_SND_MSS), for compatibility
reasons.
Note that TCP_MAXSEG socket option enforces a minimal value
of (TCP_MIN_MSS). David Miller increased this minimal value
in commit c39508d6f118 ("tcp: Make TCP_MAXSEG minimum more correct.")
from 64 to 88.
We might in the future merge TCP_MIN_SND_MSS and TCP_MIN_MSS.
CVE-2019-11479 -- tcp mss hardcoded to 48
Signed-off-by: Eric Dumazet <edumazet@google.com>
Suggested-by: Jonathan Looney <jtl@netflix.com>
Acked-by: Neal Cardwell <ncardwell@google.com>
Cc: Yuchung Cheng <ycheng@google.com>
Cc: Tyler Hicks <tyhicks@canonical.com>
Cc: Bruce Curtis <brucec@netflix.com>
Cc: Jonathan Lemon <jonathan.lemon@gmail.com>
---
Documentation/networking/ip-sysctl.txt | 8 ++++++++
include/net/netns/ipv4.h | 1 +
net/ipv4/sysctl_net_ipv4.c | 11 +++++++++++
net/ipv4/tcp_ipv4.c | 1 +
net/ipv4/tcp_output.c | 3 +--
5 files changed, 22 insertions(+), 2 deletions(-)
diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
index 288aa264ac26d98637a5bb1babc334bfc699bef1..22f6b8b1110ad20c36e7ceea6d67fd2cc938eb7b 100644
--- a/Documentation/networking/ip-sysctl.txt
+++ b/Documentation/networking/ip-sysctl.txt
@@ -255,6 +255,14 @@ tcp_base_mss - INTEGER
Path MTU discovery (MTU probing). If MTU probing is enabled,
this is the initial MSS used by the connection.
+tcp_min_snd_mss - INTEGER
+ TCP SYN and SYNACK messages usually advertise an ADVMSS option,
+ as described in RFC 1122 and RFC 6691.
+ If this ADVMSS option is smaller than tcp_min_snd_mss,
+ it is silently capped to tcp_min_snd_mss.
+
+ Default : 48 (at least 8 bytes of payload per segment)
+
tcp_congestion_control - STRING
Set the congestion control algorithm to be used for new
connections. The algorithm "reno" is always available, but
diff --git a/include/net/netns/ipv4.h b/include/net/netns/ipv4.h
index 7698460a3dd1e5070e12d406b3ee58834688cdc9..623cfbb7b8dcbb2a6d8325ec010aff78bbdf8839 100644
--- a/include/net/netns/ipv4.h
+++ b/include/net/netns/ipv4.h
@@ -117,6 +117,7 @@ struct netns_ipv4 {
#endif
int sysctl_tcp_mtu_probing;
int sysctl_tcp_base_mss;
+ int sysctl_tcp_min_snd_mss;
int sysctl_tcp_probe_threshold;
u32 sysctl_tcp_probe_interval;
diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
index fa213bd8e233b577114815ca2227f08264e7df06..b6f14af926faf80f1686549bee7154c584dc63e6 100644
--- a/net/ipv4/sysctl_net_ipv4.c
+++ b/net/ipv4/sysctl_net_ipv4.c
@@ -39,6 +39,8 @@ static int ip_local_port_range_min[] = { 1, 1 };
static int ip_local_port_range_max[] = { 65535, 65535 };
static int tcp_adv_win_scale_min = -31;
static int tcp_adv_win_scale_max = 31;
+static int tcp_min_snd_mss_min = TCP_MIN_SND_MSS;
+static int tcp_min_snd_mss_max = 65535;
static int ip_privileged_port_min;
static int ip_privileged_port_max = 65535;
static int ip_ttl_min = 1;
@@ -769,6 +771,15 @@ static struct ctl_table ipv4_net_table[] = {
.mode = 0644,
.proc_handler = proc_dointvec,
},
+ {
+ .procname = "tcp_min_snd_mss",
+ .data = &init_net.ipv4.sysctl_tcp_min_snd_mss,
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = proc_dointvec_minmax,
+ .extra1 = &tcp_min_snd_mss_min,
+ .extra2 = &tcp_min_snd_mss_max,
+ },
{
.procname = "tcp_probe_threshold",
.data = &init_net.ipv4.sysctl_tcp_probe_threshold,
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index bc86f9735f4577d50d94f42b10edb6ba95bb7a05..cfa81190a1b1af30d05f4f6cd84c05b025a6afeb 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -2628,6 +2628,7 @@ static int __net_init tcp_sk_init(struct net *net)
net->ipv4.sysctl_tcp_ecn_fallback = 1;
net->ipv4.sysctl_tcp_base_mss = TCP_BASE_MSS;
+ net->ipv4.sysctl_tcp_min_snd_mss = TCP_MIN_SND_MSS;
net->ipv4.sysctl_tcp_probe_threshold = TCP_PROBE_THRESHOLD;
net->ipv4.sysctl_tcp_probe_interval = TCP_PROBE_INTERVAL;
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 1bb1c46b4abad100622d3f101a0a3ca0a6c8e881..00c01a01b547ec67c971dc25a74c9258563cf871 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -1459,8 +1459,7 @@ static inline int __tcp_mtu_to_mss(struct sock *sk, int pmtu)
mss_now -= icsk->icsk_ext_hdr_len;
/* Then reserve room for full set of TCP options and 8 bytes of data */
- if (mss_now < TCP_MIN_SND_MSS)
- mss_now = TCP_MIN_SND_MSS;
+ mss_now = max(mss_now, sock_net(sk)->ipv4.sysctl_tcp_min_snd_mss);
return mss_now;
}
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH net 4/4] tcp: enforce tcp_min_snd_mss in tcp_mtu_probing()
From: Eric Dumazet @ 2019-06-17 17:03 UTC (permalink / raw)
To: David S . Miller
Cc: netdev, Eric Dumazet, Eric Dumazet, Greg Kroah-Hartman,
Jonathan Looney, Neal Cardwell, Tyler Hicks, Yuchung Cheng,
Bruce Curtis, Jonathan Lemon
In-Reply-To: <20190617170354.37770-1-edumazet@google.com>
If mtu probing is enabled tcp_mtu_probing() could very well end up
with a too small MSS.
Use the new sysctl tcp_min_snd_mss to make sure MSS search
is performed in an acceptable range.
CVE-2019-11479 -- tcp mss hardcoded to 48
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: Jonathan Lemon <jonathan.lemon@gmail.com>
Cc: Jonathan Looney <jtl@netflix.com>
Acked-by: Neal Cardwell <ncardwell@google.com>
Cc: Yuchung Cheng <ycheng@google.com>
Cc: Tyler Hicks <tyhicks@canonical.com>
Cc: Bruce Curtis <brucec@netflix.com>
---
net/ipv4/tcp_timer.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
index 5bad937ce779ef8dca42a26dcbb5f1d60a571c73..c801cd37cc2a9c11f2dd4b9681137755e501a538 100644
--- a/net/ipv4/tcp_timer.c
+++ b/net/ipv4/tcp_timer.c
@@ -155,6 +155,7 @@ static void tcp_mtu_probing(struct inet_connection_sock *icsk, struct sock *sk)
mss = tcp_mtu_to_mss(sk, icsk->icsk_mtup.search_low) >> 1;
mss = min(net->ipv4.sysctl_tcp_base_mss, mss);
mss = max(mss, 68 - tcp_sk(sk)->tcp_header_len);
+ mss = max(mss, net->ipv4.sysctl_tcp_min_snd_mss);
icsk->icsk_mtup.search_low = tcp_mss_to_mtu(sk, mss);
}
tcp_sync_mss(sk, icsk->icsk_pmtu_cookie);
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* Re: [PATCH net v4 1/8] ipv4/fib_frontend: Rename ip_valid_fib_dump_req, provide non-strict version
From: David Ahern @ 2019-06-17 17:06 UTC (permalink / raw)
To: Stefano Brivio
Cc: David Miller, Martin KaFai Lau, Jianlin Shi, Wei Wang,
Eric Dumazet, Matti Vaittinen, netdev
In-Reply-To: <20190617161333.29cab4d7@redhat.com>
On 6/17/19 8:13 AM, Stefano Brivio wrote:
>>
>> With strict checking (5.0 and forward):
>> - RTM_F_CLONED NOT set means dump only FIB entries
>> - RTM_F_CLONED set means dump only exceptions
>
> Okay. Should we really ignore the RFC and NLM_F_MATCH though? If we add
> field(s) to the filter, it comes almost for free, something like:
>
> if (nlh->nlmsg_flags & NLM_F_MATCH)
> filter->dump_exceptions = rtm->rtm_flags & RTM_F_CLONED;
>
> instead of:
>
> filter->dump_exceptions = rtm->rtm_flags & RTM_F_CLONED;
This is where you keep losing me. iproute2 has always set NLM_F_MATCH on
dump requests, so that flag can not be used as a discriminator here.
>
>> Without strict checking (old iproute2 on any kernel):
>> - dump all, userspace has to sort
>>
>> Kernel side this can be handled with new field, dump_exceptions, in the
>> filter that defaults to true and then is reset in the strict path if the
>> flag is not set.
>
> I guess we need to add two fields, we'll need a 'dump_routes' too.
>
> Otherwise, the dump functions can't distinguish between the three cases
> ('no strict checking', 'strict checking and RTM_F_CLONED', 'strict
> checking and no RTM_F_CLONED'). How would you do this with a single
> additional field?
>
sure, separate fields are needed for the pre-strict mode use case. So, I
take it we are converging on this:
1. non-strict mode, dump both (FIB entries and exceptions). Userspace
has to filter. This is the legacy behavior you are trying to restore.
2. strict mode:
a. dump only FIB entries if RTM_F_CLONED is not set
b. dump only exception entries if RTM_F_CLONED is set
Agreed?
Martin, others, ok with this?
^ permalink raw reply
* Re: [PATCH net-next v3] ipv4: Support multipath hashing on inner IP pkts for GRE tunnel
From: Stephen Suryaputra @ 2019-06-17 17:08 UTC (permalink / raw)
To: David Ahern; +Cc: Ido Schimmel, netdev, nikolay
In-Reply-To: <e56ca29f-8d80-b9ae-112a-4ff55847313d@gmail.com>
On Mon, Jun 17, 2019 at 09:53:06AM -0600, David Ahern wrote:
> On 6/17/19 8:39 AM, Ido Schimmel wrote:
> >
> > Do you plan to add IPv6 support? Would be good to have the same features
> > in both stacks.
>
> we really should be mandating equal support for all new changes like this.
>
I will add that.
> >
> > Also, we have tests for these sysctls under
> > tools/testing/selftests/net/forwarding/router_multipath.sh
> >
> > Can you add a test for this change as well? You'll probably need to
> > create a new file given the topology created by router_multipath.sh does
> > not include tunnels.
I never looked at the selftests scripts, but will attempt.
Stephen.
^ permalink raw reply
* Re: [PATCH] fib_semantics: Fix warning in fib_check_nh_v4_gw
From: David Ahern @ 2019-06-17 17:12 UTC (permalink / raw)
To: Vincenzo Frascino, linux-kernel, netdev; +Cc: davem, kuznet, yoshfuji
In-Reply-To: <20190617102119.56253-1-vincenzo.frascino@arm.com>
On 6/17/19 4:21 AM, Vincenzo Frascino wrote:
> Currently, the err variable in fib_check_nh_v4_gw may be used
> uninitialized leading to the warning below:
>
> fib_semantics.c: In function ‘fib_check_nh_v4_gw’:
> fib_semantics.c:1023:12: warning: ‘err’ may be used
> uninitialised in this function [-Wmaybe-uninitialized]
> if (!tbl || err) {
> ^~
>
> Initialize err to 0 to fix the warning.
>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
> Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
> Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
> ---
> net/ipv4/fib_semantics.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
already fixed in net; will make it to net-next on next merge.
^ permalink raw reply
* Re: [PATCH net 1/4] tcp: limit payload size of sacked skbs
From: Jonathan Lemon @ 2019-06-17 17:14 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S . Miller, netdev, Eric Dumazet, Greg Kroah-Hartman,
Jonathan Looney, Neal Cardwell, Tyler Hicks, Yuchung Cheng,
Bruce Curtis
In-Reply-To: <20190617170354.37770-2-edumazet@google.com>
On 17 Jun 2019, at 10:03, Eric Dumazet wrote:
> Jonathan Looney reported that TCP can trigger the following crash
> in tcp_shifted_skb() :
>
> BUG_ON(tcp_skb_pcount(skb) < pcount);
>
> This can happen if the remote peer has advertized the smallest
> MSS that linux TCP accepts : 48
>
> An skb can hold 17 fragments, and each fragment can hold 32KB
> on x86, or 64KB on PowerPC.
>
> This means that the 16bit witdh of TCP_SKB_CB(skb)->tcp_gso_segs
> can overflow.
>
> Note that tcp_sendmsg() builds skbs with less than 64KB
> of payload, so this problem needs SACK to be enabled.
> SACK blocks allow TCP to coalesce multiple skbs in the retransmit
> queue, thus filling the 17 fragments to maximal capacity.
>
> CVE-2019-11477 -- u16 overflow of TCP_SKB_CB(skb)->tcp_gso_segs
>
> Fixes: 832d11c5cd07 ("tcp: Try to restore large SKBs while SACK
> processing")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: Jonathan Looney <jtl@netflix.com>
> Acked-by: Neal Cardwell <ncardwell@google.com>
> Reviewed-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Jonathan Lemon <jonathan.lemon@gmail.com>
^ permalink raw reply
* Re: [PATCH net 2/4] tcp: tcp_fragment() should apply sane memory limits
From: Jonathan Lemon @ 2019-06-17 17:14 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S . Miller, netdev, Eric Dumazet, Greg Kroah-Hartman,
Jonathan Looney, Neal Cardwell, Tyler Hicks, Yuchung Cheng,
Bruce Curtis
In-Reply-To: <20190617170354.37770-3-edumazet@google.com>
On 17 Jun 2019, at 10:03, Eric Dumazet wrote:
> Jonathan Looney reported that a malicious peer can force a sender
> to fragment its retransmit queue into tiny skbs, inflating memory
> usage and/or overflow 32bit counters.
>
> TCP allows an application to queue up to sk_sndbuf bytes,
> so we need to give some allowance for non malicious splitting
> of retransmit queue.
>
> A new SNMP counter is added to monitor how many times TCP
> did not allow to split an skb if the allowance was exceeded.
>
> Note that this counter might increase in the case applications
> use SO_SNDBUF socket option to lower sk_sndbuf.
>
> CVE-2019-11478 : tcp_fragment, prevent fragmenting a packet when the
> socket is already using more than half the allowed space
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: Jonathan Looney <jtl@netflix.com>
> Acked-by: Neal Cardwell <ncardwell@google.com>
> Acked-by: Yuchung Cheng <ycheng@google.com>
> Reviewed-by: Tyler Hicks <tyhicks@canonical.com>
> Cc: Bruce Curtis <brucec@netflix.com>
> Cc: Jonathan Lemon <jonathan.lemon@gmail.com>
Acked-by: Jonathan Lemon <jonathan.lemon@gmail.com>
^ permalink raw reply
* Re: [PATCH net 3/4] tcp: add tcp_min_snd_mss sysctl
From: Jonathan Lemon @ 2019-06-17 17:15 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S . Miller, netdev, Eric Dumazet, Greg Kroah-Hartman,
Jonathan Looney, Neal Cardwell, Tyler Hicks, Yuchung Cheng,
Bruce Curtis
In-Reply-To: <20190617170354.37770-4-edumazet@google.com>
On 17 Jun 2019, at 10:03, Eric Dumazet wrote:
> Some TCP peers announce a very small MSS option in their SYN and/or
> SYN/ACK messages.
>
> This forces the stack to send packets with a very high network/cpu
> overhead.
>
> Linux has enforced a minimal value of 48. Since this value includes
> the size of TCP options, and that the options can consume up to 40
> bytes, this means that each segment can include only 8 bytes of payload.
>
> In some cases, it can be useful to increase the minimal value
> to a saner value.
>
> We still let the default to 48 (TCP_MIN_SND_MSS), for compatibility
> reasons.
>
> Note that TCP_MAXSEG socket option enforces a minimal value
> of (TCP_MIN_MSS). David Miller increased this minimal value
> in commit c39508d6f118 ("tcp: Make TCP_MAXSEG minimum more correct.")
> from 64 to 88.
>
> We might in the future merge TCP_MIN_SND_MSS and TCP_MIN_MSS.
>
> CVE-2019-11479 -- tcp mss hardcoded to 48
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Suggested-by: Jonathan Looney <jtl@netflix.com>
> Acked-by: Neal Cardwell <ncardwell@google.com>
> Cc: Yuchung Cheng <ycheng@google.com>
> Cc: Tyler Hicks <tyhicks@canonical.com>
> Cc: Bruce Curtis <brucec@netflix.com>
> Cc: Jonathan Lemon <jonathan.lemon@gmail.com>
Acked-by: Jonathan Lemon <jonathan.lemon@gmail.com>
^ permalink raw reply
* Re: [PATCH net 4/4] tcp: enforce tcp_min_snd_mss in tcp_mtu_probing()
From: Jonathan Lemon @ 2019-06-17 17:16 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S . Miller, netdev, Eric Dumazet, Greg Kroah-Hartman,
Jonathan Looney, Neal Cardwell, Tyler Hicks, Yuchung Cheng,
Bruce Curtis
In-Reply-To: <20190617170354.37770-5-edumazet@google.com>
On 17 Jun 2019, at 10:03, Eric Dumazet wrote:
> If mtu probing is enabled tcp_mtu_probing() could very well end up
> with a too small MSS.
>
> Use the new sysctl tcp_min_snd_mss to make sure MSS search
> is performed in an acceptable range.
>
> CVE-2019-11479 -- tcp mss hardcoded to 48
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: Jonathan Lemon <jonathan.lemon@gmail.com>
> Cc: Jonathan Looney <jtl@netflix.com>
> Acked-by: Neal Cardwell <ncardwell@google.com>
> Cc: Yuchung Cheng <ycheng@google.com>
> Cc: Tyler Hicks <tyhicks@canonical.com>
> Cc: Bruce Curtis <brucec@netflix.com>
Acked-by: Jonathan Lemon <jonathan.lemon@gmail.com>
^ permalink raw reply
* Re: [PATCH V2] net: phy: tja11xx: Add IRQ support to the driver
From: Andrew Lunn @ 2019-06-17 17:16 UTC (permalink / raw)
To: Marek Vasut
Cc: Heiner Kallweit, netdev, Florian Fainelli, Guenter Roeck,
Jean Delvare, linux-hwmon
In-Reply-To: <f9bb1f48-b69d-09b2-5b48-e3f09ce9107e@denx.de>
On Thu, Jun 13, 2019 at 05:42:53PM +0200, Marek Vasut wrote:
> On 5/30/19 1:46 AM, Marek Vasut wrote:
> > On 5/30/19 1:29 AM, Andrew Lunn wrote:
> >> On Tue, May 28, 2019 at 11:33:33PM +0200, Marek Vasut wrote:
> >>> On 5/28/19 11:22 PM, Andrew Lunn wrote:
> >>>>> The link detection on the TJA1100 (not TJA1101) seems unstable at best,
> >>>>> so I better use all the interrupt sources to nudge the PHY subsystem and
> >>>>> have it check the link change.
> >>>>
> >>>> Then it sounds like you should just ignore interrupts and stay will
> >>>> polling for the TJA1100.
> >>>
> >>> Polling for the link status change is slow(er) than the IRQ driven
> >>> operation, so I would much rather use the interrupts.
> >>
> >> I agree about the speed, but it seems like interrupts on this PHY are
> >> not so reliable. Polling always works. But unfortunately, you cannot
> >> have both interrupts and polling to fix up problems when interrupts
> >> fail. Your call, do you think interrupts really do work?
> >
> > It works fine for me this way. And mind you, it's only the TJA1100
> > that's flaky, the TJA1101 is better.
> >
> >> If you say that tja1101 works as expected, then please just use the
> >> link up/down bits for it.
> >
> > I still don't know which bits really trigger link status changes, so I'd
> > like to play it safe and just trigger on all of them.
>
> So what do we do here ?
Hi Marek
My personal preference would be to just enable what is needed. But
I won't block a patch which enables everything.
Andrew
^ permalink raw reply
* Re: [PATCH net 3/4] tcp: add tcp_min_snd_mss sysctl
From: Tyler Hicks @ 2019-06-17 17:18 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S . Miller, netdev, Eric Dumazet, Greg Kroah-Hartman,
Jonathan Looney, Neal Cardwell, Yuchung Cheng, Bruce Curtis,
Jonathan Lemon
In-Reply-To: <20190617170354.37770-4-edumazet@google.com>
On 2019-06-17 10:03:53, Eric Dumazet wrote:
> Some TCP peers announce a very small MSS option in their SYN and/or
> SYN/ACK messages.
>
> This forces the stack to send packets with a very high network/cpu
> overhead.
>
> Linux has enforced a minimal value of 48. Since this value includes
> the size of TCP options, and that the options can consume up to 40
> bytes, this means that each segment can include only 8 bytes of payload.
>
> In some cases, it can be useful to increase the minimal value
> to a saner value.
>
> We still let the default to 48 (TCP_MIN_SND_MSS), for compatibility
> reasons.
>
> Note that TCP_MAXSEG socket option enforces a minimal value
> of (TCP_MIN_MSS). David Miller increased this minimal value
> in commit c39508d6f118 ("tcp: Make TCP_MAXSEG minimum more correct.")
> from 64 to 88.
>
> We might in the future merge TCP_MIN_SND_MSS and TCP_MIN_MSS.
>
> CVE-2019-11479 -- tcp mss hardcoded to 48
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Suggested-by: Jonathan Looney <jtl@netflix.com>
> Acked-by: Neal Cardwell <ncardwell@google.com>
> Cc: Yuchung Cheng <ycheng@google.com>
> Cc: Tyler Hicks <tyhicks@canonical.com>
I've given the two sysctl patches a close review and some testing.
Acked-by: Tyler Hicks <tyhicks@canonical.com>
Tyler
> Cc: Bruce Curtis <brucec@netflix.com>
> Cc: Jonathan Lemon <jonathan.lemon@gmail.com>
> ---
> Documentation/networking/ip-sysctl.txt | 8 ++++++++
> include/net/netns/ipv4.h | 1 +
> net/ipv4/sysctl_net_ipv4.c | 11 +++++++++++
> net/ipv4/tcp_ipv4.c | 1 +
> net/ipv4/tcp_output.c | 3 +--
> 5 files changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
> index 288aa264ac26d98637a5bb1babc334bfc699bef1..22f6b8b1110ad20c36e7ceea6d67fd2cc938eb7b 100644
> --- a/Documentation/networking/ip-sysctl.txt
> +++ b/Documentation/networking/ip-sysctl.txt
> @@ -255,6 +255,14 @@ tcp_base_mss - INTEGER
> Path MTU discovery (MTU probing). If MTU probing is enabled,
> this is the initial MSS used by the connection.
>
> +tcp_min_snd_mss - INTEGER
> + TCP SYN and SYNACK messages usually advertise an ADVMSS option,
> + as described in RFC 1122 and RFC 6691.
> + If this ADVMSS option is smaller than tcp_min_snd_mss,
> + it is silently capped to tcp_min_snd_mss.
> +
> + Default : 48 (at least 8 bytes of payload per segment)
> +
> tcp_congestion_control - STRING
> Set the congestion control algorithm to be used for new
> connections. The algorithm "reno" is always available, but
> diff --git a/include/net/netns/ipv4.h b/include/net/netns/ipv4.h
> index 7698460a3dd1e5070e12d406b3ee58834688cdc9..623cfbb7b8dcbb2a6d8325ec010aff78bbdf8839 100644
> --- a/include/net/netns/ipv4.h
> +++ b/include/net/netns/ipv4.h
> @@ -117,6 +117,7 @@ struct netns_ipv4 {
> #endif
> int sysctl_tcp_mtu_probing;
> int sysctl_tcp_base_mss;
> + int sysctl_tcp_min_snd_mss;
> int sysctl_tcp_probe_threshold;
> u32 sysctl_tcp_probe_interval;
>
> diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
> index fa213bd8e233b577114815ca2227f08264e7df06..b6f14af926faf80f1686549bee7154c584dc63e6 100644
> --- a/net/ipv4/sysctl_net_ipv4.c
> +++ b/net/ipv4/sysctl_net_ipv4.c
> @@ -39,6 +39,8 @@ static int ip_local_port_range_min[] = { 1, 1 };
> static int ip_local_port_range_max[] = { 65535, 65535 };
> static int tcp_adv_win_scale_min = -31;
> static int tcp_adv_win_scale_max = 31;
> +static int tcp_min_snd_mss_min = TCP_MIN_SND_MSS;
> +static int tcp_min_snd_mss_max = 65535;
> static int ip_privileged_port_min;
> static int ip_privileged_port_max = 65535;
> static int ip_ttl_min = 1;
> @@ -769,6 +771,15 @@ static struct ctl_table ipv4_net_table[] = {
> .mode = 0644,
> .proc_handler = proc_dointvec,
> },
> + {
> + .procname = "tcp_min_snd_mss",
> + .data = &init_net.ipv4.sysctl_tcp_min_snd_mss,
> + .maxlen = sizeof(int),
> + .mode = 0644,
> + .proc_handler = proc_dointvec_minmax,
> + .extra1 = &tcp_min_snd_mss_min,
> + .extra2 = &tcp_min_snd_mss_max,
> + },
> {
> .procname = "tcp_probe_threshold",
> .data = &init_net.ipv4.sysctl_tcp_probe_threshold,
> diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
> index bc86f9735f4577d50d94f42b10edb6ba95bb7a05..cfa81190a1b1af30d05f4f6cd84c05b025a6afeb 100644
> --- a/net/ipv4/tcp_ipv4.c
> +++ b/net/ipv4/tcp_ipv4.c
> @@ -2628,6 +2628,7 @@ static int __net_init tcp_sk_init(struct net *net)
> net->ipv4.sysctl_tcp_ecn_fallback = 1;
>
> net->ipv4.sysctl_tcp_base_mss = TCP_BASE_MSS;
> + net->ipv4.sysctl_tcp_min_snd_mss = TCP_MIN_SND_MSS;
> net->ipv4.sysctl_tcp_probe_threshold = TCP_PROBE_THRESHOLD;
> net->ipv4.sysctl_tcp_probe_interval = TCP_PROBE_INTERVAL;
>
> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> index 1bb1c46b4abad100622d3f101a0a3ca0a6c8e881..00c01a01b547ec67c971dc25a74c9258563cf871 100644
> --- a/net/ipv4/tcp_output.c
> +++ b/net/ipv4/tcp_output.c
> @@ -1459,8 +1459,7 @@ static inline int __tcp_mtu_to_mss(struct sock *sk, int pmtu)
> mss_now -= icsk->icsk_ext_hdr_len;
>
> /* Then reserve room for full set of TCP options and 8 bytes of data */
> - if (mss_now < TCP_MIN_SND_MSS)
> - mss_now = TCP_MIN_SND_MSS;
> + mss_now = max(mss_now, sock_net(sk)->ipv4.sysctl_tcp_min_snd_mss);
> return mss_now;
> }
>
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
^ permalink raw reply
* Re: [PATCH net 4/4] tcp: enforce tcp_min_snd_mss in tcp_mtu_probing()
From: Tyler Hicks @ 2019-06-17 17:18 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S . Miller, netdev, Eric Dumazet, Greg Kroah-Hartman,
Jonathan Looney, Neal Cardwell, Yuchung Cheng, Bruce Curtis,
Jonathan Lemon
In-Reply-To: <20190617170354.37770-5-edumazet@google.com>
On 2019-06-17 10:03:54, Eric Dumazet wrote:
> If mtu probing is enabled tcp_mtu_probing() could very well end up
> with a too small MSS.
>
> Use the new sysctl tcp_min_snd_mss to make sure MSS search
> is performed in an acceptable range.
>
> CVE-2019-11479 -- tcp mss hardcoded to 48
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: Jonathan Lemon <jonathan.lemon@gmail.com>
> Cc: Jonathan Looney <jtl@netflix.com>
> Acked-by: Neal Cardwell <ncardwell@google.com>
> Cc: Yuchung Cheng <ycheng@google.com>
> Cc: Tyler Hicks <tyhicks@canonical.com>
As mentioned for the other sysctl patch, I've given the two sysctl
patches a close review and some testing.
Acked-by: Tyler Hicks <tyhicks@canonical.com>
Tyler
> Cc: Bruce Curtis <brucec@netflix.com>
> ---
> net/ipv4/tcp_timer.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
> index 5bad937ce779ef8dca42a26dcbb5f1d60a571c73..c801cd37cc2a9c11f2dd4b9681137755e501a538 100644
> --- a/net/ipv4/tcp_timer.c
> +++ b/net/ipv4/tcp_timer.c
> @@ -155,6 +155,7 @@ static void tcp_mtu_probing(struct inet_connection_sock *icsk, struct sock *sk)
> mss = tcp_mtu_to_mss(sk, icsk->icsk_mtup.search_low) >> 1;
> mss = min(net->ipv4.sysctl_tcp_base_mss, mss);
> mss = max(mss, 68 - tcp_sk(sk)->tcp_header_len);
> + mss = max(mss, net->ipv4.sysctl_tcp_min_snd_mss);
> icsk->icsk_mtup.search_low = tcp_mss_to_mtu(sk, mss);
> }
> tcp_sync_mss(sk, icsk->icsk_pmtu_cookie);
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox