drivers/iio/common/scmi_sensors/scmi_iio.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
Replace calls of devm_kzalloc() with devm_kcalloc() in scmi_alloc_iiodev()
and scmi_iio_set_sampling_freq_avail() for safer memory allocation with
built-in overflow protection.
Signed-off-by: Qianfeng Rong <rongqianfeng@vivo.com>
---
drivers/iio/common/scmi_sensors/scmi_iio.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/iio/common/scmi_sensors/scmi_iio.c b/drivers/iio/common/scmi_sensors/scmi_iio.c
index da516c46e057..19798ea9de3e 100644
--- a/drivers/iio/common/scmi_sensors/scmi_iio.c
+++ b/drivers/iio/common/scmi_sensors/scmi_iio.c
@@ -521,9 +521,9 @@ static int scmi_iio_set_sampling_freq_avail(struct iio_dev *iio_dev)
int i;
sensor->freq_avail =
- devm_kzalloc(&iio_dev->dev,
- sizeof(*sensor->freq_avail) *
- (sensor->sensor_info->intervals.count * 2),
+ devm_kcalloc(&iio_dev->dev,
+ sensor->sensor_info->intervals.count * 2,
+ sizeof(*sensor->freq_avail),
GFP_KERNEL);
if (!sensor->freq_avail)
return -ENOMEM;
@@ -597,8 +597,8 @@ scmi_alloc_iiodev(struct scmi_device *sdev,
iiodev->info = &scmi_iio_info;
iio_channels =
- devm_kzalloc(dev,
- sizeof(*iio_channels) * (iiodev->num_channels),
+ devm_kcalloc(dev, iiodev->num_channels,
+ sizeof(*iio_channels),
GFP_KERNEL);
if (!iio_channels)
return ERR_PTR(-ENOMEM);
--
2.34.1On Tue, Aug 19, 2025 at 11:56 AM Qianfeng Rong <rongqianfeng@vivo.com> wrote: > > Replace calls of devm_kzalloc() with devm_kcalloc() in scmi_alloc_iiodev() > and scmi_iio_set_sampling_freq_avail() for safer memory allocation with > built-in overflow protection. While this change is correct... ... > sensor->freq_avail = > - devm_kzalloc(&iio_dev->dev, > - sizeof(*sensor->freq_avail) * > - (sensor->sensor_info->intervals.count * 2), > + devm_kcalloc(&iio_dev->dev, > + sensor->sensor_info->intervals.count * 2, ...I would also switch this to use array_size() instead of explicit multiplication as it will check for boundaries that are not static in this case. > + sizeof(*sensor->freq_avail), > GFP_KERNEL); > if (!sensor->freq_avail) > return -ENOMEM; -- With Best Regards, Andy Shevchenko
在 2025/8/19 17:46, Andy Shevchenko 写道: > [You don't often get email from andy.shevchenko@gmail.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ] > > On Tue, Aug 19, 2025 at 11:56 AM Qianfeng Rong <rongqianfeng@vivo.com> wrote: >> Replace calls of devm_kzalloc() with devm_kcalloc() in scmi_alloc_iiodev() >> and scmi_iio_set_sampling_freq_avail() for safer memory allocation with >> built-in overflow protection. > While this change is correct... > > ... > >> sensor->freq_avail = >> - devm_kzalloc(&iio_dev->dev, >> - sizeof(*sensor->freq_avail) * >> - (sensor->sensor_info->intervals.count * 2), >> + devm_kcalloc(&iio_dev->dev, >> + sensor->sensor_info->intervals.count * 2, > ...I would also switch this to use array_size() instead of explicit > multiplication as it will check for boundaries that are not static in > this case. I don't understand what "will check for boundaries that are not static in this case" means. Could you explain it to me? I've experimented with the following command and found that kmalloc_array() generates fewer instructions than kmalloc(array_size()): objdump -dSl --prefix-addresses <changed module>.o > -- > With Best Regards, > Andy Shevchenko Best regards, Qianfeng
On Tue, Aug 19, 2025 at 1:08 PM Qianfeng Rong <rongqianfeng@vivo.com> wrote: > 在 2025/8/19 17:46, Andy Shevchenko 写道: > > On Tue, Aug 19, 2025 at 11:56 AM Qianfeng Rong <rongqianfeng@vivo.com> wrote: ... > > While this change is correct... > >> sensor->freq_avail = > >> - devm_kzalloc(&iio_dev->dev, > >> - sizeof(*sensor->freq_avail) * > >> - (sensor->sensor_info->intervals.count * 2), > >> + devm_kcalloc(&iio_dev->dev, > >> + sensor->sensor_info->intervals.count * 2, > > ...I would also switch this to use array_size() instead of explicit > > multiplication as it will check for boundaries that are not static in > > this case. > > I don't understand what "will check for boundaries that are not static in > this case" means. Could you explain it to me? intervals.count may be anything and of any type. Compiler may or may not proof that it holds the value less than size_t / 2 (which may be == int / 2 on 32-bit platforms). That's why it's better to use array_size(intervals.count, 2), > I've experimented with the following command and found that kmalloc_array() > generates fewer instructions than kmalloc(array_size()): > objdump -dSl --prefix-addresses <changed module>.o It's about the second parameter in devm_kcalloc(), and of course it may generate less instructions but it's irrelevant to my comment. -- With Best Regards, Andy Shevchenko
在 2025/8/19 18:13, Andy Shevchenko 写道: > [You don't often get email from andy.shevchenko@gmail.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ] > > On Tue, Aug 19, 2025 at 1:08 PM Qianfeng Rong <rongqianfeng@vivo.com> wrote: >> 在 2025/8/19 17:46, Andy Shevchenko 写道: >>> On Tue, Aug 19, 2025 at 11:56 AM Qianfeng Rong <rongqianfeng@vivo.com> wrote: > ... > >>> While this change is correct... >>>> sensor->freq_avail = >>>> - devm_kzalloc(&iio_dev->dev, >>>> - sizeof(*sensor->freq_avail) * >>>> - (sensor->sensor_info->intervals.count * 2), >>>> + devm_kcalloc(&iio_dev->dev, >>>> + sensor->sensor_info->intervals.count * 2, >>> ...I would also switch this to use array_size() instead of explicit >>> multiplication as it will check for boundaries that are not static in >>> this case. >> I don't understand what "will check for boundaries that are not static in >> this case" means. Could you explain it to me? > intervals.count may be anything and of any type. Compiler may or may > not proof that it holds the value less than size_t / 2 (which may be > == int / 2 on 32-bit platforms). That's why it's better to use > array_size(intervals.count, 2), Sorry, I just understood your reply, I will send v2 immediately. :) > >> I've experimented with the following command and found that kmalloc_array() >> generates fewer instructions than kmalloc(array_size()): >> objdump -dSl --prefix-addresses <changed module>.o > It's about the second parameter in devm_kcalloc(), and of course it > may generate less instructions but it's irrelevant to my comment. > > Best regards, Qianfeng
在 2025/8/19 18:13, Andy Shevchenko 写道:
> [You don't often get email from andy.shevchenko@gmail.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> On Tue, Aug 19, 2025 at 1:08 PM Qianfeng Rong <rongqianfeng@vivo.com> wrote:
>> 在 2025/8/19 17:46, Andy Shevchenko 写道:
>>> On Tue, Aug 19, 2025 at 11:56 AM Qianfeng Rong <rongqianfeng@vivo.com> wrote:
> ...
>
>>> While this change is correct...
>>>> sensor->freq_avail =
>>>> - devm_kzalloc(&iio_dev->dev,
>>>> - sizeof(*sensor->freq_avail) *
>>>> - (sensor->sensor_info->intervals.count * 2),
>>>> + devm_kcalloc(&iio_dev->dev,
>>>> + sensor->sensor_info->intervals.count * 2,
>>> ...I would also switch this to use array_size() instead of explicit
>>> multiplication as it will check for boundaries that are not static in
>>> this case.
>> I don't understand what "will check for boundaries that are not static in
>> this case" means. Could you explain it to me?
> intervals.count may be anything and of any type. Compiler may or may
> not proof that it holds the value less than size_t / 2 (which may be
> == int / 2 on 32-bit platforms). That's why it's better to use
> array_size(intervals.count, 2),
Thanks for taking the time to explain! The devm_kmalloc_array() function
implements a check function similar to array_size() by default.
devm_kmalloc_array():
static inline void *devm_kmalloc_array(struct device *dev, size_t n,
size_t size, gfp_t flags)
{
size_t bytes;
if (unlikely(check_mul_overflow(n, size, &bytes)))
return NULL;
return devm_kmalloc(dev, bytes, flags);
}
static inline void *devm_kcalloc(struct device *dev, size_t n, size_t
size, gfp_t flags)
{
return devm_kmalloc_array(dev, n, size, flags | __GFP_ZERO);
}
array_size():
static inline size_t __must_check size_mul(size_t factor1, size_t factor2)
{
size_t bytes;
if (check_mul_overflow(factor1, factor2, &bytes))
return SIZE_MAX;
return bytes;
}
#define array_size(a, b) size_mul(a, b)
>
>> I've experimented with the following command and found that kmalloc_array()
>> generates fewer instructions than kmalloc(array_size()):
>> objdump -dSl --prefix-addresses <changed module>.o
> It's about the second parameter in devm_kcalloc(), and of course it
> may generate less instructions but it's irrelevant to my comment.
>
>
Best regards,
Qianfeng
© 2016 - 2025 Red Hat, Inc.