Re: [4.4] FragmentSmack security fixes

From: maowenan <maowenan@huawei.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Ben Hutchings <ben.hutchings@codethink.co.uk>
Cc: Sasha Levin <Alexander.Levin@microsoft.com>,
	stable <stable@vger.kernel.org>,
	Eric Dumazet <edumazet@google.com>,
	Peter Oskolkov <posk@google.com>
Subject: Re: [4.4] FragmentSmack security fixes
Date: Sat, 9 Feb 2019 09:58:17 +0800	[thread overview]
Message-ID: <b77f0c49-5a41-48d2-5f3d-62abb1046f3d@huawei.com> (raw)
In-Reply-To: <20190207112611.GA3120@kroah.com>

On 2019/2/7 19:26, Greg Kroah-Hartman wrote:
> On Wed, Feb 06, 2019 at 10:13:26PM +0100, Greg Kroah-Hartman wrote:
>> On Tue, Feb 05, 2019 at 07:41:18PM +0000, Ben Hutchings wrote:
>>>>> 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.
>>
>> Ok, I dropped those patches from the 4.4 queue before releasing it.  Let
>> me go add them back for the moment and then I'll dig through all of this
>> over the next few days and see what it looks like...
> 
> I've reviewed all of these and they look good.  There were some
> duplications with what was in my tree, but I have taken your versions
> instead.
> 
> Mao, you will note that 4.4.173 did not get released with your patches
> in it.  I have added your signed-off-by to the same ones that Ben did
> here in this series, as the changes were minimal at most, to what you
> had.  If you have any objections to these, please let me know.
> 

It looks well.

> I'll probably just push out a -rc release for 4.4.y later today with
> these in it to get some testing and a release out so that we can get
> this issue finally resolved.
> 
> thanks,
> 
> greg k-h
> 
> .
> 