Bug 48154 - [Patch] Prevent crash on duplicate module loading
Summary: [Patch] Prevent crash on duplicate module loading
Status: RESOLVED FIXED
Alias: None
Product: p11-glue
Classification: Unclassified
Component: p11-kit (show other bugs)
Version: unspecified
Hardware: Other Linux (All)
: medium normal
Assignee: Stef Walter
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-04-01 04:51 UTC by Andreas Metzler
Modified: 2012-07-16 16:06 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments

Description Andreas Metzler 2012-04-01 04:51:51 UTC
Hello,

Ubuntu ships two patches related to https://bugs.launchpad.net/ubuntu/+source/p11-kit/+bug/911436

-------------------- Anders Kaseorg wrote -------------
This happened after a slightly broken upgrade, but it can be reproduced with

$ sudo cp /etc/pkcs11/modules/gnome-keyring-module /etc/pkcs11/modules/gnome-keyring-module.dpkg-new
[...]
$ sudo apt-get update
p11-kit: duplicate configured module: gnome-keyring-module: /usr/lib/x86_64-linux-gnu/pkcs11/gnome-keyring-pkcs11.so
E: Method https has died unexpectedly!
E: Sub-process https received a segmentation fault.

There are two bugs here, I suppose. One is that pkcs11 should ignore *.dpkg-new files (usually this would be done by reading, e.g., /etc/pkcs11/modules/*.conf instead of /etc/pkcs11/modules/*), and the other is that a duplicate file shouldn’t cause a crash.
-------------------- ----------------------------------

Patch#1, this should prevent the crash:
http://bazaar.launchpad.net/~ubuntu-branches/ubuntu/precise/p11-kit/precise/view/head:/debian/patches/duplicate-module-fix.patch
Reset module reference after unloading duplicates

Patch#2
http://bazaar.launchpad.net/~ubuntu-branches/ubuntu/precise/p11-kit/precise/view/head:/debian/patches/valid-config-files.patch
filter out invalid config file names in directory includes
 This patch filters config filenames, and only allows names that fit
 the following criteria:
 - Must start with an alphanumeric
 - Can only contain alnum, _, -, and .
 - Can not end in .dpkg*

Could these possibly be included upstream?

thanks, cu andreas
Comment 1 Stef Walter 2012-04-03 04:36:02 UTC
Thanks. The other day I fixed this in git master in a different way. Does this commit fix the issue for you? Please reopen this bug if it doesn't fix the issue.

commit ff9926b8dcead91e7fc6d08d0ca1d2d8cc982308
Author: Stef Walter <stefw@gnome.org>
Date:   Sun Apr 1 21:56:35 2012 +0200

    Fix crasher when a duplicate module is present
Comment 2 Andreas Metzler 2012-04-03 10:34:09 UTC
The patch in GIT fixes the crash for me, thanks.

What are your thought about the second patch? Could this have a place upstream, or will it need to stay in Ubuntu (and Debian)?

cu andreas
Comment 3 Stef Walter 2012-04-03 11:44:11 UTC
(In reply to comment #2)
> The patch in GIT fixes the crash for me, thanks.
> 
> What are your thought about the second patch? Could this have a place upstream,
> or will it need to stay in Ubuntu (and Debian)?
> 
> cu andreas

I think it may be worthwhile to limit the module config file names. This however is not a backwards compatible change. Since this is early on in p11-kit usage, we may be able to swing such a change.

Could you to post such a patch to the p11-glue mailing list and see if anyone balks. 

We don't want to have *.dpkg checks upstream. Two reasons:

 * More and more daemons are moving to files-in-a-directory configuration. If
   dpkg writes such duplicate files regularly, this is a systemic problem, and
   not specific to p11-kit.

 * p11-kit should probably move towards having three directories that it loads
   config files:

    1. /etc/pkcs11/modules       (root/admin configured module configs)
    2. /usr/lib/p11-kit/modules  (module configs installed by packages)
    3. ~/.pkcs11/modules         (user configured module configs)

   Stuff in (2) would be expected not to be edited by admins. They could
   override such things in (1). (2) would be installed to used by packages.
Comment 4 Stef Walter 2012-07-16 16:06:45 UTC
See bug 52158 for more work on the requiring module configs to have certain types of file names.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.