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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4f614b66-ad85-7fa3-6e6e-2a672e8148e3@huawei.com>
Date:   Wed, 15 Sep 2021 10:03:44 +0800
From:   Bixuan Cui <cuibixuan@...wei.com>
To:     Thomas Gleixner <tglx@...utronix.de>,
        <linux-kernel@...r.kernel.org>
CC:     <maz@...nel.org>, <john.wanghui@...wei.com>
Subject: Re: [PATCH -next] irqdomain: fix overflow error



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