linux-kernel - Re: [PATCH 2/2] i2c-dev: Don't block the adapter from unregistering

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: <20160706205540.GJ4127@ubuntu>
Date:	Wed, 6 Jul 2016 13:55:40 -0700
From:	Viresh Kumar <viresh.kumar@...aro.org>
To:	Jean Delvare <jdelvare@...e.de>
Cc:	Wolfram Sang <wsa@...-dreams.de>, linux-i2c@...r.kernel.org,
	linux-kernel@...r.kernel.org, gregkh@...uxfoundation.org,
	Johan Hovold <johan@...nel.org>, Alex Elder <elder@...aro.org>
Subject: Re: [PATCH 2/2] i2c-dev: Don't block the adapter from unregistering

On 06-07-16, 19:12, Jean Delvare wrote:
> Well well... I don't like this patch at all to be honest.

Sure, I didn't like it much as well. I just wanted people to comment on what
else we can do here. We don't really want to add out-of-mainline stuff here.

> My first question would be: what is keeping /dev/i2c-* open all the
> time? Originally i2c-dev was developed with development and debugging
> tools in mind (the i2c-tools suite.) The device nodes were never meant
> to be kept open for more than a few seconds.

We thought that buggy userspace shouldn't be allowed to get kernel into trouble.
Isn't that the case ?

In our case this is what happens:
- userspace opens the file descriptor
- we try to forcefully remove the module from phone (that doesn't talk to
  userspace to stop using the device).
- The module doesn't get ejected unless the app closes the fd.

> Do you have user-space i2c device drivers on your system? Which ones,

No. Its probably an app written by some of our module app developers.

> and why (I would expect all useful i2c devices to have a kernel
> driver.)

That's what we have.

> Requesting and freeing the i2c adapter for every transaction is going

Well, we are just finding it (taking a reference of it) and the dropping its
reference.

> to add a lot of overhead to all existing tools :-(

:(

> It's not like every user can open i2c device nodes and block the
> system. Only selected users should be able to open i2c device nodes
> (only root by default) so they should be responsible for not
> misbehaving.

Hmmm. The problem is that they weren't told when the module tries to go away and
so they don't know that they need to close the fd.

Also coming to the earlier thing, I though even the buggy userspace thing
shouldn't be allowed to block kernel device unregisteration.

-- 
viresh