Implement whitelist approach for carrying RW_VPD entries over powerwash/recovery |
||||||
Issue descriptionRight now we use a blacklist to delete certain RW_VPD entries on powerwash. I couldn't find code to delete RW_VPD entries on recovery. Instead, we should clear all of RW_VPD except for a well-defined whitelist.
,
Aug 3 2017
The benefit is privacy. By exposing and documenting powerwash/recovery functionality we give rise to the expectation that all state is deleted by these operations. Retaining random VPD data across powerwash/recovery may cause surprise and may put users at risk in some way that we don't foresee. To reduce the risk I'd like to come up with a list of VPD entries that we consider fair and reasonable to keep across powerwash/recovery and discard all others. Does that make sense?
,
Aug 3 2017
My concern is if this will delete any data on shipped units while users are not aware of the risk, and the data can't be recovered after that. i.e., I don't know if any partner has provisioned values that they expect to live, for example ActivateDate (this was not clearly defined in early devices).
,
Aug 3 2017
> My concern is if this will delete any data on shipped units while users > are not aware of the risk, and the data can't be recovered after that. Thank you for bringing that up! How can we find out which RW_VPD values are currently in use so that we can whitelist them? Is a codesearch sufficient or is there some factory code that we don't have access to? Btw, this is what I have found so far: ActivateDate block_devmode check_enrollment first_active_omaha_ping_sent gbind_attribute mlb_serial_number oem_can_exit_enrollment # seems unused oem_device_requisition # seems unused oem_enterprise_managed # seems unused oem_keyboard_driven_oobe # seems unused recovery_count ubind_attribute Do you have any thoughts as to what other values we need to whitelist? (Btw, we could add UMA to see whether we encounter unexpected values in RW_VPD.)
,
Aug 3 2017
Just came across this comment:
# 2. When a DUT is finalized, factory related device data in RW VPD
# like 'mlb_serial_number' and 'smt_complete' are deleted.
This seems to indicate that mlb_serial_number shouldn't be present on a shipped device thus I guess we can drop it from the list in comment #4.
,
Aug 3 2017
> How can we find out which RW_VPD values are currently in use so that we can whitelist them? Is a codesearch sufficient or is there some factory code that we don't have access to? What factory really provisioned are based on their shopfloor backend system, so we can't be for sure. May need to ask ODM. But considering that "fields that people never use in RW are probably useless", I think code search is a reasonable first step. mlb_serial_number is actually RO (optional) today, but I wonder where you see that in RW from code? Note old devices may use different VPD name for ActivateDate so please look up private overlays as well.
,
Aug 7 2017
I've searched the code some, but I'm not certain that I have covered everything. Let's see what UMA reveals. Concerning "mlb_serial_number" in RW_VPD -- I found it on my peppy test device. No idea how it got there. About old variants of "ActivateDate" -- are you aware of any? I've tried two old devices (parrot, lumpy) but they were both using "ActivateDate". On a lumpy I've found "MACAddress" (which was set to empty) and "System_UUID". But no indication of them being used anywhere in the code.
,
Aug 7 2017
Devices made in early stage (proto, EVT, DVT) may have different VPD values since they were either provisioned with testing values, or not finalized (so don't be surprised if you see weird values there). Also ODM may add any values they want - sometimes they do want it in RO, sometimes they just don't know what they were doing :p The equivalent of ActivateDate may appear in devices before Spring, which has formally included ActivateDate, but originally the key was introduced by Alex. So no, I don't know which device may have different value, but I'm indicating that devices before Spring may have chosen (or didn't implement) different names (or not).
,
Aug 7 2017
I've seen "System_UUID" on two lumpys and two snows now. Also there's "FCT1Finished"="" and "Main"="2" on the two snows.
,
Aug 7 2017
The devices look like MP to me. (They have no EVT/DVT/PVT stickers.)
,
Aug 7 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/vpd/+/d786226f9154be6d8d4f4d87d7751f95364456c1 commit d786226f9154be6d8d4f4d87d7751f95364456c1 Author: Thiemo Nagel <tnagel@chromium.org> Date: Mon Aug 07 20:50:47 2017 vpd: Add UMA for unknown RW_VPD keys Report the number of RW_VPD keys that don't match the set of well-known keys. BUG=chromium:752017 TEST=manual BRANCH=none Change-Id: I50bd7456d958f076e587d51fdd2368a0120f7888 Reviewed-on: https://chromium-review.googlesource.com/601970 Commit-Ready: Thiemo Nagel <tnagel@chromium.org> Tested-by: Thiemo Nagel <tnagel@chromium.org> Reviewed-by: Thiemo Nagel <tnagel@chromium.org> [modify] https://crrev.com/d786226f9154be6d8d4f4d87d7751f95364456c1/util/check_rw_vpd
,
Aug 8 2017
+vineeth If we want to change RW_VPD into writelist, I think we should make "register for a new VPD entry" more formally defined, following the step in go/cros-new-vpd. Vineeth, do you think this should be added to PT tracker?
,
Aug 8 2017
,
Aug 8 2017
> I've seen "System_UUID" on two lumpys and two snows now. > Also there's "FCT1Finished"="" and "Main"="2" on the two snows. Lumpy and Snow were both built by Samsung and they don't really use our software, so it's not a surprise if they added something during manufacturing and didn't clear it property. BTW, 'vpd' command didn't support "delete" in the very beginning, so I think you can ignore the FCT1Finished if it's empty. Main=2 is probably a SKU identifier and use only during their manufacturing stage so I won't worry about it, too.
,
Aug 9 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/389e496a6656ffb8da5a07133f618357611f1b20 commit 389e496a6656ffb8da5a07133f618357611f1b20 Author: Thiemo Nagel <tnagel@chromium.org> Date: Wed Aug 09 09:00:52 2017 Add Platform.Vpd.UnknownKeys histogram Code for histogram filling is being added in https://chromium-review.googlesource.com/c/601970. Bug: 752017 Change-Id: Ib2d7ce0a172570ac673af69d28c0ff7c323e7513 Reviewed-on: https://chromium-review.googlesource.com/603611 Reviewed-by: Mark Pearson <mpearson@chromium.org> Commit-Queue: Thiemo Nagel <tnagel@chromium.org> Cr-Commit-Position: refs/heads/master@{#492912} [modify] https://crrev.com/389e496a6656ffb8da5a07133f618357611f1b20/tools/metrics/histograms/histograms.xml
,
Aug 9 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/vpd/+/77587b6afc196cfd46590e9cf84fbebc5cd98b53 commit 77587b6afc196cfd46590e9cf84fbebc5cd98b53 Author: Thiemo Nagel <tnagel@chromium.org> Date: Wed Aug 09 18:48:04 2017 vpd: Add more known_keys to VPD UMA These have been found on peach_pit. BUG=chromium:752017 TEST=manual BRANCH=none Change-Id: I977866006d33ba4bb1eb450a00376fcdd8bee2bc Reviewed-on: https://chromium-review.googlesource.com/605847 Commit-Ready: Thiemo Nagel <tnagel@chromium.org> Tested-by: Thiemo Nagel <tnagel@chromium.org> Reviewed-by: Hung-Te Lin <hungte@chromium.org> [modify] https://crrev.com/77587b6afc196cfd46590e9cf84fbebc5cd98b53/util/check_rw_vpd
,
Aug 10 2017
First glimpse at the data: https://uma.googleplex.com/p/chrome/timeline_v2/?sid=d16b2340ca37beb076e8cf777a5e8f07
,
Aug 11 2017
The following revision refers to this bug: https://chrome-internal.googlesource.com/chromeos/docs/+/2b849df40e1a87c5658833bc5061a9813e5f8430 commit 2b849df40e1a87c5658833bc5061a9813e5f8430 Author: Thiemo Nagel <tnagel@chromium.org> Date: Fri Aug 11 12:53:13 2017
,
Aug 11 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/vpd/+/c2056c997cb94c59579075676684d7b4a51e10f8 commit c2056c997cb94c59579075676684d7b4a51e10f8 Author: Thiemo Nagel <tnagel@chromium.org> Date: Fri Aug 11 17:35:54 2017 vpd: Add even more known keys to VPD UMA Add "WLANID", "BT_MAC" and "BIOS" which have been found on Acer devices cyan, banjo and lars. Also break out the keys into two separate variables for documented and undocumented keys. BUG=chromium:752017 TEST=manual BRANCH=none Change-Id: I9371d489c976be09ee3cc6dfb97ecc3e644c49f6 Reviewed-on: https://chromium-review.googlesource.com/610141 Commit-Ready: Thiemo Nagel <tnagel@chromium.org> Tested-by: Thiemo Nagel <tnagel@chromium.org> Reviewed-by: Hung-Te Lin <hungte@chromium.org> [modify] https://crrev.com/c2056c997cb94c59579075676684d7b4a51e10f8/util/check_rw_vpd
,
Aug 18 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/vpd/+/5a70033e842864df7532b540c7c6ee66dc2c3866 commit 5a70033e842864df7532b540c7c6ee66dc2c3866 Author: Thiemo Nagel <tnagel@chromium.org> Date: Fri Aug 18 14:12:18 2017 vpd: Add more known keys to VPD UMA Also adding documentation on which boards the various keys have been observed. BUG=chromium:752017 TEST=manual BRANCH=none Change-Id: I56ccf3b54170d41424f6a760f8b245c6f3d4fc8f Reviewed-on: https://chromium-review.googlesource.com/618867 Commit-Ready: Thiemo Nagel <tnagel@chromium.org> Tested-by: Thiemo Nagel <tnagel@chromium.org> Reviewed-by: Hung-Te Lin <hungte@chromium.org> Reviewed-by: Thiemo Nagel <tnagel@chromium.org> [modify] https://crrev.com/5a70033e842864df7532b540c7c6ee66dc2c3866/util/check_rw_vpd
,
Aug 18 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/vpd/+/238310669b6260205ea4deb561fe34617466ab58 commit 238310669b6260205ea4deb561fe34617466ab58 Author: Thiemo Nagel <tnagel@chromium.org> Date: Fri Aug 18 16:38:19 2017 vpd: Add another known key to VPD UMA Found "oem_device_requisition" key on guado. BUG=chromium:752017 TEST=manual BRANCH=none Change-Id: I9a26c1dc9cf268b6003057130c46ab6290c90cb0 Reviewed-on: https://chromium-review.googlesource.com/620663 Commit-Ready: Thiemo Nagel <tnagel@chromium.org> Tested-by: Thiemo Nagel <tnagel@chromium.org> Reviewed-by: Hung-Te Lin <hungte@chromium.org> [modify] https://crrev.com/238310669b6260205ea4deb561fe34617466ab58/util/check_rw_vpd
,
Aug 21 2017
,
Aug 23 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/vpd/+/5621a838826454df9bdafe1efc261e5c2ce43eb9 commit 5621a838826454df9bdafe1efc261e5c2ce43eb9 Author: Thiemo Nagel <tnagel@chromium.org> Date: Wed Aug 23 19:19:08 2017 vpd: Cosmetics for check_rw_vpd Update top comment and move constants to the top to make the script more readable. Wrap script body in a main() function. No functional change. BUG=chromium:752017 TEST=manual BRANCH=none Change-Id: I2eccf2c9c35d9ec382e580f2301d176b047da317 Reviewed-on: https://chromium-review.googlesource.com/623788 Commit-Ready: Thiemo Nagel <tnagel@chromium.org> Tested-by: Thiemo Nagel <tnagel@chromium.org> Reviewed-by: Mike Frysinger <vapier@chromium.org> [modify] https://crrev.com/5621a838826454df9bdafe1efc261e5c2ce43eb9/util/check_rw_vpd
,
Aug 25 2017
,
Aug 29 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/vpd/+/61cb1e6254238ac4293bcc9581141f78852df4a2 commit 61cb1e6254238ac4293bcc9581141f78852df4a2 Author: Thiemo Nagel <tnagel@chromium.org> Date: Tue Aug 29 11:45:49 2017 vpd: Add more undocumented keys to check_rw_vpd Adding the following keys: component.has_lte (clapper) ec_status (gnawty) Also ignoring everything that starts with "factory." and slightly simplifying the control flow. BUG=chromium:752017 TEST=manual BRANCH=none Change-Id: Id8cf52853f2dd3faa61324633de95a78678b0f8a Reviewed-on: https://chromium-review.googlesource.com/637670 Commit-Ready: Thiemo Nagel <tnagel@chromium.org> Tested-by: Thiemo Nagel <tnagel@chromium.org> Reviewed-by: Hung-Te Lin <hungte@chromium.org> [modify] https://crrev.com/61cb1e6254238ac4293bcc9581141f78852df4a2/util/check_rw_vpd
,
Jan 31 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/vpd/+/0a1804831284fb990df34bc3d111fbce5176f281 commit 0a1804831284fb990df34bc3d111fbce5176f281 Author: Thiemo Nagel <tnagel@chromium.org> Date: Wed Jan 31 16:29:45 2018 vpd: Add another undocumented key to check_rw_vpd Adding battery_cto_disabled (eve). BUG=chromium:752017 TEST=manual BRANCH=none Change-Id: Ieca191f92303fb0a786ce788790fc36a1d0535bc Reviewed-on: https://chromium-review.googlesource.com/895486 Commit-Ready: Thiemo Nagel <tnagel@chromium.org> Tested-by: Thiemo Nagel <tnagel@chromium.org> Reviewed-by: Hung-Te Lin <hungte@chromium.org> [modify] https://crrev.com/0a1804831284fb990df34bc3d111fbce5176f281/util/check_rw_vpd |
||||||
►
Sign in to add a comment |
||||||
Comment 1 by hungte@chromium.org
, Aug 3 2017