diff options
author | Tom Rini <trini@konsulko.com> | 2019-11-01 09:23:21 -0400 |
---|---|---|
committer | Tom Rini <trini@konsulko.com> | 2019-11-01 09:23:21 -0400 |
commit | 82679624f9aa6d1be733c46f3555d5166b6f5b72 (patch) | |
tree | 8a99cf79bc520b833e155094ef134c0526b1f005 | |
parent | 412326d1bc2d346d7b4faad6fa547eaf065681a2 (diff) | |
parent | 5d80a1a93d42c8325d65516cc654ff6a9ceec58a (diff) |
Merge branch '2019-10-30-master-imports'
- Migrate test.py to use python3 and current pytest.
- NVMe bugfixes
- Assorted other fixes
- Android AVB updates.
55 files changed, 1499 insertions, 625 deletions
diff --git a/.azure-pipelines.yml b/.azure-pipelines.yml index d476d8d0e9..11d5a6175b 100644 --- a/.azure-pipelines.yml +++ b/.azure-pipelines.yml @@ -1,7 +1,7 @@ variables: windows_vm: vs2015-win2012r2 ubuntu_vm: ubuntu-18.04 - ci_runner_image: trini/u-boot-gitlab-ci-runner:bionic-20190912.1-03Oct2019 + ci_runner_image: trini/u-boot-gitlab-ci-runner:bionic-20191010-20Oct2019 # Add '-u 0' options for Azure pipelines, otherwise we get "permission # denied" error when it tries to "useradd -m -u 1001 vsts_azpcontainer", # since our $(ci_runner_image) user is not root. @@ -245,11 +245,6 @@ jobs: git clone --depth=1 git://github.com/swarren/uboot-test-hooks.git /tmp/uboot-test-hooks ln -s travis-ci /tmp/uboot-test-hooks/bin/`hostname` ln -s travis-ci /tmp/uboot-test-hooks/py/`hostname` - virtualenv /tmp/venv - . /tmp/venv/bin/activate - pip install pytest==2.8.7 - pip install python-subunit - pip install coverage grub-mkimage --prefix=\"\" -o ~/grub_x86.efi -O i386-efi normal echo lsefimmap lsefi lsefisystab efinet tftp minicmd grub-mkimage --prefix=\"\" -o ~/grub_x64.efi -O x86_64-efi normal echo lsefimmap lsefi lsefisystab efinet tftp minicmd mkdir ~/grub2-arm @@ -266,6 +261,9 @@ jobs: exit $ret; fi; fi + virtualenv -p /usr/bin/python3 /tmp/venv + . /tmp/venv/bin/activate + pip install -r test/py/requirements.txt export UBOOT_TRAVIS_BUILD_DIR=/tmp/.bm-work/${TEST_PY_BD}; export PATH=/opt/qemu/bin:/tmp/uboot-test-hooks/bin:/usr/bin:/bin; export PYTHONPATH=/tmp/uboot-test-hooks/py/travis-ci; diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 967abed9f2..9b295ac710 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -2,7 +2,7 @@ # Grab our configured image. The source for this is found at: # https://gitlab.denx.de/u-boot/gitlab-ci-runner -image: trini/u-boot-gitlab-ci-runner:bionic-20190912.1-03Oct2019 +image: trini/u-boot-gitlab-ci-runner:bionic-20191010-20Oct2019 # We run some tests in different order, to catch some failures quicker. stages: @@ -18,11 +18,6 @@ stages: - git clone --depth=1 git://github.com/swarren/uboot-test-hooks.git /tmp/uboot-test-hooks - ln -s travis-ci /tmp/uboot-test-hooks/bin/`hostname` - ln -s travis-ci /tmp/uboot-test-hooks/py/`hostname` - - virtualenv /tmp/venv - - . /tmp/venv/bin/activate - - pip install pytest==2.8.7 - - pip install python-subunit - - pip install coverage - grub-mkimage --prefix="" -o ~/grub_x86.efi -O i386-efi normal echo lsefimmap lsefi lsefisystab efinet tftp minicmd - grub-mkimage --prefix="" -o ~/grub_x64.efi -O x86_64-efi normal echo lsefimmap lsefi lsefisystab efinet tftp minicmd - mkdir ~/grub2-arm @@ -47,8 +42,11 @@ stages: # never prevent any test from running. That way, we can always pass # "-k something" even when $TEST_PY_TEST_SPEC doesnt need a custom # value. + - virtualenv -p /usr/bin/python3 /tmp/venv + - . /tmp/venv/bin/activate + - pip install -r test/py/requirements.txt - export UBOOT_TRAVIS_BUILD_DIR=/tmp/.bm-work/${TEST_PY_BD}; - export PATH=/opt/qemu/bin:/tmp/uboot-test-hooks/bin:/usr/bin:/bin; + export PATH=/opt/qemu/bin:/tmp/uboot-test-hooks/bin:${PATH}; export PYTHONPATH=/tmp/uboot-test-hooks/py/travis-ci; if [[ "${TEST_PY_BD}" != "" ]]; then ./test/py/test.py --bd ${TEST_PY_BD} ${TEST_PY_ID} @@ -65,11 +63,11 @@ build all 32bit ARM platforms: stage: world build script: - ret=0; - ./tools/buildman/buildman -o /tmp -P -E arm -x aarch64 || ret=$?; - if [[ $ret -ne 0 && $ret -ne 129 ]]; then - ./tools/buildman/buildman -o /tmp -sdeP; - exit $ret; - fi; + ./tools/buildman/buildman -o /tmp -P -E arm -x aarch64 || ret=$?; + if [[ $ret -ne 0 && $ret -ne 129 ]]; then + ./tools/buildman/buildman -o /tmp -sdeP; + exit $ret; + fi; build all 64bit ARM platforms: tags: [ 'all' ] @@ -79,33 +77,33 @@ build all 64bit ARM platforms: - . /tmp/venv/bin/activate - pip install pyelftools - ret=0; - ./tools/buildman/buildman -o /tmp -P -E aarch64 || ret=$?; - if [[ $ret -ne 0 && $ret -ne 129 ]]; then - ./tools/buildman/buildman -o /tmp -sdeP; - exit $ret; - fi; + ./tools/buildman/buildman -o /tmp -P -E aarch64 || ret=$?; + if [[ $ret -ne 0 && $ret -ne 129 ]]; then + ./tools/buildman/buildman -o /tmp -sdeP; + exit $ret; + fi; build all PowerPC platforms: tags: [ 'all' ] stage: world build script: - ret=0; - ./tools/buildman/buildman -o /tmp -P -E powerpc || ret=$?; - if [[ $ret -ne 0 && $ret -ne 129 ]]; then - ./tools/buildman/buildman -o /tmp -sdeP; - exit $ret; - fi; + ./tools/buildman/buildman -o /tmp -P -E powerpc || ret=$?; + if [[ $ret -ne 0 && $ret -ne 129 ]]; then + ./tools/buildman/buildman -o /tmp -sdeP; + exit $ret; + fi; build all other platforms: tags: [ 'all' ] stage: world build script: - ret=0; - ./tools/buildman/buildman -o /tmp -P -E -x arm,powerpc || ret=$?; - if [[ $ret -ne 0 && $ret -ne 129 ]]; then - ./tools/buildman/buildman -o /tmp -sdeP; - exit $ret; - fi; + ./tools/buildman/buildman -o /tmp -P -E -x arm,powerpc || ret=$?; + if [[ $ret -ne 0 && $ret -ne 129 ]]; then + ./tools/buildman/buildman -o /tmp -sdeP; + exit $ret; + fi; # QA jobs for code analytics # static code analysis with cppcheck (we can add --enable=all later) diff --git a/.travis.yml b/.travis.yml index 3aaed93346..2369da97f9 100644 --- a/.travis.yml +++ b/.travis.yml @@ -21,7 +21,9 @@ addons: - build-essential - libsdl1.2-dev - python - - python-virtualenv + - python-pyelftools + - python3-virtualenv + - python3-pip - swig - libpython-dev - iasl @@ -47,11 +49,6 @@ install: - echo -e "arc = /tmp/arc_gnu_2018.09_prebuilt_uclibc_le_archs_linux_install" >> ~/.buildman - echo -e "\n[toolchain-alias]\nsh = sh2\n" >> ~/.buildman - cat ~/.buildman - - virtualenv /tmp/venv - - . /tmp/venv/bin/activate - - pip install pytest==2.8.7 - - pip install python-subunit - - pip install pyelftools - grub-mkimage --prefix="" -o ~/grub_x86.efi -O i386-efi normal echo lsefimmap lsefi lsefisystab efinet tftp minicmd - grub-mkimage --prefix="" -o ~/grub_x64.efi -O x86_64-efi normal echo lsefimmap lsefi lsefisystab efinet tftp minicmd - mkdir ~/grub2-arm @@ -136,15 +133,6 @@ script: cp ~/grub_x64.efi $UBOOT_TRAVIS_BUILD_DIR/; cp ~/grub2-arm/usr/lib/grub2/arm-efi/grub.efi $UBOOT_TRAVIS_BUILD_DIR/grub_arm.efi; cp ~/grub2-arm64/usr/lib/grub2/arm64-efi/grub.efi $UBOOT_TRAVIS_BUILD_DIR/grub_arm64.efi; - if [[ "${TEST_PY_BD}" != "" ]]; then - ./test/py/test.py --bd ${TEST_PY_BD} ${TEST_PY_ID} - -k "${TEST_PY_TEST_SPEC:-not a_test_which_does_not_exist}" - --build-dir "$UBOOT_TRAVIS_BUILD_DIR"; - ret=$?; - if [[ $ret -ne 0 ]]; then - exit $ret; - fi; - fi; if [[ -n "${TEST_PY_TOOLS}" ]]; then PYTHONPATH="${UBOOT_TRAVIS_BUILD_DIR}/scripts/dtc/pylibfdt" PATH="${UBOOT_TRAVIS_BUILD_DIR}/scripts/dtc:${PATH}" @@ -154,6 +142,18 @@ script: PYTHONPATH="${UBOOT_TRAVIS_BUILD_DIR}/scripts/dtc/pylibfdt" PATH="${UBOOT_TRAVIS_BUILD_DIR}/scripts/dtc:${PATH}" ./tools/dtoc/dtoc -t; + fi; + if [[ "${TEST_PY_BD}" != "" ]]; then + virtualenv -p /usr/bin/python3 /tmp/venv; + . /tmp/venv/bin/activate; + pip install -r test/py/requirements.txt; + ./test/py/test.py --bd ${TEST_PY_BD} ${TEST_PY_ID} + -k "${TEST_PY_TEST_SPEC:-not a_test_which_does_not_exist}" + --build-dir "$UBOOT_TRAVIS_BUILD_DIR"; + ret=$?; + if [[ $ret -ne 0 ]]; then + exit $ret; + fi; fi matrix: @@ -346,7 +346,7 @@ define size_check limit=$$( printf "%d" $2 ); \ if test $$actual -gt $$limit; then \ echo "$1 exceeds file size limit:" >&2; \ - echo " limit: $$(printf %#x bytes $$limit) bytes" >&2; \ + echo " limit: $$(printf %#x $$limit) bytes" >&2; \ echo " actual: $$(printf %#x $$actual) bytes" >&2; \ echo " excess: $$(printf %#x $$((actual - limit))) bytes" >&2;\ exit 1; \ diff --git a/arch/arm/mach-bcm283x/include/mach/timer.h b/arch/arm/mach-bcm283x/include/mach/timer.h index 014355e759..61beb1aba1 100644 --- a/arch/arm/mach-bcm283x/include/mach/timer.h +++ b/arch/arm/mach-bcm283x/include/mach/timer.h @@ -25,9 +25,6 @@ struct bcm2835_timer_regs { u32 c2; u32 c3; }; - -extern ulong get_timer_us(ulong base); - #endif #endif @@ -15,11 +15,6 @@ #define AVB_BOOTARGS "avb_bootargs" static struct AvbOps *avb_ops; -static const char * const requested_partitions[] = {"boot", - "system", - "vendor", - NULL}; - int do_avb_init(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) { unsigned long mmc_dev; @@ -232,10 +227,12 @@ int do_avb_get_uuid(cmd_tbl_t *cmdtp, int flag, int do_avb_verify_part(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[]) { + const char * const requested_partitions[] = {"boot", NULL}; AvbSlotVerifyResult slot_result; AvbSlotVerifyData *out_data; char *cmdline; char *extra_args; + char *slot_suffix = ""; bool unlocked = false; int res = CMD_RET_FAILURE; @@ -245,9 +242,12 @@ int do_avb_verify_part(cmd_tbl_t *cmdtp, int flag, return CMD_RET_FAILURE; } - if (argc != 1) + if (argc < 1 || argc > 2) return CMD_RET_USAGE; + if (argc == 2) + slot_suffix = argv[1]; + printf("## Android Verified Boot 2.0 version %s\n", avb_version_string()); @@ -260,7 +260,7 @@ int do_avb_verify_part(cmd_tbl_t *cmdtp, int flag, slot_result = avb_slot_verify(avb_ops, requested_partitions, - "", + slot_suffix, unlocked, AVB_HASHTREE_ERROR_MODE_RESTART_AND_INVALIDATE, &out_data); @@ -420,7 +420,7 @@ static cmd_tbl_t cmd_avb[] = { U_BOOT_CMD_MKENT(read_part, 5, 0, do_avb_read_part, "", ""), U_BOOT_CMD_MKENT(read_part_hex, 4, 0, do_avb_read_part_hex, "", ""), U_BOOT_CMD_MKENT(write_part, 5, 0, do_avb_write_part, "", ""), - U_BOOT_CMD_MKENT(verify, 1, 0, do_avb_verify_part, "", ""), + U_BOOT_CMD_MKENT(verify, 2, 0, do_avb_verify_part, "", ""), #ifdef CONFIG_OPTEE_TA_AVB U_BOOT_CMD_MKENT(read_pvalue, 3, 0, do_avb_read_pvalue, "", ""), U_BOOT_CMD_MKENT(write_pvalue, 3, 0, do_avb_write_pvalue, "", ""), @@ -463,6 +463,7 @@ U_BOOT_CMD( "avb read_pvalue <name> <bytes> - read a persistent value <name>\n" "avb write_pvalue <name> <value> - write a persistent value <name>\n" #endif - "avb verify - run verification process using hash data\n" + "avb verify [slot_suffix] - run verification process using hash data\n" " from vbmeta structure\n" + " [slot_suffix] - _a, _b, etc (if vbmeta partition is slotted)\n" ); diff --git a/common/Kconfig b/common/Kconfig index 28d5e9a0cc..d9ecf79e0a 100644 --- a/common/Kconfig +++ b/common/Kconfig @@ -764,7 +764,7 @@ config SPL_LOG_CONSOLE line number are omitted. config TPL_LOG_CONSOLE - bool "Allow log output to the console in SPL" + bool "Allow log output to the console in TPL" depends on TPL_LOG default y help diff --git a/disk/part_dos.c b/disk/part_dos.c index 8ddc13b50c..83ff40d310 100644 --- a/disk/part_dos.c +++ b/disk/part_dos.c @@ -67,28 +67,39 @@ static int test_block_type(unsigned char *buffer) { int slot; struct dos_partition *p; + int part_count = 0; if((buffer[DOS_PART_MAGIC_OFFSET + 0] != 0x55) || (buffer[DOS_PART_MAGIC_OFFSET + 1] != 0xaa) ) { return (-1); } /* no DOS Signature at all */ p = (struct dos_partition *)&buffer[DOS_PART_TBL_OFFSET]; - for (slot = 0; slot < 3; slot++) { - if (p->boot_ind != 0 && p->boot_ind != 0x80) { - if (!slot && - (strncmp((char *)&buffer[DOS_PBR_FSTYPE_OFFSET], - "FAT", 3) == 0 || - strncmp((char *)&buffer[DOS_PBR32_FSTYPE_OFFSET], - "FAT32", 5) == 0)) { - return DOS_PBR; /* is PBR */ - } else { - return -1; - } - } + + /* Check that the boot indicators are valid and count the partitions. */ + for (slot = 0; slot < 4; ++slot, ++p) { + if (p->boot_ind != 0 && p->boot_ind != 0x80) + break; + if (p->sys_ind) + ++part_count; } - return DOS_MBR; /* Is MBR */ -} + /* + * If the partition table is invalid or empty, + * check if this is a DOS PBR + */ + if (slot != 4 || !part_count) { + if (!strncmp((char *)&buffer[DOS_PBR_FSTYPE_OFFSET], + "FAT", 3) || + !strncmp((char *)&buffer[DOS_PBR32_FSTYPE_OFFSET], + "FAT32", 5)) + return DOS_PBR; /* This is a DOS PBR and not an MBR */ + } + if (slot == 4) + return DOS_MBR; /* This is an DOS MBR */ + + /* This is neither a DOS MBR nor a DOS PBR */ + return -1; +} static int part_test_dos(struct blk_desc *dev_desc) { diff --git a/doc/android/avb2.txt b/doc/android/avb2.txt index a29cee1b6f..48e9297c75 100644 --- a/doc/android/avb2.txt +++ b/doc/android/avb2.txt @@ -95,6 +95,10 @@ e.g.: mmc read ${loadaddr} ${boot_start} ${boot_size}; \ bootm $loadaddr $loadaddr $fdtaddr; \ +If partitions you want to verify are slotted (have A/B suffixes), then current +slot suffix should be passed to 'avb verify' sub-command, e.g.: + +=> avb verify _a To switch on automatic generation of vbmeta partition in AOSP build, add these lines to device configuration mk file: diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c index 21a89eba5a..d10f9f0bf8 100644 --- a/drivers/ata/ahci.c +++ b/drivers/ata/ahci.c @@ -50,6 +50,8 @@ struct ahci_uc_priv *probe_ent = NULL; #define WAIT_MS_FLUSH 5000 #define WAIT_MS_LINKUP 200 +#define AHCI_CAP_S64A BIT(31) + __weak void __iomem *ahci_port_base(void __iomem *base, u32 port) { return base + 0x100 + (port * 0x80); @@ -503,9 +505,15 @@ static int ahci_fill_sg(struct ahci_uc_priv *uc_priv, u8 port, } for (i = 0; i < sg_count; i++) { - ahci_sg->addr = - cpu_to_le32((unsigned long) buf + i * MAX_DATA_BYTE_COUNT); - ahci_sg->addr_hi = 0; + /* We assume virt=phys */ + phys_addr_t pa = (unsigned long)buf + i * MAX_DATA_BYTE_COUNT; + + ahci_sg->addr = cpu_to_le32(lower_32_bits(pa)); + ahci_sg->addr_hi = cpu_to_le32(upper_32_bits(pa)); + if (ahci_sg->addr_hi && !(uc_priv->cap & AHCI_CAP_S64A)) { + printf("Error: DMA address too high\n"); + return -1; + } ahci_sg->flags_size = cpu_to_le32(0x3fffff & (buf_len < MAX_DATA_BYTE_COUNT ? (buf_len - 1) diff --git a/drivers/gpio/da8xx_gpio.c b/drivers/gpio/da8xx_gpio.c index bd79448164..0a50c68d72 100644 --- a/drivers/gpio/da8xx_gpio.c +++ b/drivers/gpio/da8xx_gpio.c @@ -342,13 +342,6 @@ int gpio_free(unsigned int gpio) } #endif -static int _gpio_direction_output(struct davinci_gpio *bank, unsigned int gpio, int value) -{ - clrbits_le32(&bank->dir, 1U << GPIO_BIT(gpio)); - gpio_set_value(gpio, value); - return 0; -} - static int _gpio_direction_input(struct davinci_gpio *bank, unsigned int gpio) { setbits_le32(&bank->dir, 1U << GPIO_BIT(gpio)); @@ -377,6 +370,13 @@ static int _gpio_get_dir(struct davinci_gpio *bank, unsigned int gpio) return in_le32(&bank->dir) & (1U << GPIO_BIT(gpio)); } +static int _gpio_direction_output(struct davinci_gpio *bank, unsigned int gpio, + int value) +{ + clrbits_le32(&bank->dir, 1U << GPIO_BIT(gpio)); + _gpio_set_value(bank, gpio, value); + return 0; +} #ifndef CONFIG_DM_GPIO void gpio_info(void) diff --git a/drivers/nvme/nvme.c b/drivers/nvme/nvme.c index ee6b581d9e..f915817aaa 100644 --- a/drivers/nvme/nvme.c +++ b/drivers/nvme/nvme.c @@ -123,6 +123,9 @@ static int nvme_setup_prps(struct nvme_dev *dev, u64 *prp2, } *prp2 = (ulong)dev->prp_pool; + flush_dcache_range((ulong)dev->prp_pool, (ulong)dev->prp_pool + + dev->prp_entry_num * sizeof(u64)); + return 0; } @@ -580,14 +583,19 @@ static int nvme_setup_io_queues(struct nvme_dev *dev) static int nvme_get_info_from_identify(struct nvme_dev *dev) { - ALLOC_CACHE_ALIGN_BUFFER(char, buf, sizeof(struct nvme_id_ctrl)); - struct nvme_id_ctrl *ctrl = (struct nvme_id_ctrl *)buf; + struct nvme_id_ctrl *ctrl; int ret; int shift = NVME_CAP_MPSMIN(dev->cap) + 12; + ctrl = memalign(dev->page_size, sizeof(struct nvme_id_ctrl)); + if (!ctrl) + return -ENOMEM; + ret = nvme_identify(dev, 0, 1, (dma_addr_t)(long)ctrl); - if (ret) + if (ret) { + free(ctrl); return -EIO; + } dev->nn = le32_to_cpu(ctrl->nn); dev->vwc = ctrl->vwc; @@ -618,6 +626,7 @@ static int nvme_get_info_from_identify(struct nvme_dev *dev) dev->max_transfer_shift = 20; } + free(ctrl); return 0; } @@ -658,16 +667,21 @@ static int nvme_blk_probe(struct udevice *udev) struct blk_desc *desc = dev_get_uclass_platdata(udev); struct nvme_ns *ns = dev_get_priv(udev); u8 flbas; - ALLOC_CACHE_ALIGN_BUFFER(char, buf, sizeof(struct nvme_id_ns)); - struct nvme_id_ns *id = (struct nvme_id_ns *)buf; struct pci_child_platdata *pplat; + struct nvme_id_ns *id; + + id = memalign(ndev->page_size, sizeof(struct nvme_id_ns)); + if (!id) + return -ENOMEM; memset(ns, 0, sizeof(*ns)); ns->dev = ndev; /* extract the namespace id from the block device name */ ns->ns_id = trailing_strtol(udev->name) + 1; - if (nvme_identify(ndev, ns->ns_id, 0, (dma_addr_t)(long)id)) + if (nvme_identify(ndev, ns->ns_id, 0, (dma_addr_t)(long)id)) { + free(id); return -EIO; + } memcpy(&ns->eui64, &id->eui64, sizeof(id->eui64)); flbas = id->flbas & NVME_NS_FLBAS_LBA_MASK; @@ -686,6 +700,7 @@ static int nvme_blk_probe(struct udevice *udev) memcpy(desc->product, ndev->serial, sizeof(ndev->serial)); memcpy(desc->revision, ndev->firmware_rev, sizeof(ndev->firmware_rev)); + free(id); return 0; } @@ -705,9 +720,8 @@ static ulong nvme_blk_rw(struct udevice *udev, lbaint_t blknr, u16 lbas = 1 << (dev->max_transfer_shift - ns->lba_shift); u64 total_lbas = blkcnt; - if (!read) - flush_dcache_range((unsigned long)buffer, - (unsigned long)buffer + total_len); + flush_dcache_range((unsigned long)buffer, + (unsigned long)buffer + total_len); c.rw.opcode = read ? nvme_cmd_read : nvme_cmd_write; c.rw.flags = 0; diff --git a/drivers/phy/phy-uclass.c b/drivers/phy/phy-uclass.c index a0ac30aa71..e201a90c8c 100644 --- a/drivers/phy/phy-uclass.c +++ b/drivers/phy/phy-uclass.c @@ -108,35 +108,55 @@ int generic_phy_get_by_name(struct udevice *dev, const char *phy_name, int generic_phy_init(struct phy *phy) { - struct phy_ops const *ops = phy_dev_ops(phy->dev); + struct phy_ops const *ops; + + if (!phy) + return 0; + ops = phy_dev_ops(phy->dev); return ops->init ? ops->init(phy) : 0; } int generic_phy_reset(struct phy *phy) { - struct phy_ops const *ops = phy_dev_ops(phy->dev); + struct phy_ops const *ops; + + if (!phy) + return 0; + ops = phy_dev_ops(phy->dev); return ops->reset ? ops->reset(phy) : 0; } int generic_phy_exit(struct phy *phy) { - struct phy_ops const *ops = phy_dev_ops(phy->dev); + struct phy_ops const *ops; + + if (!phy) + return 0; + ops = phy_dev_ops(phy->dev); return ops->exit ? ops->exit(phy) : 0; } int generic_phy_power_on(struct phy *phy) { - struct phy_ops const *ops = phy_dev_ops(phy->dev); + struct phy_ops const *ops; + + if (!phy) + return 0; + ops = phy_dev_ops(phy->dev); return ops->power_on ? ops->power_on(phy) : 0; } int generic_phy_power_off(struct phy *phy) { - struct phy_ops const *ops = phy_dev_ops(phy->dev); + struct phy_ops const *ops; + + if (!phy) + return 0; + ops = phy_dev_ops(phy->dev); return ops->power_off ? ops->power_off(phy) : 0; } diff --git a/drivers/virtio/virtio_pci_legacy.c b/drivers/virtio/virtio_pci_legacy.c index 08764ee6f2..202e5ab1d3 100644 --- a/drivers/virtio/virtio_pci_legacy.c +++ b/drivers/virtio/virtio_pci_legacy.c @@ -277,7 +277,7 @@ static int virtio_pci_notify(struct udevice *udev, struct virtqueue *vq) static int virtio_pci_bind(struct udevice *udev) { - static int num_devs; + static unsigned int num_devs; char name[20]; /* Create a unique device name for PCI type devices */ diff --git a/dts/Kconfig b/dts/Kconfig index c9ab66cccc..2bd959a7dc 100644 --- a/dts/Kconfig +++ b/dts/Kconfig @@ -44,7 +44,7 @@ config SPL_OF_CONTROL depends on SPL && OF_CONTROL help Some boards use device tree in U-Boot but only have 4KB of SRAM - which is not enough to support device tree. Enable this option to + which is not enough to support device tree. Disable this option to allow such boards to be supported by U-Boot SPL. config TPL_OF_CONTROL @@ -131,7 +131,7 @@ config OF_LIST separated by <space>. choice - prompt "SPL OF LIST compression" + prompt "OF LIST compression" depends on MULTI_DTB_FIT default MULTI_DTB_FIT_NO_COMPRESSION diff --git a/include/config_distro_bootcmd.h b/include/config_distro_bootcmd.h index 3570a32dff..fc0935fa21 100644 --- a/include/config_distro_bootcmd.h +++ b/include/config_distro_bootcmd.h @@ -189,6 +189,7 @@ "fi\0" \ \ "nvme_boot=" \ + BOOTENV_RUN_PCI_ENUM \ BOOTENV_RUN_NVME_INIT \ BOOTENV_SHARED_BLKDEV_BODY(nvme) #define BOOTENV_DEV_NVME BOOTENV_DEV_BLKDEV diff --git a/include/errno.h b/include/errno.h index ccb7869e17..3af539b9e9 100644 --- a/include/errno.h +++ b/include/errno.h @@ -12,12 +12,21 @@ extern int errno; #define __set_errno(val) do { errno = val; } while (0) +/** + * errno_str() - get description for error number + * + * @errno: error number (negative in case of error) + * Return: string describing the error. If CONFIG_ERRNO_STR is not + * defined an empty string is returned. + */ #ifdef CONFIG_ERRNO_STR const char *errno_str(int errno); #else +static const char error_message[] = ""; + static inline const char *errno_str(int errno) { - return 0; + return error_message; } #endif #endif /* _ERRNO_H */ diff --git a/include/generic-phy.h b/include/generic-phy.h index 947c582f68..95caf58341 100644 --- a/include/generic-phy.h +++ b/include/generic-phy.h @@ -270,7 +270,7 @@ static inline int generic_phy_get_by_name(struct udevice *user, const char *phy_ */ static inline bool generic_phy_valid(struct phy *phy) { - return phy->dev != NULL; + return phy && phy->dev; } #endif /*__GENERIC_PHY_H */ diff --git a/include/time.h b/include/time.h index 1e9b369be7..a1149522ed 100644 --- a/include/time.h +++ b/include/time.h @@ -13,6 +13,7 @@ unsigned long get_timer(unsigned long base); * Granularity may be larger than 1us if hardware does not support this. */ unsigned long timer_get_us(void); +uint64_t get_timer_us(uint64_t base); /* * timer_test_add_offset() diff --git a/lib/errno_str.c b/lib/errno_str.c index 0ba950e970..2e5f4a887d 100644 --- a/lib/errno_str.c +++ b/lib/errno_str.c @@ -13,7 +13,7 @@ static const char * const errno_message[] = { ERRNO_MSG(0, "Success"), ERRNO_MSG(EPERM, "Operation not permitted"), - ERRNO_MSG(ENOEN, "No such file or directory"), + ERRNO_MSG(ENOENT, "No such file or directory"), ERRNO_MSG(ESRCH, "No such process"), ERRNO_MSG(EINTR, "Interrupted system call"), ERRNO_MSG(EIO, "I/O error"), @@ -26,7 +26,7 @@ static const char * const errno_message[] = { ERRNO_MSG(ENOMEM, "Out of memory"), ERRNO_MSG(EACCES, "Permission denied"), ERRNO_MSG(EFAULT, "Bad address"), - ERRNO_MSG(ENOTBL, "Block device required"), + ERRNO_MSG(ENOTBLK, "Block device required"), ERRNO_MSG(EBUSY, "Device or resource busy"), ERRNO_MSG(EEXIST, "File exists"), ERRNO_MSG(EXDEV, "Cross-device link"), @@ -136,6 +136,8 @@ static const char * const errno_message[] = { ERRNO_MSG(EDQUOT, "Quota exceeded"), ERRNO_MSG(ENOMEDIUM, "No medium found"), ERRNO_MSG(EMEDIUMTYPE, "Wrong medium type"), + /* Message for unsupported error numbers */ + ERRNO_MSG(0, "Unknown error"), }; const char *errno_str(int errno) @@ -143,5 +145,9 @@ const char *errno_str(int errno) if (errno >= 0) return errno_message[0]; - return errno_message[abs(errno)]; + errno = -errno; + if (errno >= ARRAY_SIZE(errno_message)) + errno = ARRAY_SIZE(errno_message) - 1; + + return errno_message[errno]; } diff --git a/lib/libavb/avb_cmdline.c b/lib/libavb/avb_cmdline.c index d246699272..684c512bb9 100644 --- a/lib/libavb/avb_cmdline.c +++ b/lib/libavb/avb_cmdline.c @@ -39,6 +39,14 @@ char* avb_sub_cmdline(AvbOps* ops, char part_name[AVB_PART_NAME_MAX_SIZE]; char guid_buf[37]; + /* Don't attempt to query the partition guid unless its search string is + * present in the command line. Note: the original cmdline is used here, + * not the replaced one. See b/116010959. + */ + if (avb_strstr(cmdline, replace_str[n]) == NULL) { + continue; + } + if (!avb_str_concat(part_name, sizeof part_name, part_name_str[n], @@ -70,7 +78,15 @@ char* avb_sub_cmdline(AvbOps* ops, } } - avb_assert(ret != NULL); + /* It's possible there is no _PARTUUID for replacement above. + * Duplicate cmdline to ret for additional substitutions below. + */ + if (ret == NULL) { + ret = avb_strdup(cmdline); + if (ret == NULL) { + goto fail; + } + } /* Replace any additional substitutions. */ if (additional_substitutions != NULL) { @@ -198,21 +214,27 @@ static int cmdline_append_hex(AvbSlotVerifyData* slot_data, AvbSlotVerifyResult avb_append_options( AvbOps* ops, + AvbSlotVerifyFlags flags, AvbSlotVerifyData* slot_data, AvbVBMetaImageHeader* toplevel_vbmeta, AvbAlgorithmType algorithm_type, - AvbHashtreeErrorMode hashtree_error_mode) { + AvbHashtreeErrorMode hashtree_error_mode, + AvbHashtreeErrorMode resolved_hashtree_error_mode) { AvbSlotVerifyResult ret; const char* verity_mode; bool is_device_unlocked; AvbIOResult io_ret; - /* Add androidboot.vbmeta.device option. */ - if (!cmdline_append_option(slot_data, - "androidboot.vbmeta.device", - "PARTUUID=$(ANDROID_VBMETA_PARTUUID)")) { - ret = AVB_SLOT_VERIFY_RESULT_ERROR_OOM; - goto out; + /* Add androidboot.vbmeta.device option... except if not using a vbmeta + * partition since it doesn't make sense in that case. + */ + if (!(flags & AVB_SLOT_VERIFY_FLAGS_NO_VBMETA_PARTITION)) { + if (!cmdline_append_option(slot_data, + "androidboot.vbmeta.device", + "PARTUUID=$(ANDROID_VBMETA_PARTUUID)")) { + ret = AVB_SLOT_VERIFY_RESULT_ERROR_OOM; + goto out; + } } /* Add androidboot.vbmeta.avb_version option. */ @@ -304,7 +326,7 @@ AvbSlotVerifyResult avb_append_options( const char* dm_verity_mode; char* new_ret; - switch (hashtree_error_mode) { + switch (resolved_hashtree_error_mode) { case AVB_HASHTREE_ERROR_MODE_RESTART_AND_INVALIDATE: if (!cmdline_append_option( slot_data, "androidboot.vbmeta.invalidate_on_error", "yes")) { @@ -331,6 +353,12 @@ AvbSlotVerifyResult avb_append_options( verity_mode = "logging"; dm_verity_mode = "ignore_corruption"; break; + case AVB_HASHTREE_ERROR_MODE_MANAGED_RESTART_AND_EIO: + // Should never get here because MANAGED_RESTART_AND_EIO is + // remapped by avb_manage_hashtree_error_mode(). + avb_assert_not_reached(); + ret = AVB_SLOT_VERIFY_RESULT_ERROR_INVALID_ARGUMENT; + goto out; default: ret = AVB_SLOT_VERIFY_RESULT_ERROR_INVALID_ARGUMENT; goto out; @@ -349,6 +377,13 @@ AvbSlotVerifyResult avb_append_options( ret = AVB_SLOT_VERIFY_RESULT_ERROR_OOM; goto out; } + if (hashtree_error_mode == AVB_HASHTREE_ERROR_MODE_MANAGED_RESTART_AND_EIO) { + if (!cmdline_append_option( + slot_data, "androidboot.veritymode.managed", "yes")) { + ret = AVB_SLOT_VERIFY_RESULT_ERROR_OOM; + goto out; + } + } ret = AVB_SLOT_VERIFY_RESULT_OK; diff --git a/lib/libavb/avb_cmdline.h b/lib/libavb/avb_cmdline.h index 9af3a99994..96539d84bd 100644 --- a/lib/libavb/avb_cmdline.h +++ b/lib/libavb/avb_cmdline.h @@ -43,10 +43,12 @@ char* avb_sub_cmdline(AvbOps* ops, AvbSlotVerifyResult avb_append_options( AvbOps* ops, + AvbSlotVerifyFlags flags, AvbSlotVerifyData* slot_data, AvbVBMetaImageHeader* toplevel_vbmeta, AvbAlgorithmType algorithm_type, - AvbHashtreeErrorMode hashtree_error_mode); + AvbHashtreeErrorMode hashtree_error_mode, + AvbHashtreeErrorMode resolved_hashtree_error_mode); /* Allocates and initializes a new command line substitution list. Free with * |avb_free_cmdline_subst_list|. diff --git a/lib/libavb/avb_descriptor.c b/lib/libavb/avb_descriptor.c index fb0b305f2c..9f03b9777a 100644 --- a/lib/libavb/avb_descriptor.c +++ b/lib/libavb/avb_descriptor.c @@ -72,7 +72,11 @@ bool avb_descriptor_foreach(const uint8_t* image_data, const AvbDescriptor* dh = (const AvbDescriptor*)p; avb_assert_aligned(dh); uint64_t nb_following = avb_be64toh(dh->num_bytes_following); - uint64_t nb_total = sizeof(AvbDescriptor) + nb_following; + uint64_t nb_total = 0; + if (!avb_safe_add(&nb_total, sizeof(AvbDescriptor), nb_following)) { + avb_error("Invalid descriptor length.\n"); + goto out; + } if ((nb_total & 7) != 0) { avb_error("Invalid descriptor length.\n"); @@ -88,7 +92,10 @@ bool avb_descriptor_foreach(const uint8_t* image_data, goto out; } - p += nb_total; + if (!avb_safe_add_to((uint64_t*)(&p), nb_total)) { + avb_error("Invalid descriptor length.\n"); + goto out; + } } ret = true; diff --git a/lib/libavb/avb_ops.h b/lib/libavb/avb_ops.h index 8bbdc7c31b..6a5c589da8 100644 --- a/lib/libavb/avb_ops.h +++ b/lib/libavb/avb_ops.h @@ -18,6 +18,7 @@ extern "C" { /* Well-known names of named persistent values. */ #define AVB_NPV_PERSISTENT_DIGEST_PREFIX "avb.persistent_digest." +#define AVB_NPV_MANAGED_VERITY_MODE "avb.managed_verity_mode" /* Return codes used for I/O operations. * @@ -171,6 +172,10 @@ struct AvbOps { * * If AVB_IO_RESULT_OK is returned then |out_is_trusted| is set - * true if trusted or false if untrusted. + * + * NOTE: If AVB_SLOT_VERIFY_FLAGS_NO_VBMETA_PARTITION is passed to + * avb_slot_verify() then this operation is never used. Instead, the + * validate_public_key_for_partition() operation is used */ AvbIOResult (*validate_vbmeta_public_key)(AvbOps* ops, const uint8_t* public_key_data, @@ -231,6 +236,9 @@ struct AvbOps { * (NUL-terminated UTF-8 string). Returns the value in * |out_size_num_bytes|. * + * If the partition doesn't exist the AVB_IO_RESULT_ERROR_NO_SUCH_PARTITION + * error code should be returned. + * * Returns AVB_IO_RESULT_OK on success, otherwise an error code. */ AvbIOResult (*get_size_of_partition)(AvbOps* ops, @@ -253,9 +261,10 @@ struct AvbOps { * AVB_IO_RESULT_ERROR_NO_SUCH_VALUE. If |buffer_size| is smaller than the * size of the stored value, returns AVB_IO_RESULT_ERROR_INSUFFICIENT_SPACE. * - * This operation is currently only used to support persistent digests. If a - * device does not use persistent digests this function pointer can be set to - * NULL. + * This operation is currently only used to support persistent digests or the + * AVB_HASHTREE_ERROR_MODE_MANAGED_RESTART_AND_EIO hashtree error mode. If a + * device does not use one of these features this function pointer can be set + * to NULL. */ AvbIOResult (*read_persistent_value)(AvbOps* ops, const char* name, @@ -275,14 +284,34 @@ struct AvbOps { * AVB_IO_RESULT_ERROR_NO_SUCH_VALUE. If the |value_size| is not supported, * returns AVB_IO_RESULT_ERROR_INVALID_VALUE_SIZE. * - * This operation is currently only used to support persistent digests. If a - * device does not use persistent digests this function pointer can be set to - * NULL. + * This operation is currently only used to support persistent digests or the + * AVB_HASHTREE_ERROR_MODE_MANAGED_RESTART_AND_EIO hashtree error mode. If a + * device does not use one of these features this function pointer can be set + * to NULL. */ AvbIOResult (*write_persistent_value)(AvbOps* ops, const char* name, size_t value_size, const uint8_t* value); + + /* Like validate_vbmeta_public_key() but for when the flag + * AVB_SLOT_VERIFY_FLAGS_NO_VBMETA_PARTITION is being used. The name of the + * partition to get the public key for is passed in |partition_name|. + * + * Also returns the rollback index location to use for the partition, in + * |out_rollback_index_location|. + * + * Returns AVB_IO_RESULT_OK on success, otherwise an error code. + */ + AvbIOResult (*validate_public_key_for_partition)( + AvbOps* ops, + const char* partition, + const uint8_t* public_key_data, + size_t public_key_length, + const uint8_t* public_key_metadata, + size_t public_key_metadata_length, + bool* out_is_trusted, + uint32_t* out_rollback_index_location); }; #ifdef __cplusplus diff --git a/lib/libavb/avb_sha.h b/lib/libavb/avb_sha.h index 365aaadc2f..f5d02e09f2 100644 --- a/lib/libavb/avb_sha.h +++ b/lib/libavb/avb_sha.h @@ -31,8 +31,8 @@ extern "C" { /* Data structure used for SHA-256. */ typedef struct { uint32_t h[8]; - uint32_t tot_len; - uint32_t len; + uint64_t tot_len; + size_t len; uint8_t block[2 * AVB_SHA256_BLOCK_SIZE]; uint8_t buf[AVB_SHA256_DIGEST_SIZE]; /* Used for storing the final digest. */ } AvbSHA256Ctx; @@ -40,8 +40,8 @@ typedef struct { /* Data structure used for SHA-512. */ typedef struct { uint64_t h[8]; - uint32_t tot_len; - uint32_t len; + uint64_t tot_len; + size_t len; uint8_t block[2 * AVB_SHA512_BLOCK_SIZE]; uint8_t buf[AVB_SHA512_DIGEST_SIZE]; /* Used for storing the final digest. */ } AvbSHA512Ctx; @@ -50,7 +50,7 @@ typedef struct { void avb_sha256_init(AvbSHA256Ctx* ctx); /* Updates the SHA-256 context with |len| bytes from |data|. */ -void avb_sha256_update(AvbSHA256Ctx* ctx, const uint8_t* data, uint32_t len); +void avb_sha256_update(AvbSHA256Ctx* ctx, const uint8_t* data, size_t len); /* Returns the SHA-256 digest. */ uint8_t* avb_sha256_final(AvbSHA256Ctx* ctx) AVB_ATTR_WARN_UNUSED_RESULT; @@ -59,7 +59,7 @@ uint8_t* avb_sha256_final(AvbSHA256Ctx* ctx) AVB_ATTR_WARN_UNUSED_RESULT; void avb_sha512_init(AvbSHA512Ctx* ctx); /* Updates the SHA-512 context with |len| bytes from |data|. */ -void avb_sha512_update(AvbSHA512Ctx* ctx, const uint8_t* data, uint32_t len); +void avb_sha512_update(AvbSHA512Ctx* ctx, const uint8_t* data, size_t len); /* Returns the SHA-512 digest. */ uint8_t* avb_sha512_final(AvbSHA512Ctx* ctx) AVB_ATTR_WARN_UNUSED_RESULT; diff --git a/lib/libavb/avb_sha256.c b/lib/libavb/avb_sha256.c index d24c7015f6..86ecca57b7 100644 --- a/lib/libavb/avb_sha256.c +++ b/lib/libavb/avb_sha256.c @@ -29,6 +29,18 @@ *((str) + 0) = (uint8_t)((x) >> 24); \ } +#define UNPACK64(x, str) \ + { \ + *((str) + 7) = (uint8_t)x; \ + *((str) + 6) = (uint8_t)((uint64_t)x >> 8); \ + *((str) + 5) = (uint8_t)((uint64_t)x >> 16); \ + *((str) + 4) = (uint8_t)((uint64_t)x >> 24); \ + *((str) + 3) = (uint8_t)((uint64_t)x >> 32); \ + *((str) + 2) = (uint8_t)((uint64_t)x >> 40); \ + *((str) + 1) = (uint8_t)((uint64_t)x >> 48); \ + *((str) + 0) = (uint8_t)((uint64_t)x >> 56); \ + } + #define PACK32(str, x) \ { \ *(x) = ((uint32_t) * ((str) + 3)) | ((uint32_t) * ((str) + 2) << 8) | \ @@ -96,18 +108,18 @@ void avb_sha256_init(AvbSHA256Ctx* ctx) { static void SHA256_transform(AvbSHA256Ctx* ctx, const uint8_t* message, - unsigned int block_nb) { + size_t block_nb) { uint32_t w[64]; uint32_t wv[8]; uint32_t t1, t2; const unsigned char* sub_block; - int i; + size_t i; #ifndef UNROLL_LOOPS - int j; + size_t j; #endif - for (i = 0; i < (int)block_nb; i++) { + for (i = 0; i < block_nb; i++) { sub_block = message + (i << 6); #ifndef UNROLL_LOOPS @@ -293,9 +305,9 @@ static void SHA256_transform(AvbSHA256Ctx* ctx, } } -void avb_sha256_update(AvbSHA256Ctx* ctx, const uint8_t* data, uint32_t len) { - unsigned int block_nb; - unsigned int new_len, rem_len, tmp_len; +void avb_sha256_update(AvbSHA256Ctx* ctx, const uint8_t* data, size_t len) { + size_t block_nb; + size_t new_len, rem_len, tmp_len; const uint8_t* shifted_data; tmp_len = AVB_SHA256_BLOCK_SIZE - ctx->len; @@ -325,11 +337,11 @@ void avb_sha256_update(AvbSHA256Ctx* ctx, const uint8_t* data, uint32_t len) { } uint8_t* avb_sha256_final(AvbSHA256Ctx* ctx) { - unsigned int block_nb; - unsigned int pm_len; - unsigned int len_b; + size_t block_nb; + size_t pm_len; + uint64_t len_b; #ifndef UNROLL_LOOPS - int i; + size_t i; #endif block_nb = @@ -340,7 +352,7 @@ uint8_t* avb_sha256_final(AvbSHA256Ctx* ctx) { avb_memset(ctx->block + ctx->len, 0, pm_len - ctx->len); ctx->block[ctx->len] = 0x80; - UNPACK32(len_b, ctx->block + pm_len - 4); + UNPACK64(len_b, ctx->block + pm_len - 8); SHA256_transform(ctx, ctx->block, block_nb); diff --git a/lib/libavb/avb_sha512.c b/lib/libavb/avb_sha512.c index a5e7297aa7..b19054fc74 100644 --- a/lib/libavb/avb_sha512.c +++ b/lib/libavb/avb_sha512.c @@ -127,14 +127,14 @@ void avb_sha512_init(AvbSHA512Ctx* ctx) { static void SHA512_transform(AvbSHA512Ctx* ctx, const uint8_t* message, - unsigned int block_nb) { + size_t block_nb) { uint64_t w[80]; uint64_t wv[8]; uint64_t t1, t2; const uint8_t* sub_block; - int i, j; + size_t i, j; - for (i = 0; i < (int)block_nb; i++) { + for (i = 0; i < block_nb; i++) { sub_block = message + (i << 7); #ifdef UNROLL_LOOPS_SHA512 @@ -291,9 +291,9 @@ static void SHA512_transform(AvbSHA512Ctx* ctx, } } -void avb_sha512_update(AvbSHA512Ctx* ctx, const uint8_t* data, uint32_t len) { - unsigned int block_nb; - unsigned int new_len, rem_len, tmp_len; +void avb_sha512_update(AvbSHA512Ctx* ctx, const uint8_t* data, size_t len) { + size_t block_nb; + size_t new_len, rem_len, tmp_len; const uint8_t* shifted_data; tmp_len = AVB_SHA512_BLOCK_SIZE - ctx->len; @@ -323,12 +323,12 @@ void avb_sha512_update(AvbSHA512Ctx* ctx, const uint8_t* data, uint32_t len) { } uint8_t* avb_sha512_final(AvbSHA512Ctx* ctx) { - unsigned int block_nb; - unsigned int pm_len; - unsigned int len_b; + size_t block_nb; + size_t pm_len; + uint64_t len_b; #ifndef UNROLL_LOOPS_SHA512 - int i; + size_t i; #endif block_nb = @@ -339,7 +339,7 @@ uint8_t* avb_sha512_final(AvbSHA512Ctx* ctx) { avb_memset(ctx->block + ctx->len, 0, pm_len - ctx->len); ctx->block[ctx->len] = 0x80; - UNPACK32(len_b, ctx->block + pm_len - 4); + UNPACK64(len_b, ctx->block + pm_len - 8); SHA512_transform(ctx, ctx->block, block_nb); diff --git a/lib/libavb/avb_slot_verify.c b/lib/libavb/avb_slot_verify.c index a941850d93..c0defdf9c9 100644 --- a/lib/libavb/avb_slot_verify.c +++ b/lib/libavb/avb_slot_verify.c @@ -24,6 +24,14 @@ /* Maximum size of a vbmeta image - 64 KiB. */ #define VBMETA_MAX_SIZE (64 * 1024) +static AvbSlotVerifyResult initialize_persistent_digest( + AvbOps* ops, + const char* part_name, + const char* persistent_value_name, + size_t digest_size, + const uint8_t* initial_digest, + uint8_t* out_digest); + /* Helper function to see if we should continue with verification in * allow_verification_error=true mode if something goes wrong. See the * comments for the avb_slot_verify() function for more information. @@ -114,9 +122,26 @@ static AvbSlotVerifyResult load_full_partition(AvbOps* ops, return AVB_SLOT_VERIFY_RESULT_OK; } +/* Reads a persistent digest stored as a named persistent value corresponding to + * the given |part_name|. The value is returned in |out_digest| which must point + * to |expected_digest_size| bytes. If there is no digest stored for |part_name| + * it can be initialized by providing a non-NULL |initial_digest| of length + * |expected_digest_size|. This automatic initialization will only occur if the + * device is currently locked. The |initial_digest| may be NULL. + * + * Returns AVB_SLOT_VERIFY_RESULT_OK on success, otherwise returns an + * AVB_SLOT_VERIFY_RESULT_ERROR_* error code. + * + * If the value does not exist, is not supported, or is not populated, and + * |initial_digest| is NULL, returns + * AVB_SLOT_VERIFY_RESULT_ERROR_INVALID_METADATA. If |expected_digest_size| does + * not match the stored digest size, also returns + * AVB_SLOT_VERIFY_RESULT_ERROR_INVALID_METADATA. + */ static AvbSlotVerifyResult read_persistent_digest(AvbOps* ops, const char* part_name, size_t expected_digest_size, + const uint8_t* initial_digest, uint8_t* out_digest) { char* persistent_value_name = NULL; AvbIOResult io_ret = AVB_IO_RESULT_OK; @@ -131,30 +156,106 @@ static AvbSlotVerifyResult read_persistent_digest(AvbOps* ops, if (persistent_value_name == NULL) { return AVB_SLOT_VERIFY_RESULT_ERROR_OOM; } + io_ret = ops->read_persistent_value(ops, persistent_value_name, expected_digest_size, out_digest, &stored_digest_size); + + // If no such named persistent value exists and an initial digest value was + // given, initialize the named persistent value with the given digest. If + // initialized successfully, this will recurse into this function but with a + // NULL initial_digest. + if (io_ret == AVB_IO_RESULT_ERROR_NO_SUCH_VALUE && initial_digest) { + AvbSlotVerifyResult ret = + initialize_persistent_digest(ops, + part_name, + persistent_value_name, + expected_digest_size, + initial_digest, + out_digest); + avb_free(persistent_value_name); + return ret; + } avb_free(persistent_value_name); + if (io_ret == AVB_IO_RESULT_ERROR_OOM) { return AVB_SLOT_VERIFY_RESULT_ERROR_OOM; } else if (io_ret == AVB_IO_RESULT_ERROR_NO_SUCH_VALUE) { + // Treat a missing persistent value as a verification error, which is + // ignoreable, rather than a metadata error which is not. avb_errorv(part_name, ": Persistent digest does not exist.\n", NULL); - return AVB_SLOT_VERIFY_RESULT_ERROR_INVALID_METADATA; + return AVB_SLOT_VERIFY_RESULT_ERROR_VERIFICATION; } else if (io_ret == AVB_IO_RESULT_ERROR_INVALID_VALUE_SIZE || - io_ret == AVB_IO_RESULT_ERROR_INSUFFICIENT_SPACE || - expected_digest_size != stored_digest_size) { + io_ret == AVB_IO_RESULT_ERROR_INSUFFICIENT_SPACE) { avb_errorv( part_name, ": Persistent digest is not of expected size.\n", NULL); return AVB_SLOT_VERIFY_RESULT_ERROR_INVALID_METADATA; } else if (io_ret != AVB_IO_RESULT_OK) { avb_errorv(part_name, ": Error reading persistent digest.\n", NULL); return AVB_SLOT_VERIFY_RESULT_ERROR_IO; + } else if (expected_digest_size != stored_digest_size) { + avb_errorv( + part_name, ": Persistent digest is not of expected size.\n", NULL); + return AVB_SLOT_VERIFY_RESULT_ERROR_INVALID_METADATA; } return AVB_SLOT_VERIFY_RESULT_OK; } +static AvbSlotVerifyResult initialize_persistent_digest( + AvbOps* ops, + const char* part_name, + const char* persistent_value_name, + size_t digest_size, + const uint8_t* initial_digest, + uint8_t* out_digest) { + AvbSlotVerifyResult ret; + AvbIOResult io_ret = AVB_IO_RESULT_OK; + bool is_device_unlocked = true; + + io_ret = ops->read_is_device_unlocked(ops, &is_device_unlocked); + if (io_ret == AVB_IO_RESULT_ERROR_OOM) { + return AVB_SLOT_VERIFY_RESULT_ERROR_OOM; + } else if (io_ret != AVB_IO_RESULT_OK) { + avb_error("Error getting device lock state.\n"); + return AVB_SLOT_VERIFY_RESULT_ERROR_IO; + } + + if (is_device_unlocked) { + avb_debugv(part_name, + ": Digest does not exist, device unlocked so not initializing " + "digest.\n", + NULL); + return AVB_SLOT_VERIFY_RESULT_ERROR_VERIFICATION; + } + + // Device locked; initialize digest with given initial value. + avb_debugv(part_name, + ": Digest does not exist, initializing persistent digest.\n", + NULL); + io_ret = ops->write_persistent_value( + ops, persistent_value_name, digest_size, initial_digest); + if (io_ret == AVB_IO_RESULT_ERROR_OOM) { + return AVB_SLOT_VERIFY_RESULT_ERROR_OOM; + } else if (io_ret != AVB_IO_RESULT_OK) { + avb_errorv(part_name, ": Error initializing persistent digest.\n", NULL); + return AVB_SLOT_VERIFY_RESULT_ERROR_IO; + } + + // To ensure that the digest value was written successfully - and avoid a + // scenario where the digest is simply 'initialized' on every verify - recurse + // into read_persistent_digest to read back the written value. The NULL + // initial_digest ensures that this will not recurse again. + ret = read_persistent_digest(ops, part_name, digest_size, NULL, out_digest); + if (ret != AVB_SLOT_VERIFY_RESULT_OK) { + avb_errorv(part_name, + ": Reading back initialized persistent digest failed!\n", + NULL); + } + return ret; +} + static AvbSlotVerifyResult load_and_verify_hash_partition( AvbOps* ops, const char* const* requested_partitions, @@ -248,24 +349,16 @@ static AvbSlotVerifyResult load_and_verify_hash_partition( */ image_size = hash_desc.image_size; if (allow_verification_error) { - if (ops->get_size_of_partition == NULL) { - avb_errorv(part_name, - ": The get_size_of_partition() operation is " - "not implemented so we may not load the entire partition. " - "Please implement.", - NULL); - } else { - io_ret = ops->get_size_of_partition(ops, part_name, &image_size); - if (io_ret == AVB_IO_RESULT_ERROR_OOM) { - ret = AVB_SLOT_VERIFY_RESULT_ERROR_OOM; - goto out; - } else if (io_ret != AVB_IO_RESULT_OK) { - avb_errorv(part_name, ": Error determining partition size.\n", NULL); - ret = AVB_SLOT_VERIFY_RESULT_ERROR_IO; - goto out; - } - avb_debugv(part_name, ": Loading entire partition.\n", NULL); + io_ret = ops->get_size_of_partition(ops, part_name, &image_size); + if (io_ret == AVB_IO_RESULT_ERROR_OOM) { + ret = AVB_SLOT_VERIFY_RESULT_ERROR_OOM; + goto out; + } else if (io_ret != AVB_IO_RESULT_OK) { + avb_errorv(part_name, ": Error determining partition size.\n", NULL); + ret = AVB_SLOT_VERIFY_RESULT_ERROR_IO; + goto out; } + avb_debugv(part_name, ": Loading entire partition.\n", NULL); } ret = load_full_partition( @@ -273,19 +366,27 @@ static AvbSlotVerifyResult load_and_verify_hash_partition( if (ret != AVB_SLOT_VERIFY_RESULT_OK) { goto out; } - + // Although only one of the type might be used, we have to defined the + // structure here so that they would live outside the 'if/else' scope to be + // used later. + AvbSHA256Ctx sha256_ctx; + AvbSHA512Ctx sha512_ctx; + size_t image_size_to_hash = hash_desc.image_size; + // If we allow verification error and the whole partition is smaller than + // image size in hash descriptor, we just hash the whole partition. + if (image_size_to_hash > image_size) { + image_size_to_hash = image_size; + } if (avb_strcmp((const char*)hash_desc.hash_algorithm, "sha256") == 0) { - AvbSHA256Ctx sha256_ctx; avb_sha256_init(&sha256_ctx); avb_sha256_update(&sha256_ctx, desc_salt, hash_desc.salt_len); - avb_sha256_update(&sha256_ctx, image_buf, hash_desc.image_size); + avb_sha256_update(&sha256_ctx, image_buf, image_size_to_hash); digest = avb_sha256_final(&sha256_ctx); digest_len = AVB_SHA256_DIGEST_SIZE; } else if (avb_strcmp((const char*)hash_desc.hash_algorithm, "sha512") == 0) { - AvbSHA512Ctx sha512_ctx; avb_sha512_init(&sha512_ctx); avb_sha512_update(&sha512_ctx, desc_salt, hash_desc.salt_len); - avb_sha512_update(&sha512_ctx, image_buf, hash_desc.image_size); + avb_sha512_update(&sha512_ctx, image_buf, image_size_to_hash); digest = avb_sha512_final(&sha512_ctx); digest_len = AVB_SHA512_DIGEST_SIZE; } else { @@ -295,18 +396,21 @@ static AvbSlotVerifyResult load_and_verify_hash_partition( } if (hash_desc.digest_len == 0) { - // Expect a match to a persistent digest. + /* Expect a match to a persistent digest. */ avb_debugv(part_name, ": No digest, using persistent digest.\n", NULL); expected_digest_len = digest_len; expected_digest = expected_digest_buf; avb_assert(expected_digest_len <= sizeof(expected_digest_buf)); - ret = - read_persistent_digest(ops, part_name, digest_len, expected_digest_buf); + /* Pass |digest| as the |initial_digest| so devices not yet initialized get + * initialized to the current partition digest. + */ + ret = read_persistent_digest( + ops, part_name, digest_len, digest, expected_digest_buf); if (ret != AVB_SLOT_VERIFY_RESULT_OK) { goto out; } } else { - // Expect a match to the digest in the descriptor. + /* Expect a match to the digest in the descriptor. */ expected_digest_len = hash_desc.digest_len; expected_digest = desc_digest; } @@ -365,12 +469,6 @@ static AvbSlotVerifyResult load_requested_partitions( bool image_preloaded = false; size_t n; - if (ops->get_size_of_partition == NULL) { - avb_error("get_size_of_partition() not implemented.\n"); - ret = AVB_SLOT_VERIFY_RESULT_ERROR_INVALID_ARGUMENT; - goto out; - } - for (n = 0; requested_partitions[n] != NULL; n++) { char part_name[AVB_PART_NAME_MAX_SIZE]; AvbIOResult io_ret; @@ -441,6 +539,7 @@ static AvbSlotVerifyResult load_and_verify_vbmeta( AvbOps* ops, const char* const* requested_partitions, const char* ab_suffix, + AvbSlotVerifyFlags flags, bool allow_verification_error, AvbVBMetaImageFlags toplevel_vbmeta_flags, int rollback_index_location, @@ -467,7 +566,7 @@ static AvbSlotVerifyResult load_and_verify_vbmeta( size_t num_descriptors; size_t n; bool is_main_vbmeta; - bool is_vbmeta_partition; + bool look_for_vbmeta_footer; AvbVBMetaData* vbmeta_image_data = NULL; ret = AVB_SLOT_VERIFY_RESULT_OK; @@ -478,8 +577,20 @@ static AvbSlotVerifyResult load_and_verify_vbmeta( * rollback_index_location to determine whether we're the main * vbmeta struct. */ - is_main_vbmeta = (rollback_index_location == 0); - is_vbmeta_partition = (avb_strcmp(partition_name, "vbmeta") == 0); + is_main_vbmeta = false; + if (rollback_index_location == 0) { + if ((flags & AVB_SLOT_VERIFY_FLAGS_NO_VBMETA_PARTITION) == 0) { + is_main_vbmeta = true; + } + } + + /* Don't use footers for vbmeta partitions ('vbmeta' or + * 'vbmeta_<partition_name>'). + */ + look_for_vbmeta_footer = true; + if (avb_strncmp(partition_name, "vbmeta", avb_strlen("vbmeta")) == 0) { + look_for_vbmeta_footer = false; + } if (!avb_validate_utf8((const uint8_t*)partition_name, partition_name_len)) { avb_error("Partition name is not valid UTF-8.\n"); @@ -487,7 +598,7 @@ static AvbSlotVerifyResult load_and_verify_vbmeta( goto out; } - /* Construct full partition name. */ + /* Construct full partition name e.g. system_a. */ if (!avb_str_concat(full_partition_name, sizeof full_partition_name, partition_name, @@ -499,19 +610,15 @@ static AvbSlotVerifyResult load_and_verify_vbmeta( goto out; } - avb_debugv("Loading vbmeta struct from partition '", - full_partition_name, - "'.\n", - NULL); - - /* If we're loading from the main vbmeta partition, the vbmeta - * struct is in the beginning. Otherwise we have to locate it via a - * footer. + /* If we're loading from the main vbmeta partition, the vbmeta struct is in + * the beginning. Otherwise we may have to locate it via a footer... if no + * footer is found, we look in the beginning to support e.g. vbmeta_<org> + * partitions holding data for e.g. super partitions (b/80195851 for + * rationale). */ - if (is_vbmeta_partition) { - vbmeta_offset = 0; - vbmeta_size = VBMETA_MAX_SIZE; - } else { + vbmeta_offset = 0; + vbmeta_size = VBMETA_MAX_SIZE; + if (look_for_vbmeta_footer) { uint8_t footer_buf[AVB_FOOTER_SIZE]; size_t footer_num_read; AvbFooter footer; @@ -534,21 +641,17 @@ static AvbSlotVerifyResult load_and_verify_vbmeta( if (!avb_footer_validate_and_byteswap((const AvbFooter*)footer_buf, &footer)) { - avb_errorv(full_partition_name, ": Error validating footer.\n", NULL); - ret = AVB_SLOT_VERIFY_RESULT_ERROR_INVALID_METADATA; - goto out; - } - - /* Basic footer sanity check since the data is untrusted. */ - if (footer.vbmeta_size > VBMETA_MAX_SIZE) { - avb_errorv( - full_partition_name, ": Invalid vbmeta size in footer.\n", NULL); - ret = AVB_SLOT_VERIFY_RESULT_ERROR_INVALID_METADATA; - goto out; + avb_debugv(full_partition_name, ": No footer detected.\n", NULL); + } else { + /* Basic footer sanity check since the data is untrusted. */ + if (footer.vbmeta_size > VBMETA_MAX_SIZE) { + avb_errorv( + full_partition_name, ": Invalid vbmeta size in footer.\n", NULL); + } else { + vbmeta_offset = footer.vbmeta_offset; + vbmeta_size = footer.vbmeta_size; + } } - - vbmeta_offset = footer.vbmeta_offset; - vbmeta_size = footer.vbmeta_size; } vbmeta_buf = avb_malloc(vbmeta_size); @@ -557,6 +660,18 @@ static AvbSlotVerifyResult load_and_verify_vbmeta( goto out; } + if (vbmeta_offset != 0) { + avb_debugv("Loading vbmeta struct in footer from partition '", + full_partition_name, + "'.\n", + NULL); + } else { + avb_debugv("Loading vbmeta struct from partition '", + full_partition_name, + "'.\n", + NULL); + } + io_ret = ops->read_from_partition(ops, full_partition_name, vbmeta_offset, @@ -571,13 +686,14 @@ static AvbSlotVerifyResult load_and_verify_vbmeta( * go try to get it from the boot partition instead. */ if (is_main_vbmeta && io_ret == AVB_IO_RESULT_ERROR_NO_SUCH_PARTITION && - is_vbmeta_partition) { + !look_for_vbmeta_footer) { avb_debugv(full_partition_name, ": No such partition. Trying 'boot' instead.\n", NULL); ret = load_and_verify_vbmeta(ops, requested_partitions, ab_suffix, + flags, allow_verification_error, 0 /* toplevel_vbmeta_flags */, 0 /* rollback_index_location */, @@ -655,6 +771,8 @@ static AvbSlotVerifyResult load_and_verify_vbmeta( } } + uint32_t rollback_index_location_to_use = rollback_index_location; + /* Check if key used to make signature matches what is expected. */ if (pk_data != NULL) { if (expected_public_key != NULL) { @@ -682,9 +800,27 @@ static AvbSlotVerifyResult load_and_verify_vbmeta( pk_metadata_len = vbmeta_header.public_key_metadata_size; } - avb_assert(is_main_vbmeta); - io_ret = ops->validate_vbmeta_public_key( - ops, pk_data, pk_len, pk_metadata, pk_metadata_len, &key_is_trusted); + // If we're not using a vbmeta partition, need to use another AvbOps... + if (flags & AVB_SLOT_VERIFY_FLAGS_NO_VBMETA_PARTITION) { + io_ret = ops->validate_public_key_for_partition( + ops, + full_partition_name, + pk_data, + pk_len, + pk_metadata, + pk_metadata_len, + &key_is_trusted, + &rollback_index_location_to_use); + } else { + avb_assert(is_main_vbmeta); + io_ret = ops->validate_vbmeta_public_key(ops, + pk_data, + pk_len, + pk_metadata, + pk_metadata_len, + &key_is_trusted); + } + if (io_ret == AVB_IO_RESULT_ERROR_OOM) { ret = AVB_SLOT_VERIFY_RESULT_ERROR_OOM; goto out; @@ -709,7 +845,7 @@ static AvbSlotVerifyResult load_and_verify_vbmeta( /* Check rollback index. */ io_ret = ops->read_rollback_index( - ops, rollback_index_location, &stored_rollback_index); + ops, rollback_index_location_to_use, &stored_rollback_index); if (io_ret == AVB_IO_RESULT_ERROR_OOM) { ret = AVB_SLOT_VERIFY_RESULT_ERROR_OOM; goto out; @@ -735,7 +871,9 @@ static AvbSlotVerifyResult load_and_verify_vbmeta( if (is_main_vbmeta) { avb_assert(slot_data->num_vbmeta_images == 0); } else { - avb_assert(slot_data->num_vbmeta_images > 0); + if (!(flags & AVB_SLOT_VERIFY_FLAGS_NO_VBMETA_PARTITION)) { + avb_assert(slot_data->num_vbmeta_images > 0); + } } if (slot_data->num_vbmeta_images == MAX_NUMBER_OF_VBMETA_IMAGES) { avb_errorv(full_partition_name, ": Too many vbmeta images.\n", NULL); @@ -859,6 +997,7 @@ static AvbSlotVerifyResult load_and_verify_vbmeta( load_and_verify_vbmeta(ops, requested_partitions, ab_suffix, + flags, allow_verification_error, toplevel_vbmeta_flags, chain_desc.rollback_index_location, @@ -1019,7 +1158,11 @@ static AvbSlotVerifyResult load_and_verify_vbmeta( goto out; } - ret = read_persistent_digest(ops, part_name, digest_len, digest_buf); + ret = read_persistent_digest(ops, + part_name, + digest_len, + NULL /* initial_digest */, + digest_buf); if (ret != AVB_SLOT_VERIFY_RESULT_OK) { goto out; } @@ -1043,7 +1186,8 @@ static AvbSlotVerifyResult load_and_verify_vbmeta( } } - if (rollback_index_location >= AVB_MAX_NUMBER_OF_ROLLBACK_INDEX_LOCATIONS) { + if (rollback_index_location < 0 || + rollback_index_location >= AVB_MAX_NUMBER_OF_ROLLBACK_INDEX_LOCATIONS) { avb_errorv( full_partition_name, ": Invalid rollback_index_location.\n", NULL); ret = AVB_SLOT_VERIFY_RESULT_ERROR_INVALID_METADATA; @@ -1072,13 +1216,137 @@ out: return ret; } +static AvbIOResult avb_manage_hashtree_error_mode( + AvbOps* ops, + AvbSlotVerifyFlags flags, + AvbSlotVerifyData* data, + AvbHashtreeErrorMode* out_hashtree_error_mode) { + AvbHashtreeErrorMode ret = AVB_HASHTREE_ERROR_MODE_RESTART; + AvbIOResult io_ret = AVB_IO_RESULT_OK; + uint8_t vbmeta_digest_sha256[AVB_SHA256_DIGEST_SIZE]; + uint8_t stored_vbmeta_digest_sha256[AVB_SHA256_DIGEST_SIZE]; + size_t num_bytes_read; + + avb_assert(out_hashtree_error_mode != NULL); + avb_assert(ops->read_persistent_value != NULL); + avb_assert(ops->write_persistent_value != NULL); + + // If we're rebooting because of dm-verity corruption, make a note of + // the vbmeta hash so we can stay in 'eio' mode until things change. + if (flags & AVB_SLOT_VERIFY_FLAGS_RESTART_CAUSED_BY_HASHTREE_CORRUPTION) { + avb_debug( + "Rebooting because of dm-verity corruption - " + "recording OS instance and using 'eio' mode.\n"); + avb_slot_verify_data_calculate_vbmeta_digest( + data, AVB_DIGEST_TYPE_SHA256, vbmeta_digest_sha256); + io_ret = ops->write_persistent_value(ops, + AVB_NPV_MANAGED_VERITY_MODE, + AVB_SHA256_DIGEST_SIZE, + vbmeta_digest_sha256); + if (io_ret != AVB_IO_RESULT_OK) { + avb_error("Error writing to " AVB_NPV_MANAGED_VERITY_MODE ".\n"); + goto out; + } + ret = AVB_HASHTREE_ERROR_MODE_EIO; + io_ret = AVB_IO_RESULT_OK; + goto out; + } + + // See if we're in 'eio' mode. + io_ret = ops->read_persistent_value(ops, + AVB_NPV_MANAGED_VERITY_MODE, + AVB_SHA256_DIGEST_SIZE, + stored_vbmeta_digest_sha256, + &num_bytes_read); + if (io_ret == AVB_IO_RESULT_ERROR_NO_SUCH_VALUE || + (io_ret == AVB_IO_RESULT_OK && num_bytes_read == 0)) { + // This is the usual case ('eio' mode not set). + avb_debug("No dm-verity corruption - using in 'restart' mode.\n"); + ret = AVB_HASHTREE_ERROR_MODE_RESTART; + io_ret = AVB_IO_RESULT_OK; + goto out; + } else if (io_ret != AVB_IO_RESULT_OK) { + avb_error("Error reading from " AVB_NPV_MANAGED_VERITY_MODE ".\n"); + goto out; + } + if (num_bytes_read != AVB_SHA256_DIGEST_SIZE) { + avb_error( + "Unexpected number of bytes read from " AVB_NPV_MANAGED_VERITY_MODE + ".\n"); + io_ret = AVB_IO_RESULT_ERROR_IO; + goto out; + } + + // OK, so we're currently in 'eio' mode and the vbmeta digest of the OS + // that caused this is in |stored_vbmeta_digest_sha256| ... now see if + // the OS we're dealing with now is the same. + avb_slot_verify_data_calculate_vbmeta_digest( + data, AVB_DIGEST_TYPE_SHA256, vbmeta_digest_sha256); + if (avb_memcmp(vbmeta_digest_sha256, + stored_vbmeta_digest_sha256, + AVB_SHA256_DIGEST_SIZE) == 0) { + // It's the same so we're still in 'eio' mode. + avb_debug("Same OS instance detected - staying in 'eio' mode.\n"); + ret = AVB_HASHTREE_ERROR_MODE_EIO; + io_ret = AVB_IO_RESULT_OK; + } else { + // It did change! + avb_debug( + "New OS instance detected - changing from 'eio' to 'restart' mode.\n"); + io_ret = + ops->write_persistent_value(ops, + AVB_NPV_MANAGED_VERITY_MODE, + 0, // This clears the persistent property. + vbmeta_digest_sha256); + if (io_ret != AVB_IO_RESULT_OK) { + avb_error("Error clearing " AVB_NPV_MANAGED_VERITY_MODE ".\n"); + goto out; + } + ret = AVB_HASHTREE_ERROR_MODE_RESTART; + io_ret = AVB_IO_RESULT_OK; + } + +out: + *out_hashtree_error_mode = ret; + return io_ret; +} + +static bool has_system_partition(AvbOps* ops, const char* ab_suffix) { + char part_name[AVB_PART_NAME_MAX_SIZE]; + char* system_part_name = "system"; + char guid_buf[37]; + AvbIOResult io_ret; + + if (!avb_str_concat(part_name, + sizeof part_name, + system_part_name, + avb_strlen(system_part_name), + ab_suffix, + avb_strlen(ab_suffix))) { + avb_error("System partition name and suffix does not fit.\n"); + return false; + } + + io_ret = ops->get_unique_guid_for_partition( + ops, part_name, guid_buf, sizeof guid_buf); + if (io_ret == AVB_IO_RESULT_ERROR_NO_SUCH_PARTITION) { + avb_debug("No system partition.\n"); + return false; + } else if (io_ret != AVB_IO_RESULT_OK) { + avb_error("Error getting unique GUID for system partition.\n"); + return false; + } + + return true; +} + AvbSlotVerifyResult avb_slot_verify(AvbOps* ops, const char* const* requested_partitions, const char* ab_suffix, AvbSlotVerifyFlags flags, AvbHashtreeErrorMode hashtree_error_mode, AvbSlotVerifyData** out_data) { - AvbSlotVerifyResult ret; + AvbSlotVerifyResult ret = AVB_SLOT_VERIFY_RESULT_ERROR_INVALID_ARGUMENT; AvbSlotVerifyData* slot_data = NULL; AvbAlgorithmType algorithm_type = AVB_ALGORITHM_TYPE_NONE; bool using_boot_for_vbmeta = false; @@ -1087,14 +1355,10 @@ AvbSlotVerifyResult avb_slot_verify(AvbOps* ops, (flags & AVB_SLOT_VERIFY_FLAGS_ALLOW_VERIFICATION_ERROR); AvbCmdlineSubstList* additional_cmdline_subst = NULL; - /* Fail early if we're missing the AvbOps needed for slot verification. - * - * For now, handle get_size_of_partition() not being implemented. In - * a later release we may change that. - */ + /* Fail early if we're missing the AvbOps needed for slot verification. */ avb_assert(ops->read_is_device_unlocked != NULL); avb_assert(ops->read_from_partition != NULL); - avb_assert(ops->validate_vbmeta_public_key != NULL); + avb_assert(ops->get_size_of_partition != NULL); avb_assert(ops->read_rollback_index != NULL); avb_assert(ops->get_unique_guid_for_partition != NULL); @@ -1112,6 +1376,36 @@ AvbSlotVerifyResult avb_slot_verify(AvbOps* ops, goto fail; } + /* Make sure passed-in AvbOps support persistent values if + * asking for libavb to manage verity state. + */ + if (hashtree_error_mode == AVB_HASHTREE_ERROR_MODE_MANAGED_RESTART_AND_EIO) { + if (ops->read_persistent_value == NULL || + ops->write_persistent_value == NULL) { + avb_error( + "Persistent values required for " + "AVB_HASHTREE_ERROR_MODE_MANAGED_RESTART_AND_EIO " + "but are not implemented in given AvbOps.\n"); + ret = AVB_SLOT_VERIFY_RESULT_ERROR_INVALID_ARGUMENT; + goto fail; + } + } + + /* Make sure passed-in AvbOps support verifying public keys and getting + * rollback index location if not using a vbmeta partition. + */ + if (flags & AVB_SLOT_VERIFY_FLAGS_NO_VBMETA_PARTITION) { + if (ops->validate_public_key_for_partition == NULL) { + avb_error( + "AVB_SLOT_VERIFY_FLAGS_NO_VBMETA_PARTITION was passed but the " + "validate_public_key_for_partition() operation isn't implemented.\n"); + ret = AVB_SLOT_VERIFY_RESULT_ERROR_INVALID_ARGUMENT; + goto fail; + } + } else { + avb_assert(ops->validate_vbmeta_public_key != NULL); + } + slot_data = avb_calloc(sizeof(AvbSlotVerifyData)); if (slot_data == NULL) { ret = AVB_SLOT_VERIFY_RESULT_ERROR_OOM; @@ -1136,99 +1430,163 @@ AvbSlotVerifyResult avb_slot_verify(AvbOps* ops, goto fail; } - ret = load_and_verify_vbmeta(ops, - requested_partitions, - ab_suffix, - allow_verification_error, - 0 /* toplevel_vbmeta_flags */, - 0 /* rollback_index_location */, - "vbmeta", - avb_strlen("vbmeta"), - NULL /* expected_public_key */, - 0 /* expected_public_key_length */, - slot_data, - &algorithm_type, - additional_cmdline_subst); - if (!allow_verification_error && ret != AVB_SLOT_VERIFY_RESULT_OK) { + if (flags & AVB_SLOT_VERIFY_FLAGS_NO_VBMETA_PARTITION) { + if (requested_partitions == NULL || requested_partitions[0] == NULL) { + avb_fatal( + "Requested partitions cannot be empty when using " + "AVB_SLOT_VERIFY_FLAGS_NO_VBMETA_PARTITION"); + ret = AVB_SLOT_VERIFY_RESULT_ERROR_INVALID_ARGUMENT; + goto fail; + } + + /* No vbmeta partition, go through each of the requested partitions... */ + for (size_t n = 0; requested_partitions[n] != NULL; n++) { + ret = load_and_verify_vbmeta(ops, + requested_partitions, + ab_suffix, + flags, + allow_verification_error, + 0 /* toplevel_vbmeta_flags */, + 0 /* rollback_index_location */, + requested_partitions[n], + avb_strlen(requested_partitions[n]), + NULL /* expected_public_key */, + 0 /* expected_public_key_length */, + slot_data, + &algorithm_type, + additional_cmdline_subst); + if (!allow_verification_error && ret != AVB_SLOT_VERIFY_RESULT_OK) { + goto fail; + } + } + + } else { + /* Usual path, load "vbmeta"... */ + ret = load_and_verify_vbmeta(ops, + requested_partitions, + ab_suffix, + flags, + allow_verification_error, + 0 /* toplevel_vbmeta_flags */, + 0 /* rollback_index_location */, + "vbmeta", + avb_strlen("vbmeta"), + NULL /* expected_public_key */, + 0 /* expected_public_key_length */, + slot_data, + &algorithm_type, + additional_cmdline_subst); + if (!allow_verification_error && ret != AVB_SLOT_VERIFY_RESULT_OK) { + goto fail; + } + } + + if (!result_should_continue(ret)) { goto fail; } /* If things check out, mangle the kernel command-line as needed. */ - if (result_should_continue(ret)) { + if (!(flags & AVB_SLOT_VERIFY_FLAGS_NO_VBMETA_PARTITION)) { if (avb_strcmp(slot_data->vbmeta_images[0].partition_name, "vbmeta") != 0) { avb_assert( avb_strcmp(slot_data->vbmeta_images[0].partition_name, "boot") == 0); using_boot_for_vbmeta = true; } + } - /* Byteswap top-level vbmeta header since we'll need it below. */ - avb_vbmeta_image_header_to_host_byte_order( - (const AvbVBMetaImageHeader*)slot_data->vbmeta_images[0].vbmeta_data, - &toplevel_vbmeta); + /* Byteswap top-level vbmeta header since we'll need it below. */ + avb_vbmeta_image_header_to_host_byte_order( + (const AvbVBMetaImageHeader*)slot_data->vbmeta_images[0].vbmeta_data, + &toplevel_vbmeta); - /* Fill in |ab_suffix| field. */ - slot_data->ab_suffix = avb_strdup(ab_suffix); - if (slot_data->ab_suffix == NULL) { - ret = AVB_SLOT_VERIFY_RESULT_ERROR_OOM; - goto fail; - } + /* Fill in |ab_suffix| field. */ + slot_data->ab_suffix = avb_strdup(ab_suffix); + if (slot_data->ab_suffix == NULL) { + ret = AVB_SLOT_VERIFY_RESULT_ERROR_OOM; + goto fail; + } - /* If verification is disabled, we are done ... we specifically - * don't want to add any androidboot.* options since verification - * is disabled. + /* If verification is disabled, we are done ... we specifically + * don't want to add any androidboot.* options since verification + * is disabled. + */ + if (toplevel_vbmeta.flags & AVB_VBMETA_IMAGE_FLAGS_VERIFICATION_DISABLED) { + /* Since verification is disabled we didn't process any + * descriptors and thus there's no cmdline... so set root= such + * that the system partition is mounted. */ - if (toplevel_vbmeta.flags & AVB_VBMETA_IMAGE_FLAGS_VERIFICATION_DISABLED) { - /* Since verification is disabled we didn't process any - * descriptors and thus there's no cmdline... so set root= such - * that the system partition is mounted. - */ - avb_assert(slot_data->cmdline == NULL); + avb_assert(slot_data->cmdline == NULL); + // Devices with dynamic partitions won't have system partition. + // Instead, it has a large super partition to accommodate *.img files. + // See b/119551429 for details. + if (has_system_partition(ops, ab_suffix)) { slot_data->cmdline = avb_strdup("root=PARTUUID=$(ANDROID_SYSTEM_PARTUUID)"); - if (slot_data->cmdline == NULL) { - ret = AVB_SLOT_VERIFY_RESULT_ERROR_OOM; - goto fail; - } } else { - /* Add options - any failure in avb_append_options() is either an - * I/O or OOM error. - */ - AvbSlotVerifyResult sub_ret = avb_append_options(ops, - slot_data, - &toplevel_vbmeta, - algorithm_type, - hashtree_error_mode); - if (sub_ret != AVB_SLOT_VERIFY_RESULT_OK) { - ret = sub_ret; - goto fail; - } + // The |cmdline| field should be a NUL-terminated string. + slot_data->cmdline = avb_strdup(""); } - - /* Substitute $(ANDROID_SYSTEM_PARTUUID) and friends. */ - if (slot_data->cmdline != NULL) { - char* new_cmdline; - new_cmdline = avb_sub_cmdline(ops, - slot_data->cmdline, - ab_suffix, - using_boot_for_vbmeta, - additional_cmdline_subst); - if (new_cmdline != slot_data->cmdline) { - if (new_cmdline == NULL) { + if (slot_data->cmdline == NULL) { + ret = AVB_SLOT_VERIFY_RESULT_ERROR_OOM; + goto fail; + } + } else { + /* If requested, manage dm-verity mode... */ + AvbHashtreeErrorMode resolved_hashtree_error_mode = hashtree_error_mode; + if (hashtree_error_mode == + AVB_HASHTREE_ERROR_MODE_MANAGED_RESTART_AND_EIO) { + AvbIOResult io_ret; + io_ret = avb_manage_hashtree_error_mode( + ops, flags, slot_data, &resolved_hashtree_error_mode); + if (io_ret != AVB_IO_RESULT_OK) { + ret = AVB_SLOT_VERIFY_RESULT_ERROR_IO; + if (io_ret == AVB_IO_RESULT_ERROR_OOM) { ret = AVB_SLOT_VERIFY_RESULT_ERROR_OOM; - goto fail; } - avb_free(slot_data->cmdline); - slot_data->cmdline = new_cmdline; + goto fail; } } + slot_data->resolved_hashtree_error_mode = resolved_hashtree_error_mode; - if (out_data != NULL) { - *out_data = slot_data; - } else { - avb_slot_verify_data_free(slot_data); + /* Add options... */ + AvbSlotVerifyResult sub_ret; + sub_ret = avb_append_options(ops, + flags, + slot_data, + &toplevel_vbmeta, + algorithm_type, + hashtree_error_mode, + resolved_hashtree_error_mode); + if (sub_ret != AVB_SLOT_VERIFY_RESULT_OK) { + ret = sub_ret; + goto fail; + } + } + + /* Substitute $(ANDROID_SYSTEM_PARTUUID) and friends. */ + if (slot_data->cmdline != NULL && avb_strlen(slot_data->cmdline) != 0) { + char* new_cmdline; + new_cmdline = avb_sub_cmdline(ops, + slot_data->cmdline, + ab_suffix, + using_boot_for_vbmeta, + additional_cmdline_subst); + if (new_cmdline != slot_data->cmdline) { + if (new_cmdline == NULL) { + ret = AVB_SLOT_VERIFY_RESULT_ERROR_OOM; + goto fail; + } + avb_free(slot_data->cmdline); + slot_data->cmdline = new_cmdline; } } + if (out_data != NULL) { + *out_data = slot_data; + } else { + avb_slot_verify_data_free(slot_data); + } + avb_free_cmdline_subst_list(additional_cmdline_subst); additional_cmdline_subst = NULL; diff --git a/lib/libavb/avb_slot_verify.h b/lib/libavb/avb_slot_verify.h index 73fd70d4ce..8d0fa53693 100644 --- a/lib/libavb/avb_slot_verify.h +++ b/lib/libavb/avb_slot_verify.h @@ -51,12 +51,25 @@ typedef enum { * be used ONLY for diagnostics and debugging. It cannot be used * unless AVB_SLOT_VERIFY_FLAGS_ALLOW_VERIFICATION_ERROR is also * used. + * + * AVB_HASHTREE_ERROR_MODE_MANAGED_RESTART_AND_EIO means that either + * AVB_HASHTREE_ERROR_MODE_RESTART or AVB_HASHTREE_ERROR_MODE_EIO is used + * depending on state. This mode implements a state machine whereby + * AVB_HASHTREE_ERROR_MODE_RESTART is used by default and when + * AVB_SLOT_VERIFY_FLAGS_RESTART_CAUSED_BY_HASHTREE_CORRUPTION is passed the + * mode transitions to AVB_HASHTREE_ERROR_MODE_EIO. When a new OS has been + * detected the device transitions back to the AVB_HASHTREE_ERROR_MODE_RESTART + * mode. To do this persistent storage is needed - specifically this means that + * the passed in AvbOps will need to have the read_persistent_value() and + * write_persistent_value() operations implemented. The name of the persistent + * value used is "avb.managed_verity_mode" and 32 bytes of storage is needed. */ typedef enum { AVB_HASHTREE_ERROR_MODE_RESTART_AND_INVALIDATE, AVB_HASHTREE_ERROR_MODE_RESTART, AVB_HASHTREE_ERROR_MODE_EIO, - AVB_HASHTREE_ERROR_MODE_LOGGING + AVB_HASHTREE_ERROR_MODE_LOGGING, + AVB_HASHTREE_ERROR_MODE_MANAGED_RESTART_AND_EIO } AvbHashtreeErrorMode; /* Flags that influence how avb_slot_verify() works. @@ -80,10 +93,26 @@ typedef enum { * contents loaded from |requested_partition| will be the contents of * the entire partition instead of just the size specified in the hash * descriptor. + * + * The AVB_SLOT_VERIFY_FLAGS_RESTART_CAUSED_BY_HASHTREE_CORRUPTION flag + * should be set if using AVB_HASHTREE_ERROR_MODE_MANAGED_RESTART_AND_EIO + * and the reason the boot loader is running is because the device + * was restarted by the dm-verity driver. + * + * If the AVB_SLOT_VERIFY_FLAGS_NO_VBMETA_PARTITION flag is set then + * data won't be loaded from the "vbmeta" partition and the + * |validate_vbmeta_public_key| operation is never called. Instead, the + * vbmeta structs in |requested_partitions| are loaded and processed and the + * |validate_public_key_for_partition| operation is called for each of these + * vbmeta structs. This flag is useful when booting into recovery on a device + * not using A/B - see section "Booting into recovery" in README.md for + * more information. */ typedef enum { AVB_SLOT_VERIFY_FLAGS_NONE = 0, - AVB_SLOT_VERIFY_FLAGS_ALLOW_VERIFICATION_ERROR = (1 << 0) + AVB_SLOT_VERIFY_FLAGS_ALLOW_VERIFICATION_ERROR = (1 << 0), + AVB_SLOT_VERIFY_FLAGS_RESTART_CAUSED_BY_HASHTREE_CORRUPTION = (1 << 1), + AVB_SLOT_VERIFY_FLAGS_NO_VBMETA_PARTITION = (1 << 2), } AvbSlotVerifyFlags; /* Get a textual representation of |result|. */ @@ -188,6 +217,10 @@ typedef struct { * set to AVB_HASHTREE_ERROR_MODE_EIO, and 'logging' if it's set to * AVB_HASHTREE_ERROR_MODE_LOGGING. * + * androidboot.veritymode.managed: This is set to 'yes' only + * if hashtree validation isn't disabled and the passed-in hashtree + * error mode is AVB_HASHTREE_ERROR_MODE_MANAGED_RESTART_AND_EIO. + * * androidboot.vbmeta.invalidate_on_error: This is set to 'yes' only * if hashtree validation isn't disabled and the passed-in hashtree * error mode is AVB_HASHTREE_ERROR_MODE_RESTART_AND_INVALIDATE. @@ -203,7 +236,9 @@ typedef struct { * PARTUUID=$(ANDROID_VBMETA_PARTUUID) before substitution so it * will end up pointing to the vbmeta partition for the verified * slot. If there is no vbmeta partition it will point to the boot - * partition of the verified slot. + * partition of the verified slot. If the flag + * AVB_SLOT_VERIFY_FLAGS_NO_VBMETA_PARTITION is used, this is not + * set. * * androidboot.vbmeta.avb_version: This is set to the decimal value * of AVB_VERSION_MAJOR followed by a dot followed by the decimal @@ -228,6 +263,15 @@ typedef struct { * appropriate system partition is substituted in. Note that none of * the androidboot.* options mentioned above will be set. * + * The |resolved_hashtree_error_mode| is the the value of the passed + * avb_slot_verify()'s |hashtree_error_mode| parameter except that it never has + * the value AVB_HASHTREE_ERROR_MODE_MANAGED_RESTART_AND_EIO. If this value was + * passed in, then the restart/eio state machine is used resulting in + * |resolved_hashtree_error_mode| being set to either + * AVB_HASHTREE_ERROR_MODE_RESTART or AVB_HASHTREE_ERROR_MODE_EIO. If set to + * AVB_HASHTREE_ERROR_MODE_EIO the boot loader should present a RED warning + * screen for the user to click through before continuing to boot. + * * This struct may grow in the future without it being considered an * ABI break. */ @@ -239,6 +283,7 @@ typedef struct { size_t num_loaded_partitions; char* cmdline; uint64_t rollback_indexes[AVB_MAX_NUMBER_OF_ROLLBACK_INDEX_LOCATIONS]; + AvbHashtreeErrorMode resolved_hashtree_error_mode; } AvbSlotVerifyData; /* Calculates a digest of all vbmeta images in |data| using @@ -282,12 +327,8 @@ void avb_slot_verify_data_free(AvbSlotVerifyData* data); * ignore verification errors which is something needed in the * UNLOCKED state. See the AvbSlotVerifyFlags enumeration for details. * - * The |hashtree_error_mode| parameter should be set to the desired - * error handling mode when hashtree validation fails inside the - * HLOS. This value isn't used by libavb per se - it is forwarded to - * the HLOS through the androidboot.veritymode and - * androidboot.vbmeta.invalidate_on_error cmdline parameters. See the - * AvbHashtreeErrorMode enumeration for details. + * The |hashtree_error_mode| parameter should be set to the desired error + * handling mode. See the AvbHashtreeErrorMode enumeration for details. * * Also note that |out_data| is never set if * AVB_SLOT_VERIFY_RESULT_ERROR_OOM, AVB_SLOT_VERIFY_RESULT_ERROR_IO, diff --git a/lib/libavb/avb_sysdeps.h b/lib/libavb/avb_sysdeps.h index f032de4a2e..f52428cc62 100644 --- a/lib/libavb/avb_sysdeps.h +++ b/lib/libavb/avb_sysdeps.h @@ -53,6 +53,14 @@ int avb_memcmp(const void* src1, */ int avb_strcmp(const char* s1, const char* s2); +/* Compare |n| bytes in two strings. + * + * Return an integer less than, equal to, or greater than zero if the + * first |n| bytes of |s1| is found, respectively, to be less than, + * to match, or be greater than the first |n| bytes of |s2|. + */ +int avb_strncmp(const char* s1, const char* s2, size_t n); + /* Copy |n| bytes from |src| to |dest|. */ void* avb_memcpy(void* dest, const void* src, size_t n); diff --git a/lib/libavb/avb_sysdeps_posix.c b/lib/libavb/avb_sysdeps_posix.c index e9addc1c87..4ccf41e428 100644 --- a/lib/libavb/avb_sysdeps_posix.c +++ b/lib/libavb/avb_sysdeps_posix.c @@ -24,14 +24,12 @@ int avb_strcmp(const char* s1, const char* s2) { return strcmp(s1, s2); } -size_t avb_strlen(const char* str) { - return strlen(str); +int avb_strncmp(const char* s1, const char* s2, size_t n) { + return strncmp(s1, s2, n); } -uint32_t avb_div_by_10(uint64_t* dividend) { - uint32_t rem = (uint32_t)(*dividend % 10); - *dividend /= 10; - return rem; +size_t avb_strlen(const char* str) { + return strlen(str); } void avb_abort(void) { @@ -60,3 +58,9 @@ void* avb_malloc_(size_t size) { void avb_free(void* ptr) { free(ptr); } + +uint32_t avb_div_by_10(uint64_t* dividend) { + uint32_t rem = (uint32_t)(*dividend % 10); + *dividend /= 10; + return rem; +} diff --git a/lib/libavb/avb_vbmeta_image.c b/lib/libavb/avb_vbmeta_image.c index a7e2322b9e..384f5ac19e 100644 --- a/lib/libavb/avb_vbmeta_image.c +++ b/lib/libavb/avb_vbmeta_image.c @@ -35,17 +35,18 @@ AvbVBMetaVerifyResult avb_vbmeta_image_verify( *out_public_key_length = 0; } + /* Before we byteswap or compare Magic, ensure length is long enough. */ + if (length < sizeof(AvbVBMetaImageHeader)) { + avb_error("Length is smaller than header.\n"); + goto out; + } + /* Ensure magic is correct. */ if (avb_safe_memcmp(data, AVB_MAGIC, AVB_MAGIC_LEN) != 0) { avb_error("Magic is incorrect.\n"); goto out; } - /* Before we byteswap, ensure length is long enough. */ - if (length < sizeof(AvbVBMetaImageHeader)) { - avb_error("Length is smaller than header.\n"); - goto out; - } avb_vbmeta_image_header_to_host_byte_order((const AvbVBMetaImageHeader*)data, &h); diff --git a/lib/linux_compat.c b/lib/linux_compat.c index 6373b4451e..81ea8fb126 100644 --- a/lib/linux_compat.c +++ b/lib/linux_compat.c @@ -20,7 +20,7 @@ void *kmalloc(size_t size, int flags) void *p; p = malloc_cache_aligned(size); - if (flags & __GFP_ZERO) + if (p && flags & __GFP_ZERO) memset(p, 0, size); return p; diff --git a/lib/time.c b/lib/time.c index f5751ab162..f30fc05804 100644 --- a/lib/time.c +++ b/lib/time.c @@ -134,6 +134,20 @@ ulong __weak get_timer(ulong base) return tick_to_time(get_ticks()) - base; } +static uint64_t notrace tick_to_time_us(uint64_t tick) +{ + ulong div = get_tbclk() / 1000; + + tick *= CONFIG_SYS_HZ; + do_div(tick, div); + return tick; +} + +uint64_t __weak get_timer_us(uint64_t base) +{ + return tick_to_time_us(get_ticks()) - base; +} + unsigned long __weak notrace timer_get_us(void) { return tick_to_time(get_ticks() * 1000); diff --git a/lib/tiny-printf.c b/lib/tiny-printf.c index ebef92fc9f..62e6381961 100644 --- a/lib/tiny-printf.c +++ b/lib/tiny-printf.c @@ -366,6 +366,22 @@ int sprintf(char *buf, const char *fmt, ...) return ret; } +#if CONFIG_IS_ENABLED(LOG) +/* Note that size is ignored */ +int vsnprintf(char *buf, size_t size, const char *fmt, va_list va) +{ + struct printf_info info; + int ret; + + info.outstr = buf; + info.putc = putc_outstr; + ret = _vprintf(&info, fmt, va); + *info.outstr = '\0'; + + return ret; +} +#endif + /* Note that size is ignored */ int snprintf(char *buf, size_t size, const char *fmt, ...) { diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 373094e59e..6fcc66afb0 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -1,9 +1,11 @@ #!/usr/bin/env perl +# SPDX-License-Identifier: GPL-2.0 +# # (c) 2001, Dave Jones. (the file handling bit) # (c) 2005, Joel Schopp <jschopp@austin.ibm.com> (the ugly bit) # (c) 2007,2008, Andy Whitcroft <apw@uk.ibm.com> (new conditions, test suite) # (c) 2008-2010 Andy Whitcroft <apw@canonical.com> -# Licensed under the terms of the GNU GPL License version 2 +# (c) 2010-2018 Joe Perches <joe@perches.com> use strict; use warnings; @@ -11,6 +13,7 @@ use POSIX; use File::Basename; use Cwd 'abs_path'; use Term::ANSIColor qw(:constants); +use Encode qw(decode encode); my $P = $0; my $D = dirname(abs_path($P)); @@ -58,7 +61,9 @@ my $codespellfile = "/usr/share/codespell/dictionary.txt"; my $conststructsfile = "$D/const_structs.checkpatch"; my $typedefsfile = ""; my $color = "auto"; -my $allow_c99_comments = 1; +my $allow_c99_comments = 1; # Can be overridden by --ignore C99_COMMENT_TOLERANCE +# git output parsing needs US English output, so first set backtick child process LANGUAGE +my $git_command ='export LANGUAGE=en_US.UTF-8; git'; sub help { my ($exitcode) = @_; @@ -238,11 +243,11 @@ $check_orig = $check; my $exit = 0; +my $perl_version_ok = 1; if ($^V && $^V lt $minimum_perl_version) { + $perl_version_ok = 0; printf "$P: requires at least perl version %vd\n", $minimum_perl_version; - if (!$ignore_perl_version) { - exit(1); - } + exit(1) if (!$ignore_perl_version); } #if no filenames are given, push '-' to read patch from stdin @@ -344,9 +349,10 @@ our $Sparse = qr{ __force| __iomem| __must_check| - __init_refok| __kprobes| __ref| + __refconst| + __refdata| __rcu| __private }x; @@ -376,6 +382,7 @@ our $Attribute = qr{ __noclone| __deprecated| __read_mostly| + __ro_after_init| __kprobes| $InitAttribute| ____cacheline_aligned| @@ -461,8 +468,19 @@ our $logFunctions = qr{(?x: seq_vprintf|seq_printf|seq_puts )}; +our $allocFunctions = qr{(?x: + (?:(?:devm_)? + (?:kv|k|v)[czm]alloc(?:_node|_array)? | + kstrdup(?:_const)? | + kmemdup(?:_nul)?) | + (?:\w+)?alloc_skb(?:ip_align)? | + # dev_alloc_skb/netdev_alloc_skb, et al + dma_alloc_coherent +)}; + our $signature_tags = qr{(?xi: Signed-off-by:| + Co-developed-by:| Acked-by:| Tested-by:| Reviewed-by:| @@ -568,6 +586,27 @@ foreach my $entry (@mode_permission_funcs) { } $mode_perms_search = "(?:${mode_perms_search})"; +our %deprecated_apis = ( + "synchronize_rcu_bh" => "synchronize_rcu", + "synchronize_rcu_bh_expedited" => "synchronize_rcu_expedited", + "call_rcu_bh" => "call_rcu", + "rcu_barrier_bh" => "rcu_barrier", + "synchronize_sched" => "synchronize_rcu", + "synchronize_sched_expedited" => "synchronize_rcu_expedited", + "call_rcu_sched" => "call_rcu", + "rcu_barrier_sched" => "rcu_barrier", + "get_state_synchronize_sched" => "get_state_synchronize_rcu", + "cond_synchronize_sched" => "cond_synchronize_rcu", +); + +#Create a search pattern for all these strings to speed up a loop below +our $deprecated_apis_search = ""; +foreach my $entry (keys %deprecated_apis) { + $deprecated_apis_search .= '|' if ($deprecated_apis_search ne ""); + $deprecated_apis_search .= $entry; +} +$deprecated_apis_search = "(?:${deprecated_apis_search})"; + our $mode_perms_world_writable = qr{ S_IWUGO | S_IWOTH | @@ -845,6 +884,17 @@ sub is_maintained_obsolete { return $status =~ /obsolete/i; } +sub is_SPDX_License_valid { + my ($license) = @_; + + return 1 if (!$tree || which("python") eq "" || !(-e "$root/scripts/spdxcheck.py") || !(-e "$root/.git")); + + my $root_path = abs_path($root); + my $status = `cd "$root_path"; echo "$license" | python scripts/spdxcheck.py -`; + return 0 if ($status ne ""); + return 1; +} + my $camelcase_seeded = 0; sub seed_camelcase_includes { return if ($camelcase_seeded); @@ -856,7 +906,7 @@ sub seed_camelcase_includes { $camelcase_seeded = 1; if (-e ".git") { - my $git_last_include_commit = `git log --no-merges --pretty=format:"%h%n" -1 -- include`; + my $git_last_include_commit = `${git_command} log --no-merges --pretty=format:"%h%n" -1 -- include`; chomp $git_last_include_commit; $camelcase_cache = ".checkpatch-camelcase.git.$git_last_include_commit"; } else { @@ -884,7 +934,7 @@ sub seed_camelcase_includes { } if (-e ".git") { - $files = `git ls-files "include/*.h"`; + $files = `${git_command} ls-files "include/*.h"`; @include_files = split('\n', $files); } @@ -908,13 +958,13 @@ sub git_commit_info { return ($id, $desc) if ((which("git") eq "") || !(-e ".git")); - my $output = `git log --no-color --format='%H %s' -1 $commit 2>&1`; + my $output = `${git_command} log --no-color --format='%H %s' -1 $commit 2>&1`; $output =~ s/^\s*//gm; my @lines = split("\n", $output); return ($id, $desc) if ($#lines < 0); - if ($lines[0] =~ /^error: short SHA1 $commit is ambiguous\./) { + if ($lines[0] =~ /^error: short SHA1 $commit is ambiguous/) { # Maybe one day convert this block of bash into something that returns # all matching commit ids, but it's very slow... # @@ -958,7 +1008,7 @@ if ($git) { } else { $git_range = "-1 $commit_expr"; } - my $lines = `git log --no-color --no-merges --pretty=format:'%H %s' $git_range`; + my $lines = `${git_command} log --no-color --no-merges --pretty=format:'%H %s' $git_range`; foreach my $line (split(/\n/, $lines)) { $line =~ /^([0-9a-fA-F]{40,40}) (.*)$/; next if (!defined($1) || !defined($2)); @@ -973,6 +1023,7 @@ if ($git) { } my $vname; +$allow_c99_comments = !defined $ignore_type{"C99_COMMENT_TOLERANCE"}; for my $filename (@ARGV) { my $FILE; if ($git) { @@ -1024,11 +1075,11 @@ if (!$quiet) { hash_show_words(\%use_type, "Used"); hash_show_words(\%ignore_type, "Ignored"); - if ($^V lt 5.10.0) { + if (!$perl_version_ok) { print << "EOM" NOTE: perl $^V is not modern enough to detect all possible issues. - An upgrade to at least perl v5.10.0 is suggested. + An upgrade to at least perl $minimum_perl_version is suggested. EOM } if ($exit) { @@ -2233,10 +2284,14 @@ sub process { our $clean = 1; my $signoff = 0; + my $author = ''; + my $authorsignoff = 0; my $is_patch = 0; + my $is_binding_patch = -1; my $in_header_lines = $file ? 0 : 1; my $in_commit_log = 0; #Scanning lines before patch my $has_commit_log = 0; #Encountered lines before patch + my $commit_log_lines = 0; #Number of commit log lines my $commit_log_possible_stack_dump = 0; my $commit_log_long_line = 0; my $commit_log_has_diff = 0; @@ -2375,6 +2430,14 @@ sub process { my $rawline = $rawlines[$linenr - 1]; +# check if it's a mode change, rename or start of a patch + if (!$in_commit_log && + ($line =~ /^ mode change [0-7]+ => [0-7]+ \S+\s*$/ || + ($line =~ /^rename (?:from|to) \S+\s*$/ || + $line =~ /^diff --git a\/[\w\/\.\_\-]+ b\/\S+\s*$/))) { + $is_patch = 1; + } + #extract the line range in the file after the patch is applied if (!$in_commit_log && $line =~ /^\@\@ -\d+(?:,\d+)? \+(\d+)(,(\d+))? \@\@(.*)/) { @@ -2475,6 +2538,19 @@ sub process { $check = $check_orig; } $checklicenseline = 1; + + if ($realfile !~ /^MAINTAINERS/) { + my $last_binding_patch = $is_binding_patch; + + $is_binding_patch = () = $realfile =~ m@^(?:Documentation/devicetree/|include/dt-bindings/)@; + + if (($last_binding_patch != -1) && + ($last_binding_patch ^ $is_binding_patch)) { + WARN("DT_SPLIT_BINDING_PATCH", + "DT binding docs and includes should be a separate patch. See: Documentation/devicetree/bindings/submitting-patches.txt\n"); + } + } + next; } @@ -2486,6 +2562,18 @@ sub process { $cnt_lines++ if ($realcnt != 0); +# Verify the existence of a commit log if appropriate +# 2 is used because a $signature is counted in $commit_log_lines + if ($in_commit_log) { + if ($line !~ /^\s*$/) { + $commit_log_lines++; #could be a $signature + } + } elsif ($has_commit_log && $commit_log_lines < 2) { + WARN("COMMIT_MESSAGE", + "Missing commit description - Add an appropriate one\n"); + $commit_log_lines = 2; #warn only once + } + # Check if the commit log has what seems like a diff which can confuse patch if ($in_commit_log && !$commit_log_has_diff && (($line =~ m@^\s+diff\b.*a/[\w/]+@ && @@ -2507,10 +2595,24 @@ sub process { } } +# Check the patch for a From: + if (decode("MIME-Header", $line) =~ /^From:\s*(.*)/) { + $author = $1; + $author = encode("utf8", $author) if ($line =~ /=\?utf-8\?/i); + $author =~ s/"//g; + } + # Check the patch for a signoff: if ($line =~ /^\s*signed-off-by:/i) { $signoff++; $in_commit_log = 0; + if ($author ne '') { + my $l = $line; + $l =~ s/"//g; + if ($l =~ /^\s*signed-off-by:\s*\Q$author\E/i) { + $authorsignoff = 1; + } + } } # Check if MAINTAINERS is being updated. If so, there's probably no need to @@ -2587,6 +2689,24 @@ sub process { } else { $signatures{$sig_nospace} = 1; } + +# Check Co-developed-by: immediately followed by Signed-off-by: with same name and email + if ($sign_off =~ /^co-developed-by:$/i) { + if ($email eq $author) { + WARN("BAD_SIGN_OFF", + "Co-developed-by: should not be used to attribute nominal patch author '$author'\n" . "$here\n" . $rawline); + } + if (!defined $lines[$linenr]) { + WARN("BAD_SIGN_OFF", + "Co-developed-by: must be immediately followed by Signed-off-by:\n" . "$here\n" . $rawline); + } elsif ($rawlines[$linenr] !~ /^\s*signed-off-by:\s*(.*)/i) { + WARN("BAD_SIGN_OFF", + "Co-developed-by: must be immediately followed by Signed-off-by:\n" . "$here\n" . $rawline . "\n" .$rawlines[$linenr]); + } elsif ($1 ne $email) { + WARN("BAD_SIGN_OFF", + "Co-developed-by and Signed-off-by: name/email do not match \n" . "$here\n" . $rawline . "\n" .$rawlines[$linenr]); + } + } } # Check email subject for common tools that don't need to be mentioned @@ -2596,12 +2716,6 @@ sub process { "A patch subject line should describe the change not the tool that found it\n" . $herecurr); } -# Check for old stable address - if ($line =~ /^\s*cc:\s*.*<?\bstable\@kernel\.org\b>?.*$/i) { - ERROR("STABLE_ADDRESS", - "The 'stable' address should be 'stable\@vger.kernel.org'\n" . $herecurr); - } - # Check for unwanted Gerrit info if ($in_commit_log && $line =~ /^\s*change-id:/i) { ERROR("GERRIT_CHANGE_ID", @@ -2613,8 +2727,10 @@ sub process { ($line =~ /^\s*(?:WARNING:|BUG:)/ || $line =~ /^\s*\[\s*\d+\.\d{6,6}\s*\]/ || # timestamp - $line =~ /^\s*\[\<[0-9a-fA-F]{8,}\>\]/)) { - # stack dump address + $line =~ /^\s*\[\<[0-9a-fA-F]{8,}\>\]/) || + $line =~ /^(?:\s+\w+:\s+[0-9a-fA-F]+){3,3}/ || + $line =~ /^\s*\#\d+\s*\[[0-9a-fA-F]+\]\s*\w+ at [0-9a-fA-F]+/) { + # stack dump address styles $commit_log_possible_stack_dump = 1; } @@ -2786,6 +2902,17 @@ sub process { } } +# check for invalid commit id + if ($in_commit_log && $line =~ /(^fixes:|\bcommit)\s+([0-9a-f]{6,40})\b/i) { + my $id; + my $description; + ($id, $description) = git_commit_info($2, undef, undef); + if (!defined($id)) { + WARN("UNKNOWN_COMMIT_ID", + "Unknown commit id '$2', maybe rebased or not pulled?\n" . $herecurr); + } + } + # ignore non-hunk lines and lines being removed next if (!$hunk_line || $line =~ /^-/); @@ -2915,7 +3042,7 @@ sub process { my @compats = $rawline =~ /\"([a-zA-Z0-9\-\,\.\+_]+)\"/g; my $dt_path = $root . "/Documentation/devicetree/bindings/"; - my $vp_file = $dt_path . "vendor-prefixes.txt"; + my $vp_file = $dt_path . "vendor-prefixes.yaml"; foreach my $compat (@compats) { my $compat2 = $compat; @@ -2930,7 +3057,7 @@ sub process { next if $compat !~ /^([a-zA-Z0-9\-]+)\,/; my $vendor = $1; - `grep -Eq "^$vendor\\b" $vp_file`; + `grep -Eq "\\"\\^\Q$vendor\E,\\.\\*\\":" $vp_file`; if ( $? >> 8 ) { WARN("UNDOCUMENTED_DT_STRING", "DT compatible string vendor \"$vendor\" appears un-documented -- check $vp_file\n" . $herecurr); @@ -2954,10 +3081,24 @@ sub process { $comment = '..'; } +# check SPDX comment style for .[chsS] files + if ($realfile =~ /\.[chsS]$/ && + $rawline =~ /SPDX-License-Identifier:/ && + $rawline !~ m@^\+\s*\Q$comment\E\s*@) { + WARN("SPDX_LICENSE_TAG", + "Improper SPDX comment style for '$realfile', please use '$comment' instead\n" . $herecurr); + } + if ($comment !~ /^$/ && - $rawline !~ /^\+\Q$comment\E SPDX-License-Identifier: /) { + $rawline !~ m@^\+\Q$comment\E SPDX-License-Identifier: @) { WARN("SPDX_LICENSE_TAG", "Missing or malformed SPDX-License-Identifier tag in line $checklicenseline\n" . $herecurr); + } elsif ($rawline =~ /(SPDX-License-Identifier: .*)/) { + my $spdx_license = $1; + if (!is_SPDX_License_valid($spdx_license)) { + WARN("SPDX_LICENSE_TAG", + "'$spdx_license' is not supported in LICENSES/...\n" . $herecurr); + } } } } @@ -2965,6 +3106,14 @@ sub process { # check we are in a valid source file if not then ignore this hunk next if ($realfile !~ /\.(h|c|s|S|sh|dtsi|dts)$/); +# check for using SPDX-License-Identifier on the wrong line number + if ($realline != $checklicenseline && + $rawline =~ /\bSPDX-License-Identifier:/ && + substr($line, @-, @+ - @-) eq "$;" x (@+ - @-)) { + WARN("SPDX_LICENSE_TAG", + "Misplaced SPDX-License-Identifier tag - use line $checklicenseline instead\n" . $herecurr); + } + # line length limit (with some exclusions) # # There are a few types of lines that may extend beyond $max_line_length: @@ -3062,6 +3211,12 @@ sub process { } } +# check for assignments on the start of a line + if ($sline =~ /^\+\s+($Assignment)[^=]/) { + CHK("ASSIGNMENT_CONTINUATIONS", + "Assignment operator '$1' should be on the previous line\n" . $hereprev); + } + # check for && or || at the start of a line if ($rawline =~ /^\+\s*(&&|\|\|)/) { CHK("LOGICAL_CONTINUATIONS", @@ -3069,7 +3224,7 @@ sub process { } # check indentation starts on a tab stop - if ($^V && $^V ge 5.10.0 && + if ($perl_version_ok && $sline =~ /^\+\t+( +)(?:$c90_Keywords\b|\{\s*$|\}\s*(?:else\b|while\b|\s*$)|$Declare\s*$Ident\s*[;=])/) { my $indent = length($1); if ($indent % 8) { @@ -3082,7 +3237,7 @@ sub process { } # check multi-line statement indentation matches previous line - if ($^V && $^V ge 5.10.0 && + if ($perl_version_ok && $prevline =~ /^\+([ \t]*)((?:$c90_Keywords(?:\s+if)\s*)|(?:$Declare\s*)?(?:$Ident|\(\s*\*\s*$Ident\s*\))\s*|(?:\*\s*)*$Lval\s*=\s*$Ident\s*)\(.*(\&\&|\|\||,)\s*$/) { $prevline =~ /^\+(\t*)(.*)$/; my $oldindent = $1; @@ -3239,7 +3394,7 @@ sub process { # known declaration macros $sline =~ /^\+\s+$declaration_macros/ || # start of struct or union or enum - $sline =~ /^\+\s+(?:union|struct|enum|typedef)\b/ || + $sline =~ /^\+\s+(?:static\s+)?(?:const\s+)?(?:union|struct|enum|typedef)\b/ || # start or end of block or continuation of declaration $sline =~ /^\+\s+(?:$|[\{\}\.\#\"\?\:\(\[])/ || # bitfield continuation @@ -3771,19 +3926,48 @@ sub process { "type '$tmp' should be specified in [[un]signed] [short|int|long|long long] order\n" . $herecurr); } +# check for unnecessary <signed> int declarations of short/long/long long + while ($sline =~ m{\b($TypeMisordered(\s*\*)*|$C90_int_types)\b}g) { + my $type = trim($1); + next if ($type !~ /\bint\b/); + next if ($type !~ /\b(?:short|long\s+long|long)\b/); + my $new_type = $type; + $new_type =~ s/\b\s*int\s*\b/ /; + $new_type =~ s/\b\s*(?:un)?signed\b\s*/ /; + $new_type =~ s/^const\s+//; + $new_type = "unsigned $new_type" if ($type =~ /\bunsigned\b/); + $new_type = "const $new_type" if ($type =~ /^const\b/); + $new_type =~ s/\s+/ /g; + $new_type = trim($new_type); + if (WARN("UNNECESSARY_INT", + "Prefer '$new_type' over '$type' as the int is unnecessary\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/\b\Q$type\E\b/$new_type/; + } + } + # check for static const char * arrays. if ($line =~ /\bstatic\s+const\s+char\s*\*\s*(\w+)\s*\[\s*\]\s*=\s*/) { WARN("STATIC_CONST_CHAR_ARRAY", "static const char * array should probably be static const char * const\n" . $herecurr); - } + } + +# check for initialized const char arrays that should be static const + if ($line =~ /^\+\s*const\s+(char|unsigned\s+char|_*u8|(?:[us]_)?int8_t)\s+\w+\s*\[\s*(?:\w+\s*)?\]\s*=\s*"/) { + if (WARN("STATIC_CONST_CHAR_ARRAY", + "const array should probably be static const\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/(^.\s*)const\b/${1}static const/; + } + } # check for static char foo[] = "bar" declarations. if ($line =~ /\bstatic\s+char\s+(\w+)\s*\[\s*\]\s*=\s*"/) { WARN("STATIC_CONST_CHAR_ARRAY", "static char array declaration should probably be static const char\n" . $herecurr); - } + } # check for const <foo> const where <foo> is not a pointer or array type if ($sline =~ /\bconst\s+($BasicType)\s+const\b/) { @@ -3957,7 +4141,7 @@ sub process { # function brace can't be on same line, except for #defines of do while, # or if closed on same line - if ($^V && $^V ge 5.10.0 && + if ($perl_version_ok && $sline =~ /$Type\s*$Ident\s*$balanced_parens\s*\{/ && $sline !~ /\#\s*define\b.*do\s*\{/ && $sline !~ /}/) { @@ -4083,7 +4267,7 @@ sub process { my ($where, $prefix) = ($-[1], $1); if ($prefix !~ /$Type\s+$/ && ($where != 0 || $prefix !~ /^.\s+$/) && - $prefix !~ /[{,]\s+$/) { + $prefix !~ /[{,:]\s+$/) { if (ERROR("BRACKET_SPACE", "space prohibited before open square bracket '['\n" . $herecurr) && $fix) { @@ -4473,11 +4657,11 @@ sub process { #need space before brace following if, while, etc if (($line =~ /\(.*\)\{/ && $line !~ /\($Type\)\{/) || - $line =~ /do\{/) { + $line =~ /\b(?:else|do)\{/) { if (ERROR("SPACING", "space required before the open brace '{'\n" . $herecurr) && $fix) { - $fixed[$fixlinenr] =~ s/^(\+.*(?:do|\)))\{/$1 {/; + $fixed[$fixlinenr] =~ s/^(\+.*(?:do|else|\)))\{/$1 {/; } } @@ -4491,7 +4675,7 @@ sub process { # closing brace should have a space following it when it has anything # on the line - if ($line =~ /}(?!(?:,|;|\)))\S/) { + if ($line =~ /}(?!(?:,|;|\)|\}))\S/) { if (ERROR("SPACING", "space required after that close brace '}'\n" . $herecurr) && $fix) { @@ -4568,7 +4752,7 @@ sub process { # check for unnecessary parentheses around comparisons in if uses # when !drivers/staging or command-line uses --strict if (($realfile !~ m@^(?:drivers/staging/)@ || $check_orig) && - $^V && $^V ge 5.10.0 && defined($stat) && + $perl_version_ok && defined($stat) && $stat =~ /(^.\s*if\s*($balanced_parens))/) { my $if_stat = $1; my $test = substr($2, 1, -1); @@ -4605,7 +4789,7 @@ sub process { # return is not a function if (defined($stat) && $stat =~ /^.\s*return(\s*)\(/s) { my $spacing = $1; - if ($^V && $^V ge 5.10.0 && + if ($perl_version_ok && $stat =~ /^.\s*return\s*($balanced_parens)\s*;\s*$/) { my $value = $1; $value = deparenthesize($value); @@ -4632,7 +4816,7 @@ sub process { } # if statements using unnecessary parentheses - ie: if ((foo == bar)) - if ($^V && $^V ge 5.10.0 && + if ($perl_version_ok && $line =~ /\bif\s*((?:\(\s*){2,})/) { my $openparens = $1; my $count = $openparens =~ tr@\(@\(@; @@ -4649,7 +4833,7 @@ sub process { # avoid cases like "foo + BAR < baz" # only fix matches surrounded by parentheses to avoid incorrect # conversions like "FOO < baz() + 5" being "misfixed" to "baz() > FOO + 5" - if ($^V && $^V ge 5.10.0 && + if ($perl_version_ok && $line =~ /^\+(.*)\b($Constant|[A-Z_][A-Z0-9_]*)\s*($Compare)\s*($LvalOrFunc)/) { my $lead = $1; my $const = $2; @@ -4841,17 +5025,6 @@ sub process { while ($line =~ m{($Constant|$Lval)}g) { my $var = $1; -#gcc binary extension - if ($var =~ /^$Binary$/) { - if (WARN("GCC_BINARY_CONSTANT", - "Avoid gcc v4.3+ binary constant extension: <$var>\n" . $herecurr) && - $fix) { - my $hexval = sprintf("0x%x", oct($var)); - $fixed[$fixlinenr] =~ - s/\b$var\b/$hexval/; - } - } - #CamelCase if ($var !~ /^$Constant$/ && $var =~ /[A-Z][a-z]|[a-z][A-Z]/ && @@ -4939,6 +5112,7 @@ sub process { if (defined $define_args && $define_args ne "") { $define_args = substr($define_args, 1, length($define_args) - 2); $define_args =~ s/\s*//g; + $define_args =~ s/\\\+?//g; @def_args = split(",", $define_args); } @@ -5032,10 +5206,10 @@ sub process { next if ($arg =~ /\.\.\./); next if ($arg =~ /^type$/i); my $tmp_stmt = $define_stmt; - $tmp_stmt =~ s/\b(typeof|__typeof__|__builtin\w+|typecheck\s*\(\s*$Type\s*,|\#+)\s*\(*\s*$arg\s*\)*\b//g; + $tmp_stmt =~ s/\b(sizeof|typeof|__typeof__|__builtin\w+|typecheck\s*\(\s*$Type\s*,|\#+)\s*\(*\s*$arg\s*\)*\b//g; $tmp_stmt =~ s/\#+\s*$arg\b//g; $tmp_stmt =~ s/\b$arg\s*\#\#//g; - my $use_cnt = $tmp_stmt =~ s/\b$arg\b//g; + my $use_cnt = () = $tmp_stmt =~ /\b$arg\b/g; if ($use_cnt > 1) { CHK("MACRO_ARG_REUSE", "Macro argument reuse '$arg' - possible side-effects?\n" . "$herectx"); @@ -5074,7 +5248,7 @@ sub process { # do {} while (0) macro tests: # single-statement macros do not need to be enclosed in do while (0) loop, # macro should not end with a semicolon - if ($^V && $^V ge 5.10.0 && + if ($perl_version_ok && $realfile !~ m@/vmlinux.lds.h$@ && $line =~ /^.\s*\#\s*define\s+$Ident(\()?/) { my $ln = $linenr; @@ -5115,16 +5289,6 @@ sub process { } } -# make sure symbols are always wrapped with VMLINUX_SYMBOL() ... -# all assignments may have only one of the following with an assignment: -# . -# ALIGN(...) -# VMLINUX_SYMBOL(...) - if ($realfile eq 'vmlinux.lds.h' && $line =~ /(?:(?:^|\s)$Ident\s*=|=\s*$Ident(?:\s|$))/) { - WARN("MISSING_VMLINUX_SYMBOL", - "vmlinux.lds.h needs VMLINUX_SYMBOL() around C-visible symbols\n" . $herecurr); - } - # check for redundant bracing round if etc if ($line =~ /(^.*)\bif\b/ && $1 !~ /else\s*$/) { my ($level, $endln, @chunks) = @@ -5330,15 +5494,28 @@ sub process { } # concatenated string without spaces between elements - if ($line =~ /$String[A-Z_]/ || $line =~ /[A-Za-z0-9_]$String/) { - CHK("CONCATENATED_STRING", - "Concatenated strings should use spaces between elements\n" . $herecurr); + if ($line =~ /$String[A-Za-z0-9_]/ || $line =~ /[A-Za-z0-9_]$String/) { + if (CHK("CONCATENATED_STRING", + "Concatenated strings should use spaces between elements\n" . $herecurr) && + $fix) { + while ($line =~ /($String)/g) { + my $extracted_string = substr($rawline, $-[0], $+[0] - $-[0]); + $fixed[$fixlinenr] =~ s/\Q$extracted_string\E([A-Za-z0-9_])/$extracted_string $1/; + $fixed[$fixlinenr] =~ s/([A-Za-z0-9_])\Q$extracted_string\E/$1 $extracted_string/; + } + } } # uncoalesced string fragments if ($line =~ /$String\s*"/) { - WARN("STRING_FRAGMENTS", - "Consecutive strings are generally better as a single string\n" . $herecurr); + if (WARN("STRING_FRAGMENTS", + "Consecutive strings are generally better as a single string\n" . $herecurr) && + $fix) { + while ($line =~ /($String)(?=\s*")/g) { + my $extracted_string = substr($rawline, $-[0], $+[0] - $-[0]); + $fixed[$fixlinenr] =~ s/\Q$extracted_string\E\s*"/substr($extracted_string, 0, -1)/e; + } + } } # check for non-standard and hex prefixed decimal printf formats @@ -5374,9 +5551,14 @@ sub process { # warn about #if 0 if ($line =~ /^.\s*\#\s*if\s+0\b/) { - CHK("REDUNDANT_CODE", - "if this code is redundant consider removing it\n" . - $herecurr); + WARN("IF_0", + "Consider removing the code enclosed by this #if 0 and its #endif\n" . $herecurr); + } + +# warn about #if 1 + if ($line =~ /^.\s*\#\s*if\s+1\b/) { + WARN("IF_1", + "Consider removing the #if 1 and its #endif\n" . $herecurr); } # check for needless "if (<foo>) fn(<foo>)" uses @@ -5423,7 +5605,8 @@ sub process { my ($s, $c) = ctx_statement_block($linenr - 3, $realcnt, 0); # print("line: <$line>\nprevline: <$prevline>\ns: <$s>\nc: <$c>\n\n\n"); - if ($s =~ /(?:^|\n)[ \+]\s*(?:$Type\s*)?\Q$testval\E\s*=\s*(?:\([^\)]*\)\s*)?\s*(?:devm_)?(?:[kv][czm]alloc(?:_node|_array)?\b|kstrdup|kmemdup|(?:dev_)?alloc_skb)/) { + if ($s =~ /(?:^|\n)[ \+]\s*(?:$Type\s*)?\Q$testval\E\s*=\s*(?:\([^\)]*\)\s*)?\s*$allocFunctions\s*\(/ && + $s !~ /\b__GFP_NOWARN\b/ ) { WARN("OOM_MESSAGE", "Possible unnecessary 'out of memory' message\n" . $hereprev); } @@ -5447,7 +5630,7 @@ sub process { } # check for mask then right shift without a parentheses - if ($^V && $^V ge 5.10.0 && + if ($perl_version_ok && $line =~ /$LvalOrFunc\s*\&\s*($LvalOrFunc)\s*>>/ && $4 !~ /^\&/) { # $LvalOrFunc may be &foo, ignore if so WARN("MASK_THEN_SHIFT", @@ -5455,7 +5638,7 @@ sub process { } # check for pointer comparisons to NULL - if ($^V && $^V ge 5.10.0) { + if ($perl_version_ok) { while ($line =~ /\b$LvalOrFunc\s*(==|\!=)\s*NULL\b/g) { my $val = $1; my $equal = "!"; @@ -5544,7 +5727,7 @@ sub process { # ignore udelay's < 10, however if (! ($delay < 10) ) { CHK("USLEEP_RANGE", - "usleep_range is preferred over udelay; see Documentation/timers/timers-howto.txt\n" . $herecurr); + "usleep_range is preferred over udelay; see Documentation/timers/timers-howto.rst\n" . $herecurr); } if ($delay > 2000) { WARN("LONG_UDELAY", @@ -5556,7 +5739,7 @@ sub process { if ($line =~ /\bmsleep\s*\((\d+)\);/) { if ($1 < 20) { WARN("MSLEEP", - "msleep < 20ms can sleep for up to 20ms; see Documentation/timers/timers-howto.txt\n" . $herecurr); + "msleep < 20ms can sleep for up to 20ms; see Documentation/timers/timers-howto.rst\n" . $herecurr); } } @@ -5698,13 +5881,6 @@ sub process { "__packed is preferred over __attribute__((packed))\n" . $herecurr); } -# Check for new packed members, warn to use care - if ($realfile !~ m@\binclude/uapi/@ && - $line =~ /\b(__attribute__\s*\(\s*\(.*\bpacked|__packed)\b/) { - WARN("NEW_PACKED", - "Adding new packed members is to be done with care\n" . $herecurr); - } - # Check for __attribute__ aligned, prefer __aligned if ($realfile !~ m@\binclude/uapi/@ && $line =~ /\b__attribute__\s*\(\s*\(.*aligned/) { @@ -5712,6 +5888,18 @@ sub process { "__aligned(size) is preferred over __attribute__((aligned(size)))\n" . $herecurr); } +# Check for __attribute__ section, prefer __section + if ($realfile !~ m@\binclude/uapi/@ && + $line =~ /\b__attribute__\s*\(\s*\(.*_*section_*\s*\(\s*("[^"]*")/) { + my $old = substr($rawline, $-[1], $+[1] - $-[1]); + my $new = substr($old, 1, -1); + if (WARN("PREFER_SECTION", + "__section($new) is preferred over __attribute__((section($old)))\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/\b__attribute__\s*\(\s*\(\s*_*section_*\s*\(\s*\Q$old\E\s*\)\s*\)\s*\)/__section($new)/; + } + } + # Check for __attribute__ format(printf, prefer __printf if ($realfile !~ m@\binclude/uapi/@ && $line =~ /\b__attribute__\s*\(\s*\(\s*format\s*\(\s*printf/) { @@ -5734,7 +5922,7 @@ sub process { } # Check for __attribute__ weak, or __weak declarations (may have link issues) - if ($^V && $^V ge 5.10.0 && + if ($perl_version_ok && $line =~ /(?:$Declare|$DeclareMisordered)\s*$Ident\s*$balanced_parens\s*(?:$Attribute)?\s*;/ && ($line =~ /\b__attribute__\s*\(\s*\(.*\bweak\b/ || $line =~ /\b__weak\b/)) { @@ -5816,25 +6004,25 @@ sub process { } # check for vsprintf extension %p<foo> misuses - if ($^V && $^V ge 5.10.0 && + if ($perl_version_ok && defined $stat && $stat =~ /^\+(?![^\{]*\{\s*).*\b(\w+)\s*\(.*$String\s*,/s && $1 !~ /^_*volatile_*$/) { - my $specifier; - my $extension; - my $bad_specifier = ""; my $stat_real; my $lc = $stat =~ tr@\n@@; $lc = $lc + $linenr; for (my $count = $linenr; $count <= $lc; $count++) { + my $specifier; + my $extension; + my $bad_specifier = ""; my $fmt = get_quoted_string($lines[$count - 1], raw_line($count, 0)); $fmt =~ s/%%//g; while ($fmt =~ /(\%[\*\d\.]*p(\w))/g) { $specifier = $1; $extension = $2; - if ($extension !~ /[SsBKRraEhMmIiUDdgVCbGNOx]/) { + if ($extension !~ /[SsBKRraEhMmIiUDdgVCbGNOxt]/) { $bad_specifier = $specifier; last; } @@ -5863,7 +6051,7 @@ sub process { } # Check for misused memsets - if ($^V && $^V ge 5.10.0 && + if ($perl_version_ok && defined $stat && $stat =~ /^\+(?:.*?)\bmemset\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\,\s*$FuncArg\s*\)/) { @@ -5881,7 +6069,7 @@ sub process { } # Check for memcpy(foo, bar, ETH_ALEN) that could be ether_addr_copy(foo, bar) -# if ($^V && $^V ge 5.10.0 && +# if ($perl_version_ok && # defined $stat && # $stat =~ /^\+(?:.*?)\bmemcpy\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\,\s*ETH_ALEN\s*\)/) { # if (WARN("PREFER_ETHER_ADDR_COPY", @@ -5892,7 +6080,7 @@ sub process { # } # Check for memcmp(foo, bar, ETH_ALEN) that could be ether_addr_equal*(foo, bar) -# if ($^V && $^V ge 5.10.0 && +# if ($perl_version_ok && # defined $stat && # $stat =~ /^\+(?:.*?)\bmemcmp\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\,\s*ETH_ALEN\s*\)/) { # WARN("PREFER_ETHER_ADDR_EQUAL", @@ -5901,7 +6089,7 @@ sub process { # check for memset(foo, 0x0, ETH_ALEN) that could be eth_zero_addr # check for memset(foo, 0xFF, ETH_ALEN) that could be eth_broadcast_addr -# if ($^V && $^V ge 5.10.0 && +# if ($perl_version_ok && # defined $stat && # $stat =~ /^\+(?:.*?)\bmemset\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\,\s*ETH_ALEN\s*\)/) { # @@ -5923,7 +6111,7 @@ sub process { # } # typecasts on min/max could be min_t/max_t - if ($^V && $^V ge 5.10.0 && + if ($perl_version_ok && defined $stat && $stat =~ /^\+(?:.*?)\b(min|max)\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\)/) { if (defined $2 || defined $7) { @@ -5947,23 +6135,23 @@ sub process { } # check usleep_range arguments - if ($^V && $^V ge 5.10.0 && + if ($perl_version_ok && defined $stat && $stat =~ /^\+(?:.*?)\busleep_range\s*\(\s*($FuncArg)\s*,\s*($FuncArg)\s*\)/) { my $min = $1; my $max = $7; if ($min eq $max) { WARN("USLEEP_RANGE", - "usleep_range should not use min == max args; see Documentation/timers/timers-howto.txt\n" . "$here\n$stat\n"); + "usleep_range should not use min == max args; see Documentation/timers/timers-howto.rst\n" . "$here\n$stat\n"); } elsif ($min =~ /^\d+$/ && $max =~ /^\d+$/ && $min > $max) { WARN("USLEEP_RANGE", - "usleep_range args reversed, use min then max; see Documentation/timers/timers-howto.txt\n" . "$here\n$stat\n"); + "usleep_range args reversed, use min then max; see Documentation/timers/timers-howto.rst\n" . "$here\n$stat\n"); } } # check for naked sscanf - if ($^V && $^V ge 5.10.0 && + if ($perl_version_ok && defined $stat && $line =~ /\bsscanf\b/ && ($stat !~ /$Ident\s*=\s*sscanf\s*$balanced_parens/ && @@ -5977,7 +6165,7 @@ sub process { } # check for simple sscanf that should be kstrto<foo> - if ($^V && $^V ge 5.10.0 && + if ($perl_version_ok && defined $stat && $line =~ /\bsscanf\b/) { my $lc = $stat =~ tr@\n@@; @@ -6049,7 +6237,7 @@ sub process { } # check for function definitions - if ($^V && $^V ge 5.10.0 && + if ($perl_version_ok && defined $stat && $stat =~ /^.\s*(?:$Storage\s+)?$Type\s*($Ident)\s*$balanced_parens\s*{/s) { $context_function = $1; @@ -6081,22 +6269,22 @@ sub process { } } -# check for pointless casting of kmalloc return - if ($line =~ /\*\s*\)\s*[kv][czm]alloc(_node){0,1}\b/) { +# check for pointless casting of alloc functions + if ($line =~ /\*\s*\)\s*$allocFunctions\b/) { WARN("UNNECESSARY_CASTS", "unnecessary cast may hide bugs, see http://c-faq.com/malloc/mallocnocast.html\n" . $herecurr); } # alloc style # p = alloc(sizeof(struct foo), ...) should be p = alloc(sizeof(*p), ...) - if ($^V && $^V ge 5.10.0 && - $line =~ /\b($Lval)\s*\=\s*(?:$balanced_parens)?\s*([kv][mz]alloc(?:_node)?)\s*\(\s*(sizeof\s*\(\s*struct\s+$Lval\s*\))/) { + if ($perl_version_ok && + $line =~ /\b($Lval)\s*\=\s*(?:$balanced_parens)?\s*((?:kv|k|v)[mz]alloc(?:_node)?)\s*\(\s*(sizeof\s*\(\s*struct\s+$Lval\s*\))/) { CHK("ALLOC_SIZEOF_STRUCT", "Prefer $3(sizeof(*$1)...) over $3($4...)\n" . $herecurr); } # check for k[mz]alloc with multiplies that could be kmalloc_array/kcalloc - if ($^V && $^V ge 5.10.0 && + if ($perl_version_ok && defined $stat && $stat =~ /^\+\s*($Lval)\s*\=\s*(?:$balanced_parens)?\s*(k[mz]alloc)\s*\(\s*($FuncArg)\s*\*\s*($FuncArg)\s*,/) { my $oldfunc = $3; @@ -6125,8 +6313,9 @@ sub process { } # check for krealloc arg reuse - if ($^V && $^V ge 5.10.0 && - $line =~ /\b($Lval)\s*\=\s*(?:$balanced_parens)?\s*krealloc\s*\(\s*\1\s*,/) { + if ($perl_version_ok && + $line =~ /\b($Lval)\s*\=\s*(?:$balanced_parens)?\s*krealloc\s*\(\s*($Lval)\s*,/ && + $1 eq $3) { WARN("KREALLOC_ARG_REUSE", "Reusing the krealloc arg is almost always a bug\n" . $herecurr); } @@ -6194,7 +6383,7 @@ sub process { } # check for switch/default statements without a break; - if ($^V && $^V ge 5.10.0 && + if ($perl_version_ok && defined $stat && $stat =~ /^\+[$;\s]*(?:case[$;\s]+\w+[$;\s]*:[$;\s]*|)*[$;\s]*\bdefault[$;\s]*:[$;\s]*;/g) { my $cnt = statement_rawlines($stat); @@ -6270,6 +6459,20 @@ sub process { "please use device_initcall() or more appropriate function instead of __initcall() (see include/linux/init.h)\n" . $herecurr); } +# check for spin_is_locked(), suggest lockdep instead + if ($line =~ /\bspin_is_locked\(/) { + WARN("USE_LOCKDEP", + "Where possible, use lockdep_assert_held instead of assertions based on spin_is_locked\n" . $herecurr); + } + +# check for deprecated apis + if ($line =~ /\b($deprecated_apis_search)\b\s*\(/) { + my $deprecated_api = $1; + my $new_api = $deprecated_apis{$deprecated_api}; + WARN("DEPRECATED_API", + "Deprecated use of '$deprecated_api', prefer '$new_api' instead\n" . $herecurr); + } + # check for various structs that are normally const (ops, kgdb, device_tree) # and avoid what seem like struct definitions 'struct foo {' if ($line !~ /\bconst\b/ && @@ -6298,12 +6501,18 @@ sub process { } # likely/unlikely comparisons similar to "(likely(foo) > 0)" - if ($^V && $^V ge 5.10.0 && + if ($perl_version_ok && $line =~ /\b((?:un)?likely)\s*\(\s*$FuncArg\s*\)\s*$Compare/) { WARN("LIKELY_MISUSE", "Using $1 should generally have parentheses around the comparison\n" . $herecurr); } +# nested likely/unlikely calls + if ($line =~ /\b(?:(?:un)?likely)\s*\(\s*!?\s*(IS_ERR(?:_OR_NULL|_VALUE)?|WARN)/) { + WARN("LIKELY_MISUSE", + "nested (un)?likely() calls, $1 already uses unlikely() internally\n" . $herecurr); + } + # whine mightly about in_atomic if ($line =~ /\bin_atomic\s*\(/) { if ($realfile =~ m@^drivers/@) { @@ -6341,7 +6550,7 @@ sub process { # check for DEVICE_ATTR uses that could be DEVICE_ATTR_<FOO> # and whether or not function naming is typical and if # DEVICE_ATTR permissions uses are unusual too - if ($^V && $^V ge 5.10.0 && + if ($perl_version_ok && defined $stat && $stat =~ /\bDEVICE_ATTR\s*\(\s*(\w+)\s*,\s*\(?\s*(\s*(?:${multi_mode_perms_string_search}|0[0-7]{3,3})\s*)\s*\)?\s*,\s*(\w+)\s*,\s*(\w+)\s*\)/) { my $var = $1; @@ -6401,7 +6610,7 @@ sub process { # specific definition of not visible in sysfs. # o Ignore proc_create*(...) uses with a decimal 0 permission as that means # use the default permissions - if ($^V && $^V ge 5.10.0 && + if ($perl_version_ok && defined $stat && $line =~ /$mode_perms_search/) { foreach my $entry (@mode_permission_funcs) { @@ -6463,6 +6672,12 @@ sub process { "unknown module license " . $extracted_string . "\n" . $herecurr); } } + +# check for sysctl duplicate constants + if ($line =~ /\.extra[12]\s*=\s*&(zero|one|int_max)\b/) { + WARN("DUPLICATED_SYSCTL_CONST", + "duplicated sysctl range checking value '$1', consider using the shared one in include/linux/sysctl.h\n" . $herecurr); + } } # If we have no input at all, then there is nothing to report on @@ -6487,9 +6702,14 @@ sub process { ERROR("NOT_UNIFIED_DIFF", "Does not appear to be a unified-diff format patch\n"); } - if ($is_patch && $has_commit_log && $chk_signoff && $signoff == 0) { - ERROR("MISSING_SIGN_OFF", - "Missing Signed-off-by: line(s)\n"); + if ($is_patch && $has_commit_log && $chk_signoff) { + if ($signoff == 0) { + ERROR("MISSING_SIGN_OFF", + "Missing Signed-off-by: line(s)\n"); + } elsif (!$authorsignoff) { + WARN("NO_AUTHOR_SIGN_OFF", + "Missing Signed-off-by: line by nominal patch author '$author'\n"); + } } print report_dump(); diff --git a/test/lib/Makefile b/test/lib/Makefile index 308c61708e..b13aaca7ce 100644 --- a/test/lib/Makefile +++ b/test/lib/Makefile @@ -6,3 +6,4 @@ obj-y += cmd_ut_lib.o obj-y += hexdump.o obj-y += lmb.o obj-y += string.o +obj-$(CONFIG_ERRNO_STR) += test_errno_str.o diff --git a/test/lib/test_errno_str.c b/test/lib/test_errno_str.c new file mode 100644 index 0000000000..8a9f1fd980 --- /dev/null +++ b/test/lib/test_errno_str.c @@ -0,0 +1,46 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright (c) 2019 Heinrich Schuchardt <xypron.glpk@gmx.de> + * + * Unit tests for memory functions + * + * The architecture dependent implementations run through different lines of + * code depending on the alignment and length of memory regions copied or set. + * This has to be considered in testing. + */ + +#include <common.h> +#include <command.h> +#include <errno.h> +#include <test/lib.h> +#include <test/test.h> +#include <test/ut.h> + +/** + * lib_errno_str() - unit test for errno_str() + * + * Test errno_str() with varied alignment and length of the copied buffer. + * + * @uts: unit test state + * Return: 0 = success, 1 = failure + */ +static int lib_errno_str(struct unit_test_state *uts) +{ + const char *msg; + + msg = errno_str(1); + ut_asserteq_str("Success", msg); + + msg = errno_str(0); + ut_asserteq_str("Success", msg); + + msg = errno_str(-ENOMEM); + ut_asserteq_str("Out of memory", msg); + + msg = errno_str(-99999); + ut_asserteq_str("Unknown error", msg); + + return 0; +} + +LIB_TEST(lib_errno_str, 0); diff --git a/test/py/README.md b/test/py/README.md index 2156661d6c..3cbe01b73e 100644 --- a/test/py/README.md +++ b/test/py/README.md @@ -21,19 +21,26 @@ involves executing some binary and interacting with its stdin/stdout. You will need to implement various "hook" scripts that are called by the test suite at the appropriate time. -On Debian or Debian-like distributions, the following packages are required. -Some packages are required to execute any test, and others only for specific -tests. Similar package names should exist in other distributions. - -| Package | Version tested (Ubuntu 14.04) | -| -------------- | ----------------------------- | -| python | 2.7.5-5ubuntu3 | -| python-pytest | 2.5.1-1 | -| python-subunit | - | -| gdisk | 0.8.8-1ubuntu0.1 | -| dfu-util | 0.5-1 | -| dtc | 1.4.0+dfsg-1 | -| openssl | 1.0.1f-1ubuntu2.22 | +In order to run the testsuite at a minimum we require that both python3 and +pip for python3 be installed. All of the required python modules are +described in the requirements.txt file in this directory and can be installed +with the command ```pip install -r requirements.txt``` + +In order to execute certain tests on their supported platforms other tools +will be required. The following is an incomplete list: + +| Package | +| -------------- | +| gdisk | +| dfu-util | +| dtc | +| openssl | +| sudo OR guestmount | +| e2fsprogs | +| dosfstools | + +Please use the apporirate commands for your distribution to match these tools +up with the package that provides them. The test script supports either: @@ -45,18 +52,16 @@ The test script supports either: ### Using `virtualenv` to provide requirements -Older distributions (e.g. Ubuntu 10.04) may not provide all the required -packages, or may provide versions that are too old to run the test suite. One -can use the Python `virtualenv` script to locally install more up-to-date -versions of the required packages without interfering with the OS installation. -For example: +The recommended way to run the test suite, in order to ensure reproducibility +is to use `virtualenv` to set up the necessary environment. This can be done +via the following commands: ```bash $ cd /path/to/u-boot -$ sudo apt-get install python python-virtualenv -$ virtualenv venv +$ sudo apt-get install python3 python3-virtualenv +$ virtualenv -p /usr/bin/python3 venv $ . ./venv/bin/activate -$ pip install pytest +$ pip install -r test/py/requirements.txt ``` ## Testing sandbox diff --git a/test/py/conftest.py b/test/py/conftest.py index 00d8ef8ba9..bffee6b8a3 100644 --- a/test/py/conftest.py +++ b/test/py/conftest.py @@ -13,20 +13,16 @@ # - Implementing custom pytest markers. import atexit +import configparser import errno +import io import os import os.path import pytest -from _pytest.runner import runtestprotocol import re -import StringIO +from _pytest.runner import runtestprotocol import sys -try: - import configparser -except: - import ConfigParser as configparser - # Globals: The HTML log file, and the connection to the U-Boot console. log = None console = None @@ -169,9 +165,9 @@ def pytest_configure(config): with open(dot_config, 'rt') as f: ini_str = '[root]\n' + f.read() - ini_sio = StringIO.StringIO(ini_str) + ini_sio = io.StringIO(ini_str) parser = configparser.RawConfigParser() - parser.readfp(ini_sio) + parser.read_file(ini_sio) ubconfig.buildconfig.update(parser.items('root')) ubconfig.test_py_dir = test_py_dir @@ -431,11 +427,9 @@ def setup_boardspec(item): Nothing. """ - mark = item.get_marker('boardspec') - if not mark: - return required_boards = [] - for board in mark.args: + for boards in item.iter_markers('boardspec'): + board = boards.args[0] if board.startswith('!'): if ubconfig.board_type == board[1:]: pytest.skip('board "%s" not supported' % ubconfig.board_type) @@ -459,16 +453,14 @@ def setup_buildconfigspec(item): Nothing. """ - mark = item.get_marker('buildconfigspec') - if mark: - for option in mark.args: - if not ubconfig.buildconfig.get('config_' + option.lower(), None): - pytest.skip('.config feature "%s" not enabled' % option.lower()) - notmark = item.get_marker('notbuildconfigspec') - if notmark: - for option in notmark.args: - if ubconfig.buildconfig.get('config_' + option.lower(), None): - pytest.skip('.config feature "%s" enabled' % option.lower()) + for options in item.iter_markers('buildconfigspec'): + option = options.args[0] + if not ubconfig.buildconfig.get('config_' + option.lower(), None): + pytest.skip('.config feature "%s" not enabled' % option.lower()) + for option in item.iter_markers('notbuildconfigspec'): + option = options.args[0] + if ubconfig.buildconfig.get('config_' + option.lower(), None): + pytest.skip('.config feature "%s" enabled' % option.lower()) def tool_is_in_path(tool): for path in os.environ["PATH"].split(os.pathsep): @@ -491,10 +483,8 @@ def setup_requiredtool(item): Nothing. """ - mark = item.get_marker('requiredtool') - if not mark: - return - for tool in mark.args: + for tools in item.iter_markers('requiredtool'): + tool = tools.args[0] if not tool_is_in_path(tool): pytest.skip('tool "%s" not in $PATH' % tool) diff --git a/test/py/multiplexed_log.py b/test/py/multiplexed_log.py index 637a3bd257..545a774302 100644 --- a/test/py/multiplexed_log.py +++ b/test/py/multiplexed_log.py @@ -5,8 +5,8 @@ # Generate an HTML-formatted log file containing multiple streams of data, # each represented in a well-delineated/-structured fashion. -import cgi import datetime +import html import os.path import shutil import subprocess @@ -51,7 +51,7 @@ class LogfileStream(object): """Write data to the log stream. Args: - data: The data to write tot he file. + data: The data to write to the file. implicit: Boolean indicating whether data actually appeared in the stream, or was implicitly generated. A valid use-case is to repeat a shell prompt at the start of each separate log @@ -64,7 +64,8 @@ class LogfileStream(object): self.logfile.write(self, data, implicit) if self.chained_file: - self.chained_file.write(data) + # Chained file is console, convert things a little + self.chained_file.write((data.encode('ascii', 'replace')).decode()) def flush(self): """Flush the log stream, to ensure correct log interleaving. @@ -136,6 +137,10 @@ class RunAndLog(object): p = subprocess.Popen(cmd, cwd=cwd, stdin=None, stdout=subprocess.PIPE, stderr=subprocess.STDOUT) (stdout, stderr) = p.communicate() + if stdout is not None: + stdout = stdout.decode('utf-8') + if stderr is not None: + stderr = stderr.decode('utf-8') output = '' if stdout: if stderr: @@ -215,7 +220,7 @@ class Logfile(object): Nothing. """ - self.f = open(fn, 'wt') + self.f = open(fn, 'wt', encoding='utf-8') self.last_stream = None self.blocks = [] self.cur_evt = 1 @@ -334,7 +339,7 @@ $(document).ready(function () { data = data.replace(chr(13), '') data = ''.join((ord(c) in self._nonprint) and ('%%%02x' % ord(c)) or c for c in data) - data = cgi.escape(data) + data = html.escape(data) return data def _terminate_stream(self): diff --git a/test/py/pytest.ini b/test/py/pytest.ini index 7e400682bf..e93d010f1f 100644 --- a/test/py/pytest.ini +++ b/test/py/pytest.ini @@ -8,3 +8,6 @@ markers = boardspec: U-Boot: Describes the set of boards a test can/can't run on. buildconfigspec: U-Boot: Describes Kconfig/config-header constraints. + notbuildconfigspec: U-Boot: Describes required disabled Kconfig options. + requiredtool: U-Boot: Required host tools for a test. + slow: U-Boot: Specific test will run slowly. diff --git a/test/py/requirements.txt b/test/py/requirements.txt new file mode 100644 index 0000000000..cf251186f4 --- /dev/null +++ b/test/py/requirements.txt @@ -0,0 +1,22 @@ +atomicwrites==1.3.0 +attrs==19.3.0 +coverage==4.5.4 +extras==1.0.0 +fixtures==3.0.0 +importlib-metadata==0.23 +linecache2==1.0.0 +more-itertools==7.2.0 +packaging==19.2 +pbr==5.4.3 +pluggy==0.13.0 +py==1.8.0 +pyparsing==2.4.2 +pytest==5.2.1 +python-mimeparse==1.6.0 +python-subunit==1.3.0 +six==1.12.0 +testtools==2.3.0 +traceback2==1.4.0 +unittest2==1.1.0 +wcwidth==0.1.7 +zipp==0.6.0 diff --git a/test/py/test.py b/test/py/test.py index a5140945d4..bee88d96bc 100755 --- a/test/py/test.py +++ b/test/py/test.py @@ -1,4 +1,4 @@ -#!/usr/bin/env python2 +#!/usr/bin/env python3 # SPDX-License-Identifier: GPL-2.0 # Copyright (c) 2015 Stephen Warren @@ -7,28 +7,14 @@ # Wrapper script to invoke pytest with the directory name that contains the # U-Boot tests. -from __future__ import print_function - import os import os.path import sys - -# Get rid of argv[0] -sys.argv.pop(0) +from pkg_resources import load_entry_point # argv; py.test test_directory_name user-supplied-arguments -args = ['py.test', os.path.dirname(__file__) + '/tests'] +args = [os.path.dirname(__file__) + '/tests'] args.extend(sys.argv) -try: - os.execvp('py.test', args) -except: - # Log full details of any exception for detailed analysis - import traceback - traceback.print_exc() - # Hint to the user that they likely simply haven't installed the required - # dependencies. - print(''' -exec(py.test) failed; perhaps you are missing some dependencies? -See test/py/README.md for the list.''', file=sys.stderr) - sys.exit(1) +if __name__ == '__main__': + sys.exit(load_entry_point('pytest', 'console_scripts', 'pytest')(args)) diff --git a/test/py/tests/test_android/test_avb.py b/test/py/tests/test_android/test_avb.py index 8132423435..20ccaf6712 100644 --- a/test/py/tests/test_android/test_avb.py +++ b/test/py/tests/test_android/test_avb.py @@ -23,7 +23,8 @@ mmc_dev = 1 temp_addr = 0x90000000 temp_addr2 = 0x90002000 -@pytest.mark.buildconfigspec('cmd_avb', 'cmd_mmc') +@pytest.mark.buildconfigspec('cmd_avb') +@pytest.mark.buildconfigspec('cmd_mmc') def test_avb_verify(u_boot_console): """Run AVB 2.0 boot verification chain with avb subset of commands """ @@ -36,7 +37,8 @@ def test_avb_verify(u_boot_console): assert response.find(success_str) -@pytest.mark.buildconfigspec('cmd_avb', 'cmd_mmc') +@pytest.mark.buildconfigspec('cmd_avb') +@pytest.mark.buildconfigspec('cmd_mmc') def test_avb_mmc_uuid(u_boot_console): """Check if 'avb get_uuid' works, compare results with 'part list mmc 1' output @@ -93,7 +95,8 @@ def test_avb_is_unlocked(u_boot_console): assert response == 'Unlocked = 1' -@pytest.mark.buildconfigspec('cmd_avb', 'cmd_mmc') +@pytest.mark.buildconfigspec('cmd_avb') +@pytest.mark.buildconfigspec('cmd_mmc') def test_avb_mmc_read(u_boot_console): """Test mmc read operation """ diff --git a/test/py/tests/test_bind.py b/test/py/tests/test_bind.py index 2d48484c6a..20c6050342 100644 --- a/test/py/tests/test_bind.py +++ b/test/py/tests/test_bind.py @@ -9,11 +9,11 @@ def in_tree(response, name, uclass, drv, depth, last_child): lines = [x.strip() for x in response.splitlines()] leaf = ' ' * 4 * depth; if not last_child: - leaf = leaf + '\|' + leaf = leaf + r'\|' else: leaf = leaf + '`' leaf = leaf + '-- ' + name - line = (' *{:10.10} [0-9]* \[ [ +] \] {:20.20} {}$' + line = (r' *{:10.10} [0-9]* \[ [ +] \] {:20.20} {}$' .format(uclass, drv, leaf)) prog = re.compile(line) for l in lines: diff --git a/test/py/tests/test_efi_selftest.py b/test/py/tests/test_efi_selftest.py index d5430f9c12..ca01542088 100644 --- a/test/py/tests/test_efi_selftest.py +++ b/test/py/tests/test_efi_selftest.py @@ -59,7 +59,7 @@ def test_efi_selftest_text_input(u_boot_console): u_boot_console.run_command(cmd='setenv efi_selftest text input') output = u_boot_console.run_command(cmd='bootefi selftest', wait_for_prompt=False) - m = u_boot_console.p.expect(['To terminate type \'x\'']) + m = u_boot_console.p.expect([r'To terminate type \'x\'']) if m != 0: raise Exception('No prompt for \'text input\' test') u_boot_console.drain_console() @@ -68,7 +68,7 @@ def test_efi_selftest_text_input(u_boot_console): u_boot_console.run_command(cmd=chr(4), wait_for_echo=False, send_nl=False, wait_for_prompt=False) m = u_boot_console.p.expect( - ['Unicode char 4 \(unknown\), scan code 0 \(Null\)']) + [r'Unicode char 4 \(unknown\), scan code 0 \(Null\)']) if m != 0: raise Exception('EOT failed in \'text input\' test') u_boot_console.drain_console() @@ -76,7 +76,7 @@ def test_efi_selftest_text_input(u_boot_console): u_boot_console.run_command(cmd=chr(8), wait_for_echo=False, send_nl=False, wait_for_prompt=False) m = u_boot_console.p.expect( - ['Unicode char 8 \(BS\), scan code 0 \(Null\)']) + [r'Unicode char 8 \(BS\), scan code 0 \(Null\)']) if m != 0: raise Exception('BS failed in \'text input\' test') u_boot_console.drain_console() @@ -84,7 +84,7 @@ def test_efi_selftest_text_input(u_boot_console): u_boot_console.run_command(cmd=chr(9), wait_for_echo=False, send_nl=False, wait_for_prompt=False) m = u_boot_console.p.expect( - ['Unicode char 9 \(TAB\), scan code 0 \(Null\)']) + [r'Unicode char 9 \(TAB\), scan code 0 \(Null\)']) if m != 0: raise Exception('BS failed in \'text input\' test') u_boot_console.drain_console() @@ -92,7 +92,7 @@ def test_efi_selftest_text_input(u_boot_console): u_boot_console.run_command(cmd='a', wait_for_echo=False, send_nl=False, wait_for_prompt=False) m = u_boot_console.p.expect( - ['Unicode char 97 \(\'a\'\), scan code 0 \(Null\)']) + [r'Unicode char 97 \(\'a\'\), scan code 0 \(Null\)']) if m != 0: raise Exception('\'a\' failed in \'text input\' test') u_boot_console.drain_console() @@ -100,14 +100,14 @@ def test_efi_selftest_text_input(u_boot_console): u_boot_console.run_command(cmd=chr(27) + '[A', wait_for_echo=False, send_nl=False, wait_for_prompt=False) m = u_boot_console.p.expect( - ['Unicode char 0 \(Null\), scan code 1 \(Up\)']) + [r'Unicode char 0 \(Null\), scan code 1 \(Up\)']) if m != 0: raise Exception('UP failed in \'text input\' test') u_boot_console.drain_console() # Euro sign - u_boot_console.run_command(cmd='\xe2\x82\xac', wait_for_echo=False, + u_boot_console.run_command(cmd=b'\xe2\x82\xac'.decode(), wait_for_echo=False, send_nl=False, wait_for_prompt=False) - m = u_boot_console.p.expect(['Unicode char 8364 \(\'']) + m = u_boot_console.p.expect([r'Unicode char 8364 \(\'']) if m != 0: raise Exception('Euro sign failed in \'text input\' test') u_boot_console.drain_console() @@ -129,7 +129,7 @@ def test_efi_selftest_text_input_ex(u_boot_console): u_boot_console.run_command(cmd='setenv efi_selftest extended text input') output = u_boot_console.run_command(cmd='bootefi selftest', wait_for_prompt=False) - m = u_boot_console.p.expect(['To terminate type \'CTRL\+x\'']) + m = u_boot_console.p.expect([r'To terminate type \'CTRL\+x\'']) if m != 0: raise Exception('No prompt for \'text input\' test') u_boot_console.drain_console() @@ -138,7 +138,7 @@ def test_efi_selftest_text_input_ex(u_boot_console): u_boot_console.run_command(cmd=chr(4), wait_for_echo=False, send_nl=False, wait_for_prompt=False) m = u_boot_console.p.expect( - ['Unicode char 100 \\(\'d\'\\), scan code 0 \\(CTRL\\+Null\\)']) + [r'Unicode char 100 \(\'d\'\), scan code 0 \(CTRL\+Null\)']) if m != 0: raise Exception('EOT failed in \'text input\' test') u_boot_console.drain_console() @@ -146,7 +146,7 @@ def test_efi_selftest_text_input_ex(u_boot_console): u_boot_console.run_command(cmd=chr(8), wait_for_echo=False, send_nl=False, wait_for_prompt=False) m = u_boot_console.p.expect( - ['Unicode char 8 \(BS\), scan code 0 \(\+Null\)']) + [r'Unicode char 8 \(BS\), scan code 0 \(\+Null\)']) if m != 0: raise Exception('BS failed in \'text input\' test') u_boot_console.drain_console() @@ -154,7 +154,7 @@ def test_efi_selftest_text_input_ex(u_boot_console): u_boot_console.run_command(cmd=chr(9), wait_for_echo=False, send_nl=False, wait_for_prompt=False) m = u_boot_console.p.expect( - ['Unicode char 9 \(TAB\), scan code 0 \(\+Null\)']) + [r'Unicode char 9 \(TAB\), scan code 0 \(\+Null\)']) if m != 0: raise Exception('TAB failed in \'text input\' test') u_boot_console.drain_console() @@ -162,7 +162,7 @@ def test_efi_selftest_text_input_ex(u_boot_console): u_boot_console.run_command(cmd='a', wait_for_echo=False, send_nl=False, wait_for_prompt=False) m = u_boot_console.p.expect( - ['Unicode char 97 \(\'a\'\), scan code 0 \(Null\)']) + [r'Unicode char 97 \(\'a\'\), scan code 0 \(Null\)']) if m != 0: raise Exception('\'a\' failed in \'text input\' test') u_boot_console.drain_console() @@ -170,23 +170,23 @@ def test_efi_selftest_text_input_ex(u_boot_console): u_boot_console.run_command(cmd=chr(27) + '[A', wait_for_echo=False, send_nl=False, wait_for_prompt=False) m = u_boot_console.p.expect( - ['Unicode char 0 \(Null\), scan code 1 \(\+Up\)']) + [r'Unicode char 0 \(Null\), scan code 1 \(\+Up\)']) if m != 0: raise Exception('UP failed in \'text input\' test') u_boot_console.drain_console() # Euro sign - u_boot_console.run_command(cmd='\xe2\x82\xac', wait_for_echo=False, + u_boot_console.run_command(cmd=b'\xe2\x82\xac'.decode(), wait_for_echo=False, send_nl=False, wait_for_prompt=False) - m = u_boot_console.p.expect(['Unicode char 8364 \(\'']) + m = u_boot_console.p.expect([r'Unicode char 8364 \(\'']) if m != 0: raise Exception('Euro sign failed in \'text input\' test') u_boot_console.drain_console() # SHIFT+ALT+FN 5 - u_boot_console.run_command(cmd='\x1b\x5b\x31\x35\x3b\x34\x7e', + u_boot_console.run_command(cmd=b'\x1b\x5b\x31\x35\x3b\x34\x7e'.decode(), wait_for_echo=False, send_nl=False, wait_for_prompt=False) m = u_boot_console.p.expect( - ['Unicode char 0 \(Null\), scan code 15 \(SHIFT\+ALT\+FN 5\)']) + [r'Unicode char 0 \(Null\), scan code 15 \(SHIFT\+ALT\+FN 5\)']) if m != 0: raise Exception('SHIFT+ALT+FN 5 failed in \'text input\' test') u_boot_console.drain_console() diff --git a/test/py/tests/test_fit.py b/test/py/tests/test_fit.py index e3210ed43f..356d9a20f2 100755 --- a/test/py/tests/test_fit.py +++ b/test/py/tests/test_fit.py @@ -3,8 +3,6 @@ # # Sanity check of the FIT handling in U-Boot -from __future__ import print_function - import os import pytest import struct @@ -155,7 +153,7 @@ def test_fit(u_boot_console): src = make_fname('u-boot.dts') dtb = make_fname('u-boot.dtb') with open(src, 'w') as fd: - print(base_fdt, file=fd) + fd.write(base_fdt) util.run_and_log(cons, ['dtc', src, '-O', 'dtb', '-o', dtb]) return dtb @@ -188,7 +186,7 @@ def test_fit(u_boot_console): its = make_its(params) util.run_and_log(cons, [mkimage, '-f', its, fit]) with open(make_fname('u-boot.dts'), 'w') as fd: - print(base_fdt, file=fd) + fd.write(base_fdt) return fit def make_kernel(filename, text): diff --git a/test/py/tests/test_fpga.py b/test/py/tests/test_fpga.py index e3bb7b41c7..ca7ef8ea40 100644 --- a/test/py/tests/test_fpga.py +++ b/test/py/tests/test_fpga.py @@ -175,29 +175,29 @@ def test_fpga_load_fail(u_boot_console): f, dev, addr, bit, bit_size = load_file_from_var(u_boot_console, 'bitstream_load') for cmd in ['dump', 'load', 'loadb']: - # missing dev parameter - expected = 'fpga: incorrect parameters passed' - output = u_boot_console.run_command('fpga %s %x $filesize' % (cmd, addr)) - #assert expected in output - assert expected_usage in output - - # more parameters - 0 at the end - expected = 'fpga: more parameters passed' - output = u_boot_console.run_command('fpga %s %x %x $filesize 0' % (cmd, dev, addr)) - #assert expected in output - assert expected_usage in output - - # 0 address - expected = 'fpga: zero fpga_data address' - output = u_boot_console.run_command('fpga %s %x 0 $filesize' % (cmd, dev)) - #assert expected in output - assert expected_usage in output - - # 0 filesize - expected = 'fpga: zero size' - output = u_boot_console.run_command('fpga %s %x %x 0' % (cmd, dev, addr)) - #assert expected in output - assert expected_usage in output + # missing dev parameter + expected = 'fpga: incorrect parameters passed' + output = u_boot_console.run_command('fpga %s %x $filesize' % (cmd, addr)) + #assert expected in output + assert expected_usage in output + + # more parameters - 0 at the end + expected = 'fpga: more parameters passed' + output = u_boot_console.run_command('fpga %s %x %x $filesize 0' % (cmd, dev, addr)) + #assert expected in output + assert expected_usage in output + + # 0 address + expected = 'fpga: zero fpga_data address' + output = u_boot_console.run_command('fpga %s %x 0 $filesize' % (cmd, dev)) + #assert expected in output + assert expected_usage in output + + # 0 filesize + expected = 'fpga: zero size' + output = u_boot_console.run_command('fpga %s %x %x 0' % (cmd, dev, addr)) + #assert expected in output + assert expected_usage in output @pytest.mark.buildconfigspec('cmd_fpga') @pytest.mark.buildconfigspec('cmd_echo') diff --git a/test/py/tests/test_fs/conftest.py b/test/py/tests/test_fs/conftest.py index 9324657d21..1949f91619 100644 --- a/test/py/tests/test_fs/conftest.py +++ b/test/py/tests/test_fs/conftest.py @@ -300,38 +300,38 @@ def fs_obj_basic(request, u_boot_config): # Generate the md5sums of reads that we will test against small file out = check_output( 'dd if=%s bs=1M skip=0 count=1 2> /dev/null | md5sum' - % small_file, shell=True) + % small_file, shell=True).decode() md5val = [ out.split()[0] ] # Generate the md5sums of reads that we will test against big file # One from beginning of file. out = check_output( 'dd if=%s bs=1M skip=0 count=1 2> /dev/null | md5sum' - % big_file, shell=True) + % big_file, shell=True).decode() md5val.append(out.split()[0]) # One from end of file. out = check_output( 'dd if=%s bs=1M skip=2499 count=1 2> /dev/null | md5sum' - % big_file, shell=True) + % big_file, shell=True).decode() md5val.append(out.split()[0]) # One from the last 1MB chunk of 2GB out = check_output( 'dd if=%s bs=1M skip=2047 count=1 2> /dev/null | md5sum' - % big_file, shell=True) + % big_file, shell=True).decode() md5val.append(out.split()[0]) # One from the start 1MB chunk from 2GB out = check_output( 'dd if=%s bs=1M skip=2048 count=1 2> /dev/null | md5sum' - % big_file, shell=True) + % big_file, shell=True).decode() md5val.append(out.split()[0]) # One 1MB chunk crossing the 2GB boundary out = check_output( 'dd if=%s bs=512K skip=4095 count=2 2> /dev/null | md5sum' - % big_file, shell=True) + % big_file, shell=True).decode() md5val.append(out.split()[0]) umount_fs(mount_dir) @@ -390,7 +390,7 @@ def fs_obj_ext(request, u_boot_config): % min_file, shell=True) out = check_output( 'dd if=%s bs=1K 2> /dev/null | md5sum' - % min_file, shell=True) + % min_file, shell=True).decode() md5val = [ out.split()[0] ] # Calculate md5sum of Test Case 4 @@ -399,7 +399,7 @@ def fs_obj_ext(request, u_boot_config): check_call('dd if=%s of=%s bs=1K seek=5 count=20' % (min_file, tmp_file), shell=True) out = check_output('dd if=%s bs=1K 2> /dev/null | md5sum' - % tmp_file, shell=True) + % tmp_file, shell=True).decode() md5val.append(out.split()[0]) # Calculate md5sum of Test Case 5 @@ -408,7 +408,7 @@ def fs_obj_ext(request, u_boot_config): check_call('dd if=%s of=%s bs=1K seek=5 count=5' % (min_file, tmp_file), shell=True) out = check_output('dd if=%s bs=1K 2> /dev/null | md5sum' - % tmp_file, shell=True) + % tmp_file, shell=True).decode() md5val.append(out.split()[0]) # Calculate md5sum of Test Case 7 @@ -417,7 +417,7 @@ def fs_obj_ext(request, u_boot_config): check_call('dd if=%s of=%s bs=1K seek=20 count=20' % (min_file, tmp_file), shell=True) out = check_output('dd if=%s bs=1K 2> /dev/null | md5sum' - % tmp_file, shell=True) + % tmp_file, shell=True).decode() md5val.append(out.split()[0]) check_call('rm %s' % tmp_file, shell=True) @@ -508,8 +508,8 @@ def fs_obj_unlink(request, u_boot_config): # Test Case 2 check_call('mkdir %s/dir2' % mount_dir, shell=True) - for i in range(0, 20): - check_call('mkdir %s/dir2/0123456789abcdef%02x' + for i in range(0, 20): + check_call('mkdir %s/dir2/0123456789abcdef%02x' % (mount_dir, i), shell=True) # Test Case 4 @@ -582,11 +582,11 @@ def fs_obj_symlink(request, u_boot_config): # Generate the md5sums of reads that we will test against small file out = check_output( 'dd if=%s bs=1M skip=0 count=1 2> /dev/null | md5sum' - % small_file, shell=True) + % small_file, shell=True).decode() md5val = [out.split()[0]] out = check_output( 'dd if=%s bs=10M skip=0 count=1 2> /dev/null | md5sum' - % medium_file, shell=True) + % medium_file, shell=True).decode() md5val.extend([out.split()[0]]) umount_fs(mount_dir) diff --git a/test/py/tests/test_log.py b/test/py/tests/test_log.py index cb183444c6..75325fad61 100644 --- a/test/py/tests/test_log.py +++ b/test/py/tests/test_log.py @@ -27,9 +27,9 @@ def test_log(u_boot_console): """ for i in range(max_level): if mask & 1: - assert 'log_run() log %d' % i == lines.next() + assert 'log_run() log %d' % i == next(lines) if mask & 3: - assert 'func() _log %d' % i == lines.next() + assert 'func() _log %d' % i == next(lines) def run_test(testnum): """Run a particular test number (the 'log test' command) @@ -43,7 +43,7 @@ def test_log(u_boot_console): output = u_boot_console.run_command('log test %d' % testnum) split = output.replace('\r', '').splitlines() lines = iter(split) - assert 'test %d' % testnum == lines.next() + assert 'test %d' % testnum == next(lines) return lines def test0(): @@ -88,7 +88,7 @@ def test_log(u_boot_console): def test10(): lines = run_test(10) for i in range(7): - assert 'log_test() level %d' % i == lines.next() + assert 'log_test() level %d' % i == next(lines) # TODO(sjg@chromium.org): Consider structuring this as separate tests cons = u_boot_console diff --git a/test/py/tests/test_mmc_wr.py b/test/py/tests/test_mmc_wr.py index 8b18781eac..05e5c1ee85 100644 --- a/test/py/tests/test_mmc_wr.py +++ b/test/py/tests/test_mmc_wr.py @@ -35,7 +35,9 @@ env__mmc_wr_configs = ( """ -@pytest.mark.buildconfigspec('cmd_mmc','cmd_memory', 'cmd_random') +@pytest.mark.buildconfigspec('cmd_mmc') +@pytest.mark.buildconfigspec('cmd_memory') +@pytest.mark.buildconfigspec('cmd_random') def test_mmc_wr(u_boot_console, env__mmc_wr_config): """Test the "mmc write" command. @@ -65,41 +67,39 @@ def test_mmc_wr(u_boot_console, env__mmc_wr_config): for i in range(test_iterations): - # Generate random data - cmd = 'random %s %x' % (src_addr, count_bytes) - response = u_boot_console.run_command(cmd) - good_response = '%d bytes filled with random data' % (count_bytes) - assert good_response in response - - # Select MMC device - cmd = 'mmc dev %d' % devid - if is_emmc: - cmd += ' %d' % partid - response = u_boot_console.run_command(cmd) - assert 'no card present' not in response - if is_emmc: - partid_response = "(part %d)" % partid - else: - partid_response = "" - good_response = 'mmc%d%s is current device' % (devid, partid_response) - assert good_response in response - - # Write data - cmd = 'mmc write %s %x %x' % (src_addr, sector, count_sectors) - response = u_boot_console.run_command(cmd) - good_response = 'MMC write: dev # %d, block # %d, count %d ... %d blocks written: OK' % ( - devid, sector, count_sectors, count_sectors) - assert good_response in response - - # Read data - cmd = 'mmc read %s %x %x' % (dst_addr, sector, count_sectors) - response = u_boot_console.run_command(cmd) - good_response = 'MMC read: dev # %d, block # %d, count %d ... %d blocks read: OK' % ( - devid, sector, count_sectors, count_sectors) - assert good_response in response - - # Compare src and dst data - cmd = 'cmp.b %s %s %x' % (src_addr, dst_addr, count_bytes) - response = u_boot_console.run_command(cmd) - good_response = 'Total of %d byte(s) were the same' % (count_bytes) - assert good_response in response + # Generate random data + cmd = 'random %s %x' % (src_addr, count_bytes) + response = u_boot_console.run_command(cmd) + good_response = '%d bytes filled with random data' % (count_bytes) + assert good_response in response + + # Select MMC device + cmd = 'mmc dev %d' % devid + if is_emmc: + cmd += ' %d' % partid + response = u_boot_console.run_command(cmd) + assert 'no card present' not in response + if is_emmc: + partid_response = "(part %d)" % partid + else: + partid_response = "" + good_response = 'mmc%d%s is current device' % (devid, partid_response) + assert good_response in response + + # Write data + cmd = 'mmc write %s %x %x' % (src_addr, sector, count_sectors) + response = u_boot_console.run_command(cmd) + good_response = 'MMC write: dev # %d, block # %d, count %d ... %d blocks written: OK' % (devid, sector, count_sectors, count_sectors) + assert good_response in response + + # Read data + cmd = 'mmc read %s %x %x' % (dst_addr, sector, count_sectors) + response = u_boot_console.run_command(cmd) + good_response = 'MMC read: dev # %d, block # %d, count %d ... %d blocks read: OK' % (devid, sector, count_sectors, count_sectors) + assert good_response in response + + # Compare src and dst data + cmd = 'cmp.b %s %s %x' % (src_addr, dst_addr, count_bytes) + response = u_boot_console.run_command(cmd) + good_response = 'Total of %d byte(s) were the same' % (count_bytes) + assert good_response in response diff --git a/test/py/tests/test_ut.py b/test/py/tests/test_ut.py index 62037d2c45..6c7b8dd2b3 100644 --- a/test/py/tests/test_ut.py +++ b/test/py/tests/test_ut.py @@ -10,14 +10,14 @@ def test_ut_dm_init(u_boot_console): fn = u_boot_console.config.source_dir + '/testflash.bin' if not os.path.exists(fn): - data = 'this is a test' - data += '\x00' * ((4 * 1024 * 1024) - len(data)) + data = b'this is a test' + data += b'\x00' * ((4 * 1024 * 1024) - len(data)) with open(fn, 'wb') as fh: fh.write(data) fn = u_boot_console.config.source_dir + '/spi.bin' if not os.path.exists(fn): - data = '\x00' * (2 * 1024 * 1024) + data = b'\x00' * (2 * 1024 * 1024) with open(fn, 'wb') as fh: fh.write(data) diff --git a/test/py/u_boot_spawn.py b/test/py/u_boot_spawn.py index b011a3e3da..6991b78cca 100644 --- a/test/py/u_boot_spawn.py +++ b/test/py/u_boot_spawn.py @@ -42,10 +42,7 @@ class Spawn(object): self.after = '' self.timeout = None # http://stackoverflow.com/questions/7857352/python-regex-to-match-vt100-escape-sequences - # Note that re.I doesn't seem to work with this regex (or perhaps the - # version of Python in Ubuntu 14.04), hence the inclusion of a-z inside - # [] instead. - self.re_vt100 = re.compile('(\x1b\[|\x9b)[^@-_a-z]*[@-_a-z]|\x1b[@-_a-z]') + self.re_vt100 = re.compile(r'(\x1b\[|\x9b)[^@-_]*[@-_]|\x1b[@-_]', re.I) (self.pid, self.fd) = pty.fork() if self.pid == 0: @@ -113,7 +110,7 @@ class Spawn(object): Nothing. """ - os.write(self.fd, data) + os.write(self.fd, data.encode(errors='replace')) def expect(self, patterns): """Wait for the sub-process to emit specific data. @@ -171,7 +168,7 @@ class Spawn(object): events = self.poll.poll(poll_maxwait) if not events: raise Timeout() - c = os.read(self.fd, 1024) + c = os.read(self.fd, 1024).decode(errors='replace') if not c: raise EOFError() if self.logfile_read: diff --git a/tools/fit_image.c b/tools/fit_image.c index 5aca634b5e..0201cc44d8 100644 --- a/tools/fit_image.c +++ b/tools/fit_image.c @@ -229,6 +229,7 @@ static int fit_write_images(struct image_tool_params *params, char *fdt) for (cont = params->content_head; cont; cont = cont->next) { if (cont->type != IH_TYPE_FLATDT) continue; + typename = genimg_get_type_short_name(cont->type); snprintf(str, sizeof(str), "%s-%d", FIT_FDT_PROP, ++upto); fdt_begin_node(fdt, str); @@ -253,6 +254,8 @@ static int fit_write_images(struct image_tool_params *params, char *fdt) fdt_property_string(fdt, FIT_TYPE_PROP, FIT_RAMDISK_PROP); fdt_property_string(fdt, FIT_OS_PROP, genimg_get_os_short_name(params->os)); + fdt_property_string(fdt, FIT_ARCH_PROP, + genimg_get_arch_short_name(params->arch)); ret = fdt_property_file(params, fdt, FIT_DATA_PROP, params->fit_ramdisk); |