lkml.org 
[lkml]   [2017]   [May]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [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@suse.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

    \
     
     \ /
      Last update: 2017-05-17 17:39    [W:3.468 / U:1.180 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site