stable.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Ben Hutchings <ben.hutchings@codethink.co.uk>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Sasha Levin <Alexander.Levin@microsoft.com>,
	stable <stable@vger.kernel.org>,
	Eric Dumazet <edumazet@google.com>,
	Peter Oskolkov <posk@google.com>, Mao Wenan <maowenan@huawei.com>
Subject: Re: [4.4] FragmentSmack security fixes
Date: Tue, 05 Feb 2019 19:41:18 +0000	[thread overview]
Message-ID: <1549395678.2925.236.camel@codethink.co.uk> (raw)
In-Reply-To: <20190205184105.GA22198@kroah.com>

On Tue, 2019-02-05 at 19:41 +0100, Greg Kroah-Hartman wrote:
> On Tue, Feb 05, 2019 at 06:26:23PM +0000, Ben Hutchings wrote:
> > This is a backport of upstream changes to fix the FragmentSmack (CVE-
> > 2018-5391) vulnerability.
> > 
> > Peter Oskolkov checked an earlier version of this backport, but I have
> > since rebased and added another 3 commits to it.  I tested with the
> > ip_defrag.sh self-test that he added upstream, and it passed.  I have
> > included the fix that is currently queued for the 4.9, 4.14 and 4.19
> > branches.
> 
> That's a lot of patches, some of which I have already queued up in the
> next 4.4 release which will happen in a day or so.  Are they all still
> needed after the changes there are merged?

Ah, yes, a lot of the changes are already in your queue and I'm not
certain that all of mine are needed.  However I can say that the
changes currently in the queue are not correct:

* The ip_defrag.sh self-test fails: in the ipv4 non-overlap case, after
 a few seconds, recv() returns an EAGAIN error. If I modify the script
to continue running the other cases, however, they pass.

* There is a reference leak which prevents the new network namespaces
being torn down ("unregister_netdevice: waiting for lo to become free.
Usage count = 61").  (I see similar warnings with my backport, but the
number gradually decreases and they stop after 

* Shutdown hangs.

Ben.

-- 
Ben Hutchings, Software Developer                         Codethink Ltd
https://www.codethink.co.uk/                 Dale House, 35 Dale Street
                                     Manchester, M1 2HF, United Kingdom
On Tue, 2019-02-05 at 19:41 +0100, Greg Kroah-Hartman wrote:
> On Tue, Feb 05, 2019 at 06:26:23PM +0000, Ben Hutchings wrote:
> > This is a backport of upstream changes to fix the FragmentSmack (CVE-
> > 2018-5391) vulnerability.
> > 
> > Peter Oskolkov checked an earlier version of this backport, but I have
> > since rebased and added another 3 commits to it.  I tested with the
> > ip_defrag.sh self-test that he added upstream, and it passed.  I have
> > included the fix that is currently queued for the 4.9, 4.14 and 4.19
> > branches.
> 
> That's a lot of patches, some of which I have already queued up in the
> next 4.4 release which will happen in a day or so.  Are they all still
> needed after the changes there are merged?

Ah, yes, a lot of the fragment-handling changes are already in your
queue and I'm not certain that all of mine are needed.  However I don't
think the changes in your queue are complete and correct.  When I run
the ip_defrag.sh self-test:

1. The ipv4 non-overlap case fails after a few seconds, with recv()
returning an EAGAIN error. If I modify the script to continue after an
error, the other cases do pass, however.  This is not a regression from
4.4.172, but with my changes all cases pass.

2. There is a reference leak which prevents the new network namespaces
being cleaned up ("unregister_netdevice: waiting for lo to become free.
Usage count = 61").  With 4.4.172 or with my changes applied, the
warnings appear, but only for about a minute with the number gradually
decreasing.  So this is a regression.

3. If I run the test again, it hangs.  Shutting down the VM also hangs.
 I think this is related to the previous issue.  Again, this is a
regression.

Ben.

-- 
Ben Hutchings, Software Developer                         Codethink Ltd
https://www.codethink.co.uk/                 Dale House, 35 Dale Street
                                     Manchester, M1 2HF, United Kingdom

  reply	other threads:[~2019-02-05 19:41 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-05 18:26 [4.4] FragmentSmack security fixes Ben Hutchings
2019-02-05 18:41 ` Greg Kroah-Hartman
2019-02-05 19:41   ` Ben Hutchings [this message]
2019-02-06 21:13     ` Greg Kroah-Hartman
2019-02-07 11:26       ` Greg Kroah-Hartman
2019-02-09  1:58         ` maowenan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1549395678.2925.236.camel@codethink.co.uk \
    --to=ben.hutchings@codethink.co.uk \
    --cc=Alexander.Levin@microsoft.com \
    --cc=edumazet@google.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=maowenan@huawei.com \
    --cc=posk@google.com \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).