linux-kernel - Re: [PATCH v3 8/9] i2c: Support dynamic address translation

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: <2099546.fqZRxdAdAU@fw-rgant>
Date: Tue, 10 Dec 2024 16:21:19 +0100
From: Romain Gantois <romain.gantois@...tlin.com>
To: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>
Cc: Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
 Kory Maincent <kory.maincent@...tlin.com>, linux-i2c@...r.kernel.org,
 linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
 linux-media@...r.kernel.org, linux-gpio@...r.kernel.org,
 Wolfram Sang <wsa+renesas@...g-engineering.com>,
 Luca Ceresoli <luca.ceresoli@...tlin.com>,
 Andi Shyti <andi.shyti@...nel.org>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
 Derek Kiernan <derek.kiernan@....com>, Dragan Cvetic <dragan.cvetic@....com>,
 Arnd Bergmann <arnd@...db.de>,
 Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
 Mauro Carvalho Chehab <mchehab@...nel.org>,
 Linus Walleij <linus.walleij@...aro.org>,
 Bartosz Golaszewski <brgl@...ev.pl>, Cosmin Tanislav <demonsingur@...il.com>
Subject: Re: [PATCH v3 8/9] i2c: Support dynamic address translation

Hi Tomi,

On lundi 9 décembre 2024 13:42:29 heure normale d’Europe centrale Romain 
Gantois wrote:
> Hi Tomi,
> 
...
> > This fails with:
> > 
> > WARNING: CPU: 1 PID: 360 at lib/list_debug.c:35
> > __list_add_valid_or_report+0xe4/0x100
> > 
> > as the i2c_atr_create_c2a() calls list_add(), but i2c_atr_attach_addr(),
> > which is changed to use i2c_atr_create_c2a(), also calls list_add().
> > 
> > Also, if you add i2c_atr_create_c2a() which hides the allocation and
> > list_add, I think it makes sense to add a i2c_atr_destroy_c2a() to
> > revert that.
> > 
> > There's also a memory error "BUG: KASAN: slab-use-after-free in
> > __lock_acquire+0xc4/0x375c" (see below) when unloading the ub960 or
> > ub953 driver. I haven't looked at that yet.
> 
> I think I've found what's causing this KASAN splat.  i2c_atr_del_adapter is
> freeing it's alias pool before setting atr->adapter[chan_id] to NULL. So
> there's a time window during which bus notifications can trigger a call
> to i2c_atr_attach_addr, leading to a UAF on the alias pool struct.

It seems like my initial theory was wrong. The bus notifier wouldn't trigger 
after the adapter has been removed.

However, the "alias_pool->shared" flag is not set anywhere in the i2c-atr 
module! So a more likely theory is that the common alias pool is being
freed when the first channel is deleted. Thus, the second channel is
still referencing a freed alias pool during it's deletion, hence the UAF.

Properly setting the "shared" flag of alias pools owned by the i2c_atr struct
should fix this.

Thanks,

-- 
Romain Gantois, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com