[Intel-wired-lan] [PATCH net v1] i40e: fix PTP pins verification

[Paul Menzel]

Dear Andrii,


Am 30.03.23 um 09:17 schrieb Andrii Staikov:
> Fix PTP pins verification not to contain tainted arguments.
> As a new PTP pins configuration is provided by a user, it
> may contain tainted arguments that are out of bounds for the
> list of possible values that can lead to a potential security threat.
> Change pin's state name from 'invalid' to 'empty' for more
> clarification.

Please use the full line width of 75 characters per line.

Also, how did you test this?

> Fixes: 1050713026a0 ("i40e: add support for PTP external synchronization clock")
> Signed-off-by: Andrii Staikov <andrii.staikov at intel.com>
> ---
>   drivers/net/ethernet/intel/i40e/i40e_ptp.c | 16 ++++++++++++----
>   1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_ptp.c b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
> index c37abbb3cd06..78e7c705cd89 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_ptp.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
> @@ -49,7 +49,7 @@ static struct ptp_pin_desc sdp_desc[] = {
>   
>   enum i40e_ptp_gpio_pin_state {
>   	end = -2,
> -	invalid,
> +	empty,
>   	off,
>   	in_A,
>   	in_B,
> @@ -1078,11 +1078,19 @@ static int i40e_ptp_set_pins(struct i40e_pf *pf,
>   	else if (pin_caps == CAN_DO_PINS)
>   		return 0;
>   
> -	if (pins->sdp3_2 == invalid)
> +	if ((pins->sdp3_2 < empty || pins->sdp3_2 > out_B) ||
> +	    (pins->sdp3_3 < empty || pins->sdp3_3 > out_B) ||
> +	    (pins->gpio_4 < empty || pins->gpio_4 > out_B)) {
> +		dev_warn(&pf->pdev->dev,
> +			 "The provided PTP configuration set contains meaningless values that may potentially pose a safety threat.\n");

Would it be better to tell the user the wrong argument?

> +		return -EPERM;
> +	}
> +
> +	if (pins->sdp3_2 == empty)
>   		pins->sdp3_2 = pf->ptp_pins->sdp3_2;
> -	if (pins->sdp3_3 == invalid)
> +	if (pins->sdp3_3 == empty)
>   		pins->sdp3_3 = pf->ptp_pins->sdp3_3;
> -	if (pins->gpio_4 == invalid)
> +	if (pins->gpio_4 == empty)
>   		pins->gpio_4 = pf->ptp_pins->gpio_4;
>   	while (i40e_ptp_pin_led_allowed_states[i].sdp3_2 != end) {
>   		if (pins->sdp3_2 == i40e_ptp_pin_led_allowed_states[i].sdp3_2 &&


Kind regards,

Paul