yes, tipc_l2_device_event() only change MTU of bearer rather than the MTU of link, tipc_enable_l2_media() will be the right place to test a tiny MTU.
Qian Sent from my iPhone On 5 Nov 2016, at 00:00, Jon Maloy <jon.ma...@ericsson.com> wrote: >> -----Original Message----- >> From: netdev-ow...@vger.kernel.org [mailto:netdev-ow...@vger.kernel.org] >> On Behalf Of ?? >> Sent: Friday, 04 November, 2016 03:24 >> To: Jon Maloy <jon.ma...@ericsson.com>; Ben Hutchings >> <b...@decadent.org.uk>; Ying Xue <ying.x...@gmail.com> >> Cc: netdev@vger.kernel.org; Eric Dumazet <eduma...@google.com> >> Subject: Re: [PATCH net] tipc: Guard against tiny MTU in tipc_msg_build() >> >> Hi, >> I think both tipc_l2_device_event() and tipc_enable_l2_media() need to >> refuse a >> tiny MTU for TIPC bearers. > > Right, except that when looking into the code for tipc_l2_device_event() I > realize that it currently doesn't try to re-adapt to a new MTU at all. It > just calls tipc_reset_bearer(), which I suspect has changed somewhere along > the road to ignore the MTU. So, you only need to change > tipc_enable_l2_media(). > > ///jon > >> >> tipc_l2_device_event() used to update the TIPC MTU value when executing a >> command like 'ifconfig eth0 MTU 1 up'. >> tipc_enable_l2_media() will be invoked when the TIPC network created. >> >> Thanks. >> >> Qian Zhang >> MarvelTeam Qihoo 360 >> >> >> >> -----邮件原件----- >> 发件人: Jon Maloy [mailto:jon.ma...@ericsson.com] >> 发送时间: 2016年11月1日 19:37 >> 收件人: 张谦; Ben Hutchings; Ying Xue >> 抄送: netdev@vger.kernel.org; Eric Dumazet >> 主题: RE: [PATCH net] tipc: Guard against tiny MTU in tipc_msg_build() >> >> Hi, >> I think we all agreed in the end that this is a possible, but highly >> implausible, >> scenario, and rather as a point of exploit than a functional bug. >> The solution is very simple, and described further down in this mail thread. >> I have >> not done anything to it yet, but you are welcome to contribute. >> >> BR >> ///jon >> >> >>> -----Original Message----- >>> From: 张谦 [mailto:zhangqia...@360.cn] >>> Sent: Tuesday, 01 November, 2016 02:35 >>> To: Ben Hutchings <b...@decadent.org.uk>; Jon Maloy >>> <jon.ma...@ericsson.com>; Ying Xue <ying.x...@gmail.com> >>> Cc: netdev@vger.kernel.org; Eric Dumazet <eduma...@google.com> >>> Subject: Re: [PATCH net] tipc: Guard against tiny MTU in >>> tipc_msg_build() >>> >>> Hi all, >>> I have accomplished a PoC can help you to confirm this issue. >>> >>> And two weeks passed from the last mail, can you tell me the progress >>> of the patch to this flaw? >>> >>> Thanks. >>> >>> Qian Zhang >>> Marvel Team Qihoo 360 >>> >>> >>> -----邮件原件----- >>> 发件人: Ben Hutchings [mailto:b...@decadent.org.uk] >>> 发送时间: 2016年10月21日 23:00 >>> 收件人: Jon Maloy; Ying Xue >>> 抄送: netdev@vger.kernel.org; 张谦; Eric Dumazet >>> 主题: Re: [PATCH net] tipc: Guard against tiny MTU in tipc_msg_build() >>> >>> On Fri, 2016-10-21 at 14:57 +0000, Jon Maloy wrote: >>>>> -----Original Message----- >>>>>>> From: Ben Hutchings [mailto:b...@decadent.org.uk] >>>>> Sent: Thursday, 20 October, 2016 12:40 >>>>>>> To: Jon Maloy <jon.ma...@ericsson.com>; Ying Xue >>>>>>> <ying.x...@gmail.com> >>>>>>>>> Cc: netdev@vger.kernel.org; Qian Zhang >>>>>>>>> <zhangqia...@360.cn>; Eric Dumazet >>>>>>> <eduma...@google.com> >>>>> 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: >>>>> [...] >>>>>>> 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 >>>>>>> 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). >>>>>> INT_H_SIZE is 40 bytes, so you are in reality testing for >>>>>> whether we have an mtu >>>>> >>>>> < 84 bytes. >>>>>> 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. >
