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]
Message-ID: <7275436b02f9551807f68784d4f4ebaf0adbc35e.camel@mellanox.com>
Date:   Thu, 14 Nov 2019 23:57:42 +0000
From:   Saeed Mahameed <saeedm@...lanox.com>
To:     "richardcochran@...il.com" <richardcochran@...il.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
CC:     "christopher.s.hall@...el.com" <christopher.s.hall@...el.com>,
        Eugenia Emantayev <eugenia@...lanox.com>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "sergei.shtylyov@...entembedded.com" 
        <sergei.shtylyov@...entembedded.com>,
        Feras Daoud <ferasda@...lanox.com>,
        "stefan.sorensen@...ctralink.com" <stefan.sorensen@...ctralink.com>,
        "aaron.f.brown@...el.com" <aaron.f.brown@...el.com>,
        "brandon.streiff@...com" <brandon.streiff@...com>,
        "jacob.e.keller@...el.com" <jacob.e.keller@...el.com>,
        "jeffrey.t.kirsher@...el.com" <jeffrey.t.kirsher@...el.com>,
        "intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>,
        "felipe.balbi@...ux.intel.com" <felipe.balbi@...ux.intel.com>
Subject: Re: [PATCH net 02/13] net: reject PTP periodic output requests with
 unsupported flags

On Thu, 2019-11-14 at 10:44 -0800, Richard Cochran wrote:
> From: Jacob Keller <jacob.e.keller@...el.com>
> 
> Commit 823eb2a3c4c7 ("PTP: add support for one-shot output")
> introduced
> a new flag for the PTP periodic output request ioctl. This flag is
> not
> currently supported by any driver.
> 
> Fix all drivers which implement the periodic output request ioctl to
> explicitly reject any request with flags they do not understand. This
> ensures that the driver does not accidentally misinterpret the
> PTP_PEROUT_ONE_SHOT flag, or any new flag introduced in the future.
> 
> This is important for forward compatibility: if a new flag is
> introduced, the driver should reject requests to enable the flag
> until
> the driver has actually been modified to support the flag in
> question.

LGTM, just there might be a problem with old tools that didn't clear
the flags upon request and expected PTP periodic .. they will stop to
work with new kernel, am i am not sure if such tools do exist.

But the fact now that we have PTP_PEROUT_ONE_SHOT, we need to trust
both driver and tools to do the right thing.

What are the tools to test PTP_PEROUT_ONE_SHOT ? to support this in
mlx5 it is just a matter of a flipping a bit.

Reviewed-by: Saeed Mahameed <saeedm@...lanox.com>

> 
> Cc: Felipe Balbi <felipe.balbi@...ux.intel.com>
> Cc: David S. Miller <davem@...emloft.net>
> Cc: Christopher Hall <christopher.s.hall@...el.com>
> Signed-off-by: Jacob Keller <jacob.e.keller@...el.com>
> Signed-off-by: Richard Cochran <richardcochran@...il.com>
> Tested-by: Aaron Brown <aaron.f.brown@...el.com>
> ---
>  drivers/net/ethernet/broadcom/tg3.c                 | 4 ++++
>  drivers/net/ethernet/intel/igb/igb_ptp.c            | 4 ++++
>  drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c | 4 ++++
>  drivers/net/ethernet/microchip/lan743x_ptp.c        | 4 ++++
>  drivers/net/ethernet/renesas/ravb_ptp.c             | 4 ++++
>  drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c    | 4 ++++
>  drivers/net/phy/dp83640.c                           | 3 +++
>  7 files changed, 27 insertions(+)
> 
> diff --git a/drivers/net/ethernet/broadcom/tg3.c
> b/drivers/net/ethernet/broadcom/tg3.c
> index 77f3511b97de..ca3aa1250dd1 100644
> --- a/drivers/net/ethernet/broadcom/tg3.c
> +++ b/drivers/net/ethernet/broadcom/tg3.c
> @@ -6280,6 +6280,10 @@ static int tg3_ptp_enable(struct
> ptp_clock_info *ptp,
>  
>  	switch (rq->type) {
>  	case PTP_CLK_REQ_PEROUT:
> +		/* Reject requests with unsupported flags */
> +		if (rq->perout.flags)
> +			return -EOPNOTSUPP;
> +
>  		if (rq->perout.index != 0)
>  			return -EINVAL;
>  
> diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c
> b/drivers/net/ethernet/intel/igb/igb_ptp.c
> index fd3071f55bd3..4997963149f6 100644
> --- a/drivers/net/ethernet/intel/igb/igb_ptp.c
> +++ b/drivers/net/ethernet/intel/igb/igb_ptp.c
> @@ -551,6 +551,10 @@ static int igb_ptp_feature_enable_i210(struct
> ptp_clock_info *ptp,
>  		return 0;
>  
>  	case PTP_CLK_REQ_PEROUT:
> +		/* Reject requests with unsupported flags */
> +		if (rq->perout.flags)
> +			return -EOPNOTSUPP;
> +
>  		if (on) {
>  			pin = ptp_find_pin(igb->ptp_clock,
> PTP_PF_PEROUT,
>  					   rq->perout.index);
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c
> b/drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c
> index 0059b290e095..cff6b60de304 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c
> @@ -290,6 +290,10 @@ static int mlx5_perout_configure(struct
> ptp_clock_info *ptp,
>  	if (!MLX5_PPS_CAP(mdev))
>  		return -EOPNOTSUPP;
>  
> +	/* Reject requests with unsupported flags */
> +	if (rq->perout.flags)
> +		return -EOPNOTSUPP;
> +
>  	if (rq->perout.index >= clock->ptp_info.n_pins)
>  		return -EINVAL;
>  
> diff --git a/drivers/net/ethernet/microchip/lan743x_ptp.c
> b/drivers/net/ethernet/microchip/lan743x_ptp.c
> index 57b26c2acf87..e8fe9a90fe4f 100644
> --- a/drivers/net/ethernet/microchip/lan743x_ptp.c
> +++ b/drivers/net/ethernet/microchip/lan743x_ptp.c
> @@ -429,6 +429,10 @@ static int lan743x_ptp_perout(struct
> lan743x_adapter *adapter, int on,
>  	int pulse_width = 0;
>  	int perout_bit = 0;
>  
> +	/* Reject requests with unsupported flags */
> +	if (perout->flags)
> +		return -EOPNOTSUPP;
> +
>  	if (!on) {
>  		lan743x_ptp_perout_off(adapter);
>  		return 0;
> diff --git a/drivers/net/ethernet/renesas/ravb_ptp.c
> b/drivers/net/ethernet/renesas/ravb_ptp.c
> index 9a42580693cb..638f1fc2166f 100644
> --- a/drivers/net/ethernet/renesas/ravb_ptp.c
> +++ b/drivers/net/ethernet/renesas/ravb_ptp.c
> @@ -211,6 +211,10 @@ static int ravb_ptp_perout(struct ptp_clock_info
> *ptp,
>  	unsigned long flags;
>  	int error = 0;
>  
> +	/* Reject requests with unsupported flags */
> +	if (req->flags)
> +		return -EOPNOTSUPP;
> +
>  	if (req->index)
>  		return -EINVAL;
>  
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
> b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
> index df638b18b72c..0989e2bb6ee3 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
> @@ -140,6 +140,10 @@ static int stmmac_enable(struct ptp_clock_info
> *ptp,
>  
>  	switch (rq->type) {
>  	case PTP_CLK_REQ_PEROUT:
> +		/* Reject requests with unsupported flags */
> +		if (rq->perout.flags)
> +			return -EOPNOTSUPP;
> +
>  		cfg = &priv->pps[rq->perout.index];
>  
>  		cfg->start.tv_sec = rq->perout.start.sec;
> diff --git a/drivers/net/phy/dp83640.c b/drivers/net/phy/dp83640.c
> index 6580094161a9..04ad77758920 100644
> --- a/drivers/net/phy/dp83640.c
> +++ b/drivers/net/phy/dp83640.c
> @@ -491,6 +491,9 @@ static int ptp_dp83640_enable(struct
> ptp_clock_info *ptp,
>  		return 0;
>  
>  	case PTP_CLK_REQ_PEROUT:
> +		/* Reject requests with unsupported flags */
> +		if (rq->perout.flags)
> +			return -EOPNOTSUPP;
>  		if (rq->perout.index >= N_PER_OUT)
>  			return -EINVAL;
>  		return periodic_output(clock, rq, on, rq-
> >perout.index);

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ