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: <YhyGp0zu9OBOWt/M@hatter.bewilderbeest.net>
Date:   Mon, 28 Feb 2022 00:24:07 -0800
From:   Zev Weiss <zev@...ilderbeest.net>
To:     Guenter Roeck <linux@...ck-us.net>
Cc:     linux-hwmon@...r.kernel.org, Jean Delvare <jdelvare@...e.com>,
        Renze Nicolai <renze@...lus.nl>, openbmc@...ts.ozlabs.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/5] hwmon: (nct6775) Rearrange attr-group initialization

On Sun, Feb 27, 2022 at 07:01:32AM PST, Guenter Roeck wrote:
>On 2/26/22 05:30, Zev Weiss wrote:
>>We now track the number of attribute groups in nct6775_data, as a
>>measure to simplify handling differences in the set of enabled
>>attribute groups between nct6775 drivers (platform & i2c).  As a side
>>effect, we also reduce the amount of IS_ERR()/PTR_ERR() boilerplate a
>>bit.
>>
>>Signed-off-by: Zev Weiss <zev@...ilderbeest.net>
>>---
>>  drivers/hwmon/nct6775.c | 84 ++++++++++++++++++++---------------------
>>  1 file changed, 42 insertions(+), 42 deletions(-)
>>
>>diff --git a/drivers/hwmon/nct6775.c b/drivers/hwmon/nct6775.c
>>index 2b91f7e05126..139b2fe5ca4d 100644
>>--- a/drivers/hwmon/nct6775.c
>>+++ b/drivers/hwmon/nct6775.c
>>@@ -1198,6 +1198,7 @@ struct nct6775_data {
>>  	const char *name;
>>  	const struct attribute_group *groups[7];
>>+	u8 num_groups;
>>  	u16 reg_temp[5][NUM_TEMP]; /* 0=temp, 1=temp_over, 2=temp_hyst,
>>  				    * 3=temp_crit, 4=temp_lcrit
>>@@ -1405,10 +1406,18 @@ struct sensor_template_group {
>>  	int base;
>>  };
>>-static struct attribute_group *
>>-nct6775_create_attr_group(struct device *dev,
>>-			  const struct sensor_template_group *tg,
>>-			  int repeat)
>>+static int nct6775_add_attr_group(struct nct6775_data *data, const struct attribute_group *group)
>>+{
>>+	/* Need to leave a NULL terminator at the end of data->groups */
>>+	if (WARN_ON(data->num_groups == ARRAY_SIZE(data->groups) - 1))
>>+		return -ENOSPC;
>>+
>
>At work we are struggling with a whopping 500,000+ (!) WARN backtraces
>_each day_. I would be happy if you send me patches removing some of
>those, but I am not inclined to accept patches adding them. If people
>don't notice that the driver doesn't load, they won't notice the warning
>either, and it will just add to all the other warning backtrace noise.
>

Okay, I can remove that -- I mostly just added it out of concern that 
"no space left on device" would be a fairly confusing error for someone 
to potentially end up with modprobe reporting, and some further 
indication of what went wrong could perhaps make it less mystifying 
(though yes, with any luck it can hopefully remain unreachable in 
practice as long as data->groups gets expanded when needed).

I'd certainly also be open to suggestions of a more appropriate errno 
value to return in that case, though I couldn't find one that seemed 
clearly better to me.  ENOMEM seemed vaguely more appropriate in some 
ways given that it's an in-memory array that's full rather than a 
storage device, but it's also definitely not the usual ENOMEM meaning of 
a dynamic allocation failure due to memory pressure, so...(shrug).  I 
think FreedBSD's got an EDOOFUS errno value, but as far as I can see 
Linux doesn't have one for indicating a purely internal error like this.


Thanks,
Zev

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ