Re: [PATCH -next] irqdomain: fix overflow error

[Bixuan Cui <cuibixuan@huawei.com>]

On 2021/9/14 19:56, Thomas Gleixner wrote:
> On Wed, Sep 08 2021 at 09:46, Bixuan Cui wrote:
>> In function ‘kmalloc_node’,
>>     inlined from ‘kzalloc_node.constprop’ at ./include/linux/slab.h:743:9,
>>     inlined from ‘__irq_domain_add’ at kernel/irq/irqdomain.c:153:9:
>> ./include/linux/slab.h:618:9: error: argument 1 value ‘18446744073709551615’ exceeds maximum object size 9223372036854775807 [-Werror=alloc-size-larger-than=]
>>   return __kmalloc_node(size, flags, node);
>>
>> The 'size' can be negative here, which will then get turned into a giant
>> size argument for kzalloc_node(). Changing the size to 'unsigned int'
>> instead seems more appropriate.
> What's more appropriate about that?
We call struct_size(domain, revmap, size) in __irq_domain_add() for calculations.

The struct_size() is implemented in include/linux/overflow.h
static inline __must_check size_t __ab_c_size(size_t a, size_t b, size_t c)
{
        size_t bytes;

The 'size' is passed to __ab_c_size(), the input parameter is 'size_t'(unsigned int).


On the other hand, I looked at all the code that calls __irq_domain_add(), such as:
include/linux/irqdomain.h:
static inline struct irq_domain *irq_domain_create_linear(struct fwnode_handle *fwnode,
                                         unsigned int size,
                                         const struct irq_domain_ops *ops,
                                         void *host_data)
{
        return __irq_domain_add(fwnode, size, size, 0, ops, host_data);

or
static inline struct irq_domain *irq_domain_add_linear(struct device_node *of_node,
                                         unsigned int size,
                                         const struct irq_domain_ops *ops,
                                         void *host_data)
{
        return __irq_domain_add(of_node_to_fwnode(of_node), size, size, 0, ops, host_data);

And kernel/irq/irqdomain.c
struct irq_domain *irq_domain_create_simple(struct fwnode_handle *fwnode,
                                            unsigned int size,
                                            unsigned int first_irq,
                                            const struct irq_domain_ops *ops,
                                            void *host_data)
{
        struct irq_domain *domain;

        domain = __irq_domain_add(fwnode, size, size, 0, ops, host_data);

All 'size' passed to __irq_domain_add() are unsigned int.

So I think it's more appropriate to replace it with unsigned int.


Thanks,
Bixuan Cui