Hi Priit,
On Wed, Jul 10, 2019 at 11:42 AM Eugeniu Rosca erosca@de.adit-jv.com wrote:
Hi Priit,
On Tue, Jul 09, 2019 at 02:52:56PM +0300, Priit Laes wrote:
From: Priit Laes priit.laes@paf.com
Add u-boot specific getvar "extension" to fetch u-boot environment variables via `fastboot getvar uboot:var`. This makes it possible to gather certain data (like mac addresses) in an automated way during initial fastboot flashing for inventory purposes:
$ fastboot getvar uboot:ethaddr uboot:ethaddr: 12:23:45:56:78:90
Output is currently truncated at 64 bytes, but this is good enough for my own requirements.
Signed-off-by: Priit Laes priit.laes@paf.com
drivers/fastboot/fb_getvar.c | 14 ++++++++++++++
[..]
+static void getvar_ubootenv(char *var_parameter, char *response) +{
const char *value = env_get(var_parameter);This would bring a lot of flexibility to the users. My only concern is that it exposes to the outside world an internal U-Boot API (env_get) which might have weaknesses (or might acquire them in time). I am not sure how env_get() behaves in below corner cases:
- NULL pointer
- empty string
- hugely long string
IMHO the internal APIs are usually not designed to sustain high level of stress/fuzziness in their input parameters. Once made available to the users of fastboot tool, this will open room for more experiments to the CVE seekers.
Another observation is that this patch will contribute with a deviation from the vanilla fastboot protocol (which might be fine).
Since there are already multiple getvar functions which fetch the information from the U-Boot environment, I wonder if all of these could be centralized in a table like below:
{ /* fastboot U-Boot */ { "serialno", "serial#" }, { "product", "board" }, { "platform", "platform" }, { "ethaddr", "ethaddr" }, { "anything", "anything" }, }
The upsides of this approach:
- Unification, readability, decreased code size
The downsides:
- Inflexible for getting arbitrary environment variables
I agree with Eugeniu on his points about security and deviation from fastboot spec (can be found in AOSP). Instead of exposing the whole U-Boot environment, I can suggest you to expose only actually needed variables for your platform. In fact, we already have a mechanism exactly for that, called "variable overrides". It's documented in [1]. Basically you just need to add "fastboot.xxx" variables to your environment (can be don e.g. in your board_late_init() function), and you'll be able to obtain those via "fastboot getvar xxx". You can see an example in patches [2,3].
[1] https://gitlab.denx.de/u-boot/u-boot/blob/v2019.07/doc/README.android-fastbo... [2] https://gitlab.denx.de/u-boot/u-boot/commit/fa24eca1f20a037d2dcbd1eae7ac8b2e... [3] https://gitlab.denx.de/u-boot/u-boot/commit/8bd29623b5223e880e7be475243a2bdb...
-- Best Regards, Eugeniu.