Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.37 as permitted sender) client-ip=23.128.96.37;
Date:   Sun, 8 Oct 2023 12:57:40 +0200
From:   Borislav Petkov <bp@alien8.de>
To:     Qiuxu Zhuo <qiuxu.zhuo@intel.com>
Cc:     Tony Luck <tony.luck@intel.com>, Lili Li <lili.li@intel.com>,
        James Morse <james.morse@arm.com>,
        Mauro Carvalho Chehab <mchehab@kernel.org>,
        Robert Richter <rric@kernel.org>, linux-edac@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/3] EDAC/igen6: Fix slab-use-after-free in
 igen6_unregister_mci()
Message-ID: <20231008105740.GAZSKLJMLfbiDbZlm8@fat_crate.local>
References: <20231008080231.51917-1-qiuxu.zhuo@intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <20231008080231.51917-1-qiuxu.zhuo@intel.com>
Precedence: bulk

On Sun, Oct 08, 2023 at 04:02:29PM +0800, Qiuxu Zhuo wrote:
> When unloading the igen6_edac driver, the EDAC core wrongly kfreed
> 'pvt_info,' which was private data and managed by the igen6_edac
> driver. This resulted in a slab-use-after-free bug. Fix it by adding
> a flag to indicate whether 'pvt_info' is managed by the EDAC core.
> The EDAC core will only kfree 'pvt_info' when the flag is set to true.

That's because your silly driver is wrongly allocating stuff:

igen6_probe() allocates the whole pvt struct and then
igen6_register_mci() assigns it piece-meal-wise to each MC's ->pvt_info.

On the unreg path, you then call edac_mc_free(), it frees ->mct_info and
then you do wonder why it complains when you call kfree(igen6_pvt) in
igen6_remove().

You should do the exact opposite of the allocation steps on the unreg
path and it'll all work fine. Definitely not add ugly hacks to the
EDAC core.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette