Messages in this thread | | | From | Srinivas Neeli <> | Subject | RE: Query on possible bug in the can_create_echo_skb() API | Date | Wed, 28 Aug 2019 11:02:43 +0000 |
| |
Hi,
Case 1: can_put_echo_skb(); -> skb = can_create_echo_skb(skb); -> return skb;
In can_create_echo_skb() not using the shared_skb, so we are returning the old skb. Storing the return value in "skb". But it's a pointer, for storing that need double pointer. Instead of double-pointer using a single pointer. In this scenario it's ok , we are returning the same SKB.
Case 2: can_put_echo_skb(skb, ndev, priv->tx_head % priv->tx_max); -> skb = can_create_echo_skb(skb); -> can_skb_set_owner(nskb, skb->sk); - Returning nskb;
shared_skb scenario: In share-skb case “can_create_echo_skb(skb);” returning "new skb". For storing new skb need a double pointer.
Providing an example for overcoming above issue. Example: can_put_echo_skb(struct sk_buff **skb,struct net_device *dev,unsigned int idx);
If you ok with this change, I will send a patch.
Thanks Srinivas Neeli
> -----Original Message----- > From: Marc Kleine-Budde <mkl@pengutronix.de> > Sent: Wednesday, August 28, 2019 1:03 AM > To: Srinivas Neeli <sneeli@xilinx.com>; wg@grandegger.com > Cc: Srinivas Goud <sgoud@xilinx.com>; Naga Sureshkumar Relli > <nagasure@xilinx.com>; Appana Durga Kedareswara Rao > <appanad@xilinx.com>; linux-can@vger.kernel.org > Subject: Re: Query on possible bug in the can_create_echo_skb() API > > Hello Srinivas Neeli, > > please don't send HTML messages to the kernel mailinglists. > > On 8/21/19 12:51 PM, Srinivas Neeli wrote: > > While walking through the CAN core layer dev.c file in the > > can_put_echo_skb() API [1], Seems to be there is a race condition in > > the > > can_create_echo_skb() API, more details below > > > > If the skb is a shared skb, we are overwriting the skb pointer [2] in > > the can_create_echo_skb() API and returning the new skb back. > > Where and how is the skb pointer overwritten? Can you explain a bit more. > > > If the core layer/drivers use this skb it is not valid any more (it > > may lead to crash/oops). > > > > > > > > A possible solution for this issue would make the function input > > argument should be double-pointer. > > > > Please correct me if my analyzation is wrong. > > Can you provide a patch of your proposed changes? > > regards, > Marc > > -- > Pengutronix e.K. | Marc Kleine-Budde | > Industrial Linux Solutions | Phone: +49-231-2826-924 | > Vertretung West/Dortmund | Fax: +49-5121-206917-5555 | > Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
| |