[PATCH net 0/6] net_tstamp: Validate hwtstamp_config completely before applying it

[PATCH net 0/6] net_tstamp: Validate hwtstamp_config completely before applying it

[Ben Hutchings]

This series fixes very similar bugs in 6 implementations of
SIOCSHWTSTAMP.

Ben.

Ben Hutchings (6):
  tg3: Validate hwtstamp_config completely before applying it
  e1000e: Validate hwtstamp_config completely before applying it
  pch_gbe: Validate hwtstamp_config completely before applying it
  stmmac: Validate hwtstamp_config completely before applying it
  ti_cpsw: Validate hwtstamp_config completely before applying it
  ixp4xx_eth: Validate hwtstamp_config completely before applying it

 drivers/net/ethernet/broadcom/tg3.c                  | 16 +++++++---------
 drivers/net/ethernet/intel/e1000e/netdev.c           | 14 ++++++--------
 drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c | 12 +++---------
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c    | 12 +++---------
 drivers/net/ethernet/ti/cpsw.c                       | 18 ++++++++----------
 drivers/net/ethernet/xscale/ixp4xx_eth.c             | 12 +++---------
 6 files changed, 30 insertions(+), 54 deletions(-)


-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

[PATCH net 1/6] tg3: Validate hwtstamp_config completely before applying it

[Ben Hutchings]

tg3_hwtstamp_ioctl() should validate all fields of hwtstamp_config
before making any changes.  Currently it sets the TX configuration
before validating the rx_filter field.

Compile-tested only.

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
 drivers/net/ethernet/broadcom/tg3.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index 12d961c4ebca..d3a700f86f1a 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -13598,16 +13598,9 @@ static int tg3_hwtstamp_ioctl(struct net_device *dev,
 	if (stmpconf.flags)
 		return -EINVAL;
 
-	switch (stmpconf.tx_type) {
-	case HWTSTAMP_TX_ON:
-		tg3_flag_set(tp, TX_TSTAMP_EN);
-		break;
-	case HWTSTAMP_TX_OFF:
-		tg3_flag_clear(tp, TX_TSTAMP_EN);
-		break;
-	default:
+	if (stmpconf.tx_type != HWTSTAMP_TX_ON &&
+	    stmpconf.tx_type != HWTSTAMP_TX_OFF)
 		return -ERANGE;
-	}
 
 	switch (stmpconf.rx_filter) {
 	case HWTSTAMP_FILTER_NONE:
@@ -13669,6 +13662,11 @@ static int tg3_hwtstamp_ioctl(struct net_device *dev,
 		tw32(TG3_RX_PTP_CTL,
 		     tp->rxptpctl | TG3_RX_PTP_CTL_HWTS_INTERLOCK);
 
+	if (stmpconf.tx_type == HWTSTAMP_TX_ON)
+		tg3_flag_set(tp, TX_TSTAMP_EN);
+	else
+		tg3_flag_clear(tp, TX_TSTAMP_EN);
+
 	return copy_to_user(ifr->ifr_data, &stmpconf, sizeof(stmpconf)) ?
 		-EFAULT : 0;
 }


-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

[PATCH net 2/6] e1000e: Validate hwtstamp_config completely before applying it

[Ben Hutchings]

e1000e_hwtstamp_ioctl() should validate all fields of hwtstamp_config
before making any changes.  Currently it copies the configuration to
the e1000_adapter structure before validating it at all.

Change e1000e_config_hwtstamp() to take a pointer to the
hwstamp_config and to copy the config after validating it.

Compile-tested only.

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
 drivers/net/ethernet/intel/e1000e/netdev.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index 4ef786775acb..f02816575369 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -3482,10 +3482,10 @@ s32 e1000e_get_base_timinca(struct e1000_adapter *adapter, u32 *timinca)
  * specified. Matching the kind of event packet is not supported, with the
  * exception of "all V2 events regardless of level 2 or 4".
  **/
-static int e1000e_config_hwtstamp(struct e1000_adapter *adapter)
+static int e1000e_config_hwtstamp(struct e1000_adapter *adapter,
+				  struct hwtstamp_config *config)
 {
 	struct e1000_hw *hw = &adapter->hw;
-	struct hwtstamp_config *config = &adapter->hwtstamp_config;
 	u32 tsync_tx_ctl = E1000_TSYNCTXCTL_ENABLED;
 	u32 tsync_rx_ctl = E1000_TSYNCRXCTL_ENABLED;
 	u32 rxmtrl = 0;
@@ -3586,6 +3586,8 @@ static int e1000e_config_hwtstamp(struct e1000_adapter *adapter)
 		return -ERANGE;
 	}
 
+	adapter->hwtstamp_config = *config;
+
 	/* enable/disable Tx h/w time stamping */
 	regval = er32(TSYNCTXCTL);
 	regval &= ~E1000_TSYNCTXCTL_ENABLED;
@@ -3874,7 +3876,7 @@ void e1000e_reset(struct e1000_adapter *adapter)
 	e1000e_reset_adaptive(hw);
 
 	/* initialize systim and reset the ns time counter */
-	e1000e_config_hwtstamp(adapter);
+	e1000e_config_hwtstamp(adapter, &adapter->hwtstamp_config);
 
 	/* Set EEE advertisement as appropriate */
 	if (adapter->flags2 & FLAG2_HAS_EEE) {
@@ -5797,14 +5799,10 @@ static int e1000e_hwtstamp_ioctl(struct net_device *netdev, struct ifreq *ifr)
 	if (copy_from_user(&config, ifr->ifr_data, sizeof(config)))
 		return -EFAULT;
 
-	adapter->hwtstamp_config = config;
-
-	ret_val = e1000e_config_hwtstamp(adapter);
+	ret_val = e1000e_config_hwtstamp(adapter, &config);
 	if (ret_val)
 		return ret_val;
 
-	config = adapter->hwtstamp_config;
-
 	switch (config.rx_filter) {
 	case HWTSTAMP_FILTER_PTP_V2_L4_SYNC:
 	case HWTSTAMP_FILTER_PTP_V2_L2_SYNC:


-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

[PATCH net 3/6] pch_gbe: Validate hwtstamp_config completely before applying it

[Ben Hutchings]

hwtstamp_ioctl() should validate all fields of hwtstamp_config
before making any changes.  Currently it sets the TX configuration
before validating the rx_filter field.

Compile-tested only.

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
 drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
index 5a0f04c2c813..27ffe0ebf0a6 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
@@ -245,16 +245,8 @@ static int hwtstamp_ioctl(struct net_device *netdev, struct ifreq *ifr, int cmd)
 	/* Get ieee1588's dev information */
 	pdev = adapter->ptp_pdev;
 
-	switch (cfg.tx_type) {
-	case HWTSTAMP_TX_OFF:
-		adapter->hwts_tx_en = 0;
-		break;
-	case HWTSTAMP_TX_ON:
-		adapter->hwts_tx_en = 1;
-		break;
-	default:
+	if (cfg.tx_type != HWTSTAMP_TX_OFF && cfg.tx_type != HWTSTAMP_TX_ON)
 		return -ERANGE;
-	}
 
 	switch (cfg.rx_filter) {
 	case HWTSTAMP_FILTER_NONE:
@@ -284,6 +276,8 @@ static int hwtstamp_ioctl(struct net_device *netdev, struct ifreq *ifr, int cmd)
 		return -ERANGE;
 	}
 
+	adapter->hwts_tx_en = cfg.tx_type == HWTSTAMP_TX_ON;
+
 	/* Clear out any old time stamps. */
 	pch_ch_event_write(pdev, TX_SNAPSHOT_LOCKED | RX_SNAPSHOT_LOCKED);
 


-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

[PATCH net 4/6] stmmac: Validate hwtstamp_config completely before applying it

[Ben Hutchings]

stmmac_hwtstamp_ioctl() should validate all fields of hwtstamp_config
before making any changes.  Currently it sets the TX configuration
before validating the rx_filter field.

Compile-tested only.

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 8d4ccd35a016..8a7a23a84ac5 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -435,16 +435,9 @@ static int stmmac_hwtstamp_ioctl(struct net_device *dev, struct ifreq *ifr)
 	if (config.flags)
 		return -EINVAL;
 
-	switch (config.tx_type) {
-	case HWTSTAMP_TX_OFF:
-		priv->hwts_tx_en = 0;
-		break;
-	case HWTSTAMP_TX_ON:
-		priv->hwts_tx_en = 1;
-		break;
-	default:
+	if (config.tx_type != HWTSTAMP_TX_OFF &&
+	    config.tx_type != HWTSTAMP_TX_ON)
 		return -ERANGE;
-	}
 
 	if (priv->adv_ts) {
 		switch (config.rx_filter) {
@@ -576,6 +569,7 @@ static int stmmac_hwtstamp_ioctl(struct net_device *dev, struct ifreq *ifr)
 		}
 	}
 	priv->hwts_rx_en = ((config.rx_filter == HWTSTAMP_FILTER_NONE) ? 0 : 1);
+	priv->hwts_tx_en = config.tx_type == HWTSTAMP_TX_ON;
 
 	if (!priv->hwts_tx_en && !priv->hwts_rx_en)
 		priv->hw->ptp->config_hw_tstamping(priv->ioaddr, 0);


-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

[PATCH net 5/6] ti_cpsw: Validate hwtstamp_config completely before applying it

[Ben Hutchings]

cpsw_hwtstamp_ioctl() should validate all fields of hwtstamp_config,
and the hardware version, before making any changes.  Currently it
sets the TX configuration before validating the rx_filter field
or that the hardware supports timestamping.

Also correct the error code for hardware versions that don't
support timestamping.  ENOTSUPP is used by the NFS implementation
and is not part of userland API; we want EOPNOTSUPP (which glibc
also calls ENOTSUP, with one 'P').

Untested as I don't have a cross-compiler to hand.

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
 drivers/net/ethernet/ti/cpsw.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index cc3ce557e4aa..8bda244df0fb 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -1321,6 +1321,10 @@ static int cpsw_hwtstamp_ioctl(struct net_device *dev, struct ifreq *ifr)
 	struct cpts *cpts = priv->cpts;
 	struct hwtstamp_config cfg;
 
+	if (priv->version != CPSW_VERSION_1 &&
+	    priv->version != CPSW_VERSION_2)
+		return -EOPNOTSUPP;
+
 	if (copy_from_user(&cfg, ifr->ifr_data, sizeof(cfg)))
 		return -EFAULT;
 
@@ -1328,16 +1332,8 @@ static int cpsw_hwtstamp_ioctl(struct net_device *dev, struct ifreq *ifr)
 	if (cfg.flags)
 		return -EINVAL;
 
-	switch (cfg.tx_type) {
-	case HWTSTAMP_TX_OFF:
-		cpts->tx_enable = 0;
-		break;
-	case HWTSTAMP_TX_ON:
-		cpts->tx_enable = 1;
-		break;
-	default:
+	if (cfg.tx_type != HWTSTAMP_TX_OFF && cfg.tx_type != HWTSTAMP_TX_ON)
 		return -ERANGE;
-	}
 
 	switch (cfg.rx_filter) {
 	case HWTSTAMP_FILTER_NONE:
@@ -1364,6 +1360,8 @@ static int cpsw_hwtstamp_ioctl(struct net_device *dev, struct ifreq *ifr)
 		return -ERANGE;
 	}
 
+	cpts->tx_enable = cfg.tx_type == HWTSTAMP_TX_ON;
+
 	switch (priv->version) {
 	case CPSW_VERSION_1:
 		cpsw_hwtstamp_v1(priv);
@@ -1372,7 +1370,7 @@ static int cpsw_hwtstamp_ioctl(struct net_device *dev, struct ifreq *ifr)
 		cpsw_hwtstamp_v2(priv);
 		break;
 	default:
-		return -ENOTSUPP;
+		WARN_ON(1);
 	}
 
 	return copy_to_user(ifr->ifr_data, &cfg, sizeof(cfg)) ? -EFAULT : 0;


-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

[PATCH net 6/6] ixp4xx_eth: Validate hwtstamp_config completely before applying it

[Ben Hutchings]

hwtstamp_ioctl() should validate all fields of hwtstamp_config
before making any changes.  Currently it sets the TX configuration
before validating the rx_filter field.

Untested as I don't have a cross-compiler to hand.

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
 drivers/net/ethernet/xscale/ixp4xx_eth.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/xscale/ixp4xx_eth.c b/drivers/net/ethernet/xscale/ixp4xx_eth.c
index e78802e75ea6..bcc224a83734 100644
--- a/drivers/net/ethernet/xscale/ixp4xx_eth.c
+++ b/drivers/net/ethernet/xscale/ixp4xx_eth.c
@@ -389,16 +389,8 @@ static int hwtstamp_ioctl(struct net_device *netdev, struct ifreq *ifr, int cmd)
 	ch = PORT2CHANNEL(port);
 	regs = (struct ixp46x_ts_regs __iomem *) IXP4XX_TIMESYNC_BASE_VIRT;
 
-	switch (cfg.tx_type) {
-	case HWTSTAMP_TX_OFF:
-		port->hwts_tx_en = 0;
-		break;
-	case HWTSTAMP_TX_ON:
-		port->hwts_tx_en = 1;
-		break;
-	default:
+	if (cfg.tx_type != HWTSTAMP_TX_OFF && cfg.tx_type != HWTSTAMP_TX_ON)
 		return -ERANGE;
-	}
 
 	switch (cfg.rx_filter) {
 	case HWTSTAMP_FILTER_NONE:
@@ -416,6 +408,8 @@ static int hwtstamp_ioctl(struct net_device *netdev, struct ifreq *ifr, int cmd)
 		return -ERANGE;
 	}
 
+	port->hwts_tx_en = cfg.tx_type == HWTSTAMP_TX_ON;
+
 	/* Clear out any old time stamps. */
 	__raw_writel(TX_SNAPSHOT_LOCKED | RX_SNAPSHOT_LOCKED,
 		     &regs->channel[ch].ch_event);

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: [PATCH net 1/6] tg3: Validate hwtstamp_config completely before applying it

[Nithin Nayak Sujir]

On 11/13/2013 04:40 PM, Ben Hutchings wrote:
> tg3_hwtstamp_ioctl() should validate all fields of hwtstamp_config
> before making any changes.  Currently it sets the TX configuration
> before validating the rx_filter field.
>
> Compile-tested only.
>
> Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
> ---
>   drivers/net/ethernet/broadcom/tg3.c | 16 +++++++---------
>   1 file changed, 7 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
> index 12d961c4ebca..d3a700f86f1a 100644
> --- a/drivers/net/ethernet/broadcom/tg3.c
> +++ b/drivers/net/ethernet/broadcom/tg3.c
> @@ -13598,16 +13598,9 @@ static int tg3_hwtstamp_ioctl(struct net_device *dev,
>   	if (stmpconf.flags)
>   		return -EINVAL;
>
> -	switch (stmpconf.tx_type) {
> -	case HWTSTAMP_TX_ON:
> -		tg3_flag_set(tp, TX_TSTAMP_EN);
> -		break;
> -	case HWTSTAMP_TX_OFF:
> -		tg3_flag_clear(tp, TX_TSTAMP_EN);
> -		break;
> -	default:
> +	if (stmpconf.tx_type != HWTSTAMP_TX_ON &&
> +	    stmpconf.tx_type != HWTSTAMP_TX_OFF)
>   		return -ERANGE;
> -	}
>
>   	switch (stmpconf.rx_filter) {
>   	case HWTSTAMP_FILTER_NONE:
> @@ -13669,6 +13662,11 @@ static int tg3_hwtstamp_ioctl(struct net_device *dev,
>   		tw32(TG3_RX_PTP_CTL,
>   		     tp->rxptpctl | TG3_RX_PTP_CTL_HWTS_INTERLOCK);
>
> +	if (stmpconf.tx_type == HWTSTAMP_TX_ON)
> +		tg3_flag_set(tp, TX_TSTAMP_EN);
> +	else
> +		tg3_flag_clear(tp, TX_TSTAMP_EN);
> +
>   	return copy_to_user(ifr->ifr_data, &stmpconf, sizeof(stmpconf)) ?
>   		-EFAULT : 0;
>   }
>
>

Thanks Ben.

Acked-by: Nithin Nayak Sujir <nsujir@broadcom.com>

Re: [PATCH net 0/6] net_tstamp: Validate hwtstamp_config completely before applying it

[David Miller]

From: Ben Hutchings <bhutchings@solarflare.com>
Date: Thu, 14 Nov 2013 00:39:02 +0000

> This series fixes very similar bugs in 6 implementations of
> SIOCSHWTSTAMP.

This looks fine to me, I'll just way a day or so for driver maintainers
to ACK.

Re: [PATCH net 3/6] pch_gbe: Validate hwtstamp_config completely before applying it

[Richard Cochran]

On Thu, Nov 14, 2013 at 12:42:50AM +0000, Ben Hutchings wrote:
> --- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
> +++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
> @@ -245,16 +245,8 @@ static int hwtstamp_ioctl(struct net_device *netdev, struct ifreq *ifr, int cmd)
>  	/* Get ieee1588's dev information */
>  	pdev = adapter->ptp_pdev;
>  
> -	switch (cfg.tx_type) {
> -	case HWTSTAMP_TX_OFF:
> -		adapter->hwts_tx_en = 0;
> -		break;
> -	case HWTSTAMP_TX_ON:
> -		adapter->hwts_tx_en = 1;
> -		break;
> -	default:
> +	if (cfg.tx_type != HWTSTAMP_TX_OFF && cfg.tx_type != HWTSTAMP_TX_ON)
>  		return -ERANGE;
> -	}
>  
>  	switch (cfg.rx_filter) {
>  	case HWTSTAMP_FILTER_NONE:
> @@ -284,6 +276,8 @@ static int hwtstamp_ioctl(struct net_device *netdev, struct ifreq *ifr, int cmd)
>  		return -ERANGE;
>  	}
>  
> +	adapter->hwts_tx_en = cfg.tx_type == HWTSTAMP_TX_ON;
> +

But now there is no way to turn transmit time stamping off?

>  	/* Clear out any old time stamps. */
>  	pch_ch_event_write(pdev, TX_SNAPSHOT_LOCKED | RX_SNAPSHOT_LOCKED);

Thanks,
Richard

Re: [PATCH net 3/6] pch_gbe: Validate hwtstamp_config completely before applying it

[Richard Cochran]

On Thu, Nov 14, 2013 at 08:21:37AM +0100, Richard Cochran wrote:
> On Thu, Nov 14, 2013 at 12:42:50AM +0000, Ben Hutchings wrote:
> >  
> > +	adapter->hwts_tx_en = cfg.tx_type == HWTSTAMP_TX_ON;
> > +
> 
> But now there is no way to turn transmit time stamping off?

Sorry, overlooked that '==' operator. Tricky.

Thanks,
Richard

Re: [PATCH net 3/6] pch_gbe: Validate hwtstamp_config completely before applying it

[David Miller]

From: Richard Cochran <richardcochran@gmail.com>
Date: Thu, 14 Nov 2013 08:21:37 +0100

> On Thu, Nov 14, 2013 at 12:42:50AM +0000, Ben Hutchings wrote:
>> @@ -284,6 +276,8 @@ static int hwtstamp_ioctl(struct net_device *netdev, struct ifreq *ifr, int cmd)
>>  		return -ERANGE;
>>  	}
>>  
>> +	adapter->hwts_tx_en = cfg.tx_type == HWTSTAMP_TX_ON;
>> +
> 
> But now there is no way to turn transmit time stamping off?

He's assigning a boolean, the result of the test:

	cfg.tx_type == HWTSTAMP_TX_ON

to hwts_tx_en, he's not assigning HWTSTAMP_TX_ON to it.

Re: [PATCH net 5/6] ti_cpsw: Validate hwtstamp_config completely before applying it

[Mugunthan V N]

On Thursday 14 November 2013 06:17 AM, Ben Hutchings wrote:
> cpsw_hwtstamp_ioctl() should validate all fields of hwtstamp_config,
> and the hardware version, before making any changes.  Currently it
> sets the TX configuration before validating the rx_filter field
> or that the hardware supports timestamping.
>
> Also correct the error code for hardware versions that don't
> support timestamping.  ENOTSUPP is used by the NFS implementation
> and is not part of userland API; we want EOPNOTSUPP (which glibc
> also calls ENOTSUP, with one 'P').
>
> Untested as I don't have a cross-compiler to hand.
>
> Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
> ---

Acked-by: Mugunthan V N <mugunthanvnm@gmail.com>

Regards
Mugunthan V N

Re: [PATCH net 0/6] net_tstamp: Validate hwtstamp_config completely before applying it

[Richard Cochran]

On Thu, Nov 14, 2013 at 12:39:02AM +0000, Ben Hutchings wrote:
> This series fixes very similar bugs in 6 implementations of
> SIOCSHWTSTAMP.
> 
> Ben.
> 
> Ben Hutchings (6):
>   tg3: Validate hwtstamp_config completely before applying it
>   e1000e: Validate hwtstamp_config completely before applying it
>   pch_gbe: Validate hwtstamp_config completely before applying it
>   stmmac: Validate hwtstamp_config completely before applying it
>   ti_cpsw: Validate hwtstamp_config completely before applying it
>   ixp4xx_eth: Validate hwtstamp_config completely before applying it

For the series,

Acked-by: Richard Cochran <richardcochran@gmail.com>

Re: [E1000-devel] [PATCH net 2/6] e1000e: Validate hwtstamp_config completely before applying it

[Jeff Kirsher]

[-- Attachment #1: Type: text/plain, Size: 662 bytes --]

On Thu, 2013-11-14 at 00:41 +0000, Ben Hutchings wrote:
> e1000e_hwtstamp_ioctl() should validate all fields of hwtstamp_config
> before making any changes.  Currently it copies the configuration to
> the e1000_adapter structure before validating it at all.
> 
> Change e1000e_config_hwtstamp() to take a pointer to the
> hwstamp_config and to copy the config after validating it.
> 
> Compile-tested only.
> 
> Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
> ---
>  drivers/net/ethernet/intel/e1000e/netdev.c | 14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)

Acked-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH net 0/6] net_tstamp: Validate hwtstamp_config completely before applying it

[David Miller]

From: Ben Hutchings <bhutchings@solarflare.com>
Date: Thu, 14 Nov 2013 00:39:02 +0000

> This series fixes very similar bugs in 6 implementations of
> SIOCSHWTSTAMP.

Series applied, thanks Ben.

Re: [PATCH net 4/6] stmmac: Validate hwtstamp_config completely before applying it

[Giuseppe CAVALLARO]

On 11/14/2013 1:44 AM, Ben Hutchings wrote:
> stmmac_hwtstamp_ioctl() should validate all fields of hwtstamp_config
> before making any changes.  Currently it sets the TX configuration
> before validating the rx_filter field.
>
> Compile-tested only.

Thx Ben for this patch.

Rayagond, can you do any run-time tests?

peppe

>
> Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
> ---
>   drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 12 +++---------
>   1 file changed, 3 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 8d4ccd35a016..8a7a23a84ac5 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -435,16 +435,9 @@ static int stmmac_hwtstamp_ioctl(struct net_device *dev, struct ifreq *ifr)
>   	if (config.flags)
>   		return -EINVAL;
>
> -	switch (config.tx_type) {
> -	case HWTSTAMP_TX_OFF:
> -		priv->hwts_tx_en = 0;
> -		break;
> -	case HWTSTAMP_TX_ON:
> -		priv->hwts_tx_en = 1;
> -		break;
> -	default:
> +	if (config.tx_type != HWTSTAMP_TX_OFF &&
> +	    config.tx_type != HWTSTAMP_TX_ON)
>   		return -ERANGE;
> -	}
>
>   	if (priv->adv_ts) {
>   		switch (config.rx_filter) {
> @@ -576,6 +569,7 @@ static int stmmac_hwtstamp_ioctl(struct net_device *dev, struct ifreq *ifr)
>   		}
>   	}
>   	priv->hwts_rx_en = ((config.rx_filter == HWTSTAMP_FILTER_NONE) ? 0 : 1);
> +	priv->hwts_tx_en = config.tx_type == HWTSTAMP_TX_ON;
>
>   	if (!priv->hwts_tx_en && !priv->hwts_rx_en)
>   		priv->hw->ptp->config_hw_tstamping(priv->ioaddr, 0);
>
>

Re: [PATCH net 4/6] stmmac: Validate hwtstamp_config completely before applying it

[Rayagond K]

On Tue, Nov 19, 2013 at 10:56 AM, Giuseppe CAVALLARO
<peppe.cavallaro@st.com> wrote:
> On 11/14/2013 1:44 AM, Ben Hutchings wrote:
>>
>> stmmac_hwtstamp_ioctl() should validate all fields of hwtstamp_config
>> before making any changes.  Currently it sets the TX configuration
>> before validating the rx_filter field.
>>
>> Compile-tested only.
>
>
> Thx Ben for this patch.
>
> Rayagond, can you do any run-time tests?

I don't have HW now. But next week I will be getting GMAC 4.0, so I
can do similar changes in that driver and do run-time test.

>
> peppe
>
>
>>
>> Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
>> ---
>>   drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 12 +++---------
>>   1 file changed, 3 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> index 8d4ccd35a016..8a7a23a84ac5 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> @@ -435,16 +435,9 @@ static int stmmac_hwtstamp_ioctl(struct net_device
>> *dev, struct ifreq *ifr)
>>         if (config.flags)
>>                 return -EINVAL;
>>
>> -       switch (config.tx_type) {
>> -       case HWTSTAMP_TX_OFF:
>> -               priv->hwts_tx_en = 0;
>> -               break;
>> -       case HWTSTAMP_TX_ON:
>> -               priv->hwts_tx_en = 1;
>> -               break;
>> -       default:
>> +       if (config.tx_type != HWTSTAMP_TX_OFF &&
>> +           config.tx_type != HWTSTAMP_TX_ON)
>>                 return -ERANGE;
>> -       }
>>
>>         if (priv->adv_ts) {
>>                 switch (config.rx_filter) {
>> @@ -576,6 +569,7 @@ static int stmmac_hwtstamp_ioctl(struct net_device
>> *dev, struct ifreq *ifr)
>>                 }
>>         }
>>         priv->hwts_rx_en = ((config.rx_filter == HWTSTAMP_FILTER_NONE) ? 0
>> : 1);
>> +       priv->hwts_tx_en = config.tx_type == HWTSTAMP_TX_ON;
>>
>>         if (!priv->hwts_tx_en && !priv->hwts_rx_en)
>>                 priv->hw->ptp->config_hw_tstamping(priv->ioaddr, 0);
>>
>>
>