> not entirely clear why ... it should be using /var for things).  that path is not visible to debugd.

I'm not the original author of dump_vpd_log, but I believe the reason there was because there are lots of programs need to access VPD before encrypted /var is mounted, for example the boot time recovery / wiping etc.

For modern systems, vpd is also available in /sys/firmware/vpd. Can you just include that in debugd?

Meanwhile, I think the dump_vpd_log has been improved a lot and many programs reading VPD have been changed to use vpd_get_value, so maybe we can consider about moving the cache to /var.

This is likely the root cause for this bug: https://bugs.chromium.org/p/chromium/issues/detail?id=874286

Not positive, but may this also be affecting this bug where mosys can't execute "platform brand" on some, but not all, devices?

https://bugs.chromium.org/p/chromium/issues/detail?id=874824

In any case, bug 874286 is release blocking for M69. Can we put a short-term fix in for M69 by adding the missing path to debugd?

the only thing that calls `dump_vpd_log` any more:
- vpd-log.conf (boot-services)
- activate_date.conf (system-services)
- UE (doesn't run before boot-services)
- login-manager code (long after boot-services)
- debugd (rlz callbacks)
- check-rw-vpd.conf (system-services)
- update_rw_vpd (chromeos-postinst)

that leaves `vpd_get_value` usage which doesn't seem to be called early on, but if it is, it has logic to fallback to non-cached results.

so while it might have been needed in previous versions, i don't think that's true anymore.

mosys doesn't go through debugd, so it seems unlikely that any failures in it when run outside of debugd are related to this

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/25cdbe759388ae27e61f969eb3c64804a37194bf

commit 25cdbe759388ae27e61f969eb3c64804a37194bf
Author: Mike Frysinger <vapier@chromium.org>
Date: Wed Aug 29 23:09:16 2018

debugd: hack in /mnt path for vpd

BUG=chromium:876838, chromium:874286
TEST=started debugd and it could view the vpd path

Change-Id: Ic718ae9f34b836935c9cbc08366f0a41145987f5
Reviewed-on: https://chromium-review.googlesource.com/1186165
Commit-Ready: Mike Frysinger <vapier@chromium.org>
Tested-by: Mike Frysinger <vapier@chromium.org>
Reviewed-by: Luis Hector Chavez <lhchavez@chromium.org>

[modify] https://crrev.com/25cdbe759388ae27e61f969eb3c64804a37194bf/debugd/src/main.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/bea0f705e4ff48bededf970f6337be913312e08f

commit bea0f705e4ff48bededf970f6337be913312e08f
Author: Mike Frysinger <vapier@chromium.org>
Date: Fri Aug 31 05:02:35 2018

debugd: hack in /mnt path for vpd

BUG=chromium:876838, chromium:874286
TEST=started debugd and it could view the vpd path

Change-Id: Ic718ae9f34b836935c9cbc08366f0a41145987f5
Reviewed-on: https://chromium-review.googlesource.com/1186165
(cherry picked from commit 25cdbe759388ae27e61f969eb3c64804a37194bf)
Reviewed-on: https://chromium-review.googlesource.com/1199002
Reviewed-by: Mike Frysinger <vapier@chromium.org>
Tested-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/bea0f705e4ff48bededf970f6337be913312e08f/debugd/src/main.cc

Are we Ok with the hack, or do we still need to fix something here?

vpd needs rewriting imo to not use /mnt/stateful_partition/unencrypted.  it should stick to /var like every other standard program.

allowing symlinks in /var/log is allowing people to abuse and exploit the system (see issue 916152).  we're not going to allow any program have an exception to symlinks under /var.  so we need to rewrite vpd logic.

does it actually need to write to things under /mnt/stateful_partition/unencrypted ?  /var is available once it's mounted by chreomeos_startup, so it's available to boot-services and system-services and everyone after that.

I think we have programs trying to fetch VPD in chromeos_startup (for example clobbering, by wiping / enter_dev / leave_dev / update_*firmware ...)

And it used to *not* work due to how dump_vpd was implemented, and again, I think we have improved dump_vpd so it should not be a problem anymore.

So yes, if we can't allow VPD as white-listed symlinks anymore then we should try to fix this (and hope not causing further issues).

Can we put symlink in /mnt/stateful_partition/unencrypted for transition?

A quick search on cs/ shows that the unencrypted path is used by:
 src/platform/vpd/util/dump_vpd_log
 chromium/src/chromeos/chromeos_paths.cc
 src/platform/vpd/util/vpd_get_value
 src/chromeos/system/statistics_provider.cc
 src/platform2/regions/cros_region_data
 
src/third_party/autotest/files/server/site_tests/platform_Vpd/platform_Vpd.py
 src/platform2/debugd/src/main.cc
 src/third_party/autotest/files/server/site_tests/network_WiFi_RegDomain/network_WiFi_RegDomain.py
 src/platform2/power_manager/init/systemd/powerd.service
 src/platform2/power_manager/init/upstart/powerd.conf

It may be hard to change all these components at one time.

Currently dump_vpd_log puts files in:

 /mnt/stateful_partition/unencrypted/cache/vpd:
  full-v2.txt  (synlink: /var/cache/vpd/full-v2.txt)
  vpd_echo.txt (symlink: /var/cache/echo/vpd_echo.txt)
  filtered.txt (symlink: /var/log/vpd_2.0.txt)

One question: what files do debugd want to get?
Due to privacy reason in ECHO, I'd assume debugd should only fetch filtered.txt?

Or should we move everything created by dump_vpd_log to /var/cache/vpd and let debugd selectively upload files it wants>

And, can we still set /var/cache/echo/vpd_echo.txt a symlink to ../vpd/vpd_echo?

+stephenlin for ECHO stuff

or should we just do mount-bind instead symlink as quick migration?

a mount-bind would be acceptable to help transition things.  although we might run into the same problem that originally filed this bug: if a daemon that creates a unique mount namespace doesn't also include that bind mount, it won't be able to access its contents.

you're right that the /var/cache/echo/vpd_echo.txt symlink also needs to be transitioned, but we don't need to do them all at once (in case it helps).  we can file a sep bug if they can be converted independently.

I think we want to clarify:

1. When collecting feedback logs, is it fine to include ECHO (@tnagle/mnissler/stephenlin?)

2. If not, then filtered.txt would be the only VPD debugd can access.

For that case with minimal changes, I'll probably say let's do mount-bind instead of symlink for that one file.

1) Privacy should chime in, but I'd say it's not ok to transmit the ECHO code in a feedback log since it is a unique identifier. But, Feedback is sent with user permission so not positive (though I understand some feedback is sent automatically).

2) W.r.t. filtered.txt being the only VPD debugd can access, debugd does make calls to the vpd tool as well as dump_vpd_log here:
https://cs.corp.google.com/eureka_internal/external/gwifi/platform2/debugd/src/debugd_dbus_adaptor.cc?rcl=0&l=445