lkml.org 
[lkml]   [2023]   [Sep]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH] iio: adc: stm32-adc: replace deprecated strncpy
On Thu, Sep 21, 2023 at 04:54:00AM +0000, Justin Stitt wrote:
> `strncpy` is deprecated for use on NUL-terminated destination strings [1].
>
> We should prefer more robust and less ambiguous string interfaces.
>
> We expect adc->chan_name[val] to be NUL-terminated based on ch_name's
> use within functions that expect NUL-terminated strings like strncmp and
> printf-likes:
> | if (!strncmp(stm32_adc_ic[i].name, ch_name, STM32_ADC_CH_SZ)) {
> | /* Check internal channel availability */
> | switch (i) {
> | case STM32_ADC_INT_CH_VDDCORE:
> | if (!adc->cfg->regs->or_vddcore.reg)
> | dev_warn(&indio_dev->dev,
> | "%s channel not available\n", ch_name);
> ...
>
> There is no evidence that NUL-padding is needed either.

Agreed -- it's used as a C string everywhere I can see.

>
> Considering the above, a suitable replacement is `strscpy` [2] due to
> the fact that it guarantees NUL-termination on the destination buffer
> without unnecessarily NUL-padding. If, for any reason, NUL-padding _is_
> required we should go for `strscpy_pad`.
>
> Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1]
> Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2]
> Link: https://github.com/KSPP/linux/issues/90
> Cc: linux-hardening@vger.kernel.org
> Signed-off-by: Justin Stitt <justinstitt@google.com>
> ---
> Note: build-tested
> ---
> drivers/iio/adc/stm32-adc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
> index f7613efb870d..9cdcf396d901 100644
> --- a/drivers/iio/adc/stm32-adc.c
> +++ b/drivers/iio/adc/stm32-adc.c
> @@ -2209,7 +2209,7 @@ static int stm32_adc_generic_chan_init(struct iio_dev *indio_dev,
> ret = -EINVAL;
> goto err;
> }
> - strncpy(adc->chan_name[val], name, STM32_ADC_CH_SZ);
> + strscpy(adc->chan_name[val], name, STM32_ADC_CH_SZ);

I still prefer sizeof($dst), but yes, these are the same:

char chan_name[STM32_ADC_CH_MAX][STM32_ADC_CH_SZ];

If this needs a v2, please improve the Subject, but it is technically
correct, so:

Reviewed-by: Kees Cook <keescook@chromium.org>

:)

-Kees
--
Kees Cook

\
 
 \ /
  Last update: 2023-09-24 05:16    [W:0.230 / U:0.336 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site