lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Mon, 20 Jul 2020 11:25:50 -0400 From: Murali Karicheri <m-karicheri2@...com> To: Grygorii Strashko <grygorii.strashko@...com>, <davem@...emloft.net>, <kuba@...nel.org>, <netdev@...r.kernel.org>, <linux-kernel@...r.kernel.org>, <nsekhar@...com>, <vinicius.gomes@...el.com> Subject: Re: [PATCH 1/2 v2] net: hsr: fix incorrect lsdu size in the tag of HSR frames for small frames Hi Grygoii, On 7/20/20 10:08 AM, Murali Karicheri wrote: > Grygorii, > > On 7/17/20 1:39 PM, Grygorii Strashko wrote: >> >> >> On 17/07/2020 17:55, Murali Karicheri wrote: >>> For small Ethernet frames with size less than minimum size 66 for HSR >>> vs 60 for regular Ethernet frames, hsr driver currently doesn't pad the >>> frame to make it minimum size. This results in incorrect LSDU size being >>> populated in the HSR tag for these frames. Fix this by padding the frame >>> to the minimum size applicable for HSR. >>> >>> Signed-off-by: Murali Karicheri <m-karicheri2@...com> >>> --- >>> no change from original version >>> Sending this bug fix ahead of PRP patch series as per comment >>> net/hsr/hsr_forward.c | 3 +++ >>> 1 file changed, 3 insertions(+) >>> >>> Sending this bug fix ahead of PRP patch series as per comment >>> diff --git a/net/hsr/hsr_forward.c b/net/hsr/hsr_forward.c >>> index ed13760463de..e42fd356f073 100644 >>> --- a/net/hsr/hsr_forward.c >>> +++ b/net/hsr/hsr_forward.c >>> @@ -127,6 +127,9 @@ static void hsr_fill_tag(struct sk_buff *skb, >>> struct hsr_frame_info *frame, >>> int lane_id; >>> int lsdu_size; >>> + /* pad to minimum packet size which is 60 + 6 (HSR tag) */ >>> + skb_put_padto(skb, ETH_ZLEN + HSR_HLEN); >> >> It may fail. >> And i worry that it might be not the right place to do that >> (if packet is small it will be called for every copy of the packet). >> May be it has to be done once when packet enters LRE device? >> > A better place may be to add it at the beginning of > hsr_fill_frame_info() at which point there is one copy and after that > code enters hsr_forward_do() to replicate. I don't think we can place it > anywhere before that code. > > hsr_dev_xmit() > - hsr_forward_skb() > - hsr_fill_frame_info() > > Inside hsr_fill_frame_info() we could do > > if (ethhdr->h_proto == htons(ETH_P_8021Q)) { > frame->is_vlan = true; > /* FIXME: */ > netdev_warn_once(skb->dev, "VLAN not yet supported"); > } > + min_size = ETH_ZLEN + HSR_HLEN; > + if (frame->is_vlan) > + min_size += 4; > + ret = skb_put_padto(skb, min_size)) > + if (ret) > + return ret; > > At this point, it will be ready to tag the frame. Frame will be either a > supervision frame which is already tagged or standard frame from upper > layer. Either case, padto() is required. So looks like the right place > to avoid doing it twice. > > And packet would get dropped at the caller if skb_put_padto() fails. So > we could return the return value to the caller. > > This also eliminates similar padto() call in > send_hsr_supervision_frame() as well. > > What do you think? Dave has already applied the patch. So I will send a follow up patch that fixes the original issue to check for the return type. The patch to optimize this at the correct location (second part of your concern) can be a separate patch that will send as a follow up to the PRP series as that code touches this as well. So better to optimize this after the PRP series IMO. I plan to fix similar issue in the PRP series as well ( not checking for return type) and re-spin. Don't want to delay merge of PRP series for this optimization that can be addressed separately as I have said. Murali > > Murali >>> + >>> if (port->type == HSR_PT_SLAVE_A) >>> lane_id = 0; >>> else >>> >> > -- Murali Karicheri Texas Instruments
Powered by blists - more mailing lists