Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
References: <20221107202114.823975-1-y.oudjana@protonmail.com>
 <166811478949.1075638.10692622037510321259.b4-ty@kernel.org>
From:   Aidan MacDonald <aidanmacdonald.0x0@gmail.com>
To:     Mark Brown <broonie@kernel.org>
Cc:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        "Rafael J. Wysocki" <rafael@kernel.org>,
        Yassine Oudjana <yassine.oudjana@gmail.com>,
        linux-kernel@vger.kernel.org,
        Yassine Oudjana <y.oudjana@protonmail.com>
Subject: Re: [PATCH] regmap-irq: Use the new num_config_regs property in
 regmap_add_irq_chip_fwnode
Date:   Sat, 12 Nov 2022 13:52:35 +0000
In-reply-to: <166811478949.1075638.10692622037510321259.b4-ty@kernel.org>
Message-ID: <hXJCiiSHxRbOCrOlFifkWudu1LTcK1TM@localhost>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk


Hi Mark,

Mark Brown <broonie@kernel.org> writes:

> On Mon, 7 Nov 2022 23:21:14 +0300, Yassine Oudjana wrote:
>> From: Yassine Oudjana <y.oudjana@protonmail.com>
>>
>> Commit faa87ce9196d ("regmap-irq: Introduce config registers for irq
>> types") added the num_config_regs, then commit 9edd4f5aee84 ("regmap-irq:
>> Deprecate type registers and virtual registers") suggested to replace
>> num_type_reg with it. However, regmap_add_irq_chip_fwnode wasn't modified
>> to use the new property. Later on, commit 255a03bb1bb3 ("ASoC: wcd9335:
>> Convert irq chip to config regs") removed the old num_type_reg property
>> from the WCD9335 driver's struct regmap_irq_chip, causing a null pointer
>> dereference in regmap_irq_set_type when it tried to index d->type_buf as
>> it was never allocated in regmap_add_irq_chip_fwnode:
>>
>> [...]
>
> Applied to
>
>    https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regmap.git for-next
>
> Thanks!
>
> [1/1] regmap-irq: Use the new num_config_regs property in regmap_add_irq_chip_fwnode
>       commit: 84498d1fb35de6ab71bdfdb6270a464fb4a0951b
>

Um, this does prevent the null deref, but the fix is in the wrong place:
d->type_buf shouldn't be accessed in this configuration, the bug is in
regmap_irq_set_type(). The access should be guarded by
"if (d->chip->type_in_mask || d->chip->num_type_reg)" to prevent the
NULL deref.

The analysis in the commit message is inaccurate,

>> However, regmap_add_irq_chip_fwnode wasn't modified to use the
>> new property.

the proposed fix is just wasting memory, since type_buf isn't used
except for the erroneous write -- the write shouldn't happen at all.

Regards,
Aidan