| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Sun, 27 May 2018 10:58:35 +0200
From: Michael Tuexen <michael.tuexen@...chi.franken.de>
To: Dmitry Vyukov <dvyukov@...gle.com>
Cc: Neil Horman <nhorman@...driver.com>,
Xin Long <lucien.xin@...il.com>,
network dev <netdev@...r.kernel.org>,
linux-sctp@...r.kernel.org, David Miller <davem@...emloft.net>,
David Ahern <dsa@...ulusnetworks.com>,
Eric Dumazet <edumazet@...gle.com>,
Marcelo Ricardo Leitner <marcelo.leitner@...il.com>,
syzkaller <syzkaller@...glegroups.com>
Subject: Re: [PATCH net] sctp: not allow to set rto_min with a value below 200
msecs
> On 26. May 2018, at 17:50, Dmitry Vyukov <dvyukov@...gle.com> wrote:
>
> On Sat, May 26, 2018 at 5:42 PM, Michael Tuexen
> <michael.tuexen@...chi.franken.de> wrote:
>>> On 25. May 2018, at 21:13, Neil Horman <nhorman@...driver.com> wrote:
>>>
>>> On Sat, May 26, 2018 at 01:41:02AM +0800, Xin Long wrote:
>>>> syzbot reported a rcu_sched self-detected stall on CPU which is caused
>>>> by too small value set on rto_min with SCTP_RTOINFO sockopt. With this
>>>> value, hb_timer will get stuck there, as in its timer handler it starts
>>>> this timer again with this value, then goes to the timer handler again.
>>>>
>>>> This problem is there since very beginning, and thanks to Eric for the
>>>> reproducer shared from a syzbot mail.
>>>>
>>>> This patch fixes it by not allowing to set rto_min with a value below
>>>> 200 msecs, which is based on TCP's, by either setsockopt or sysctl.
>>>>
>>>> Reported-by: syzbot+3dcd59a1f907245f891f@...kaller.appspotmail.com
>>>> Suggested-by: Marcelo Ricardo Leitner <marcelo.leitner@...il.com>
>>>> Signed-off-by: Xin Long <lucien.xin@...il.com>
>>>> ---
>>>> include/net/sctp/constants.h | 1 +
>>>> net/sctp/socket.c | 10 +++++++---
>>>> net/sctp/sysctl.c | 3 ++-
>>>> 3 files changed, 10 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/include/net/sctp/constants.h b/include/net/sctp/constants.h
>>>> index 20ff237..2ee7a7b 100644
>>>> --- a/include/net/sctp/constants.h
>>>> +++ b/include/net/sctp/constants.h
>>>> @@ -277,6 +277,7 @@ enum { SCTP_MAX_GABS = 16 };
>>>> #define SCTP_RTO_INITIAL (3 * 1000)
>>>> #define SCTP_RTO_MIN (1 * 1000)
>>>> #define SCTP_RTO_MAX (60 * 1000)
>>>> +#define SCTP_RTO_HARD_MIN 200
>>>>
>>>> #define SCTP_RTO_ALPHA 3 /* 1/8 when converted to right shifts. */
>>>> #define SCTP_RTO_BETA 2 /* 1/4 when converted to right shifts. */
>>>> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
>>>> index ae7e7c6..6ef12c7 100644
>>>> --- a/net/sctp/socket.c
>>>> +++ b/net/sctp/socket.c
>>>> @@ -3029,7 +3029,8 @@ static int sctp_setsockopt_nodelay(struct sock *sk, char __user *optval,
>>>> * be changed.
>>>> *
>>>> */
>>>> -static int sctp_setsockopt_rtoinfo(struct sock *sk, char __user *optval, unsigned int optlen)
>>>> +static int sctp_setsockopt_rtoinfo(struct sock *sk, char __user *optval,
>>>> + unsigned int optlen)
>>>> {
>>>> struct sctp_rtoinfo rtoinfo;
>>>> struct sctp_association *asoc;
>>>> @@ -3056,10 +3057,13 @@ static int sctp_setsockopt_rtoinfo(struct sock *sk, char __user *optval, unsigne
>>>> else
>>>> rto_max = asoc ? asoc->rto_max : sp->rtoinfo.srto_max;
>>>>
>>>> - if (rto_min)
>>>> + if (rto_min) {
>>>> + if (rto_min < SCTP_RTO_HARD_MIN)
>>>> + return -EINVAL;
>>>> rto_min = asoc ? msecs_to_jiffies(rto_min) : rto_min;
>>>> - else
>>>> + } else {
>>>> rto_min = asoc ? asoc->rto_min : sp->rtoinfo.srto_min;
>>>> + }
>>>>
>>>> if (rto_min > rto_max)
>>>> return -EINVAL;
>>>> diff --git a/net/sctp/sysctl.c b/net/sctp/sysctl.c
>>>> index 33ca5b7..7ec854a 100644
>>>> --- a/net/sctp/sysctl.c
>>>> +++ b/net/sctp/sysctl.c
>>>> @@ -52,6 +52,7 @@ static int rto_alpha_min = 0;
>>>> static int rto_beta_min = 0;
>>>> static int rto_alpha_max = 1000;
>>>> static int rto_beta_max = 1000;
>>>> +static int rto_hard_min = SCTP_RTO_HARD_MIN;
>>>>
>>>> static unsigned long max_autoclose_min = 0;
>>>> static unsigned long max_autoclose_max =
>>>> @@ -116,7 +117,7 @@ static struct ctl_table sctp_net_table[] = {
>>>> .maxlen = sizeof(unsigned int),
>>>> .mode = 0644,
>>>> .proc_handler = proc_sctp_do_rto_min,
>>>> - .extra1 = &one,
>>>> + .extra1 = &rto_hard_min,
>>>> .extra2 = &init_net.sctp.rto_max
>>>> },
>>>> {
>>>> --
>>>> 2.1.0
>>>>
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
>>>> the body of a message to majordomo@...r.kernel.org
>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>>
>>> Patch looks fine, you probably want to note this hard minimum in man(7) sctp as
>>> well
>>>
>> I'm aware of some signalling networks which use RTO.min of smaller values than 200ms.
>> So could this be reduced?
>
> Hi Michael,
>
> What value do they use?
I have seen values of
RTO.Min = 50ms
RTO.Max = 200ms
RTO.Initial = 100ms
Best regards
Michael
>
> Xin, Neil, is there more principled way of ensuring that a timer won't
> cause a hard CPU stall? There are slow machines and there are slow
> kernels (in particular syzbot kernel has tons of debug configs
> enabled). 200ms _should_ not cause problems because we did not see
> them with tcp. But it's hard to say what's the low limit as we are
> trying to put a hard upper bound on execution time of a complex
> section of code. Is there something like cond_resched for timers?
Powered by blists - more mailing lists