All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.