Re: [PATCH v7 3/4] iio: adc: ltc2378: Enable high-speed data capture

[Marcelo Schmitt <marcelo.schmitt1@gmail.com>]

On 07/16, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Integer divide-by-zero leading to kernel panic in `ltc2378_update_conversion_rate` when a sampling frequency of 1 Hz is requested and the PWM driver rounds the period length above 1 second.
> --
> 
> --- Patch [3]: [PATCH v7 3/4] iio: adc: ltc2378: Enable high-speed data capture ---
> commit ef54f2c79f2cac1649922032814adc7fcc5f9684
> Author: Marcelo Schmitt <marcelo.schmitt@analog.com>
> Subject: iio: adc: ltc2378: Enable high-speed data capture
> 
> This commit enables high-speed data capture for the ltc2378 ADC driver by
> utilizing SPI transfer offloading. It allows data acquisition at faster sample
> rates up to 2 MSPS.
> 
> > diff --git a/drivers/iio/adc/ltc2378.c b/drivers/iio/adc/ltc2378.c
> > index 5600c820c7f45..7d29f021cb7b4 100644
> > --- a/drivers/iio/adc/ltc2378.c
> > +++ b/drivers/iio/adc/ltc2378.c
> 
> [ ... ]
> 
> > +	/*
> > +	 * Configure SPI offload PWM trigger.
> > +	 * The trigger should fire after tBUSYLH + tCONV + tDSDOBUSYL.
> > +	 * Minimum time needed: TBUSYLH (13ns) + TCONV (part-specific) + TDSDOBUSYL (5ns)
> > +	 *
> > +	 * Use the same period as CNV PWM to avoid timing issues.
> > +	 * Convert back from period to frequency for the SPI offload API.
> > +	 */
> > +	offload_period_ns = cnv_wf.period_length_ns;
> > +	config.periodic.frequency_hz = div_u64(HZ_PER_GHZ, offload_period_ns);
> > +	min_read_offset = LTC2378_TBUSYLH_NS + st->info->tconv_ns + LTC2378_TDSDOBUSYL_NS;
> > +	offload_offset_ns = min_read_offset;
> > +	count = 100;
> > +	do {
> > +		config.periodic.offset_ns = offload_offset_ns;
> > +		ret = spi_offload_trigger_validate(st->offload_trigger, &config);
> > +		if (ret)
> > +			return ret;
> > +		offload_offset_ns += 10;
> > +	} while (count-- && config.periodic.offset_ns < min_read_offset);
> > +
> > +	/* Check the minimum CNV to SCLK delay is met */
> > +	if (config.periodic.offset_ns < min_read_offset)
> > +		return -EDOM;
> > +
> > +	/* Check the PWM periods remain the same */
> > +	offload_period_ns = div64_u64(HZ_PER_GHZ, config.periodic.frequency_hz);
> 
> [Severity: High]
> Could this result in a kernel panic due to an integer division by zero?
> 
> If a user writes 1 to the IIO in_voltage_sampling_frequency sysfs attribute,
> ltc2378_update_conversion_rate() is called with freq_Hz = 1, and the initial
> requested PWM period is calculated as 1,000,000,000 ns.
> 
> If the underlying hardware PWM driver rounds the period slightly upwards
> (e.g., to 1,000,000,001 ns) during pwm_round_waveform_might_sleep() due to
> clock granularity, offload_period_ns will exceed HZ_PER_GHZ.
> 
> This causes config.periodic.frequency_hz = div_u64(HZ_PER_GHZ, offload_period_ns)
> to evaluate to 0.
> 
> When div64_u64(HZ_PER_GHZ, config.periodic.frequency_hz) is subsequently called,
> it divides by zero.

If we have cnv_wf.period_length_ns == NSEC_PER_SEC + 1 after the offset_ns loop
and config.periodic.frequency_hz == 0 as suggested, then spi_offload_trigger_validate()
will fail the frequency_hz check and ltc2378_update_conversion_rate() errors
out with -EINVAL before reaching div64_u64(HZ_PER_GHZ, config.periodic.frequency_hz).
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/spi/spi-offload-trigger-pwm.c?h=v7.2-rc3#n48

> 
> > +	if (cnv_wf.period_length_ns != offload_period_ns)
> > +		return -EDOM;
> > +
> > +	st->offload_trigger_config = config;
> > +	st->cnv_wf = cnv_wf;
> > +	st->cnv_Hz = DIV_ROUND_CLOSEST_ULL(HZ_PER_GHZ, cnv_wf.period_length_ns);
> > +
> > +	return 0;
> > +}
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/cover.1784235595.git.marcelo.schmitt@analog.com?part=3