From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dan Carpenter Date: Thu, 04 Apr 2019 06:35:21 +0000 Subject: Re: [PATCH] Bluetooth: hci_event: potential out of bounds parsing ADV events Message-Id: <20190404063521.GF32590@kadam> List-Id: References: <20190330072511.GA5502@kadam> <20190402063313.GA32613@kadam> <20190402201322.GG32613@kadam> In-Reply-To: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Cong Wang Cc: Tomas Bortoli , Marcel Holtmann , Jaganath Kanakkassery , Johan Hedberg , linux-bluetooth , kernel-janitors@vger.kernel.org On Wed, Apr 03, 2019 at 03:51:18PM -0700, Cong Wang wrote: > On Tue, Apr 2, 2019 at 1:15 PM Dan Carpenter 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... :/ regards, dan carpenter