linux-kernel - Re: [PATCH v3] fsi: Aspeed: Fix a potential double free

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: <79bdf543-4882-360e-567f-493e84f09d6b@wanadoo.fr>
Date:   Mon, 21 Feb 2022 19:08:32 +0100
From:   Christophe JAILLET <christophe.jaillet@...adoo.fr>
To:     Joel Stanley <joel@....id.au>
Cc:     Jeremy Kerr <jk@...abs.org>,
        Alistar Popple <alistair@...ple.id.au>,
        Eddie James <eajames@...ux.ibm.com>,
        Andrew Jeffery <andrew@...id.au>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        kernel-janitors@...r.kernel.org,
        Guenter Roeck <linux@...ck-us.net>, linux-fsi@...ts.ozlabs.org,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>,
        linux-aspeed <linux-aspeed@...ts.ozlabs.org>
Subject: Re: [PATCH v3] fsi: Aspeed: Fix a potential double free

Le 21/02/2022 à 10:24, Joel Stanley a écrit :
> Hi Christophe,
> 
> Thanks for the patch.
> 
> On Sun, 9 Jan 2022 at 21:56, Christophe JAILLET
> <christophe.jaillet@...adoo.fr> wrote:
>>
>> A struct device can never be devm_alloc()'ed.
>> Here, it is embedded in "struct fsi_master", and "struct fsi_master" is
>> embedded in "struct fsi_master_aspeed".
>>
>> Since "struct device" is embedded, the data structure embedding it must be
>> released with the release function, as is already done here.
>>
>> So use kzalloc() instead of devm_kzalloc() when allocating "aspeed" and
>> update all error handling branches accordingly.
> 
> This looks like a problem with the design of the fsi master structure.
> It's a common pattern to devm_alloc the platform devices as they are
> probed, the fsi masters all embed a copy of struct fsi_master, which
> as you say embeds struct device.
> 
> Can we learn from other bus drivers (eg i2c?) how we should lay out
> these structures?
> 
Hi,
I won't do it myself.

This goes beyond my knowledge and without the possibility to test it, it 
would be just some random trial and error (as I did in the first broken 
version of this patch).

CJ