[PATCH v2] staging: comedi: ni_mio_common: protect register write overflow

Ian Abbott abbotti at mev.co.uk
Thu Oct 4 10:53:03 UTC 2018


On 03/10/18 21:54, Spencer E. Olson wrote:
> Fixes two problems introduced as early as
> commit 03aef4b6dc12  ("Staging: comedi: add ni_mio_common code"):
> (1) Ensures that the last four bits of NISTC_RTSI_TRIGB_OUT_REG register is
>      not unduly overwritten on e-series devices.  On e-series devices, the
>      first three of the last four bits are reserved.  The last bit defines
>      the output selection of the RGOUT0 pin, otherwise known as
>      RTSI_Sub_Selection.  For m-series devices, these last four bits are
>      indeed used as the output selection of the RTSI7 pin (and the
>      RTSI_Sub_Selection bit for the RGOUT0 pin is moved to the
>      RTSI_Trig_Direction register.
> (2) Allows all 4 RTSI_BRD lines to be treated as valid sources for RTSI
>      lines.
> 
> This patch also cleans up the ni_get_rtsi_routing command for readability.
> 
> Fixes: 03aef4b6dc12  ("Staging: comedi: add ni_mio_common code")
> Signed-off-by: Spencer E. Olson <olsonse at umich.edu>
> ---
> 
> Changes:
>    - [PATCH v2]: Adds missing brace that had been corrected in a later patchset
>      but should have been correct here.  This patch is now independent and
>      complete.
> 
>   .../staging/comedi/drivers/ni_mio_common.c    | 24 +++++++++++++------
>   1 file changed, 17 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/staging/comedi/drivers/ni_mio_common.c b/drivers/staging/comedi/drivers/ni_mio_common.c
> index 4dee2fc37aed..44fcb3790113 100644
> --- a/drivers/staging/comedi/drivers/ni_mio_common.c
> +++ b/drivers/staging/comedi/drivers/ni_mio_common.c
> @@ -4980,7 +4980,10 @@ static int ni_valid_rtsi_output_source(struct comedi_device *dev,
>   	case NI_RTSI_OUTPUT_G_SRC0:
>   	case NI_RTSI_OUTPUT_G_GATE0:
>   	case NI_RTSI_OUTPUT_RGOUT0:
> -	case NI_RTSI_OUTPUT_RTSI_BRD_0:
> +	case NI_RTSI_OUTPUT_RTSI_BRD(0):
> +	case NI_RTSI_OUTPUT_RTSI_BRD(1):
> +	case NI_RTSI_OUTPUT_RTSI_BRD(2):
> +	case NI_RTSI_OUTPUT_RTSI_BRD(3):
>   		return 1;
>   	case NI_RTSI_OUTPUT_RTSI_OSC:
>   		return (devpriv->is_m_series) ? 1 : 0;
> @@ -5001,11 +5004,18 @@ static int ni_set_rtsi_routing(struct comedi_device *dev,
>   		devpriv->rtsi_trig_a_output_reg |= NISTC_RTSI_TRIG(chan, src);
>   		ni_stc_writew(dev, devpriv->rtsi_trig_a_output_reg,
>   			      NISTC_RTSI_TRIGA_OUT_REG);
> -	} else if (chan < 8) {
> +	} else if (chan < NISTC_RTSI_TRIG_NUM_CHAN(devpriv->is_m_series)) {
>   		devpriv->rtsi_trig_b_output_reg &= ~NISTC_RTSI_TRIG_MASK(chan);
>   		devpriv->rtsi_trig_b_output_reg |= NISTC_RTSI_TRIG(chan, src);
>   		ni_stc_writew(dev, devpriv->rtsi_trig_b_output_reg,
>   			      NISTC_RTSI_TRIGB_OUT_REG);
> +	} else if (chan != NISTC_RTSI_TRIG_OLD_CLK_CHAN) {
> +		/* probably should never reach this, since the
> +		 * ni_valid_rtsi_output_source above errors out if chan is too
> +		 * high
> +		 */
> +		dev_err(dev->class_dev, "%s: unknown rtsi channel\n", __func__);
> +		return -EINVAL;
>   	}
>   	return 2;
>   }
> @@ -5021,12 +5031,12 @@ static unsigned int ni_get_rtsi_routing(struct comedi_device *dev,
>   	} else if (chan < NISTC_RTSI_TRIG_NUM_CHAN(devpriv->is_m_series)) {
>   		return NISTC_RTSI_TRIG_TO_SRC(chan,
>   					      devpriv->rtsi_trig_b_output_reg);
> -	} else {
> -		if (chan == NISTC_RTSI_TRIG_OLD_CLK_CHAN)
> -			return NI_RTSI_OUTPUT_RTSI_OSC;
> -		dev_err(dev->class_dev, "bug! should never get here?\n");
> -		return 0;
> +	} else if (chan == NISTC_RTSI_TRIG_OLD_CLK_CHAN) {
> +		return NI_RTSI_OUTPUT_RTSI_OSC;
>   	}
> +
> +	dev_err(dev->class_dev, "%s: unknown rtsi channel\n", __func__);
> +	return -EINVAL;

I'm not really sure what it should return for an invalid channel, since 
this value gets written to the data[1] of the INSN_CONFIG_GET_ROUTING 
instruction which doesn't return an error.

Perhaps it would be better if INSN_CONFIG_GET_ROUTING returned an error 
for an out of range channel, but that's a change for another day.

>   }
>   
>   static int ni_rtsi_insn_config(struct comedi_device *dev,
> 

Reviewed-by: Ian Abbott <abbotti at mev.co.uk>

-- 
-=( Ian Abbott <abbotti at mev.co.uk> || Web: www.mev.co.uk )=-
-=( MEV Ltd. is a company registered in England & Wales. )=-
-=( Registered number: 02862268.  Registered address:    )=-
-=( 15 West Park Road, Bramhall, STOCKPORT, SK7 3JZ, UK. )=-


More information about the devel mailing list