[fprint] [PATCH 0/8] [RFC] Build drivers as shared objects

Kunal Gangakhedkar kunal.gangakhedkar at gmail.com
Mon Oct 8 20:43:39 PDT 2012


Resending - since earlier mail got blocked from the list due to size >40KB.

On Monday 08 Oct 2012 5:54:06 PM Bastien Nocera wrote:
> On Mon, 2012-10-08 at 21:03 +0530, Kunal Gangakhedkar wrote:
> > I'm of the opinion that the less amount of un-necessary code loaded in
> > memory, the better it is.
> > 
> If it's not actually executed, the code stays on the disk, not in
> memory. Modern OSes will do that.

As per my understanding, the moment fp_init() is called, entire libfprint 
(with all the baggage of drivers) is loaded into memory - and not piece-by-
piece. 

This is exactly what I'm pitching for with my patchset that we split it up so 
that it can be loaded with only required pieces.

What parts of the memory image are swapped out are not in our control, but 
they are still executable memory image.

Also, the primary user of libfprint is still fprintd - a daemon started at 
boot time. If it keeps pinging the device for fingerprint status (I don't know 
- haven't looked at fprintd or any of the GUIs yet), libfprint is very much 
active in memory.

> How is it an attack vector if the code isn't executed? If you get to the
> point where you can execute arbitrary code, it's very very likely that
> you can also replace the memory contents with something else.

Any code that resides in memory is potentially executable.
This is why you have protection schemes like ASLR and stack protector.

Instead, if your library/executable resides only on disk (not in swap, but as 
a file on disk), it's actually not executing and hence, safe (unless you can 
patch up the executable on disk). The attempts to modify on-disk files can 
easily be hindered with systems like tripwire (or simple sha1 checksums 
comparison from known good media). Such is not the case with code in memory.

>  29 files changed, 678 insertions(+), 59 deletions(-)
> 
> Yeah, not sure about it reducing complexity.

If you look closely, a lot of it is build system changes.
Moreover, only stats don't actually convey anything about "complexity".
Since when *only* stats have been considered as metric of "complexity"?

Since you are talking about stats, let me put it in perspective:

1. refactoring of drivers:
 libfprint/drivers/aes1610.c   |   22 +++++++++++++++++++++-
 libfprint/drivers/aes2501.c   |   18 ++++++++++++++++++
 libfprint/drivers/aes4000.c   |   18 ++++++++++++++++++
 libfprint/drivers/fdu2000.c   |   19 +++++++++++++++++++
 libfprint/drivers/upeke2.c    |   17 +++++++++++++++++
 libfprint/drivers/upeksonly.c |   18 ++++++++++++++++++
 libfprint/drivers/upekts.c    |   16 ++++++++++++++++
 libfprint/drivers/uru4000.c   |   18 ++++++++++++++++++
 libfprint/drivers/vcom5s.c    |   17 +++++++++++++++++
 libfprint/drivers/vfs101.c    |   19 +++++++++++++++++++
 libfprint/drivers/vfs301.c    |   18 ++++++++++++++++++
 11 files changed, 199 insertions(+), 1 deletion(-)

This includes only defining entry/exit point functions which are guarded with 
#ifdef .... #endif - *NOTHING* of the original code is touched in the drivers.

2. Build related changes:
 configure.ac                  |   18 ++++++-
 examples/Makefile.am          |    6 +++
 libfprint/Makefile.am         |   19 ++++++-
 libfprint/drivers/Makefile.am |  119 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 159 insertions(+), 3 deletions(-)

3. Marking internal API with API_INTERNAL + some small indentation fixes in 
*existing* libfprint code:
 libfprint/async.c       |   26 +++++++++++++-------------
 libfprint/core.c        |   18 +++++++++++++++---
 libfprint/data.c        |   10 ++++++----
 libfprint/drv.c         |   19 ++++++++++---------
 libfprint/gdkpixbuf.c   |    3 ++-
 libfprint/imagemagick.c |    3 ++-
 libfprint/img.c         |   18 +++++++++---------
 libfprint/imgdev.c      |   23 +++++++++++++----------
 libfprint/poll.c        |   10 +++++-----
 9 files changed, 75 insertions(+), 55 deletions(-)

which, again, *doesn't* touch anything in the code part, except add 
API_INTERNAL qualifier in front of the function prototype.

4. Actual new code:
 examples/test_driver.c |   32 +++++++++++
 libfprint/module.c     |  148 
++++++++++++++++++++++++++++++++++++++++++++++++
 libfprint/module.h     |   57 +++++++++++++++++++
 3 files changed, 237 insertions(+)

FWIW, it took me less than 2 hrs to code the dynamic loader in C and rest of 
the time fixing up the build process - since I'm not an ardent user of 
autotools (I rather prefer to hack up Makefiles manually - a lot more flexible).

I wanted to make sure it's still possible to build libfprint as it is now - 
with driver code built-in.

So, if you don't configure with '--enable-dynamic-drivers', you do go back to 
existing way - the dynamic loader (module.c) is not even compiled and included 
in libfprint in that case.

Another point, any kind of refactoring effort does end up touching a lot of 
code - so, it cannot be used as a measure of complexity.

I think you'd agree with me that this patchset does reduce complexity if you 
look at the code proper and not just stats.

> Not debating that. If you have code to simplify the drivers handling,
> I'm interested.

For which, you need cleaner *interfaces* - which is what I'm trying to bring 
here.
As I mentioned earlier, the driver knows the best what services and interfaces 
it offers - let it take care of registering required services/interfaces.

> You're moving the complexity to the users of libfprint. Furthermore,
> enumerating the supported devices means loading all the device drivers.

I think I did mention in my first mail that this can be done by using a post-
install script with a separate program.

You don't need to do it every so often, do you?
How many times do you change your fingerprint device to require enumeration 
with drivers?

Again, I'm trying to stick to UNIX philosophy - each program does one and only 
one job but does it well.

> See the point I mentioned about the internal drivers API not being with
> any guarantees of stability.

*Somebody* has to maintain the drivers whenever there is change in internal 
API - whether they're built into libfprint or as separate modules.
So, this point is moot in my PoV.

Please note that I'm *not* suggesting or even supporting out-of-tree 
development here.

> I'm willing to take patches to reduce the complexity of the different
> driver modules.

I hope it will come in future, but like I said, it needs cleaner interfaces 
and data structures. Consider this patchset as the beginning ;)

BTW, I'm not asking it to be merged in master just now.
I expect people to just take a look at it right now - hence, calling it RFC.

Hope this helps.

Thanks,
Kunal


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/fprint/attachments/20121009/d0c0b837/attachment-0001.html>


More information about the fprint mailing list