Hi Priit, On Wed, Jul 10, 2019 at 11:42 AM Eugeniu Rosca <ero...@de.adit-jv.com> wrote: > > Hi Priit, > > On Tue, Jul 09, 2019 at 02:52:56PM +0300, Priit Laes wrote: > > From: Priit Laes <priit.l...@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.l...@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-fastboot#L90 [2] https://gitlab.denx.de/u-boot/u-boot/commit/fa24eca1f20a037d2dcbd1eae7ac8b2ecb1b0423 [3] https://gitlab.denx.de/u-boot/u-boot/commit/8bd29623b5223e880e7be475243a2bdb987aba38 > -- > Best Regards, > Eugeniu. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot