| Message ID | 20240830080135.3544948-1-lihongbo22@huawei.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [-next] ALSA: pcm: replace simple_strtoul to kstrtoul | expand |
On Fri, 30 Aug 2024 10:01:35 +0200, Hongbo Li wrote: > > As mentioned in [1], "...simple_strtol(), simple_strtoll(), > simple_strtoul(), and simple_strtoull() functions explicitly > ignore overflows, which may lead to unexpected results in callers." > Hence, the use of those functions is discouraged. > > This patch replace the use of the simple_strtoul with the safer > alternatives kstrtoul. > > [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#simple-strtol-simple-strtoll-simple-strtoul-simple-strtoull > > Signed-off-by: Hongbo Li <lihongbo22@huawei.com> > --- > sound/core/pcm_memory.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/sound/core/pcm_memory.c b/sound/core/pcm_memory.c > index 8e4c68e3bbd0..fea8bed02604 100644 > --- a/sound/core/pcm_memory.c > +++ b/sound/core/pcm_memory.c > @@ -185,6 +185,7 @@ static void snd_pcm_lib_preallocate_proc_write(struct snd_info_entry *entry, > char line[64], str[64]; > size_t size; > struct snd_dma_buffer new_dmab; > + int ret; > > guard(mutex)(&substream->pcm->open_mutex); > if (substream->runtime) { > @@ -193,8 +194,10 @@ static void snd_pcm_lib_preallocate_proc_write(struct snd_info_entry *entry, > } > if (!snd_info_get_line(buffer, line, sizeof(line))) { > snd_info_get_str(str, line, sizeof(str)); > - size = simple_strtoul(str, NULL, 10) * 1024; > - if ((size != 0 && size < 8192) || size > substream->dma_max) { > + ret = kstrtoul(str, 10, &size); > + size *= 1024; > + if ((ret != 0) || (size != 0 && size < 8192) || The parentheses around "ret != 0" is superfluous. And you can set the value to buffer->error, too. That said, it's better not to put the ret value check in that if block, but put another check right after kstrtoul() call. thanks, Takashi
buffer->error On 2024/8/30 16:22, Takashi Iwai wrote: > On Fri, 30 Aug 2024 10:01:35 +0200, > Hongbo Li wrote: >> >> As mentioned in [1], "...simple_strtol(), simple_strtoll(), >> simple_strtoul(), and simple_strtoull() functions explicitly >> ignore overflows, which may lead to unexpected results in callers." >> Hence, the use of those functions is discouraged. >> >> This patch replace the use of the simple_strtoul with the safer >> alternatives kstrtoul. >> >> [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#simple-strtol-simple-strtoll-simple-strtoul-simple-strtoull >> >> Signed-off-by: Hongbo Li <lihongbo22@huawei.com> >> --- >> sound/core/pcm_memory.c | 7 +++++-- >> 1 file changed, 5 insertions(+), 2 deletions(-) >> >> diff --git a/sound/core/pcm_memory.c b/sound/core/pcm_memory.c >> index 8e4c68e3bbd0..fea8bed02604 100644 >> --- a/sound/core/pcm_memory.c >> +++ b/sound/core/pcm_memory.c >> @@ -185,6 +185,7 @@ static void snd_pcm_lib_preallocate_proc_write(struct snd_info_entry *entry, >> char line[64], str[64]; >> size_t size; >> struct snd_dma_buffer new_dmab; >> + int ret; >> >> guard(mutex)(&substream->pcm->open_mutex); >> if (substream->runtime) { >> @@ -193,8 +194,10 @@ static void snd_pcm_lib_preallocate_proc_write(struct snd_info_entry *entry, >> } >> if (!snd_info_get_line(buffer, line, sizeof(line))) { >> snd_info_get_str(str, line, sizeof(str)); >> - size = simple_strtoul(str, NULL, 10) * 1024; >> - if ((size != 0 && size < 8192) || size > substream->dma_max) { >> + ret = kstrtoul(str, 10, &size); >> + size *= 1024; >> + if ((ret != 0) || (size != 0 && size < 8192) || > > The parentheses around "ret != 0" is superfluous. > And you can set the value to buffer->error, too. > > That said, it's better not to put the ret value check in that if > block, but put another check right after kstrtoul() call. > Do you mean : buffer->error = kstrtoul(str, 10, &size); if (buffer->error != 0) return; size *= 1024; if (size != 0 && size < 8192)) { buffer->error = -EINVAL; return; } Thanks, Hongbo > > thanks, > > Takashi
On Sat, 31 Aug 2024 03:19:06 +0200, Hongbo Li wrote: > > buffer->error > > On 2024/8/30 16:22, Takashi Iwai wrote: > > On Fri, 30 Aug 2024 10:01:35 +0200, > > Hongbo Li wrote: > >> > >> As mentioned in [1], "...simple_strtol(), simple_strtoll(), > >> simple_strtoul(), and simple_strtoull() functions explicitly > >> ignore overflows, which may lead to unexpected results in callers." > >> Hence, the use of those functions is discouraged. > >> > >> This patch replace the use of the simple_strtoul with the safer > >> alternatives kstrtoul. > >> > >> [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#simple-strtol-simple-strtoll-simple-strtoul-simple-strtoull > >> > >> Signed-off-by: Hongbo Li <lihongbo22@huawei.com> > >> --- > >> sound/core/pcm_memory.c | 7 +++++-- > >> 1 file changed, 5 insertions(+), 2 deletions(-) > >> > >> diff --git a/sound/core/pcm_memory.c b/sound/core/pcm_memory.c > >> index 8e4c68e3bbd0..fea8bed02604 100644 > >> --- a/sound/core/pcm_memory.c > >> +++ b/sound/core/pcm_memory.c > >> @@ -185,6 +185,7 @@ static void snd_pcm_lib_preallocate_proc_write(struct snd_info_entry *entry, > >> char line[64], str[64]; > >> size_t size; > >> struct snd_dma_buffer new_dmab; > >> + int ret; > >> guard(mutex)(&substream->pcm->open_mutex); > >> if (substream->runtime) { > >> @@ -193,8 +194,10 @@ static void snd_pcm_lib_preallocate_proc_write(struct snd_info_entry *entry, > >> } > >> if (!snd_info_get_line(buffer, line, sizeof(line))) { > >> snd_info_get_str(str, line, sizeof(str)); > >> - size = simple_strtoul(str, NULL, 10) * 1024; > >> - if ((size != 0 && size < 8192) || size > substream->dma_max) { > >> + ret = kstrtoul(str, 10, &size); > >> + size *= 1024; > >> + if ((ret != 0) || (size != 0 && size < 8192) || > > > > The parentheses around "ret != 0" is superfluous. > > And you can set the value to buffer->error, too. > > > > That said, it's better not to put the ret value check in that if > > block, but put another check right after kstrtoul() call. > > > Do you mean : > > buffer->error = kstrtoul(str, 10, &size); > if (buffer->error != 0) > return; > size *= 1024; > if (size != 0 && size < 8192)) { > buffer->error = -EINVAL; > return; > } Yes, something like that. thanks, Takashi
diff --git a/sound/core/pcm_memory.c b/sound/core/pcm_memory.c index 8e4c68e3bbd0..fea8bed02604 100644 --- a/sound/core/pcm_memory.c +++ b/sound/core/pcm_memory.c @@ -185,6 +185,7 @@ static void snd_pcm_lib_preallocate_proc_write(struct snd_info_entry *entry, char line[64], str[64]; size_t size; struct snd_dma_buffer new_dmab; + int ret; guard(mutex)(&substream->pcm->open_mutex); if (substream->runtime) { @@ -193,8 +194,10 @@ static void snd_pcm_lib_preallocate_proc_write(struct snd_info_entry *entry, } if (!snd_info_get_line(buffer, line, sizeof(line))) { snd_info_get_str(str, line, sizeof(str)); - size = simple_strtoul(str, NULL, 10) * 1024; - if ((size != 0 && size < 8192) || size > substream->dma_max) { + ret = kstrtoul(str, 10, &size); + size *= 1024; + if ((ret != 0) || (size != 0 && size < 8192) || + size > substream->dma_max) { buffer->error = -EINVAL; return; }
As mentioned in [1], "...simple_strtol(), simple_strtoll(), simple_strtoul(), and simple_strtoull() functions explicitly ignore overflows, which may lead to unexpected results in callers." Hence, the use of those functions is discouraged. This patch replace the use of the simple_strtoul with the safer alternatives kstrtoul. [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#simple-strtol-simple-strtoll-simple-strtoul-simple-strtoull Signed-off-by: Hongbo Li <lihongbo22@huawei.com> --- sound/core/pcm_memory.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)