linux-kernel - Re: [PATCH v3 2/9] media: i2c: ds90ub960: Replace aliased clients list with bitmap

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

Message-ID: <1743994.6779kEWLuL@fw-rgant>
Date: Tue, 03 Dec 2024 09:48:57 +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>
Subject:
 Re: [PATCH v3 2/9] media: i2c: ds90ub960: Replace aliased clients list with
 bitmap

Hi,

On vendredi 29 novembre 2024 14:46:38 heure normale d’Europe centrale Tomi 
Valkeinen wrote:
> Hi,
> 
> On 25/11/2024 10:45, Romain Gantois wrote:
> > The ds90ub960 driver currently uses a list of i2c_client structs to keep
> > track of used I2C address translator (ATR) alias slots for each RX port.
...
> >   		dev_err(dev, "rx%u: alias pool exhausted\n", rxport->nport);
> >   		return -EADDRNOTAVAIL;
> >   	
> >   	}
> > 
> > -	rxport->aliased_clients[reg_idx] = client;
> > +	set_bit(reg_idx, rxport->alias_use_mask);
> > 
> >   	ub960_rxport_write(priv, chan_id, UB960_RR_SLAVE_ID(reg_idx),
> >   	
> >   			   client->addr << 1);
> > 
> > @@ -1063,18 +1059,15 @@ static void ub960_atr_detach_client(struct i2c_atr
> > *atr, u32 chan_id,> 
> >   	struct device *dev = &priv->client->dev;
> >   	unsigned int reg_idx;
> > 
> > -	for (reg_idx = 0; reg_idx < ARRAY_SIZE(rxport->aliased_clients);
> > reg_idx++) { -		if (rxport->aliased_clients[reg_idx] == client)
> > -			break;
> > -	}
> > +	reg_idx = find_first_zero_bit(rxport->alias_use_mask,
> > UB960_MAX_PORT_ALIASES);
> The old code went through the alias table to find the matching client,
> so that it can be removed. The new code... Tries to find the first
> unused entry in the mask, to... free it?
> 
> I'm not sure how this is supposed to work, or how the driver even could
> manage with just a bit mask. The driver needs to remove the one that was
> assigned in ub960_atr_attach_addr(), so it somehow has to find the same
> entry using the address or the alias.

Indeed, there is an issue here. Tracking client addresses is still required, 
so the correct change would be to convert aliased_clients to aliased_addrs.

Thanks,

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