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
| ||
|
Date: Sat, 13 May 2017 20:46:20 +0200 From: "Luis R. Rodriguez" <mcgrof@...nel.org> To: "Luis R. Rodriguez" <mcgrof@...nel.org>, AKASHI Takahiro <takahiro.akashi@...aro.org>, wagi@...om.org Cc: gregkh@...uxfoundation.org, dwmw2@...radead.org, rafal@...ecki.pl, arend.vanspriel@...adcom.com, rjw@...ysocki.net, yi1.li@...ux.intel.com, atull@...nsource.altera.com, moritz.fischer@...us.com, pmladek@...e.com, johannes.berg@...el.com, emmanuel.grumbach@...el.com, luciano.coelho@...el.com, kvalo@...eaurora.org, luto@...nel.org, dhowells@...hat.com, pjones@...hat.com, linux-kernel@...r.kernel.org Subject: Re: [PATCH v6 3/5] test: add new driver_data load tester On Fri, May 12, 2017 at 05:52:18PM +0200, Luis R. Rodriguez wrote: > On Fri, May 12, 2017 at 09:20:24AM +0900, AKASHI Takahiro wrote: > > On Thu, May 11, 2017 at 08:26:29PM +0200, Luis R. Rodriguez wrote: > > > On Thu, May 11, 2017 at 07:46:27PM +0900, AKASHI Takahiro wrote: > > > > On Fri, Apr 28, 2017 at 03:45:35AM +0200, Luis R. Rodriguez wrote: > > > > > > > diff --git a/tools/testing/selftests/firmware/driver_data.sh b/tools/testing/selftests/firmware/driver_data.sh > > > > > ... > > > > > > > +ALL_TESTS="$ALL_TESTS 0012:1:1" > > > > > > > +ALL_TESTS="$ALL_TESTS 0013:1:1" > > > > > > > > > > > > Do you have good reasons for "the number of times" here? > > > > > > > > > > Just that 1 was not enough and more than 10 seemed too much. As is the tests > > > > > > > > In my opinion, "1," or "2" given the nature of firmware caching, is good > > > > enough, but it's up to you. > > > > > > No, firmware caching deserves its own test unit on its own, but that will be > > > enabled through a separate test once we move DRIVER_DATA_PRIV_REQ_NO_CACHE > > > to DRIVER_DATA_REQ_NO_CACHE (a private feature to a publicly available > > > enabled feature). This will be enabled for the upcoming work which enables > > > FGPA firmware upload which will first enable request_firmware_into_buf() > > > through the driver data API which uses this. > > > > I thought of implementing NO_CACHE option by myself, but came up with > > no practical use cases other than my test case :) > > FWIW, my understanding is that such work is underway. I think it may be > ready first than your signature work so it may make sense to coordinate > with that developer on their work as you might be able to make use of their > tests, etc. > > > > > BTW, firmware caching is a bit annoying in my signature tests > > > > because caching will bypass the verification checks when we iterates > > > > tests with different conditions. > > > > > > 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 ? > > > > Let me explain my test scenario: > > case (a): firmware w/o signature for signature-not-required driver > > case (b): firmware w/o signature for signature-required driver > > ... and so on > > > > Case (a) and (b) are exercised with the same firmware blob but with > > different 'test_config's of test_driver_data driver. > > > > First do case (a) and it succeeds, caching the blob, then do case (b). > > It should fail, but actually succeeds due to firmware caching. > > > > Yes, we can change the order, (b) then (a), to make tests run correctly. > > I see. OK we should consider if firmware requirements can ever vary for > a file. If not then a cached firmware file can have a sig_check which > any request can fill in. Upon request firmware, if the cache is present > *but* the sig check requirements differ (in this simple case where > signature requirements cannot vary) then the cache present will not > suffice so we should allow a full new file check through to allow > to fill the sig_check. If signatures requirements can vary then we > could have a linked list of signature requirements and at cache check > time we'd have to check if they match the requirements. > > Just one way to try to address this I think. But hey this is all firmware > signing related. I suppose a more relevant question to this thread is if > there might be current criteria or criteria being introduced in this patch > series which is similar to the signature check which should also be > extended in these sorts of caching checks. > > > But again, when we iterate this set of test cases under "-c" repeatedly, > > this will happen again as you can imagine. > > Makes sense. > > > Thanks to firmware caching, each of iterations of tests is not executed > > under the exactly same condition. That is the problem. > > Sure, specially since asynchornous lookups won't be serialized we can't > expect any proper order for which request goes in first. If the extra > caching requirements were however taken into consideration I would expect > this to not matter though. Note that a bug was just opened on multiple fw requests and only one completes fine, it sounds like a real bug, and if so sounds related to this topical case you are testing against as well. https://bugzilla.kernel.org/show_bug.cgi?id=195477 It has a proposed patch I have not yet had a chance to properly review / test: https://bugzilla.kernel.org/attachment.cgi?id=256493&action=diff&collapsed=&headers=1&format=raw Luis
Powered by blists - more mailing lists