[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 7/9] hw/sd/sdcard: Do not allow invalid SD card sizes
From: |
Markus Armbruster |
Subject: |
Re: [PATCH v2 7/9] hw/sd/sdcard: Do not allow invalid SD card sizes |
Date: |
Tue, 14 Jul 2020 11:40:07 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) |
Peter Maydell <peter.maydell@linaro.org> writes:
> On Mon, 13 Jul 2020 at 19:32, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>>
>> 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.
>>
>
>> + error_setg(errp, "Invalid SD card size: %s", blk_size_str);
>> + g_free(blk_size_str);
>> +
>> + blk_size_str = size_to_str(blk_size_aligned);
>> + error_append_hint(errp,
>> + "SD card size has to be a power of 2, e.g.
>> %s.\n"
>> + "You can resize disk images with "
>> + "'qemu-img resize <imagefile> <new-size>'\n"
>> + "(note that this will lose data if you make
>> the "
>> + "image smaller than it currently is).\n",
>> + blk_size_str);
>> + g_free(blk_size_str);
>
> Some places that create multi-line hints with error_append_hint()
> do it by calling it once per line, eg in target/arm/cpu64.c:
> error_setg(errp, "cannot disable sve128");
> error_append_hint(errp, "Disabling sve128 results in all "
> "vector lengths being disabled.\n");
> error_append_hint(errp, "With SVE enabled, at least one "
> "vector length must be enabled.\n");
>
> Some places don't, eg in block/vhdx-log.c:
> error_append_hint(errp, "To replay the log, run:\n"
> "qemu-img check -r all '%s'\n",
> bs->filename);
Both fine.
> Most places terminate with a '\n', but some places don't, eg
> crypto/block-luks.c:
> error_append_hint(errp, "Failed to write to keyslot %i", keyslot);
> return -1;
This is a bug.
> The documentation says
> * May be called multiple times. The resulting hint should end with a
> * newline.
>
> which isn't very clear -- you can call it multiple times, but
> must you, if it's multiline?
If that was required, my doc comment would demand it.
> I assume that "should end with a newline" means "must end
> with a newline", and places like block-luks.c are bugs.
If you construct a hint with multiple calls, individual calls need not
terminate with a newline, but the resulting hint still should.
I readily admit that my doc comments sometimes require a "lawyerly"
reading :)
> Markus, do you know what the intended API here is?
>
> It looks like the implementation just tacks the hint
> string onto the end of any existing hint string, in
> which case multiple-line strings are fine and the same
> behaviour as calling the function multiple times.
That's the intended behavior.
> (I had assumed we might be accumulating an array of strings,
> or requiring multiline strings to be multiple calls so we
> could have the argument not need to be \n-terminated,
> to match error_setg(), but both those assumptions
> are obviously wrong.)
Would also be workable, if slightly less flexible.
Yes, error_setg() and error_append_hint() differ on newlines.
error_setg() constructs an error message. These get reported in a rigid
format that does not afford line breaks in the middle.
error_append_hint() builds up a hint. Hints are free-form text.
Embedded line breaks are fine.
In both cases, I decided not to treat newline at the end any different
from newlines anywhere else. Thus, no newlines at all with
error_setg(), and newlines after every line including the last one with
error_append_hint().
> Anyway, I guess this multiline-message usage is something
> we do already and it will DTRT, so
Correct.
Hope this helps!
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>
>
> thanks
> -- PMM
- Re: [PATCH v2 3/9] tests/acceptance/boot_linux: Tag tests using a SD card with 'device:sd', (continued)
- [PATCH v2 4/9] tests/acceptance/boot_linux: Expand SD card image to power of 2, Philippe Mathieu-Daudé, 2020/07/13
- [PATCH v2 5/9] hw/sd/sdcard: Restrict Class 6 commands to SCSD cards, Philippe Mathieu-Daudé, 2020/07/13
- [PATCH v2 6/9] hw/sd/sdcard: Simplify realize() a bit, Philippe Mathieu-Daudé, 2020/07/13
- [PATCH v2 7/9] hw/sd/sdcard: Do not allow invalid SD card sizes, Philippe Mathieu-Daudé, 2020/07/13
- [PATCH v2 8/9] hw/sd/sdcard: Update coding style to make checkpatch.pl happy, Philippe Mathieu-Daudé, 2020/07/13
- [PATCH v2 9/9] hw/sd/sdcard: Do not switch to ReceivingData if address is invalid, Philippe Mathieu-Daudé, 2020/07/13
- Re: [PATCH v2 0/9] hw/sd/sdcard: Fix CVE-2020-13253, Philippe Mathieu-Daudé, 2020/07/14