Re: [PATCH net] tipc: Guard against tiny MTU in tipc_msg_build()

On Fri, 2016-10-21 at 14:57 +0000, Jon Maloy wrote:

quoted

-----Original Message-----

quoted

quoted

From: Ben Hutchings [mailto:ben@decadent.org.uk]

Sent: Thursday, 20 October, 2016 12:40

quoted

quoted

To: Jon Maloy <redacted>; Ying Xue <redacted>

quoted

quoted

Cc: netdev@vger.kernel.org; Qian Zhang <redacted>; Eric Dumazet

[off-list ref]

Subject: Re: [PATCH net] tipc: Guard against tiny MTU in tipc_msg_build()

On Thu, 2016-10-20 at 14:51 +0000, Jon Maloy wrote:
[...]

quoted

quoted

At this point we're about to copy INT_H_SIZE + mhsz bytes into the
first fragment.  If that's already limited to be less than or equal to
MAX_H_SIZE, comparing with MAX_H_SIZE would be fine.  But if

MAX_H_SIZE

quoted

quoted

is the maximum value of mhsz, that won't be good enough.

MAX_H_SIZE is 60 bytes, but in practice you will never see an mhsz larger than

the biggest header we are actually using, which is MCAST_H_SIZE (==44 bytes).

quoted

INT_H_SIZE is 40 bytes, so you are in reality testing for whether we have an mtu

< 84 bytes.

quoted

You won't find any interfaces or protocols that come even close to this

limitation, so to me this test is redundant.

But I can easily create such an interface:

$ unshare -n -U -r
# ip l set lo mtu 1

Ben.

It won't be very useful though. But I assume you mean it could be a
possible exploit,

Exactly.

 and I suspect a few other things would break both in TIPC and in
other stacks if you do anything like that. I think the solution to
this is not to fix all possible places in the code where this can go
wrong, but rather to have a generic test where we refuse to attach
bearers/interfaces offering an mtu < e.g. 1000 bytes. This can easily
be done in tipc_enable_l2_media().

Yes.

Ben.

-- 
Ben Hutchings
One of the nice things about standards is that there are so many of
them.