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]
Date:	Sun, 7 Sep 2014 23:50:39 +0200
From:	Richard Cochran <richardcochran@...il.com>
To:	Alexander Duyck <alexander.h.duyck@...el.com>
Cc:	netdev@...r.kernel.org, davem@...emloft.net, willemb@...gle.com
Subject: Re: [PATCH net-next v4 2/3] net-timestamp: Make the clone operation
 stand-alone from phy timestamping

Just saw this now, was away on vacation, so sorry for the delay...

On Thu, Sep 04, 2014 at 01:31:35PM -0400, Alexander Duyck wrote:
> v2: Renamed function to skb_clone_sk.
>     Added destructor to call sock_put instead of doing it ourselves.
>     Dropped freeing functionality from skb_complete_tx_timestamp.

...

> diff --git a/drivers/net/phy/dp83640.c b/drivers/net/phy/dp83640.c
> index d5991ac..87648b3 100644
> --- a/drivers/net/phy/dp83640.c
> +++ b/drivers/net/phy/dp83640.c
> @@ -1148,7 +1148,7 @@ static void dp83640_remove(struct phy_device *phydev)
>  		kfree_skb(skb);
>  
>  	while ((skb = skb_dequeue(&dp83640->tx_queue)) != NULL)
> -		skb_complete_tx_timestamp(skb, NULL);
> +		kfree_skb(skb);

The way the code was before, there was a clear usage pattern for
phy_driver.txtstamp() and skb_complete_tx_timestamp() which was also
documented in the comment to the latter.

Now, we have drivers freeing buffers allocated by the stack.  I
thought it was cleaner to have the same layer allocate and free the
clone. Even if you say that this new way is just fine, still you
should correct the comment to reflect the new pattern.

Thanks,
Richard
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists