On 02.09.25 18:24, Jan Kiszka wrote: > On 02.09.25 18:20, Philippe Mathieu-Daudé wrote: >> On 2/9/25 18:14, Philippe Mathieu-Daudé wrote: >>> On 2/9/25 18:00, Cédric Le Goater wrote: >>>> On 9/2/25 17:55, Philippe Mathieu-Daudé wrote: >>>>> On 2/9/25 17:47, Cédric Le Goater wrote: >>>>>> On 9/2/25 17:45, Philippe Mathieu-Daudé wrote: >>>>>>> On 2/9/25 17:43, Philippe Mathieu-Daudé wrote: >>>>>>>> On 2/9/25 17:34, Jan Kiszka wrote: >>>>>>>>> On 02.09.25 17:06, Philippe Mathieu-Daudé wrote: >>>>>>>>>> On 1/9/25 07:56, Jan Kiszka wrote: >>>>>>>>>>> From: Jan Kiszka <[email protected]> >>>>>>>>>>> >>>>>>>>>>> The power-of-2 rule applies to the user data area, not the >>>>>>>>>>> complete >>>>>>>>>>> block image. The latter can be concatenation of boot partition >>>>>>>>>>> images >>>>>>>>>>> and the user data. >>>>>>>>>>> >>>>>>>>>>> Signed-off-by: Jan Kiszka <[email protected]> >>>>>>>>>>> Reviewed-by: Philippe Mathieu-Daudé <[email protected]> >>>>>>>>>>> --- >>>>>>>>>>> hw/sd/sd.c | 2 +- >>>>>>>>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>>>>>>>> >>>>>>>>>>> diff --git a/hw/sd/sd.c b/hw/sd/sd.c >>>>>>>>>>> index 8c290595f0..16aee210b4 100644 >>>>>>>>>>> --- a/hw/sd/sd.c >>>>>>>>>>> +++ b/hw/sd/sd.c >>>>>>>>>>> @@ -2789,7 +2789,7 @@ static void sd_realize(DeviceState *dev, >>>>>>>>>>> Error >>>>>>>>>>> **errp) >>>>>>>>>>> return; >>>>>>>>>>> } >>>>>>>>>>> - blk_size = blk_getlength(sd->blk); >>>>>>>>>>> + blk_size = blk_getlength(sd->blk) - sd- >>>>>>>>>>>> boot_part_size * 2; >>>>>>>>>>> if (blk_size > 0 && !is_power_of_2(blk_size)) { >>>>>>>>>>> int64_t blk_size_aligned = pow2ceil(blk_size); >>>>>>>>>>> char *blk_size_str; >>>>>>>>>> >>>>>>>>>> This seems to break the tests/functional/arm/ >>>>>>>>>> test_aspeed_rainier.py >>>>>>>>>> test due to mmc-p10bmc-20240617.qcow2 size: >>>>>>>>>> >>>>>>>>>> Command: /builds/qemu-project/qemu/build/qemu-system-arm - >>>>>>>>>> display none - >>>>>>>>>> vga none -chardev socket,id=mon,fd=5 -mon >>>>>>>>>> chardev=mon,mode=control - >>>>>>>>>> machine rainier-bmc -chardev socket,id=console,fd=10 -serial >>>>>>>>>> chardev:console -drive file=/builds/qemu-project/qemu/ >>>>>>>>>> functional- cache/ >>>>>>>>>> download/ >>>>>>>>>> d523fb478d2b84d5adc5658d08502bc64b1486955683814f89c6137518acd90b,if=sd,id=sd2,index=2 >>>>>>>>>> -net nic -net user -snapshot >>>>>>>>>> Output: qemu-system-arm: Invalid SD card size: 16 GiB >>>>>>>>>> SD card size has to be a power of 2, e.g. 16 GiB. >>>>>>>>>> >>>>>>>>>> https://gitlab.com/qemu-project/qemu/-/jobs/11217561316 >>>>>>>>>> >>>>>>>>> >>>>>>>>> Hmm, then the test was always wrong as well. I suspect the >>>>>>>>> aspeed is >>>>>>>>> enabling boot partitions by default, and the image was created >>>>>>>>> to pass >>>>>>>>> the wrong alignment check. Where / by whom is the image maintained? >>>>>>>> >>>>>>>> Cédric Le Goater (Cc'ed). >>>>>>>> >>>>>>>> The test comes from: >>>>>>>> https://lore.kernel.org/qemu-devel/4d1777d6-0195-4ecb- >>>>>>>> [email protected]/ >>>>>>>> >>>>>>>> Maybe also relevant to your suspicion: >>>>>>>> https://lore.kernel.org/qemu-devel/e401d119-402e-0edd- >>>>>>>> [email protected]/ >>> >>> [*] >>> >>>>>>> >>>>>>> Digging further: >>>>>>> https://lore.kernel.org/qemu- >>>>>>> devel/[email protected]/ >>>>>>> >>>>>> >>>>>> yes commit c078298301a8 might have some impact there. >>>>> >>>>> With Jan patch, your script doesn't need anymore the >>>>> >>>>> echo "Fixing size to keep qemu happy..." >>>>> >>>>> kludge. >>>> >>>> which script ? >>> >>> The one you pasted in [*]: >>> >>> -- >>> #!/bin/sh >>> >>> URLBASE=https://jenkins.openbmc.org/view/latest/job/latest-master/ >>> label=docker-builder,target=witherspoon-tacoma/lastSuccessfulBuild/ >>> artifact/openbmc/build/tmp/deploy/images/witherspoon-tacoma/ >>> >>> IMAGESIZE=128 >>> OUTFILE=mmc.img >>> >>> FILES="u-boot.bin u-boot-spl.bin obmc-phosphor-image-witherspoon- >>> tacoma.wic.xz" >>> >>> for file in ${FILES}; do >>> >>> if test -f ${file}; then >>> echo "${file}: Already downloaded" >>> else >>> echo "${file}: Downloading" >>> wget -nv ${URLBASE}/${file} >>> fi >>> done >>> >>> echo >>> >>> echo "Creating empty image..." >>> dd status=none if=/dev/zero of=${OUTFILE} bs=1M count=${IMAGESIZE} >>> echo "Adding SPL..." >>> dd status=none if=u-boot-spl.bin of=${OUTFILE} conv=notrunc >>> echo "Adding u-boot..." >>> dd status=none if=u-boot.bin of=${OUTFILE} conv=notrunc bs=1K seek=64 >>> echo "Adding userdata..." >>> unxz -c obmc-phosphor-image-witherspoon-tacoma.wic.xz | dd >>> status=progress of=${OUTFILE} conv=notrunc bs=1M seek=2 >>> echo "Fixing size to keep qemu happy..." >>> truncate --size 16G ${OUTFILE} >>> >>> echo "Done!" >>> echo >>> echo " qemu-system-arm -M tacoma-bmc -nographic -drive >>> file=mmc.img,if=sd,index=2,format=raw" >>> --- >> >> FTR the alignment check was added to shut up fuzzed CVEs in commit >> a9bcedd15a5 ("hw/sd/sdcard: Do not allow invalid SD card sizes"): >> >> QEMU allows to create SD card with unrealistic sizes. This could >> work, but some guests (at least Linux) consider sizes that are not >> a power of 2 as a firmware bug and fix the card size to the next >> power of 2. >> >> While the possibility to use small SD card images has been seen as >> a feature, it became a bug with CVE-2020-13253, where the guest is >> able to do OOB read/write accesses past the image size end. >> >> In a pair of commits we will fix CVE-2020-13253 as: >> >> Read command is rejected if BLOCK_LEN_ERROR or ADDRESS_ERROR >> occurred and no data transfer is performed. >> >> Write command is rejected if BLOCK_LEN_ERROR or ADDRESS_ERROR >> occurred and no data transfer is performed. >> >> WP_VIOLATION errors are not modified: the error bit is set, we >> stay in receive-data state, wait for a stop command. All further >> data transfer is ignored. See the check on sd->card_status at >> the beginning of sd_read_data() and sd_write_data(). >> >> While this is the correct behavior, in case QEMU create smaller SD >> cards, guests still try to access past the image size end, and QEMU >> considers this is an invalid address, thus "all further data >> transfer is ignored". This is wrong and make the guest looping until >> eventually timeouts. >> >> Fix by not allowing invalid SD card sizes (suggesting the expected >> size as a hint): >> >> $ qemu-system-arm -M orangepi-pc -drive >> file=rootfs.ext2,if=sd,format=raw >> qemu-system-arm: Invalid SD card size: 60 MiB >> SD card size has to be a power of 2, e.g. 64 MiB. >> You can resize disk images with 'qemu-img resize <imagefile> <new- >> size>' >> (note that this will lose data if you make the image smaller than >> it currently is). >> >> >> I expect us to be safe and able to deal with non-pow2 regions if we use >> QEMUSGList from the "system/dma.h" API. But this is a rework nobody had >> time to do so far. > > We have to tell two things apart: partitions sizes on the one side and > backing storage sizes. The partitions sizes are (to my reading) clearly > defined in the spec, and the user partition (alone!) has to be power of > 2. The boot and RPMB partitions are multiples of 128K. The sum of them > all is nowhere limited to power of 2 or even only multiples of 128K. >
Re-reading the part of the device capacity, the rules are more complex: - power of two up to 2 GB - multiple of 512 bytes beyond that So that power-of-two enforcement was and still is likely too strict. But I still see no indication, neither in the existing eMMC code of QEMU nor the spec, that the boot and RPMB partition sizes are included in that. Jan -- Siemens AG, Foundational Technologies Linux Expert Center
