summaryrefslogtreecommitdiff
path: root/common/image-fit.c
AgeCommit message (Collapse)AuthorFilesLines
2021-09-15image: Avoid erroneous double byte-swap in CRC valueAlexandru Gagniuc1-9/+0
The hash algorithm selection was streamlined in commit 92055e138f28 ("image: Drop if/elseif hash selection in calculate_hash()"). Said commit kept the call to cpu_to_uimage() to convert the CRC to big endian format. This would have been correct when calling crc32_wd(). However, the ->hash_func_ws member of crc32 points to crc32_wd_buf(), which already converts the CRC to big endian. On a little endian host, doing both conversions results in a little-endian CRC. This is incorrect. To remedy this, simply drop the call to cpu_to_uimage(), thus only doing the byte-order conversion once. Fixes: 92055e138f28 ("image: Drop if/elseif hash selection in calculate_hash()") Tested-by: Tom Rini <trini@konsulko.com> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
2021-09-08image: Drop if/elseif hash selection in calculate_hash()Alexandru Gagniuc1-26/+19
calculate_hash() would try to select the appropriate hashing function by a if/elseif contruct. But that is exactly why hash_lookup_algo() exists, so use it instead. This does mean that we now have to 'select HASH' to make sure we get the hash_lookup_algo() symbol. However, the change makes sense because even basic FITs will have to deal with "hash" nodes. My only concern is that the 'select SPL_HASH' might cause some platform to grow above its SPL size allowance Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com> [trini: Make FSL_CAAM be implied only on ARM && SPL] Signed-off-by: Tom Rini <trini@konsulko.com>
2021-08-02global: Convert simple_strtoul() with hex to hextoul()Simon Glass1-1/+1
It is a pain to have to specify the value 16 in each call. Add a new hextoul() function and update the code to use it. Add a proper comment to simple_strtoul() while we are here. Signed-off-by: Simon Glass <sjg@chromium.org>
2021-07-23fit: Allow external data for FDTsJohn Keeping1-1/+2
Switch to fit_image_get_data_and_size() for consistency with all other data loaded from FIT. Signed-off-by: John Keeping <john@metanate.com> Reviewed-by: Simon Glass <sjg@chromium.org>
2021-07-21image: Allow @ in node names when not using signaturesSimon Glass1-1/+1
If signature verification is not in use we don't need to worry about the risk of using @ in node names. Update fit_image_verify() to allow it if the function is not enabled. Signed-off-by: Simon Glass <sjg@chromium.org>
2021-07-16image: Drop IMAGE_ENABLE_BEST_MATCHSimon Glass1-1/+1
This is not needed with Kconfig, since we can use IS_ENABLED() easily enough. Drop it. Signed-off-by: Simon Glass <sjg@chromium.org> Reviewed-by: Alexandru Gagniuc <mr.nuke.me@gmail.com> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
2021-07-16image: Drop IMAGE_ENABLE_SHAxxxSimon Glass1-3/+3
We already have a host Kconfig for these SHA options. Use CONFIG_IS_ENABLED(SHAxxx) directly in the code shared with the host build, so we can drop the unnecessary indirections. Signed-off-by: Simon Glass <sjg@chromium.org> Reviewed-by: Alexandru Gagniuc <mr.nuke.me@gmail.com> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
2021-07-16image: Drop IMAGE_ENABLE_SHA1Simon Glass1-1/+1
We already have a host Kconfig for SHA1. Use CONFIG_IS_ENABLED(SHA1) directly in the code shared with the host build, so we can drop the unnecessary indirection. Signed-off-by: Simon Glass <sjg@chromium.org> Reviewed-by: Alexandru Gagniuc <mr.nuke.me@gmail.com> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
2021-07-16fit: Load DTO into temporary buffer and ignore load addressMarek Vasut1-4/+28
The current fitImage DTO implementation expects each fitImage image subnode containing DTO to have 'load' property, pointing somewhere into memory where the DTO will be loaded. The address in the 'load' property must be different then the base DT load address and there must be sufficient amount of space between those two addresses. Selecting and using such hard-coded addresses is fragile, error prone and difficult to port even across devices with the same SoC and different DRAM sizes. The DTO cannot be applied in-place because fdt_overlay_apply_verbose() modifies the DTO when applying it onto the base DT, so if the DTO was used in place within the fitImage, call to fdt_overlay_apply_verbose() would corrupt the fitImage. Instead of copying the DTO to a specific hard-coded load address, allocate a buffer, copy the DTO into that buffer, apply the DTO onto the base DT, and free the buffer. The upside of this approach is that it is no longer necessary to select and hard-code specific DTO load address into the DTO. The slight downside is the new malloc()/free() overhead for each DTO, but that is negligible (*). (*) on iMX8MM/MN and STM32MP1 Signed-off-by: Marek Vasut <marex@denx.de> Cc: Pantelis Antoniou <pantelis.antoniou@konsulko.com> Cc: Simon Glass <sjg@chromium.org> Reviewed-by: Simon Glass <sjg@chromium.org> [trini: Add <linux/sizes.h>] Signed-off-by: Tom Rini <trini@konsulko.com>
2021-06-11common: fit: Update board_fit_image_post_process() to pass fit and node_offsetLokesh Vutla1-1/+1
board_fit_image_post_process() passes only start and size of the image, but type of the image is not passed. So pass fit and node_offset, to derive information about image to be processed. Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com> Reviewed-by: Tom Rini <trini@konsulko.com> Signed-off-by: Tero Kristo <kristo@kernel.org>
2021-04-14image-fit: Accept OP-TEE images when booting a FITAlexandru Gagniuc1-0/+2
OP-TEE images are normally packaged with type = "tee; os = "tee"; However, fit_image_load() thinks that is somehow invalid. However if they were declared as type = "kernel", os = "linux", fit_image_load() would happily accept them and allow the boot to continue. There is no technical limitation to excluding "tee". Allowing "tee" images is useful in a boot flow where OP-TEE is executed before linux. In fact, I think it's unintuitive for a "load"ing function to also do parsing and contain a bunch ad-hoc heuristics that only its caller might know. But I don't make the rules, I just write fixes. In more polite terms: refactoring the fit_image API is beyond the scope of this change. Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com> Reviewed-by: Simon Glass <sjg@chromium.org>
2021-04-14image-fit: Accept IH_TYPE_FIRMWARE in fit_image_load() as validAlexandru Gagniuc1-0/+2
Consider the following FIT: images { whipple {}; }; configurations { conf-1 { firmware = "whipple"; }; }; Getting the 'firmware' image with fit_image_load() is not possible, as it doesn't understand 'firmware =' properties. Although one could pass IH_TYPE_FIRMWARE for 'image_type', this needs to be converted to a "firmware" string for FDT lookup -- exactly what this change does. Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com> Reviewed-by: Simon Glass <sjg@chromium.org>
2021-03-30Merge tag 'v2021.04-rc5' into nextTom Rini1-1/+1
Prepare v2021.04-rc5
2021-03-27sandbox: image: Allow sandbox to load any imageSimon Glass1-0/+4
Sandbox is special in that it is used for testing and it does not match any particular target architecture. Allow it to load an image from any architecture, so that 'bootm' can be used as needed. Signed-off-by: Simon Glass <sjg@chromium.org>
2021-03-17image: Avoid -ENODATA in host toolsSimon Glass1-1/+1
Unfortunately -ENODATA is not available in OpenBSD. Use -EBADMSG instead, to indicate a missing timestamp. Fixes: c5819701a3d image: Adjust the workings of fit_check_format() Signed-off-by: Simon Glass <sjg@chromium.org> Reviewed-by: Mark Kettenis <kettenis@openbsd.org>
2021-02-16image: Check for unit addresses in FITsSimon Glass1-4/+52
Using unit addresses in a FIT is a security risk. Add a check for this and disallow it. CVE-2021-27138 Signed-off-by: Simon Glass <sjg@chromium.org> Reported-by: Bruce Monroe <bruce.monroe@intel.com> Reported-by: Arie Haenel <arie.haenel@intel.com> Reported-by: Julien Lenoir <julien.lenoir@intel.com>
2021-02-16image: Add an option to do a full check of the FITSimon Glass1-0/+16
Some strange modifications of the FIT can introduce security risks. Add an option to check it thoroughly, using libfdt's fdt_check_full() function. Enable this by default if signature verification is enabled. CVE-2021-27097 Signed-off-by: Simon Glass <sjg@chromium.org> Reported-by: Bruce Monroe <bruce.monroe@intel.com> Reported-by: Arie Haenel <arie.haenel@intel.com> Reported-by: Julien Lenoir <julien.lenoir@intel.com>
2021-02-16image: Adjust the workings of fit_check_format()Simon Glass1-26/+20
At present this function does not accept a size for the FIT. This means that it must be read from the FIT itself, introducing potential security risk. Update the function to include a size parameter, which can be invalid, in which case fit_check_format() calculates it. For now no callers pass the size, but this can be updated later. Also adjust the return value to an error code so that all the different types of problems can be distinguished by the user. Signed-off-by: Simon Glass <sjg@chromium.org> Reported-by: Bruce Monroe <bruce.monroe@intel.com> Reported-by: Arie Haenel <arie.haenel@intel.com> Reported-by: Julien Lenoir <julien.lenoir@intel.com>
2021-02-16fit: Don't allow verification of images with @ nodesSimon Glass1-5/+15
When searching for a node called 'fred', any unit address appended to the name is ignored by libfdt, meaning that 'fred' can match 'fred@1'. This means that we cannot be sure that the node originally intended is the one that is used. Disallow use of nodes with unit addresses. Update the forge test also, since it uses @ addresses. CVE-2021-27138 Signed-off-by: Simon Glass <sjg@chromium.org> Reported-by: Bruce Monroe <bruce.monroe@intel.com> Reported-by: Arie Haenel <arie.haenel@intel.com> Reported-by: Julien Lenoir <julien.lenoir@intel.com>
2021-02-02common: Drop asm/global_data.h from common headerSimon Glass1-0/+1
Move this out of the common header and include it only where needed. In a number of cases this requires adding "struct udevice;" to avoid adding another large header or in other cases replacing / adding missing header files that had been pulled in, very indirectly. Finally, we have a few cases where we did not need to include <asm/global_data.h> at all, so remove that include. Signed-off-by: Simon Glass <sjg@chromium.org> Signed-off-by: Tom Rini <trini@konsulko.com>
2021-02-02common: Kconfig.boot: Add FIT_PRINT config optionRavik Hasija1-2/+2
Config allows to disable printing contents of fitImage to optimize boottime. Signed-off-by: Ravik Hasija <rahasij@linux.microsoft.com> Reviewed-by: Simon Glass <sjg@chromium.org>
2021-01-23image-fit: Fix FIT_CIPHER linkingJoel Stanley1-0/+15
When CONFIG_FIT_CIPHER=y and CONFIG_FIT_SIGNATURE=n is there is no implementation of image_get_host_blob for mkimage/dumpimage: /usr/bin/ld: tools/common/image-cipher.o: in function `fit_image_decrypt_data': image-cipher.c:(.text+0x9a): undefined reference to `image_get_host_blob' Move the implementation to a common file so it can be shaed between image-cipher.c and image-fit-sig.c. Signed-off-by: Joel Stanley <joel@jms.id.au>
2021-01-13image-fit: fit_check_format check for valid FDTHeinrich Schuchardt1-0/+6
fit_check_format() must check that the buffer contains a flattened device tree before calling any device tree library functions. Failure to do may cause segmentation faults. Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
2021-01-05image: support board_fit_config_name_matchSebastian Reichel1-6/+13
Support reusing board_fit_config_name_match() to automatically select a sensible default configuration for booting fitImages using 'bootm'. Reviewed-by: Simon Glass <sjg@chromium.org> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
2021-01-05image: cleanup pre-processor usageSebastian Reichel1-26/+20
Replace most #ifdef checks for USE_HOSTCC and CONFIG_* with normal if instructions. Reviewed-by: Simon Glass <sjg@chromium.org> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
2020-10-27spl: fdt: Record load/entry fit-images entries in 64bit formatMichal Simek1-5/+6
The commit 9f45aeb93727 ("spl: fit: implement fdt_record_loadable") which introduced fdt_record_loadable() state there spl_fit.c is not 64bit safe. Based on my tests on Xilinx ZynqMP zcu102 platform there shouldn't be a problem to record these addresses in 64bit format. The patch adds support for systems which need to load images above 4GB. Signed-off-by: Michal Simek <michal.simek@xilinx.com> Reviewed-by: Simon Glass <sjg@chromium.org>
2020-06-12Add support for SHA384 and SHA512Reuben Dowle1-0/+9
The current recommendation for best security practice from the US government is to use SHA384 for TOP SECRET [1]. This patch adds support for SHA384 and SHA512 in the hash command, and also allows FIT images to be hashed with these algorithms, and signed with sha384,rsaXXXX and sha512,rsaXXXX The SHA implementation is adapted from the linux kernel implementation. [1] Commercial National Security Algorithm Suite http://www.iad.gov/iad/programs/iad-initiatives/cnsa-suite.cfm Signed-off-by: Reuben Dowle <reuben.dowle@4rf.com>
2020-05-19common: Drop log.h from common headerSimon Glass1-0/+1
Move this header out of the common header. Signed-off-by: Simon Glass <sjg@chromium.org>
2020-05-19common: Drop image.h from common headerSimon Glass1-0/+1
Move this uncommon header out of the common header. Signed-off-by: Simon Glass <sjg@chromium.org>
2020-04-08Merge tag 'xilinx-for-v2020.07' of ↵Tom Rini1-23/+6
https://gitlab.denx.de/u-boot/custodians/u-boot-microblaze into next Xilinx changes for v2020.07 common: - Align ENV_FAT_INTERFACE - Fix MAC address source print log - Improve based autodetection code xilinx: - Enable netconsole Microblaze: - Setup default ENV_OFFSET/ENV_SECT_SIZE Zynq: - Multiple DT updates/fixes - Use DEVICE_TREE environment variable for DTB selection - Switch to single zynq configuration - Enable NOR flash via DM - Minor SPL print removal - Enable i2c mux driver ZynqMP: - Print multiboot register - Enable cache commands in mini mtest - Multiple DT updates/fixes - Fix firmware probing when driver is not enabled - Specify 3rd backup RAM boot mode in SPL - Add SPL support for zcu102 v1.1 and zcu111 revA - Redesign debug uart enabling and psu_init delay - Enable full u-boot run from EL3 - Enable u-boot.itb generation without ATF with U-Boot in EL3 Versal: - Enable distro default - Enable others SPI flashes - Enable systems without DDR Drivers: - Gem: - Flush memory after freeing - Handle mdio bus separately - Watchdog: - Get rid of unused global data pointer - Enable window watchdog timer - Serial: - Change reinitialization logic in zynq serial driver Signed-off-by: Tom Rini <trini@konsulko.com>
2020-04-01image: Use constants for 'required' and 'key-name-hint'Simon Glass1-3/+3
These are used in multiple places so update them to use a shared #define. Signed-off-by: Simon Glass <sjg@chromium.org> Reviewed-by: Philippe Reynes <philippe.reynes@softathome.com>
2020-04-01image: Be a little more verbose when checking signaturesSimon Glass1-1/+1
It is useful to be a little more specific about what is being checked. Update a few messages to help with this. Signed-off-by: Simon Glass <sjg@chromium.org>
2020-04-01image: Correct comment for fit_conf_get_node()Simon Glass1-18/+0
This should mention that conf_uname can be NULL and should be in the header file. Fix this. Signed-off-by: Simon Glass <sjg@chromium.org>
2020-04-01Merge branch 'next' of git://git.denx.de/u-boot-usb into nextTom Rini1-2/+4
2020-03-31image-fit: Allow loading FIT image for VxWorksLihua Zhao1-1/+2
This adds the check against IH_OS_VXWORKS during FIT image load, to allow loading FIT image for VxWorks. Signed-off-by: Lihua Zhao <lihua.zhao@windriver.com> Signed-off-by: Bin Meng <bmeng.cn@gmail.com> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
2020-03-13fit: check return value of fit_image_get_data_size()Heinrich Schuchardt1-2/+4
GCC-10 reports: In file included from tools/common/image-fit.c:1: include/image.h: In function ‘fit_image_get_data_and_size’: ./tools/../common/image-fit.c:1015:9: warning: ‘len’ may be used uninitialized in this function [-Wmaybe-uninitialized] 1015 | *size = len; | ~~~~~~^~~~~ ./tools/../common/image-fit.c:996:6: note: ‘len’ was declared here 996 | int len; | ^~~ Add the missing check of the return value of fit_image_get_data_size(). Fixes: c3c863880479 ("add FIT data-position & data-offset property support") Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> Reviewed-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
2020-03-12lib: rsa: decouple rsa from FIT image verificationAKASHI Takahiro1-3/+3
Introduce new configuration, CONFIG_RSA_VERIFY which will decouple building RSA functions from FIT verification and allow for adding a RSA-based signature verification for other file formats, in particular PE file for UEFI secure boot. Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> Reviewed-by: Simon Glass <sjg@chromium.org>
2020-01-17u-boot: fit: add support to decrypt fit with aesPhilippe Reynes1-0/+63
This commit add to u-boot the support to decrypt fit image encrypted with aes. The FIT image contains the key name and the IV name. Then u-boot look for the key and IV in his device tree and decrypt images before moving to the next stage. Signed-off-by: Philippe Reynes <philippe.reynes@softathome.com>
2020-01-17mkimage: fit: add support to encrypt image with aesPhilippe Reynes1-0/+27
This commit add the support of encrypting image with aes in mkimage. To enable the ciphering, a node cipher with a reference to a key and IV (Initialization Vector) must be added to the its file. Then mkimage add the encrypted image to the FIT and add the key and IV to the u-boot device tree. Signed-off-by: Philippe Reynes <philippe.reynes@softathome.com>
2020-01-07image: Add IH_OS_EFI for EFI chain-load bootCristian Ciocaltea1-1/+2
Add a new OS type to be used for chain-loading an EFI compatible firmware or boot loader like GRUB2, possibly in a verified boot scenario. Bellow is sample ITS file that generates a FIT image supporting secure boot. Please note the presence of 'os = "efi";' line, which identifies the currently introduced OS type: / { #address-cells = <1>; images { efi-grub { description = "GRUB EFI"; data = /incbin/("bootarm.efi"); type = "kernel_noload"; arch = "arm"; os = "efi"; compression = "none"; load = <0x0>; entry = <0x0>; hash-1 { algo = "sha256"; }; }; }; configurations { default = "config-grub"; config-grub { kernel = "efi-grub"; signature-1 { algo = "sha256,rsa2048"; sign-images = "kernel"; }; }; }; }; Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@gmail.com> Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
2019-12-03crc32: Use the crc.h header for crc functionsSimon Glass1-0/+1
Drop inclusion of crc.h in common.h and use the correct header directly instead. With this we can drop the conflicting definition in fw_env.h and rely on the crc.h header, which is already included. Signed-off-by: Simon Glass <sjg@chromium.org> Reviewed-by: Tom Rini <trini@konsulko.com>
2019-08-07fit: Do not automatically decompress ramdisk imagesJulius Werner1-4/+9
The Linux ramdisk should always be decompressed by the kernel itself, not by U-Boot. Therefore, the 'compression' node in the FIT image should always be set to "none" for ramdisk images, since the only point of using that node is if you want U-Boot to do the decompression itself. Yet some systems populate the node to the compression algorithm used by the kernel instead. This used to be ignored, but now that we support decompression of all image types it becomes a problem. Since ramdisks should never be decompressed by U-Boot anyway, this patch adds a special exception for them to avoid these issues. Still, setting the 'compression' node like that is wrong in the first place, so we still want to print out a warning so that third-party distributions doing this can notice and fix it. Signed-off-by: Julius Werner <jwerner@chromium.org> Reviewed-by: Heiko Schocher <hs@denx.de> Tested-by: Heiko Schocher <hs@denx.de> Reviewed-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
2019-07-29fit: Support compat string property in configuration nodeJulius Werner1-28/+39
This patch adds support for an optional optimization to compatible string matching where the compatible string property from the root node of the kernel FDT can be copied into the configuration node of the FIT image. This is most useful when using compressed FDTs or when using FDT overlays, where the traditional extraction of the compatible string from the kernel FDT itself is not easily possible. Signed-off-by: Julius Werner <jwerner@chromium.org> Reviewed-by: Simon Glass <sjg@chromium.org>
2019-07-29fit: Support compression for non-kernel components (e.g. FDT)Julius Werner1-34/+52
This patch adds support for compressing non-kernel image nodes in a FIT image (kernel nodes could already be compressed previously). This can reduce the size of FIT images and therefore improve boot times (especially when an image bundles many different kernel FDTs). The images will automatically be decompressed on load. This patch does not support extracting compatible strings from compressed FDTs, so it's not very helpful in conjunction with CONFIG_FIT_BEST_MATCH yet, but it can already be used in environments that select the configuration to load explicitly. Signed-off-by: Julius Werner <jwerner@chromium.org> Reviewed-by: Simon Glass <sjg@chromium.org> Reviewed-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
2019-04-23fit: load all fragments from the extra configurationsPeter Ujfalusi1-0/+12
Currently only the first fdt is loaded from the extra configuration of FIT image. If the configuration have multiple fdt, load them all as well. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com> Reviewed-by: Lokesh Vutla <lokeshvutla@ti.com>
2018-12-03rsa: add a structure for the paddingPhilippe Reynes1-0/+5
The rsa signature use a padding algorithm. By default, we use the padding pkcs-1.5. In order to add some new padding algorithm, we add a padding framework to manage several padding algorithm. The choice of the padding is done in the file .its. Signed-off-by: Philippe Reynes <philippe.reynes@softathome.com> Reviewed-by: Simon Glass <sjg@chromium.org>
2018-07-10fit: Verify all configuration signaturesMarek Vasut1-12/+14
Rather than verifying configuration signature of the configuration node containing the kernel image types, verify all configuration nodes, even those that do not contain kernel images. This is useful when the nodes contain ie. standalone OSes or U-Boot. Signed-off-by: Marek Vasut <marex@denx.de> Cc: Tom Rini <trini@konsulko.com> Cc: Pantelis Antoniou <pantelis.antoniou@konsulko.com> Cc: Simon Glass <sjg@chromium.org> Reviewed-by: Simon Glass <sjg@chromium.org>
2018-05-27add FIT data-position & data-offset property supportKelvin Cheung1-3/+51
Add FIT data-position & data-offset property support for bootm, which were already supported in SPL. Signed-off-by: Kelvin Cheung <keguang.zhang@gmail.com>
2018-05-24fit: Add standalone image type handlingMarek Vasut1-0/+2
Just add IH_TYPE_STANDALONE to fit_get_image_type_property(). Signed-off-by: Marek Vasut <marex@denx.de> Cc: Pantelis Antoniou <pantelis.antoniou@konsulko.com> Cc: Simon Glass <sjg@chromium.org> Reviewed-by: Simon Glass <sjg@chromium.org>
2018-05-24fit: Add empty fit_print_contents() and fit_image_print()Marek Vasut1-1/+3
These functions may be needed in SPL, so add empty variants of them if CONFIG_SPL_FIT_PRINT is disabled. Signed-off-by: Marek Vasut <marex@denx.de> Cc: Pantelis Antoniou <pantelis.antoniou@konsulko.com> Cc: Simon Glass <sjg@chromium.org> Reviewed-by: Simon Glass <sjg@chromium.org>