netdev - Re: Query on possible bug in the can_create_echo

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] [day] [month] [year] [list]

Message-ID: <3a8c557f-adb5-144d-00f1-1b1c608129dc@pengutronix.de>
Date:   Wed, 28 Aug 2019 14:15:51 +0200
From:   Marc Kleine-Budde <mkl@...gutronix.de>
To:     Srinivas Neeli <sneeli@...inx.com>,
        "wg@...ndegger.com" <wg@...ndegger.com>
Cc:     Srinivas Goud <sgoud@...inx.com>,
        Naga Sureshkumar Relli <nagasure@...inx.com>,
        Appana Durga Kedareswara Rao <appanad@...inx.com>,
        "linux-can@...r.kernel.org" <linux-can@...r.kernel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: Query on possible bug in the can_create_echo_skb() API

On 8/28/19 1:02 PM, Srinivas Neeli wrote:
> 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);

Now I get what you mean.

> If you ok with this change, I will send a patch.

I think the can_put_echo_skb() API needs clarification. The driver is
not allowed to touch the skb any more after can_put_echo_skb(). We
should review the driver for this.

thanks,
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   |



Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)