MIME-Version: 1.0
References: <20190330072511.GA5502@kadam> <f1baaa05-bd1d-f2cf-410a-7073e85412d3@gmail.com>
 <CAM_iQpVNqyoBirtKg-rbOHsOZCZ0Oz5PTE2sFQnHQnv6s7g2gg@mail.gmail.com>
 <20190402063313.GA32613@kadam> <CAM_iQpW=X2X5A873nSPM7yLBiWZM+rsJk+uMpiOioYPFsFP9vg@mail.gmail.com>
 <20190402201322.GG32613@kadam> <CAM_iQpWjwOwawBN3yiZ0F-B-HDSOW8KGfpLGhAsf5yE0A=wtDA@mail.gmail.com>
 <20190404063521.GF32590@kadam>
In-Reply-To: <20190404063521.GF32590@kadam>
From:   Cong Wang <xiyou.wangcong@gmail.com>
Date:   Fri, 5 Apr 2019 09:28:57 -0700
Message-ID: <CAM_iQpUgv5HhiPV2P+swHbSfnnhWNTNCLnNAK=WCTcp3HZVTmQ@mail.gmail.com>
Subject: Re: [PATCH] Bluetooth: hci_event: potential out of bounds parsing ADV events
To:     Dan Carpenter <dan.carpenter@oracle.com>
Cc:     Tomas Bortoli <tomasbortoli@gmail.com>,
        Marcel Holtmann <marcel@holtmann.org>,
        Jaganath Kanakkassery <jaganath.k.os@gmail.com>,
        Johan Hedberg <johan.hedberg@gmail.com>,
        linux-bluetooth <linux-bluetooth@vger.kernel.org>,
        kernel-janitors@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Sender: linux-bluetooth-owner@vger.kernel.org
Precedence: bulk

On Wed, Apr 3, 2019 at 11:35 PM Dan Carpenter <dan.carpenter@oracle.com> wrote:
>
> On Wed, Apr 03, 2019 at 03:51:18PM -0700, Cong Wang wrote:
> > On Tue, Apr 2, 2019 at 1:15 PM Dan Carpenter <dan.carpenter@oracle.com> wrote:
> > >
> > > On Tue, Apr 02, 2019 at 10:42:38AM -0700, Cong Wang wrote:
> > > > > Btw, get rid of all the likely/unlikely() macros.  Then the other style
> > > > > comment would be don't move the "ev = (void *)skb->data;" assignments
> > > > > around.  It's ok to say:
> > > >
> > > >
> > > > Similarly, pskb_may_pull() may reallocate skb's, although very unlikely
> > > > for bluetooth case (skb's are linear). At least it doesn't harm anything
> > > > we move the skb->data dereference after pskb_may_pull().
> > > >
> > >
> > > It harms readability.
> >
> > Why? I can't see how it harms readability if you have pskb_may_pull()
> > in mind that it potentially reallocates skb->data.
>
> You're making the code more complicated because you're pretending that
> we didn't linearize the skb data already...  :/

I am not pretending it, I just want to use a standard networking API
which covers all cases.