linux-kernel - Re: [lm-sensors] [PATCH] therm_throt.c: Fix error handling in thermal_throttle_add

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]

Date:	Sun, 5 Sep 2010 09:35:12 -0700
From:	Guenter Roeck <guenter.roeck@...csson.com>
To:	Ingo Molnar <mingo@...e.hu>
CC:	Fenghua Yu <fenghua.yu@...el.com>,
	Hidetoshi Seto <seto.hidetoshi@...fujitsu.com>,
	linux-kernel <linux-kernel@...r.kernel.org>,
	lm-sensors <lm-sensors@...sensors.org>,
	H Peter Anvin <hpa@...or.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Len Brown <lenb@...nel.org>,
	Jin Dongming <jin.dongming@...css.fujitsu.com>
Subject: Re: [lm-sensors] [PATCH] therm_throt.c: Fix error handling in
 thermal_throttle_add_dev

On Sun, Sep 05, 2010 at 08:32:29AM -0400, Ingo Molnar wrote:
> 
> * Fenghua Yu <fenghua.yu@...el.com> wrote:
> 
> > Or other options could be:
> > 
> > 1. Just calling sysfs_add_file_to_group() without collecting returned error and
> > return 0 at the end (driver/pci/pcie/aspm.c does like this). The drawback is
> > there is no error logged if an unlikely errorr occurs. But user can see some
> > files are missing in sysfs.
> >
> > 2. Or collect errors in err1, err2, etc for each sysfs_add_file_to_group. At
> > the end, return -ENODEV(??) if any err1, err2, etc is not 0. This option makes
> > code unreasonable complex to handle unlikely errors.
> 
> Well, the usual way to handle errors is to abort the operation when it 
> occurs, and return the error code that sysfs_add_file_to_group() gave.
> 
> The error is not 'fatal' but missing sysfs files sure are confusing, and 
> might break user-land which depends on them. So we should either 
> initialize a driver fully - or not intialize it at all.
> 
> Now, a sub-case is the question whether to emit something more than the 
> return code from sysfs_add_file_to_group(). If it's exceedingly rare 
> (and subsequently poorly tested) then adding a WARN_ON_ONCE(ret) is OK - 
> but that error code should be returned.
> 
> Am i missing any detail?
> 
Unless I am missing something, the problem here is that if driver initialization
code returns an error, the driver won't be installed. If device entries are retained,
this will ultimately cause the kernel to crash.

>From my perspective, the usual handling is just fine - ie not install the driver
at all. Everything else just causes confusion.

Guenter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/