linux-kernel - Re: [PATCH 04/11] staging: iio: adt7316: fix handling of dac high resolution option

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: <20181213220146.GA2496@r2700x.localdomain>
Date:   Thu, 13 Dec 2018 15:01:46 -0700
From:   Jeremy Fertic <jeremyfertic@...il.com>
To:     Dan Carpenter <dan.carpenter@...cle.com>
Cc:     Jonathan Cameron <jic23@...nel.org>, devel@...verdev.osuosl.org,
        Lars-Peter Clausen <lars@...afoo.de>,
        Michael Hennerich <Michael.Hennerich@...log.com>,
        linux-iio@...r.kernel.org,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        linux-kernel@...r.kernel.org,
        Peter Meerwald-Stadler <pmeerw@...erw.net>,
        Hartmut Knaack <knaack.h@....de>
Subject: Re: [PATCH 04/11] staging: iio: adt7316: fix handling of dac high
 resolution option

On Wed, Dec 12, 2018 at 11:23:16AM +0300, Dan Carpenter wrote:
> On Tue, Dec 11, 2018 at 05:54:56PM -0700, Jeremy Fertic wrote:
> > @@ -651,10 +649,12 @@ static ssize_t adt7316_store_da_high_resolution(struct device *dev,
> >  	u8 config3;
> >  	int ret;
> >  
> > +	if (chip->id == ID_ADT7318 || chip->id == ID_ADT7519)
> > +		return -EPERM;
> 
> return -EINVAL is more appropriate than -EPERM.
> 
> regards,
> dan carpenter
> 

I chose -EPERM because the driver uses it quite a few times in similar
circumstances. At least with this driver, -EINVAL is used when the user
attempts to write data that would never be valid. -EPERM is used when
either the current device settings prevent some functionality from being
used, or the device never supports that functionality. This patch is the
latter, that these two chip ids never support this function.

I'll change to -EINVAL in a v2 series, but I wonder if I should hold off
on a separate patch for other instances in this driver since it will be
undergoing a substantial refactoring. Is there any rule of thumb about
when -EPERM is appropriate for a driver, or is -EINVAL almost always the
better option?