linux-kernel - Re: [PATCH v6 3/5] test: add new driver

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: <20170517153840.GO17314@wotan.suse.de>
Date:   Wed, 17 May 2017 17:38:40 +0200
From:   "Luis R. Rodriguez" <mcgrof@...nel.org>
To:     AKASHI Takahiro <takahiro.akashi@...aro.org>,
        "Luis R. Rodriguez" <mcgrof@...nel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Daniel Wagner <wagi@...om.org>,
        David Woodhouse <dwmw2@...radead.org>, rafal@...ecki.pl,
        Arend Van Spriel <arend.vanspriel@...adcom.com>,
        "Rafael J. Wysocki" <rjw@...ysocki.net>,
        "Li, Yi" <yi1.li@...ux.intel.com>, atull@...nsource.altera.com,
        moritz.fischer@...us.com, Petr Mladek <pmladek@...e.com>,
        Johannes Berg <johannes.berg@...el.com>,
        Emmanuel Grumbach <emmanuel.grumbach@...el.com>,
        Luciano Coelho <luciano.coelho@...el.com>,
        Kalle Valo <kvalo@...eaurora.org>,
        Andy Lutomirski <luto@...nel.org>,
        David Howells <dhowells@...hat.com>,
        Peter Jones <pjones@...hat.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v6 3/5] test: add new driver_data load tester

On Wed, May 17, 2017 at 06:08:36PM +0900, AKASHI Takahiro wrote:
> On Fri, May 12, 2017 at 05:59:23PM +0200, Luis R. Rodriguez wrote:
> > On Fri, May 12, 2017 at 09:28:47AM +0900, AKASHI Takahiro wrote:
> > > On Thu, May 11, 2017 at 11:32:30AM -0700, Luis R. Rodriguez wrote:
> > > > On Thu, May 11, 2017 at 11:26 AM, Luis R. Rodriguez <mcgrof@...e.com> wrote:
> > > > >
> > > > > It would seems to make sense to me to only need to verify files when read
> > > > > for the first time, once its cache I don't see why we would re-verify them ?
> > > > 
> > > > To be clear, the fw cache feature reads the files from the fs prior to
> > > > suspend, and then uses the in-memory cache on resume. So it would make
> > > > sense to me only to rely on fw verification on resume then when the fw
> > > > cache is used ?
> > > 
> > > Good point. I was thinking of need for verification on resume.
> > 
> > From what we have discussed so far it would seem to me only necessary
> > for a sig_check_ok (if we accept a file can have only one signature
> > requirement) for a cache entry, and if its not set but a lookup needs
> > a sig check it can do a full fs lookup. If such a lookup succeeded
> > then it can fill the sig_check_ok in, provided the file contents
> > match of course, given the file could have changed under the hood
> > between the last file cache lookup (if the file did change that puts
> > us at odd with the first lookup, but since its an update and no sig
> > check is required, I guess it is fine to use its contents).
> > 
> > > As cache is not protected 
> > 
> > Cache should be protected, it should be const and if its not we should fix that.
> 
> Yeah, but
> 
> > > and visible to the kernel,
> > 
> > You mean it is visible to the kernel ?
> 
> your current implementation doesn't provide any write protection.

The cache was implemented long ago by someone other than myself. Patches
are welcomed.

> > >  some malware might want to rewrite it :)
> > 
> > Right, we want to be pedantic about that sort of stuff and signature
> > verification can help here but those benefits should carry their own
> > weight. We should do what we can without file signature verification to
> > protect the cache.
> > 
> > The cache is short lived though, it exists only during suspend/resume.
> 
> I found out why my test cases fail:
> trigger_config_sync() in test_driver_data.c always enables REQ_KEEP flag
> and so cached data (firmware_buf->data) has not been cleaned up.
> I haven't fixed it in my test environment although I pointed it out before.

Ah, perhaps the later revision of the test driver I posted captured this fix?

> But the issue on write protection is still there wehn REQ_KEEP is used.

Cache of firmware is a feature internal to the firmware_class, so it can only
be fixed there. Patches welcomed.

  Luis