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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <8b8ce122-f16b-4207-b03b-f74b15756ae7@icloud.com>
Date: Mon, 12 Aug 2024 23:32:44 +0800
From: Zijun Hu <zijun_hu@...oud.com>
To: Przemek Kitszel <przemyslaw.kitszel@...el.com>,
 Dan Williams <dan.j.williams@...el.com>
Cc: linux-kernel@...r.kernel.org, linux-cxl@...r.kernel.org,
 linux1394-devel@...ts.sourceforge.net, netdev@...r.kernel.org,
 Zijun Hu <quic_zijuhu@...cinc.com>,
 Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
 "Rafael J. Wysocki" <rafael@...nel.org>, Davidlohr Bueso
 <dave@...olabs.net>, Jonathan Cameron <jonathan.cameron@...wei.com>,
 Dave Jiang <dave.jiang@...el.com>,
 Alison Schofield <alison.schofield@...el.com>,
 Vishal Verma <vishal.l.verma@...el.com>, Ira Weiny <ira.weiny@...el.com>,
 Takashi Sakamoto <o-takashi@...amocchi.jp>, Timur Tabi <timur@...nel.org>,
 "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>
Subject: Re: [PATCH 3/5] cxl/region: Prevent device_find_child() from
 modifying caller's match data

On 2024/8/12 20:54, Przemek Kitszel wrote:
> On 8/11/24 02:18, Zijun Hu wrote:
>> From: Zijun Hu <quic_zijuhu@...cinc.com>
>>
>> It does not make sense for match_free_decoder() as device_find_child()'s
>> match function to modify caller's match data, 
> 
> match_free_decoder() is just doing that, treating caller's match data as
> a piece of memory to store their int.
> (So it is hard to tell it does not make sense "for [it] ... to").
> 

Thanks for reply (^^)

The ultimate goal is to make device_find_child() have below prototype:

struct device *device_find_child(struct device *dev, const void *data,
		int (*match)(struct device *dev, const void *data));

Why ?

(1) It does not make sense, also does not need to, for such device
finding operation to modify caller's match data which is mainly
used for comparison.

(2) It will make the API's match function parameter have the same
signature as all other APIs (bus|class|driver)_find_device().


My idea is that:
use device_find_child() for READ only accessing caller's match data.

use below API if need to Modify caller's data as
constify_device_find_child_helper() does.
int device_for_each_child(struct device *dev, void *data,
                    int (*fn)(struct device *dev, void *data));


So match_free_decoder() is not proper as device_find_child()'s
match function.

>> fixed by using
>> constify_device_find_child_helper() instead of device_find_child().
> 
> I don't like the constify... name, I would go with something like
> device_find_child_mut() or similar.
> 

What about below alternative option i ever thought about ?

Don't introduce API constify_device_find_child_helper() at all, and
change in involved driver such as cxl/core/region.c directly.

>>
>> Signed-off-by: Zijun Hu <quic_zijuhu@...cinc.com>
>> ---
>>   drivers/cxl/core/region.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
>> index 21ad5f242875..266231d69dff 100644
>> --- a/drivers/cxl/core/region.c
>> +++ b/drivers/cxl/core/region.c
>> @@ -849,7 +849,8 @@ cxl_region_find_decoder(struct cxl_port *port,
>>           dev = device_find_child(&port->dev, &cxlr->params,
>>                       match_auto_decoder);
>>       else
>> -        dev = device_find_child(&port->dev, &id, match_free_decoder);
>> +        dev = constify_device_find_child_helper(&port->dev, &id,
>> +                            match_free_decoder);
>>       if (!dev)
>>           return NULL;
>>       /*
>>
> 


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ