[Intel-wired-lan] Patch for iproute2

[Intel-wired-lan] Patch for iproute2

[Piatkowski, Robert]

Hello,
I want to submit a patch for iproute2 - taken from https://github.com/eworm-de/iproute2 - we use this version as it contains pretty recent XFRM offload configuration capability.

During development, we found a stack buffer overflow bug that causes mass deletion of states command to fail under some circumstances. So we developed a fix for that (see attachment).

The fix was reviewed by Don Skidmore, and he suggested to forward it to you. I tried to make the description sound similar to other commits' description in the repository.

Is the fix OK in the current state? What are the next steps for me?

Robert
--------------------------------------------------------------------

Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | Kapital zakladowy 200.000 PLN.

Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiekolwiek
przegladanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by
others is strictly prohibited.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osuosl.org/pipermail/intel-wired-lan/attachments/20180117/d4e185a2/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-fix-stack-buffer-overflow-when-removing-many-states.patch
Type: application/octet-stream
Size: 1404 bytes
Desc: 0001-fix-stack-buffer-overflow-when-removing-many-states.patch
URL: <http://lists.osuosl.org/pipermail/intel-wired-lan/attachments/20180117/d4e185a2/attachment.obj>

[Intel-wired-lan] Patch for iproute2

[Shannon Nelson]

On 1/17/2018 4:45 AM, Piatkowski, Robert wrote:
> Hello,
> 
> I want to submit a patch for iproute2 ? taken from 
> https://github.com/eworm-de/iproute2 - we use this version as it 
> contains pretty recent XFRM offload configuration capability.
> 
> During development, we found a stack buffer overflow bug that causes 
> mass deletion of states command to fail under some circumstances. So we 
> developed a fix for that (see attachment).
> 
> The fix was reviewed by Don Skidmore, and he suggested to forward it to 
> you. I tried to make the description sound similar to other commits? 
> description in the repository.
> 
> Is the fix OK in the current state? What are the next steps for me?
> 
> Robert

As it is much easier to respond to a patch when it is in the actual 
email, I've pasted it in below.

 > From 759b7832d917c822b98f8db73b03891150d316a4 Mon Sep 17 00:00:00 2001
 > From: Robert Piatkowski <robert.piatkowski@intel.com>
 > Date: Thu, 11 Jan 2018 11:12:55 -0500
 > Subject: [PATCH] fix stack buffer overflow when removing many states

The subject should name the source tree (iproute2) and the module (xfrm) 
being updated, something like this:
[PATCH iproute2] xfrm: fix stack buffer overflow when removing many states

I imagine Jeff can take this through his/Intel's dev-queue tree, but it 
should end up in the netdev mailing list and copied to Stephen Hemminger 
<stephen@networkplumber.org> and David Ahern <dsahern@gmail.com> 
(iproute2 maintainers) and to
Steffen Klassert <steffen.klassert@secunet.com> (xfrm maintainer).

 >
 > Fix the case then many SAs were deleted using the

s/then/where/
s/were/are/

 > xfrm delall command causing it to fail with various messages,
 > mainly "State buffer overflow".
 >
 > The code did try to delete all the SAs it got from XFRM at once,
 > not keeping in mind the buffer size for the delete command.
 > Now it simply fills up the buffer as much as it can,
 > repeating the deletion a few times - the loop responsible for this
 > is already there in xfrm_state_list_or_deleteall.
 >
 > This has been previously fixed for policies in
 > commit d5eb0564da494e618ad4654abd93461e5e8bbae0.
 >
 > Signed-off-by: Robert Piatkowski <rober.piatkowski@intel.com>
 > Signed-off-by: Don Skidmore <donald.c.skidmore@intel.com>

The patch itself looks fine to me.
Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>


 > ---
 >  ip/xfrm_state.c | 3 +++
 >  1 file changed, 3 insertions(+)
 >
 > diff --git a/ip/xfrm_state.c b/ip/xfrm_state.c
 > index 85d959cc..08052ffd 100644
 > --- a/ip/xfrm_state.c
 > +++ b/ip/xfrm_state.c
 > @@ -1103,6 +1103,9 @@ static int xfrm_state_keep(const struct 
sockaddr_nl *who,
 >  		return -1;
 >  	}
 >
 > +	if (xb->offset + NLMSG_LENGTH(sizeof(*xsid)) > xb->size)
 > +		return 0;
 > +
 >  	new_n = (struct nlmsghdr *)(xb->buf + xb->offset);
 > 	new_n->nlmsg_len = NLMSG_LENGTH(sizeof(*xsid));
 > 	new_n->nlmsg_flags = NLM_F_REQUEST;
 > --
 > 2.13.6