linux-kernel - Re: [1/2] lis3: Add device owner

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: <20100924202404.GA20813@ericsson.com>
Date:	Fri, 24 Sep 2010 13:24:04 -0700
From:	Guenter Roeck <guenter.roeck@...csson.com>
To:	Yuri Ershov <ext-yuri.ershov@...ia.com>
CC:	eric.piel@...mplin-utc.net, samu.p.onkalo@...ia.com,
	akpm@...ux-foundation.org, daniel@...aq.de,
	ext-yuri.kululin@...ia.com, lm-sensors@...sensors.org,
	linux-kernel@...r.kernel.org
Subject: Re: [1/2] lis3: Add device owner

On Fri, Aug 27, 2010 at 02:33:20PM -0000, Yuri Ershov wrote:
> Add device owner and change /dev/freefall file operations owner
> according to the used driver
> 
> Signed-off-by: Yuri Kululin <ext-yuri.kululin@...ia.com>
> 
> ---
> drivers/hwmon/lis3lv02d.c |    8 ++++++--
>  drivers/hwmon/lis3lv02d.h |    2 ++
>  2 files changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hwmon/lis3lv02d.c b/drivers/hwmon/lis3lv02d.c
> index e278f0e..0f1bd36 100644
> --- a/drivers/hwmon/lis3lv02d.c
> +++ b/drivers/hwmon/lis3lv02d.c
> @@ -591,8 +591,7 @@ static int lis3lv02d_misc_fasync(int fd, struct file *file, int on)
>  	return fasync_helper(fd, file, on, &lis3_dev.async_queue);
>  }
>  
> -static const struct file_operations lis3lv02d_misc_fops = {
> -	.owner   = THIS_MODULE,
> +static struct file_operations lis3lv02d_misc_fops = {
>  	.llseek  = no_llseek,
>  	.read    = lis3lv02d_misc_read,
>  	.open    = lis3lv02d_misc_open,
> @@ -1007,6 +1006,11 @@ int lis3lv02d_init_device(struct lis3lv02d *dev)
>  		goto out;
>  	}
>  
> +	if (dev->owner)
> +		lis3lv02d_misc_fops.owner = dev->owner;
> +	else
> +		lis3lv02d_misc_fops.owner = THIS_MODULE;
> +
If you retained the static assignment to .owner above, you would not need
the else part here.

Also, this depends on patch#2, which actually sets dev->owner.
So the problem isn't really addressed w/o patch #2. Given that,
I am not sure if it makes sense to have two separate patches.

If both patches were applied as one, you might not need the if ()
above in the first place. And if you do, you might still not have solved
the problem completely since .owner would not be set correctly
if dev->owner is NULL. 

On a higher level, reassigning the owner like this seems to be quite uncommon,
at least in hwmon. I would like to see an Acked-by from Eric to ensure
that the fix is correct.

Thanks,
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/