| Message ID | 20180920144706.7504-1-christian.gmeiner@gmail.com |
|---|---|
| State | Superseded |
| Delegated to: | Bin Meng |
| Headers | show |
| Series | [U-Boot] fs: cbfs: remove wrong header validation | expand |
+Simon, Hi Christian, On Thu, Sep 20, 2018 at 10:47 PM Christian Gmeiner <christian.gmeiner@gmail.com> wrote: > > Coreboot does not contain such a check: > https://github.com/coreboot/coreboot/blob/eeb4e20b2f6d786c92fe3efb30817e90389a2bfe/src/commonlib/cbfs.c#L64 > > Before this change cbfsinit failed with 'Bad CBFS file'. After this change all cbfs commands > are working as expected. > > Signed-off-by: Christian Gmeiner <christian.gmeiner@gmail.com> > --- > fs/cbfs/cbfs.c | 6 +----- > 1 file changed, 1 insertion(+), 5 deletions(-) > > diff --git a/fs/cbfs/cbfs.c b/fs/cbfs/cbfs.c > index 0dce639b49..2a581f0c18 100644 > --- a/fs/cbfs/cbfs.c > +++ b/fs/cbfs/cbfs.c > @@ -96,11 +96,7 @@ static int file_cbfs_next_file(u8 *start, u32 size, u32 align, > } > > swap_file_header(&header, fileHeader); > - if (header.offset < sizeof(struct cbfs_fileheader) || > - header.offset > header.len) { > - file_cbfs_result = CBFS_BAD_FILE; > - return -1; > - } It looks to me the existing codes were doing some sanity checks. Can you elaborate why this is failing on your board? In your coreboot reference, I don't see exactly how U-Boot codes are connected to the coreboot one. > + > newNode->next = NULL; > newNode->type = header.type; > newNode->data = start + header.offset; > -- Regards, Bin
Hi Bin, Finally I have some time to look deeper into this issue. > > On Thu, Sep 20, 2018 at 10:47 PM Christian Gmeiner > <christian.gmeiner@gmail.com> wrote: > > > > Coreboot does not contain such a check: > > https://github.com/coreboot/coreboot/blob/eeb4e20b2f6d786c92fe3efb30817e90389a2bfe/src/commonlib/cbfs.c#L64 > > > > Before this change cbfsinit failed with 'Bad CBFS file'. After this change all cbfs commands > > are working as expected. > > > > Signed-off-by: Christian Gmeiner <christian.gmeiner@gmail.com> > > --- > > fs/cbfs/cbfs.c | 6 +----- > > 1 file changed, 1 insertion(+), 5 deletions(-) > > > > diff --git a/fs/cbfs/cbfs.c b/fs/cbfs/cbfs.c > > index 0dce639b49..2a581f0c18 100644 > > --- a/fs/cbfs/cbfs.c > > +++ b/fs/cbfs/cbfs.c > > @@ -96,11 +96,7 @@ static int file_cbfs_next_file(u8 *start, u32 size, u32 align, > > } > > > > swap_file_header(&header, fileHeader); > > - if (header.offset < sizeof(struct cbfs_fileheader) || > > - header.offset > header.len) { > > - file_cbfs_result = CBFS_BAD_FILE; > > - return -1; > > - } > > It looks to me the existing codes were doing some sanity checks. Can > you elaborate why this is failing on your board? In your coreboot > reference, I don't see exactly how U-Boot codes are connected to the > coreboot one. > This has nothing to do with my board at all - I can easily reproduce this issue under qemu: qemu-system-i386 -bios build/coreboot.rom -hda /dev/zero -serial stdio -display none WARNING: Image format was not specified for '/dev/zero' and probing guessed raw. Automatically detecting the format is dangerous for raw images, write operations on block 0 will be restricted. Specify the 'raw' format explicitly to remove the restrictions. coreboot-4.8-2510-g303a4bfd4a Wed Dec 12 02:20:54 UTC 2018 bootblock starting... CBFS: 'Master Header Locator' located CBFS at [1fa0200:2000000) CBFS: Locating 'fallback/romstage' CBFS: Found @ offset 80 size 3c04 coreboot-4.8-2510-g303a4bfd4a Wed Dec 12 02:20:54 UTC 2018 romstage starting... CBMEM: IMD: root @ 07fff000 254 entries. IMD: root @ 07ffec00 62 entries. CBFS: 'Master Header Locator' located CBFS at [1fa0200:2000000) CBFS: Locating 'fallback/ramstage' CBFS: Found @ offset 3d00 size acb2 Decompressing stage fallback/ramstage @ 0x07fbcfc0 (128664 bytes) Loading module at 07fbd000 with entry 07fbd000. filesize: 0x15750 memsize: 0x1f658 Processing 1257 relocs. Offset value of 0x071bd000 coreboot-4.8-2510-g303a4bfd4a Wed Dec 12 02:20:54 UTC 2018 ramstage starting... Enumerating buses... CPU_CLUSTER: 0 enabled DOMAIN: 0000 enabled QEMU: firmware config interface detected QEMU: max_cpus is 1 CPU: APIC: 00 enabled scan_bus: scanning of bus CPU_CLUSTER: 0 took 0 usecs PCI: pci_scan_bus for bus 00 PCI: 00:00.0 [8086/1237] enabled PCI: 00:01.0 [8086/7000] enabled PCI: 00:01.1 [8086/7010] enabled PCI: 00:01.3 [8086/7113] enabled PCI: 00:02.0 [1234/1111] enabled PCI: 00:03.0 [8086/100e] enabled scan_bus: scanning of bus PCI: 00:01.0 took 0 usecs scan_bus: scanning of bus PCI: 00:01.3 took 0 usecs scan_bus: scanning of bus DOMAIN: 0000 took 0 usecs scan_bus: scanning of bus Root Device took 0 usecs done found VGA at PCI: 00:02.0 Setting up VGA for PCI: 00:02.0 Setting PCI_BRIDGE_CTL_VGA for bridge DOMAIN: 0000 Setting PCI_BRIDGE_CTL_VGA for bridge Root Device Allocating resources... Reading resources... QEMU: 11 files in fw_cfg QEMU: bootorder [size=0] QEMU: etc/acpi/rsdp [size=36] QEMU: etc/acpi/tables [size=131072] QEMU: etc/boot-fail-wait [size=4] QEMU: etc/e820 [size=20] QEMU: etc/smbios/smbios-anchor [size=31] QEMU: etc/smbios/smbios-tables [size=320] QEMU: etc/system-states [size=6] QEMU: etc/table-loader [size=4096] QEMU: etc/tpm/log [size=0] QEMU: genroms/kvmvapic.bin [size=9216] QEMU: e820/ram: 0x00000000 +0x08000000 QEMU: reserve ioports 0x0510-0x0511 [firmware-config] QEMU: reserve ioports 0x5658-0x5658 [vmware-port] QEMU: reserve ioports 0xae00-0xae0f [pci-hotplug] QEMU: reserve ioports 0xaf00-0xaf1f [cpu-hotplug] QEMU: reserve ioports 0xafe0-0xafe3 [piix4-gpe0] Done reading resources. Setting resources... PCI: 00:01.1 20 <- [0x0000005840 - 0x000000584f] size 0x00000010 gran 0x04 io PCI: 00:02.0 10 <- [0x00fd000000 - 0x00fdffffff] size 0x01000000 gran 0x18 prefmem PCI: 00:02.0 18 <- [0x00fe070000 - 0x00fe070fff] size 0x00001000 gran 0x0c mem PCI: 00:02.0 30 <- [0x00fe060000 - 0x00fe06ffff] size 0x00010000 gran 0x10 romem PCI: 00:03.0 10 <- [0x00fe040000 - 0x00fe05ffff] size 0x00020000 gran 0x11 mem PCI: 00:03.0 14 <- [0x0000005800 - 0x000000583f] size 0x00000040 gran 0x06 io PCI: 00:03.0 30 <- [0x00fe000000 - 0x00fe03ffff] size 0x00040000 gran 0x12 romem Done setting resources. Done allocating resources. Enabling resources... PCI: 00:00.0 cmd <- 00 PCI: 00:01.0 cmd <- 00 PCI: 00:01.1 cmd <- 01 PCI: 00:01.3 cmd <- 00 PCI: 00:02.0 cmd <- 03 PCI: 00:03.0 cmd <- 03 done. Initializing devices... Root Device init ... CPU_CLUSTER: 0 init ... Initializing CPU #0 CPU: vendor Intel device 663 CPU: family 06, model 06, stepping 03 Setting up local APIC... apic_id: 0x00 done. CPU #0 initialized PCI: 00:00.0 init ... Assigning IRQ 10 to 0:1.3 Assigning IRQ 11 to 0:3.0 PCI: 00:01.0 init ... RTC Init PCI: 00:01.1 init ... IDE: Primary IDE interface: on IDE: Secondary IDE interface: on IDE: Access to legacy IDE ports: off PCI: 00:02.0 init ... PCI: 00:03.0 init ... Devices initialized Finalize devices... Devices finalized Copying Interrupt Routing Table to 0x000f0000... done. Copying Interrupt Routing Table to 0x07fb3000... done. PIRQ table: 128 bytes. QEMU: found ACPI tables in fw_cfg. QEMU: loading "etc/acpi/rsdp" to 0x7f8f000 (len 36) QEMU: loading "etc/acpi/tables" to 0x7f8f040 (len 131072) QEMU: loaded ACPI tables from fw_cfg. ACPI tables: 131136 bytes. smbios_write_tables: 07f8e000 DOMAIN: 0000 (QEMU Northbridge i440fx) QEMU: found smbios tables in fw_cfg (len 320). QEMU: coreboot type0 table found at 0x7f8e020. QEMU: loading smbios tables to 0x7f8e062 SMBIOS tables: 418 bytes. Writing table forward entry at 0x00000500 Wrote coreboot table at: 00000500, 0x10 bytes, checksum b7e3 Writing coreboot table at 0x07fb4000 0. 0000000000000000-0000000000000fff: CONFIGURATION TABLES 1. 0000000000001000-000000000009ffff: RAM 2. 00000000000c0000-0000000007f8dfff: RAM 3. 0000000007f8e000-0000000007fbcfff: CONFIGURATION TABLES 4. 0000000007fbd000-0000000007fdcfff: RAMSTAGE 5. 0000000007fdd000-0000000007ffffff: CONFIGURATION TABLES 6. 00000000ff800000-00000000ffffffff: RESERVED CBFS: 'Master Header Locator' located CBFS at [1fa0200:2000000) FMAP: Found "FLASH" version 1.1 at 1fa0000. FMAP: base = fe000000 size = 2000000 #areas = 3 Wrote coreboot table at: 07fb4000, 0x264 bytes, checksum f4f2 coreboot table: 636 bytes. IMD ROOT 0. 07fff000 00001000 IMD SMALL 1. 07ffe000 00001000 CONSOLE 2. 07fde000 00020000 TIME STAMP 3. 07fdd000 00000910 RAMSTAGE 4. 07fbc000 00021000 COREBOOT 5. 07fb4000 00008000 IRQ TABLE 6. 07fb3000 00001000 ACPI 7. 07f8f000 00024000 SMBIOS 8. 07f8e000 00000800 IMD small region: IMD ROOT 0. 07ffec00 00000400 CAR GLOBALS 1. 07ffe9c0 00000240 COREBOOTFWD 2. 07ffe980 00000028 CBFS: 'Master Header Locator' located CBFS at [1fa0200:2000000) CBFS: Locating 'fallback/payload' CBFS: Found @ offset 10000 size 354b5 Checking segment from ROM address 0xfffb0238 Checking segment from ROM address 0xfffb0254 Loading segment from ROM address 0xfffb0238 code (compression=1) New segment dstaddr 0x01110000 memsize 0x793df srcaddr 0xfffb0270 filesize 0x3547d Loading Segment: addr: 0x01110000 memsz: 0x00000000000793df filesz: 0x000000000003547d using LZMA Loading segment from ROM address 0xfffb0254 Entry Point 0x01110015 Jumping to boot code at 01110015(07fb4000) U-Boot 2019.01-rc1-00277-gee168783ae (Dec 12 2018 - 15:10:41 +0100) CPU: x86, vendor Intel, device 663h DRAM: 127.4 MiB MMC: Video: No video mode configured in coreboot! Video: No video mode configured in coreboot! Model: coreboot x86 payload Net: e1000: 52:54:00:12:34:56 Warning: e1000#0 using MAC address from ROM eth0: e1000#0 No controllers found Finalizing coreboot Hit any key to stop autoboot: 0 => cbfsinit Bad CBFS file. => So lets try to connect u-boot and coreboot sources in that area. file_cbfs_next_file(..): https://git.denx.de/?p=u-boot.git;a=blob;f=fs/cbfs/cbfs.c;h=0dce639b49ce095ac3eede2ee9c3138f1e0af3bb;hb=HEAD#l76 cbfs_for_each_file(..): https://github.com/coreboot/coreboot/blob/eeb4e20b2f6d786c92fe3efb30817e90389a2bfe/src/commonlib/cbfs.c#L64 Both ot these two functions do more or less the same: look for files in cbfs with the only difference that u-boots impl. does some validations that are not done in coreboot: https://git.denx.de/?p=u-boot.git;a=blob;f=fs/cbfs/cbfs.c;h=0dce639b49ce095ac3eede2ee9c3138f1e0af3bb;hb=HEAD#l99 With this patch cbfsinit works and cbfsinfo shows the expected output: => cbfsinfo CBFS version: 0x31313132 ROM size: 0x2000000 Boot block size: 0x4 CBFS size: 0x5fdfc Alignment: 64 Offset: 0x1fa0200 What is needed to get further with this patch? -- greets -- Christian Gmeiner, MSc https://christian-gmeiner.info
Am Mi., 12. Dez. 2018 um 15:27 Uhr schrieb Christian Gmeiner <christian.gmeiner@gmail.com>: > > Hi Bin, > > Finally I have some time to look deeper into this issue. > > > > > On Thu, Sep 20, 2018 at 10:47 PM Christian Gmeiner > > <christian.gmeiner@gmail.com> wrote: > > > > > > Coreboot does not contain such a check: > > > https://github.com/coreboot/coreboot/blob/eeb4e20b2f6d786c92fe3efb30817e90389a2bfe/src/commonlib/cbfs.c#L64 > > > > > > Before this change cbfsinit failed with 'Bad CBFS file'. After this change all cbfs commands > > > are working as expected. > > > > > > Signed-off-by: Christian Gmeiner <christian.gmeiner@gmail.com> > > > --- > > > fs/cbfs/cbfs.c | 6 +----- > > > 1 file changed, 1 insertion(+), 5 deletions(-) > > > > > > diff --git a/fs/cbfs/cbfs.c b/fs/cbfs/cbfs.c > > > index 0dce639b49..2a581f0c18 100644 > > > --- a/fs/cbfs/cbfs.c > > > +++ b/fs/cbfs/cbfs.c > > > @@ -96,11 +96,7 @@ static int file_cbfs_next_file(u8 *start, u32 size, u32 align, > > > } > > > > > > swap_file_header(&header, fileHeader); > > > - if (header.offset < sizeof(struct cbfs_fileheader) || > > > - header.offset > header.len) { > > > - file_cbfs_result = CBFS_BAD_FILE; > > > - return -1; > > > - } > > > > It looks to me the existing codes were doing some sanity checks. Can > > you elaborate why this is failing on your board? In your coreboot > > reference, I don't see exactly how U-Boot codes are connected to the > > coreboot one. > > > > This has nothing to do with my board at all - I can easily reproduce > this issue under qemu: > > qemu-system-i386 -bios build/coreboot.rom -hda /dev/zero -serial > stdio -display none > WARNING: Image format was not specified for '/dev/zero' and probing guessed raw. > Automatically detecting the format is dangerous for raw > images, write operations on block 0 will be restricted. > Specify the 'raw' format explicitly to remove the restrictions. > > > coreboot-4.8-2510-g303a4bfd4a Wed Dec 12 02:20:54 UTC 2018 bootblock starting... > CBFS: 'Master Header Locator' located CBFS at [1fa0200:2000000) > CBFS: Locating 'fallback/romstage' > CBFS: Found @ offset 80 size 3c04 > > > coreboot-4.8-2510-g303a4bfd4a Wed Dec 12 02:20:54 UTC 2018 romstage starting... > CBMEM: > IMD: root @ 07fff000 254 entries. > IMD: root @ 07ffec00 62 entries. > CBFS: 'Master Header Locator' located CBFS at [1fa0200:2000000) > CBFS: Locating 'fallback/ramstage' > CBFS: Found @ offset 3d00 size acb2 > Decompressing stage fallback/ramstage @ 0x07fbcfc0 (128664 bytes) > Loading module at 07fbd000 with entry 07fbd000. filesize: 0x15750 > memsize: 0x1f658 > Processing 1257 relocs. Offset value of 0x071bd000 > > > coreboot-4.8-2510-g303a4bfd4a Wed Dec 12 02:20:54 UTC 2018 ramstage starting... > Enumerating buses... > CPU_CLUSTER: 0 enabled > DOMAIN: 0000 enabled > QEMU: firmware config interface detected > QEMU: max_cpus is 1 > CPU: APIC: 00 enabled > scan_bus: scanning of bus CPU_CLUSTER: 0 took 0 usecs > PCI: pci_scan_bus for bus 00 > PCI: 00:00.0 [8086/1237] enabled > PCI: 00:01.0 [8086/7000] enabled > PCI: 00:01.1 [8086/7010] enabled > PCI: 00:01.3 [8086/7113] enabled > PCI: 00:02.0 [1234/1111] enabled > PCI: 00:03.0 [8086/100e] enabled > scan_bus: scanning of bus PCI: 00:01.0 took 0 usecs > scan_bus: scanning of bus PCI: 00:01.3 took 0 usecs > scan_bus: scanning of bus DOMAIN: 0000 took 0 usecs > scan_bus: scanning of bus Root Device took 0 usecs > done > found VGA at PCI: 00:02.0 > Setting up VGA for PCI: 00:02.0 > Setting PCI_BRIDGE_CTL_VGA for bridge DOMAIN: 0000 > Setting PCI_BRIDGE_CTL_VGA for bridge Root Device > Allocating resources... > Reading resources... > QEMU: 11 files in fw_cfg > QEMU: bootorder [size=0] > QEMU: etc/acpi/rsdp [size=36] > QEMU: etc/acpi/tables [size=131072] > QEMU: etc/boot-fail-wait [size=4] > QEMU: etc/e820 [size=20] > QEMU: etc/smbios/smbios-anchor [size=31] > QEMU: etc/smbios/smbios-tables [size=320] > QEMU: etc/system-states [size=6] > QEMU: etc/table-loader [size=4096] > QEMU: etc/tpm/log [size=0] > QEMU: genroms/kvmvapic.bin [size=9216] > QEMU: e820/ram: 0x00000000 +0x08000000 > QEMU: reserve ioports 0x0510-0x0511 [firmware-config] > QEMU: reserve ioports 0x5658-0x5658 [vmware-port] > QEMU: reserve ioports 0xae00-0xae0f [pci-hotplug] > QEMU: reserve ioports 0xaf00-0xaf1f [cpu-hotplug] > QEMU: reserve ioports 0xafe0-0xafe3 [piix4-gpe0] > Done reading resources. > Setting resources... > PCI: 00:01.1 20 <- [0x0000005840 - 0x000000584f] size 0x00000010 gran 0x04 io > PCI: 00:02.0 10 <- [0x00fd000000 - 0x00fdffffff] size 0x01000000 gran > 0x18 prefmem > PCI: 00:02.0 18 <- [0x00fe070000 - 0x00fe070fff] size 0x00001000 gran 0x0c mem > PCI: 00:02.0 30 <- [0x00fe060000 - 0x00fe06ffff] size 0x00010000 gran 0x10 romem > PCI: 00:03.0 10 <- [0x00fe040000 - 0x00fe05ffff] size 0x00020000 gran 0x11 mem > PCI: 00:03.0 14 <- [0x0000005800 - 0x000000583f] size 0x00000040 gran 0x06 io > PCI: 00:03.0 30 <- [0x00fe000000 - 0x00fe03ffff] size 0x00040000 gran 0x12 romem > Done setting resources. > Done allocating resources. > Enabling resources... > PCI: 00:00.0 cmd <- 00 > PCI: 00:01.0 cmd <- 00 > PCI: 00:01.1 cmd <- 01 > PCI: 00:01.3 cmd <- 00 > PCI: 00:02.0 cmd <- 03 > PCI: 00:03.0 cmd <- 03 > done. > Initializing devices... > Root Device init ... > CPU_CLUSTER: 0 init ... > Initializing CPU #0 > CPU: vendor Intel device 663 > CPU: family 06, model 06, stepping 03 > Setting up local APIC... apic_id: 0x00 done. > CPU #0 initialized > PCI: 00:00.0 init ... > Assigning IRQ 10 to 0:1.3 > Assigning IRQ 11 to 0:3.0 > PCI: 00:01.0 init ... > RTC Init > PCI: 00:01.1 init ... > IDE: Primary IDE interface: on > IDE: Secondary IDE interface: on > IDE: Access to legacy IDE ports: off > PCI: 00:02.0 init ... > PCI: 00:03.0 init ... > Devices initialized > Finalize devices... > Devices finalized > Copying Interrupt Routing Table to 0x000f0000... done. > Copying Interrupt Routing Table to 0x07fb3000... done. > PIRQ table: 128 bytes. > QEMU: found ACPI tables in fw_cfg. > QEMU: loading "etc/acpi/rsdp" to 0x7f8f000 (len 36) > QEMU: loading "etc/acpi/tables" to 0x7f8f040 (len 131072) > QEMU: loaded ACPI tables from fw_cfg. > ACPI tables: 131136 bytes. > smbios_write_tables: 07f8e000 > DOMAIN: 0000 (QEMU Northbridge i440fx) > QEMU: found smbios tables in fw_cfg (len 320). > QEMU: coreboot type0 table found at 0x7f8e020. > QEMU: loading smbios tables to 0x7f8e062 > SMBIOS tables: 418 bytes. > Writing table forward entry at 0x00000500 > Wrote coreboot table at: 00000500, 0x10 bytes, checksum b7e3 > Writing coreboot table at 0x07fb4000 > 0. 0000000000000000-0000000000000fff: CONFIGURATION TABLES > 1. 0000000000001000-000000000009ffff: RAM > 2. 00000000000c0000-0000000007f8dfff: RAM > 3. 0000000007f8e000-0000000007fbcfff: CONFIGURATION TABLES > 4. 0000000007fbd000-0000000007fdcfff: RAMSTAGE > 5. 0000000007fdd000-0000000007ffffff: CONFIGURATION TABLES > 6. 00000000ff800000-00000000ffffffff: RESERVED > CBFS: 'Master Header Locator' located CBFS at [1fa0200:2000000) > FMAP: Found "FLASH" version 1.1 at 1fa0000. > FMAP: base = fe000000 size = 2000000 #areas = 3 > Wrote coreboot table at: 07fb4000, 0x264 bytes, checksum f4f2 > coreboot table: 636 bytes. > IMD ROOT 0. 07fff000 00001000 > IMD SMALL 1. 07ffe000 00001000 > CONSOLE 2. 07fde000 00020000 > TIME STAMP 3. 07fdd000 00000910 > RAMSTAGE 4. 07fbc000 00021000 > COREBOOT 5. 07fb4000 00008000 > IRQ TABLE 6. 07fb3000 00001000 > ACPI 7. 07f8f000 00024000 > SMBIOS 8. 07f8e000 00000800 > IMD small region: > IMD ROOT 0. 07ffec00 00000400 > CAR GLOBALS 1. 07ffe9c0 00000240 > COREBOOTFWD 2. 07ffe980 00000028 > CBFS: 'Master Header Locator' located CBFS at [1fa0200:2000000) > CBFS: Locating 'fallback/payload' > CBFS: Found @ offset 10000 size 354b5 > Checking segment from ROM address 0xfffb0238 > Checking segment from ROM address 0xfffb0254 > Loading segment from ROM address 0xfffb0238 > code (compression=1) > New segment dstaddr 0x01110000 memsize 0x793df srcaddr 0xfffb0270 > filesize 0x3547d > Loading Segment: addr: 0x01110000 memsz: 0x00000000000793df filesz: > 0x000000000003547d > using LZMA > Loading segment from ROM address 0xfffb0254 > Entry Point 0x01110015 > Jumping to boot code at 01110015(07fb4000) > > > U-Boot 2019.01-rc1-00277-gee168783ae (Dec 12 2018 - 15:10:41 +0100) > > CPU: x86, vendor Intel, device 663h > DRAM: 127.4 MiB > MMC: > Video: No video mode configured in coreboot! > Video: No video mode configured in coreboot! > Model: coreboot x86 payload > Net: e1000: 52:54:00:12:34:56 > > Warning: e1000#0 using MAC address from ROM > eth0: e1000#0 > No controllers found > Finalizing coreboot > Hit any key to stop autoboot: 0 > => cbfsinit > Bad CBFS file. > => > > > So lets try to connect u-boot and coreboot sources in that area. > > file_cbfs_next_file(..): > https://git.denx.de/?p=u-boot.git;a=blob;f=fs/cbfs/cbfs.c;h=0dce639b49ce095ac3eede2ee9c3138f1e0af3bb;hb=HEAD#l76 > cbfs_for_each_file(..): > https://github.com/coreboot/coreboot/blob/eeb4e20b2f6d786c92fe3efb30817e90389a2bfe/src/commonlib/cbfs.c#L64 > > Both ot these two functions do more or less the same: look for files > in cbfs with the only difference that u-boots impl. does some > validations that > are not done in coreboot: > https://git.denx.de/?p=u-boot.git;a=blob;f=fs/cbfs/cbfs.c;h=0dce639b49ce095ac3eede2ee9c3138f1e0af3bb;hb=HEAD#l99 > > With this patch cbfsinit works and cbfsinfo shows the expected output: > => cbfsinfo > > CBFS version: 0x31313132 > ROM size: 0x2000000 > Boot block size: 0x4 > CBFS size: 0x5fdfc > Alignment: 64 > Offset: 0x1fa0200 > > > What is needed to get further with this patch? > ping
Hi Christian, On Tue, Dec 18, 2018 at 4:18 PM Christian Gmeiner <christian.gmeiner@gmail.com> wrote: > > Am Mi., 12. Dez. 2018 um 15:27 Uhr schrieb Christian Gmeiner > <christian.gmeiner@gmail.com>: > > > > Hi Bin, > > > > Finally I have some time to look deeper into this issue. > > > > > > > > On Thu, Sep 20, 2018 at 10:47 PM Christian Gmeiner > > > <christian.gmeiner@gmail.com> wrote: > > > > > > > > Coreboot does not contain such a check: > > > > https://github.com/coreboot/coreboot/blob/eeb4e20b2f6d786c92fe3efb30817e90389a2bfe/src/commonlib/cbfs.c#L64 > > > > > > > > Before this change cbfsinit failed with 'Bad CBFS file'. After this change all cbfs commands > > > > are working as expected. > > > > > > > > Signed-off-by: Christian Gmeiner <christian.gmeiner@gmail.com> > > > > --- > > > > fs/cbfs/cbfs.c | 6 +----- > > > > 1 file changed, 1 insertion(+), 5 deletions(-) > > > > > > > > diff --git a/fs/cbfs/cbfs.c b/fs/cbfs/cbfs.c > > > > index 0dce639b49..2a581f0c18 100644 > > > > --- a/fs/cbfs/cbfs.c > > > > +++ b/fs/cbfs/cbfs.c > > > > @@ -96,11 +96,7 @@ static int file_cbfs_next_file(u8 *start, u32 size, u32 align, > > > > } > > > > > > > > swap_file_header(&header, fileHeader); > > > > - if (header.offset < sizeof(struct cbfs_fileheader) || > > > > - header.offset > header.len) { > > > > - file_cbfs_result = CBFS_BAD_FILE; > > > > - return -1; > > > > - } > > > > > > It looks to me the existing codes were doing some sanity checks. Can > > > you elaborate why this is failing on your board? In your coreboot > > > reference, I don't see exactly how U-Boot codes are connected to the > > > coreboot one. > > > > > > > This has nothing to do with my board at all - I can easily reproduce > > this issue under qemu: > > > > qemu-system-i386 -bios build/coreboot.rom -hda /dev/zero -serial > > stdio -display none > > WARNING: Image format was not specified for '/dev/zero' and probing guessed raw. > > Automatically detecting the format is dangerous for raw > > images, write operations on block 0 will be restricted. > > Specify the 'raw' format explicitly to remove the restrictions. > > > > > > coreboot-4.8-2510-g303a4bfd4a Wed Dec 12 02:20:54 UTC 2018 bootblock starting... > > CBFS: 'Master Header Locator' located CBFS at [1fa0200:2000000) > > CBFS: Locating 'fallback/romstage' > > CBFS: Found @ offset 80 size 3c04 > > > > > > coreboot-4.8-2510-g303a4bfd4a Wed Dec 12 02:20:54 UTC 2018 romstage starting... > > CBMEM: > > IMD: root @ 07fff000 254 entries. > > IMD: root @ 07ffec00 62 entries. > > CBFS: 'Master Header Locator' located CBFS at [1fa0200:2000000) > > CBFS: Locating 'fallback/ramstage' > > CBFS: Found @ offset 3d00 size acb2 > > Decompressing stage fallback/ramstage @ 0x07fbcfc0 (128664 bytes) > > Loading module at 07fbd000 with entry 07fbd000. filesize: 0x15750 > > memsize: 0x1f658 > > Processing 1257 relocs. Offset value of 0x071bd000 > > > > > > coreboot-4.8-2510-g303a4bfd4a Wed Dec 12 02:20:54 UTC 2018 ramstage starting... > > Enumerating buses... > > CPU_CLUSTER: 0 enabled > > DOMAIN: 0000 enabled > > QEMU: firmware config interface detected > > QEMU: max_cpus is 1 > > CPU: APIC: 00 enabled > > scan_bus: scanning of bus CPU_CLUSTER: 0 took 0 usecs > > PCI: pci_scan_bus for bus 00 > > PCI: 00:00.0 [8086/1237] enabled > > PCI: 00:01.0 [8086/7000] enabled > > PCI: 00:01.1 [8086/7010] enabled > > PCI: 00:01.3 [8086/7113] enabled > > PCI: 00:02.0 [1234/1111] enabled > > PCI: 00:03.0 [8086/100e] enabled > > scan_bus: scanning of bus PCI: 00:01.0 took 0 usecs > > scan_bus: scanning of bus PCI: 00:01.3 took 0 usecs > > scan_bus: scanning of bus DOMAIN: 0000 took 0 usecs > > scan_bus: scanning of bus Root Device took 0 usecs > > done > > found VGA at PCI: 00:02.0 > > Setting up VGA for PCI: 00:02.0 > > Setting PCI_BRIDGE_CTL_VGA for bridge DOMAIN: 0000 > > Setting PCI_BRIDGE_CTL_VGA for bridge Root Device > > Allocating resources... > > Reading resources... > > QEMU: 11 files in fw_cfg > > QEMU: bootorder [size=0] > > QEMU: etc/acpi/rsdp [size=36] > > QEMU: etc/acpi/tables [size=131072] > > QEMU: etc/boot-fail-wait [size=4] > > QEMU: etc/e820 [size=20] > > QEMU: etc/smbios/smbios-anchor [size=31] > > QEMU: etc/smbios/smbios-tables [size=320] > > QEMU: etc/system-states [size=6] > > QEMU: etc/table-loader [size=4096] > > QEMU: etc/tpm/log [size=0] > > QEMU: genroms/kvmvapic.bin [size=9216] > > QEMU: e820/ram: 0x00000000 +0x08000000 > > QEMU: reserve ioports 0x0510-0x0511 [firmware-config] > > QEMU: reserve ioports 0x5658-0x5658 [vmware-port] > > QEMU: reserve ioports 0xae00-0xae0f [pci-hotplug] > > QEMU: reserve ioports 0xaf00-0xaf1f [cpu-hotplug] > > QEMU: reserve ioports 0xafe0-0xafe3 [piix4-gpe0] > > Done reading resources. > > Setting resources... > > PCI: 00:01.1 20 <- [0x0000005840 - 0x000000584f] size 0x00000010 gran 0x04 io > > PCI: 00:02.0 10 <- [0x00fd000000 - 0x00fdffffff] size 0x01000000 gran > > 0x18 prefmem > > PCI: 00:02.0 18 <- [0x00fe070000 - 0x00fe070fff] size 0x00001000 gran 0x0c mem > > PCI: 00:02.0 30 <- [0x00fe060000 - 0x00fe06ffff] size 0x00010000 gran 0x10 romem > > PCI: 00:03.0 10 <- [0x00fe040000 - 0x00fe05ffff] size 0x00020000 gran 0x11 mem > > PCI: 00:03.0 14 <- [0x0000005800 - 0x000000583f] size 0x00000040 gran 0x06 io > > PCI: 00:03.0 30 <- [0x00fe000000 - 0x00fe03ffff] size 0x00040000 gran 0x12 romem > > Done setting resources. > > Done allocating resources. > > Enabling resources... > > PCI: 00:00.0 cmd <- 00 > > PCI: 00:01.0 cmd <- 00 > > PCI: 00:01.1 cmd <- 01 > > PCI: 00:01.3 cmd <- 00 > > PCI: 00:02.0 cmd <- 03 > > PCI: 00:03.0 cmd <- 03 > > done. > > Initializing devices... > > Root Device init ... > > CPU_CLUSTER: 0 init ... > > Initializing CPU #0 > > CPU: vendor Intel device 663 > > CPU: family 06, model 06, stepping 03 > > Setting up local APIC... apic_id: 0x00 done. > > CPU #0 initialized > > PCI: 00:00.0 init ... > > Assigning IRQ 10 to 0:1.3 > > Assigning IRQ 11 to 0:3.0 > > PCI: 00:01.0 init ... > > RTC Init > > PCI: 00:01.1 init ... > > IDE: Primary IDE interface: on > > IDE: Secondary IDE interface: on > > IDE: Access to legacy IDE ports: off > > PCI: 00:02.0 init ... > > PCI: 00:03.0 init ... > > Devices initialized > > Finalize devices... > > Devices finalized > > Copying Interrupt Routing Table to 0x000f0000... done. > > Copying Interrupt Routing Table to 0x07fb3000... done. > > PIRQ table: 128 bytes. > > QEMU: found ACPI tables in fw_cfg. > > QEMU: loading "etc/acpi/rsdp" to 0x7f8f000 (len 36) > > QEMU: loading "etc/acpi/tables" to 0x7f8f040 (len 131072) > > QEMU: loaded ACPI tables from fw_cfg. > > ACPI tables: 131136 bytes. > > smbios_write_tables: 07f8e000 > > DOMAIN: 0000 (QEMU Northbridge i440fx) > > QEMU: found smbios tables in fw_cfg (len 320). > > QEMU: coreboot type0 table found at 0x7f8e020. > > QEMU: loading smbios tables to 0x7f8e062 > > SMBIOS tables: 418 bytes. > > Writing table forward entry at 0x00000500 > > Wrote coreboot table at: 00000500, 0x10 bytes, checksum b7e3 > > Writing coreboot table at 0x07fb4000 > > 0. 0000000000000000-0000000000000fff: CONFIGURATION TABLES > > 1. 0000000000001000-000000000009ffff: RAM > > 2. 00000000000c0000-0000000007f8dfff: RAM > > 3. 0000000007f8e000-0000000007fbcfff: CONFIGURATION TABLES > > 4. 0000000007fbd000-0000000007fdcfff: RAMSTAGE > > 5. 0000000007fdd000-0000000007ffffff: CONFIGURATION TABLES > > 6. 00000000ff800000-00000000ffffffff: RESERVED > > CBFS: 'Master Header Locator' located CBFS at [1fa0200:2000000) > > FMAP: Found "FLASH" version 1.1 at 1fa0000. > > FMAP: base = fe000000 size = 2000000 #areas = 3 > > Wrote coreboot table at: 07fb4000, 0x264 bytes, checksum f4f2 > > coreboot table: 636 bytes. > > IMD ROOT 0. 07fff000 00001000 > > IMD SMALL 1. 07ffe000 00001000 > > CONSOLE 2. 07fde000 00020000 > > TIME STAMP 3. 07fdd000 00000910 > > RAMSTAGE 4. 07fbc000 00021000 > > COREBOOT 5. 07fb4000 00008000 > > IRQ TABLE 6. 07fb3000 00001000 > > ACPI 7. 07f8f000 00024000 > > SMBIOS 8. 07f8e000 00000800 > > IMD small region: > > IMD ROOT 0. 07ffec00 00000400 > > CAR GLOBALS 1. 07ffe9c0 00000240 > > COREBOOTFWD 2. 07ffe980 00000028 > > CBFS: 'Master Header Locator' located CBFS at [1fa0200:2000000) > > CBFS: Locating 'fallback/payload' > > CBFS: Found @ offset 10000 size 354b5 > > Checking segment from ROM address 0xfffb0238 > > Checking segment from ROM address 0xfffb0254 > > Loading segment from ROM address 0xfffb0238 > > code (compression=1) > > New segment dstaddr 0x01110000 memsize 0x793df srcaddr 0xfffb0270 > > filesize 0x3547d > > Loading Segment: addr: 0x01110000 memsz: 0x00000000000793df filesz: > > 0x000000000003547d > > using LZMA > > Loading segment from ROM address 0xfffb0254 > > Entry Point 0x01110015 > > Jumping to boot code at 01110015(07fb4000) > > > > > > U-Boot 2019.01-rc1-00277-gee168783ae (Dec 12 2018 - 15:10:41 +0100) > > > > CPU: x86, vendor Intel, device 663h > > DRAM: 127.4 MiB > > MMC: > > Video: No video mode configured in coreboot! > > Video: No video mode configured in coreboot! > > Model: coreboot x86 payload > > Net: e1000: 52:54:00:12:34:56 > > > > Warning: e1000#0 using MAC address from ROM > > eth0: e1000#0 > > No controllers found > > Finalizing coreboot > > Hit any key to stop autoboot: 0 > > => cbfsinit > > Bad CBFS file. > > => > > > > > > So lets try to connect u-boot and coreboot sources in that area. > > > > file_cbfs_next_file(..): > > https://git.denx.de/?p=u-boot.git;a=blob;f=fs/cbfs/cbfs.c;h=0dce639b49ce095ac3eede2ee9c3138f1e0af3bb;hb=HEAD#l76 > > cbfs_for_each_file(..): > > https://github.com/coreboot/coreboot/blob/eeb4e20b2f6d786c92fe3efb30817e90389a2bfe/src/commonlib/cbfs.c#L64 > > > > Both ot these two functions do more or less the same: look for files > > in cbfs with the only difference that u-boots impl. does some > > validations that > > are not done in coreboot: > > https://git.denx.de/?p=u-boot.git;a=blob;f=fs/cbfs/cbfs.c;h=0dce639b49ce095ac3eede2ee9c3138f1e0af3bb;hb=HEAD#l99 > > > > With this patch cbfsinit works and cbfsinfo shows the expected output: > > => cbfsinfo > > > > CBFS version: 0x31313132 > > ROM size: 0x2000000 > > Boot block size: 0x4 > > CBFS size: 0x5fdfc > > Alignment: 64 > > Offset: 0x1fa0200 > > > > > > What is needed to get further with this patch? > > > > ping Sorry, I meant to take some time to have a look at this. Will do this week. Regards, Bin
Hi Christian, On Tue, Dec 18, 2018 at 4:57 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > Hi Christian, > > On Tue, Dec 18, 2018 at 4:18 PM Christian Gmeiner > <christian.gmeiner@gmail.com> wrote: > > > > Am Mi., 12. Dez. 2018 um 15:27 Uhr schrieb Christian Gmeiner > > <christian.gmeiner@gmail.com>: > > > > > > Hi Bin, > > > > > > Finally I have some time to look deeper into this issue. > > > > > > > > > > > On Thu, Sep 20, 2018 at 10:47 PM Christian Gmeiner > > > > <christian.gmeiner@gmail.com> wrote: > > > > > > > > > > Coreboot does not contain such a check: > > > > > https://github.com/coreboot/coreboot/blob/eeb4e20b2f6d786c92fe3efb30817e90389a2bfe/src/commonlib/cbfs.c#L64 > > > > > > > > > > Before this change cbfsinit failed with 'Bad CBFS file'. After this change all cbfs commands > > > > > are working as expected. > > > > > > > > > > Signed-off-by: Christian Gmeiner <christian.gmeiner@gmail.com> > > > > > --- > > > > > fs/cbfs/cbfs.c | 6 +----- > > > > > 1 file changed, 1 insertion(+), 5 deletions(-) > > > > > > > > > > diff --git a/fs/cbfs/cbfs.c b/fs/cbfs/cbfs.c > > > > > index 0dce639b49..2a581f0c18 100644 > > > > > --- a/fs/cbfs/cbfs.c > > > > > +++ b/fs/cbfs/cbfs.c > > > > > @@ -96,11 +96,7 @@ static int file_cbfs_next_file(u8 *start, u32 size, u32 align, > > > > > } > > > > > > > > > > swap_file_header(&header, fileHeader); > > > > > - if (header.offset < sizeof(struct cbfs_fileheader) || > > > > > - header.offset > header.len) { > > > > > - file_cbfs_result = CBFS_BAD_FILE; > > > > > - return -1; > > > > > - } > > > > > > > > It looks to me the existing codes were doing some sanity checks. Can > > > > you elaborate why this is failing on your board? In your coreboot > > > > reference, I don't see exactly how U-Boot codes are connected to the > > > > coreboot one. > > > > > > > > > > This has nothing to do with my board at all - I can easily reproduce > > > this issue under qemu: > > > [snip] > > > > > > > > > What is needed to get further with this patch? > > > > > > > ping > > Sorry, I meant to take some time to have a look at this. Will do this week. > I have looked at this patch. It looks we just need remove the "header.offset > header.len" check. The other check is still valid. During the investigation, I've noticed some other issues in the cbfs codes. I've included your patch in my series [1]. Please have a look. [1] http://patchwork.ozlabs.org/project/uboot/list/?series=83355 Regards, Bin
Hi Bin, this patch reminds me of one I sent some time ago: http://patchwork.ozlabs.org/patch/873666/ I forgot about it, so didn't follow up on the comments, but iirc this fixed a 'Bad CBFS file' error for me too. Does that work for you? Regards, Andre On 22/12/2018 10:58, Bin Meng wrote: > Hi Christian, > > On Tue, Dec 18, 2018 at 4:57 PM Bin Meng <bmeng.cn@gmail.com> wrote: >> >> Hi Christian, >> >> On Tue, Dec 18, 2018 at 4:18 PM Christian Gmeiner >> <christian.gmeiner@gmail.com> wrote: >>> >>> Am Mi., 12. Dez. 2018 um 15:27 Uhr schrieb Christian Gmeiner >>> <christian.gmeiner@gmail.com>: >>>> >>>> Hi Bin, >>>> >>>> Finally I have some time to look deeper into this issue. >>>> >>>>> >>>>> On Thu, Sep 20, 2018 at 10:47 PM Christian Gmeiner >>>>> <christian.gmeiner@gmail.com> wrote: >>>>>> >>>>>> Coreboot does not contain such a check: >>>>>> https://github.com/coreboot/coreboot/blob/eeb4e20b2f6d786c92fe3efb30817e90389a2bfe/src/commonlib/cbfs.c#L64 >>>>>> >>>>>> Before this change cbfsinit failed with 'Bad CBFS file'. After this change all cbfs commands >>>>>> are working as expected. >>>>>> >>>>>> Signed-off-by: Christian Gmeiner <christian.gmeiner@gmail.com> >>>>>> --- >>>>>> fs/cbfs/cbfs.c | 6 +----- >>>>>> 1 file changed, 1 insertion(+), 5 deletions(-) >>>>>> >>>>>> diff --git a/fs/cbfs/cbfs.c b/fs/cbfs/cbfs.c >>>>>> index 0dce639b49..2a581f0c18 100644 >>>>>> --- a/fs/cbfs/cbfs.c >>>>>> +++ b/fs/cbfs/cbfs.c >>>>>> @@ -96,11 +96,7 @@ static int file_cbfs_next_file(u8 *start, u32 size, u32 align, >>>>>> } >>>>>> >>>>>> swap_file_header(&header, fileHeader); >>>>>> - if (header.offset < sizeof(struct cbfs_fileheader) || >>>>>> - header.offset > header.len) { >>>>>> - file_cbfs_result = CBFS_BAD_FILE; >>>>>> - return -1; >>>>>> - } >>>>> >>>>> It looks to me the existing codes were doing some sanity checks. Can >>>>> you elaborate why this is failing on your board? In your coreboot >>>>> reference, I don't see exactly how U-Boot codes are connected to the >>>>> coreboot one. >>>>> >>>> >>>> This has nothing to do with my board at all - I can easily reproduce >>>> this issue under qemu: >>>> > > [snip] > >>>> >>>> >>>> What is needed to get further with this patch? >>>> >>> >>> ping >> >> Sorry, I meant to take some time to have a look at this. Will do this week. >> > > I have looked at this patch. It looks we just need remove the > "header.offset > header.len" check. The other check is still valid. > > During the investigation, I've noticed some other issues in the cbfs > codes. I've included your patch in my series [1]. Please have a look. > > [1] http://patchwork.ozlabs.org/project/uboot/list/?series=83355 > > Regards, > Bin > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > https://lists.denx.de/listinfo/u-boot >
Hi Andre, On Sat, Dec 22, 2018 at 7:07 PM Andre Heider <a.heider@gmail.com> wrote: > > Hi Bin, > > this patch reminds me of one I sent some time ago: > http://patchwork.ozlabs.org/patch/873666/ > > I forgot about it, so didn't follow up on the comments, but iirc this > fixed a 'Bad CBFS file' error for me too. Does that work for you? > I checked your patch. It can "fix" the problem, but the approach your patch used is not 100% correct. Please check my series instead. [snip] Regards, Bin
diff --git a/fs/cbfs/cbfs.c b/fs/cbfs/cbfs.c index 0dce639b49..2a581f0c18 100644 --- a/fs/cbfs/cbfs.c +++ b/fs/cbfs/cbfs.c @@ -96,11 +96,7 @@ static int file_cbfs_next_file(u8 *start, u32 size, u32 align, } swap_file_header(&header, fileHeader); - if (header.offset < sizeof(struct cbfs_fileheader) || - header.offset > header.len) { - file_cbfs_result = CBFS_BAD_FILE; - return -1; - } + newNode->next = NULL; newNode->type = header.type; newNode->data = start + header.offset;
Coreboot does not contain such a check: https://github.com/coreboot/coreboot/blob/eeb4e20b2f6d786c92fe3efb30817e90389a2bfe/src/commonlib/cbfs.c#L64 Before this change cbfsinit failed with 'Bad CBFS file'. After this change all cbfs commands are working as expected. Signed-off-by: Christian Gmeiner <christian.gmeiner@gmail.com> --- fs/cbfs/cbfs.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)