Re: [PATCH v4 06/21] hwmon: (mr75203) fix multi-channel voltage reading

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 9/6/2022 5:10 PM, Andy Shevchenko wrote:
On Tue, Sep 06, 2022 at 08:33:41AM +0000, Eliav Farber wrote:
Fix voltage allocation and reading to support all channels in all VMs.
Prior to this change allocation and reading were done only for the first
channel in each VM.
This change counts the total number of channels for allocation, and takes
into account the channel offset when reading the sample data register.

...

+             total_ch = ch_num * vm_num;
+             in_config = devm_kcalloc(dev, total_ch + 1,
                                       sizeof(*in_config), GFP_KERNEL);

Strictly speaking this should be `size_add(size_mul(...) ...)` construction
from overflow.h.

               total_ch = size_mul(ch_num, vm_num);
               in_config = devm_kcalloc(dev, size_add(total_ch, 1),
                                        sizeof(*in_config), GFP_KERNEL);

Alternatively before doing all these, add a check

               if (array3_size(ch_num, vm_num, sizeof(*in_config)) < SIZE_MAX - sizeof(*in_config))
                       return -EOVERFLOW;

But this is a bit monstrous. Seems like the above looks and feels better.

Also for backporting purposes perhaps it's fine to do without using those macro helpers.
According to the driver code total_ch is a u32 variable while vm_num
and ch_num are both limited to a value of 31:

#define VM_NUM_MSK GENMASK(20, 16)
#define VM_NUM_SFT 16
#define CH_NUM_MSK GENMASK(31, 24)
#define CH_NUM_SFT 24

In addition the PVT Controller Series 3+ Specification mentions that
the actual maximum values are even smaller – 8 for vm_num and 16 for
ch_num.
Therefore we are very far from a scenario of an overflow.
Do you still think overflow protection in necessary?

--
Thanks, Eliav



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux