From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754201AbdEQJEg (ORCPT ); Wed, 17 May 2017 05:04:36 -0400 Received: from mail-pg0-f44.google.com ([74.125.83.44]:33962 "EHLO mail-pg0-f44.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754076AbdEQJE1 (ORCPT ); Wed, 17 May 2017 05:04:27 -0400 Date: Wed, 17 May 2017 18:08:36 +0900 From: AKASHI Takahiro To: "Luis R. Rodriguez" Cc: Greg Kroah-Hartman , Daniel Wagner , David Woodhouse , rafal@milecki.pl, Arend Van Spriel , "Rafael J. Wysocki" , "Li, Yi" , atull@opensource.altera.com, moritz.fischer@ettus.com, Petr Mladek , Johannes Berg , Emmanuel Grumbach , Luciano Coelho , Kalle Valo , Andy Lutomirski , David Howells , Peter Jones , "linux-kernel@vger.kernel.org" Subject: Re: [PATCH v6 3/5] test: add new driver_data load tester Message-ID: <20170517090832.GA31374@linaro.org> Mail-Followup-To: AKASHI Takahiro , "Luis R. Rodriguez" , Greg Kroah-Hartman , Daniel Wagner , David Woodhouse , rafal@milecki.pl, Arend Van Spriel , "Rafael J. Wysocki" , "Li, Yi" , atull@opensource.altera.com, moritz.fischer@ettus.com, Petr Mladek , Johannes Berg , Emmanuel Grumbach , Luciano Coelho , Kalle Valo , Andy Lutomirski , David Howells , Peter Jones , "linux-kernel@vger.kernel.org" References: <20170330032514.17173-1-mcgrof@kernel.org> <20170330032514.17173-4-mcgrof@kernel.org> <20170411083248.GE15139@linaro.org> <20170428014535.GO28800@wotan.suse.de> <20170511104625.GF22134@linaro.org> <20170511182629.GY28800@wotan.suse.de> <20170512002846.GH22134@linaro.org> <20170512155923.GC28800@wotan.suse.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170512155923.GC28800@wotan.suse.de> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 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. > > 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. But the issue on write protection is still there wehn REQ_KEEP is used. Thanks, -Takahiro AKASHI > Luis