netdev - Re: [PATCH net-next v4 3/5] net: hdlc_fr: Improve the initial checks when we receive an skb

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAF=yD-+fQMZxSWT-_XLvdO9bQA_8xTMry49WA-ZsrcOQcz6H2A@mail.gmail.com>
Date:   Fri, 30 Oct 2020 17:20:04 -0400
From:   Willem de Bruijn <willemdebruijn.kernel@...il.com>
To:     Xie He <xie.he.0141@...il.com>
Cc:     Jakub Kicinski <kuba@...nel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Network Development <netdev@...r.kernel.org>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        Krzysztof Halasa <khc@...waw.pl>
Subject: Re: [PATCH net-next v4 3/5] net: hdlc_fr: Improve the initial checks
 when we receive an skb

On Fri, Oct 30, 2020 at 3:21 PM Xie He <xie.he.0141@...il.com> wrote:
>
> On Fri, Oct 30, 2020 at 9:31 AM Willem de Bruijn
> <willemdebruijn.kernel@...il.com> wrote:
> >
> > > Add an fh->ea2 check to the initial checks in fr_rx. fh->ea2 == 1 means
> > > the second address byte is the final address byte. We only support the
> > > case where the address length is 2 bytes.
> >
> > Can you elaborate a bit for readers not intimately familiar with the codebase?
> >
> > Is there something in the following code that has this implicit
> > assumption on 2-byte address lengths?
>
> Yes, the address length must be 2 bytes, otherwise the 3rd and 4th
> bytes would not be the control and protocol fields as we assumed in
> the code.
>
> The frame format is specified in RFC 2427
> (https://tools.ietf.org/html/rfc2427). We can see the overall frame
> format on Page 3. If the address length is longer than 2 bytes, all
> the following fields will be shifted behind.

Thanks for that context. If it's not captured in the code, it would be
great to include in the commit message.

>From a quick scan, RFC 2427 does not appear to actually define the
Q.922 address. For that I ended up reading ITU-T doc "Q.922 : ISDN
data link layer specification for frame mode bearer services", section
3.2.