Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) client-ip=2620:137:e000::3:7;
Date:   Wed, 29 Nov 2023 16:21:31 +0300
From:   Dan Carpenter <dan.carpenter@linaro.org>
To:     James Tai <james.tai@realtek.com>
Cc:     Thomas Gleixner <tglx@linutronix.de>,
        Marc Zyngier <maz@kernel.org>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Conor Dooley <conor+dt@kernel.org>,
        linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
        kernel test robot <lkp@intel.com>,
        Dan Carpenter <error27@gmail.com>
Subject: Re: [PATCH v3 2/6] irqchip: Add interrupt controller support for
 Realtek DHC SoCs
Message-ID: <8f87bfca-ab5f-4b32-a400-a90d09b64cf1@suswa.mountain>
References: <20231129054339.3054202-1-james.tai@realtek.com>
 <20231129054339.3054202-3-james.tai@realtek.com>
 <d94c79bf-04c4-4e87-bd7e-a8755508ac89@suswa.mountain>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <d94c79bf-04c4-4e87-bd7e-a8755508ac89@suswa.mountain>
Precedence: bulk

On Wed, Nov 29, 2023 at 11:21:06AM +0300, Dan Carpenter wrote:
> > +int realtek_intc_probe(struct platform_device *pdev, const struct realtek_intc_info *info)
> > +{
> > +	struct realtek_intc_data *data;
> > +	struct device *dev = &pdev->dev;
> > +	struct device_node *node = dev->of_node;
> > +	int ret, i;
> > +
> > +	data = devm_kzalloc(dev, struct_size(data, subset_data, info->cfg_num), GFP_KERNEL);
> > +	if (!data)
> > +		return -ENOMEM;
> > +
> > +	data->base = of_iomap(node, 0);
> > +	if (!data->base) {
> > +		ret = -ENOMEM;
> > +		goto out_cleanup;
> 
> devm_ allocations are cleaned up automatically so there is no need to
> call devm_kfree() before returning.
> 
> regards,
> dan carpenter
> 
> > +	}
> > +
> > +	data->info = info;
> > +
> > +	raw_spin_lock_init(&data->lock);
> > +
> > +	data->domain = irq_domain_add_linear(node, 32, &realtek_intc_domain_ops, data);

Btw, as I was testing the other static checker warning for <= 0, my
static checker really wants this irq_domain_add_linear() to be cleaned
up on the error path.

Otherwise it probably leads to a use after free because we free data
(automatically or manually) but it's still on a list somewhere.

> > +	if (!data->domain) {
> > +		ret = -ENOMEM;
> > +		goto out_cleanup;
> > +	}
> > +
> > +	data->subset_data_num = info->cfg_num;
> > +	for (i = 0; i < info->cfg_num; i++) {
> > +		ret = realtek_intc_subset(node, data, i);
> > +		if (ret) {
> > +			WARN(ret, "failed to init subset %d: %d", i, ret);
> > +			ret = -ENOMEM;
> > +			goto out_cleanup;

This error path.

regards,
dan carpenter


> > +		}
> > +	}
> > +
> > +	platform_set_drvdata(pdev, data);
> > +
> > +	return 0;
> > +
> > +out_cleanup:
> > +
> > +	if (data->base)
> > +		iounmap(data->base);
> > +
> > +	devm_kfree(dev, data);
> > +
> > +	return ret;
> > +}