linux-kernel - Re: [PATCH v3] bus: hisi_lpc: Avoid use-after-free from probe failure

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] [day] [month] [year] [list]

Message-ID: <bda4130a-411e-cb51-5736-a51226c3a99f@huawei.com>
Date:   Mon, 17 Jun 2019 16:04:31 +0100
From:   John Garry <john.garry@...wei.com>
To:     <xuwei5@...wei.com>
CC:     <bhelgaas@...gle.com>, <linuxarm@...wei.com>, <arm@...nel.org>,
        <linux-kernel@...r.kernel.org>, <joe@...ches.com>
Subject: Re: [PATCH v3] bus: hisi_lpc: Avoid use-after-free from probe failure

On 17/06/2019 12:15, John Garry wrote:
> If, after registering a logical PIO range, the driver probe later fails,
> the logical PIO range memory will be released automatically.
>
> This causes an issue, in that the logical PIO range is not unregistered
> and the released range memory may be later referenced.
>
> As an interim fix, allocate the logical PIO range with kzalloc() to avoid
> this memory being freed.
>
> Further memory will not be leaked from later attempts to probe the
> driver, as any attempts to allocate logical PIO ranges will fail, and we
> would release the 'range' memory.
>
> The correct fix for this problem would be to tear down what had been setup
> during the probe, i.e. unregister the logical PIO range, but this is not
> supported.
>
> Support for unregistering logical PIO ranges will need be to added in
> future.
>
> Fixes: adf38bb0b5956 ("HISI LPC: Support the LPC host on Hip06/Hip07 with DT bindings")
> Signed-off-by: John Garry <john.garry@...wei.com>
> ---
>

I just realized that there is another bug in this driver, regarding 
unbinding the driver from the device.

@xuwei, please ignore this patch. I need to fix this all properly, 
including adding the logical PIO unregister method.

John

> Change in v2:
> - update commit message
>
> Change to v1:
> - add comment, as advised by Joe Perches
>
> diff --git a/drivers/bus/hisi_lpc.c b/drivers/bus/hisi_lpc.c
> index 19d7b6ff2f17..5f0130a693fe 100644
> --- a/drivers/bus/hisi_lpc.c
> +++ b/drivers/bus/hisi_lpc.c
> @@ -599,7 +599,8 @@ static int hisi_lpc_probe(struct platform_device *pdev)
>  	if (IS_ERR(lpcdev->membase))
>  		return PTR_ERR(lpcdev->membase);
>
> -	range = devm_kzalloc(dev, sizeof(*range), GFP_KERNEL);
> +	/* Logical PIO may reference 'range' memory even if the probe fails */
> +	range = kzalloc(sizeof(*range), GFP_KERNEL);
>  	if (!range)
>  		return -ENOMEM;
>
> @@ -610,6 +611,7 @@ static int hisi_lpc_probe(struct platform_device *pdev)
>  	ret = logic_pio_register_range(range);
>  	if (ret) {
>  		dev_err(dev, "register IO range failed (%d)!\n", ret);
> +		kfree(range);
>  		return ret;
>  	}
>  	lpcdev->io_host = range;
>