Regarding commit a9bcedd (SD card size has to be power of 2)

Regarding commit a9bcedd (SD card size has to be power of 2)

[Tom Yan]

Hi philmd (and others),

So I just noticed your commit of requiring the size of an emulated SD
card to be a power of 2, when I was trying to emulate one for an
actual one (well, it's a microSD, but still), as it errored out.

You claim that the kernel will consider it to be a firmware bug and
"correct" the capacity by rounding it up. Could you provide a concrete
reference to the code that does such a thing? I'm not ruling out that
some crazy code could have gone upstream because some reviewers might
not be doing their job right, but if that really happened, it's a
kernel bug/regression and qemu should not do an equally-crazy thing to
"fix" it.

No offense but what you claimed really sounds absurd and ridiculous.
Although I don't have hundreds of SD cards in hand, I owned quite a
few at least, like most people do, with capacities ranging from ~2G to
~128G, and I don't even recall seeing a single one that has the
capacity being a power of 2. (Just like vendors of HDDs and SSDs, they
literally never do that AFAICT, for whatever reasons.)

Besides, even if there's a proper reason for the kernel to "fix" the
capacity, there's no reason for it to round it up either, because
obviously there will never be actual storage for the "virtual blocks".
I've never seen such a behavior so far either with the "mmcblk" hosts
I've used so far.

Regards,
Tom

Re: Regarding commit a9bcedd (SD card size has to be power of 2)

[Warner Losh]

[-- Attachment #1: Type: text/plain, Size: 1899 bytes --]

On Mon, Jun 7, 2021 at 2:31 AM Tom Yan <tom.ty89@gmail.com> wrote:

> Hi philmd (and others),
>
> So I just noticed your commit of requiring the size of an emulated SD
> card to be a power of 2, when I was trying to emulate one for an
> actual one (well, it's a microSD, but still), as it errored out.
>
> You claim that the kernel will consider it to be a firmware bug and
> "correct" the capacity by rounding it up. Could you provide a concrete
> reference to the code that does such a thing? I'm not ruling out that
> some crazy code could have gone upstream because some reviewers might
> not be doing their job right, but if that really happened, it's a
> kernel bug/regression and qemu should not do an equally-crazy thing to
> "fix" it.
>
> No offense but what you claimed really sounds absurd and ridiculous.
> Although I don't have hundreds of SD cards in hand, I owned quite a
> few at least, like most people do, with capacities ranging from ~2G to
> ~128G, and I don't even recall seeing a single one that has the
> capacity being a power of 2. (Just like vendors of HDDs and SSDs, they
> literally never do that AFAICT, for whatever reasons.)
>
> Besides, even if there's a proper reason for the kernel to "fix" the
> capacity, there's no reason for it to round it up either, because
> obviously there will never be actual storage for the "virtual blocks".
> I've never seen such a behavior so far either with the "mmcblk" hosts
> I've used so far.
>

Some data points: I have several 224GB SD cards. FreeBSD specifically uses
a size
just a little smaller than the rated size because so many capacity points
are a bit
smaller (1GB cards also tend to be only 1% larger 1E9 bytes, nowhere near a
power
of two). FreeBSD's kernel never adjusts the size of SD or MMC cards, and
there's
nothing in the Simplified SD standard nor in the various MMC standards
requiring a
power of two.

Warner

[-- Attachment #2: Type: text/html, Size: 2474 bytes --]

Re: Regarding commit a9bcedd (SD card size has to be power of 2)

[Daniel P. Berrangé]

On Mon, Jun 07, 2021 at 04:29:54PM +0800, Tom Yan wrote:
> Hi philmd (and others),
> 
> So I just noticed your commit of requiring the size of an emulated SD
> card to be a power of 2, when I was trying to emulate one for an
> actual one (well, it's a microSD, but still), as it errored out.
> 
> You claim that the kernel will consider it to be a firmware bug and
> "correct" the capacity by rounding it up. Could you provide a concrete
> reference to the code that does such a thing? I'm not ruling out that
> some crazy code could have gone upstream because some reviewers might
> not be doing their job right, but if that really happened, it's a
> kernel bug/regression and qemu should not do an equally-crazy thing to
> "fix" it.

I looked back at the original threads for details, but didn't
find any aside from this short message saying it broke Linux:

  https://www.mail-archive.com/qemu-devel@nongnu.org/msg720737.html

Philippe, do you have more details on the problem hit, or pointer
to where the power-of-2 restriction is in Linux ?

> No offense but what you claimed really sounds absurd and ridiculous.
> Although I don't have hundreds of SD cards in hand, I owned quite a
> few at least, like most people do, with capacities ranging from ~2G to
> ~128G, and I don't even recall seeing a single one that has the
> capacity being a power of 2. (Just like vendors of HDDs and SSDs, they
> literally never do that AFAICT, for whatever reasons.)

Yes, this does feel pretty odd to me too, based on the real physical
SD cards I've used with Linux non-power-2 sizes.

Also in general QEMU shouldn't be enforcing restrictions based on
guest behaviour, it should follow the hardware specs. If the
hardware spec doesn't mandate power-of-2 sizes, then QEMU shoud
not require that, even if some guest OS has added an artificial
restriction of its own.

> Besides, even if there's a proper reason for the kernel to "fix" the
> capacity, there's no reason for it to round it up either, because
> obviously there will never be actual storage for the "virtual blocks".
> I've never seen such a behavior so far either with the "mmcblk" hosts
> I've used so far.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: Regarding commit a9bcedd (SD card size has to be power of 2)

[Philippe Mathieu-Daudé]

Hi,

On 6/23/21 11:28 AM, Daniel P. Berrangé wrote:
> On Mon, Jun 07, 2021 at 04:29:54PM +0800, Tom Yan wrote:
>> Hi philmd (and others),
>>
>> So I just noticed your commit of requiring the size of an emulated SD
>> card to be a power of 2, when I was trying to emulate one for an
>> actual one (well, it's a microSD, but still), as it errored out.
>>
>> You claim that the kernel will consider it to be a firmware bug and
>> "correct" the capacity by rounding it up. Could you provide a concrete
>> reference to the code that does such a thing? I'm not ruling out that
>> some crazy code could have gone upstream because some reviewers might
>> not be doing their job right, but if that really happened, it's a
>> kernel bug/regression and qemu should not do an equally-crazy thing to
>> "fix" it.
> 
> I looked back at the original threads for details, but didn't
> find any aside from this short message saying it broke Linux:
> 
>   https://www.mail-archive.com/qemu-devel@nongnu.org/msg720737.html
> 
> Philippe, do you have more details on the problem hit, or pointer
> to where the power-of-2 restriction is in Linux ?

Sorry for not responding soon enough, too many things.

I wrote patches to address Tom's problem, but couldn't fix all
the cases yet. So far the problem is not Linux but firmwares
announcing pow2 to Linux without checking card layout.

It is hard to make everybody happy, security users and odd firmwares.

I came out with a larger series to be able to classify QEMU API /
devices code as security sensible or not, and use of some unsafe
API to taint some security mode (so far only displaying a warning).
If the security mode is tainted (use of unsafe device, unsafe config,
unsafe feature), then users shouldn't expect safety in the guest.

That way I could have classified the SD card model as unsafe and not
bothered various users by restricting to pow2 card sizes.

>> No offense but what you claimed really sounds absurd and ridiculous.
>> Although I don't have hundreds of SD cards in hand, I owned quite a
>> few at least, like most people do, with capacities ranging from ~2G to
>> ~128G, and I don't even recall seeing a single one that has the
>> capacity being a power of 2. (Just like vendors of HDDs and SSDs, they
>> literally never do that AFAICT, for whatever reasons.)
> 
> Yes, this does feel pretty odd to me too, based on the real physical
> SD cards I've used with Linux non-power-2 sizes.
> 
> Also in general QEMU shouldn't be enforcing restrictions based on
> guest behaviour, it should follow the hardware specs. If the
> hardware spec doesn't mandate power-of-2 sizes, then QEMU shoud
> not require that, even if some guest OS has added an artificial
> restriction of its own.

The comment is misleading, the restriction was to answer CVE vuln.

>> Besides, even if there's a proper reason for the kernel to "fix" the
>> capacity, there's no reason for it to round it up either, because
>> obviously there will never be actual storage for the "virtual blocks".
>> I've never seen such a behavior so far either with the "mmcblk" hosts
>> I've used so far.

Help, reproducible configurations and patches to improve are always
welcomed.

Regards,

Phil.

Re: Regarding commit a9bcedd (SD card size has to be power of 2)

[Michal Suchánek]

On Wed, Jun 23, 2021 at 12:59:45PM +0200, Philippe Mathieu-Daudé wrote:
> Hi,
> 
> On 6/23/21 11:28 AM, Daniel P. Berrangé wrote:
> > On Mon, Jun 07, 2021 at 04:29:54PM +0800, Tom Yan wrote:
> >> Hi philmd (and others),
> >>
> >> So I just noticed your commit of requiring the size of an emulated SD
> >> card to be a power of 2, when I was trying to emulate one for an
> >> actual one (well, it's a microSD, but still), as it errored out.
> >>
> >> You claim that the kernel will consider it to be a firmware bug and
> >> "correct" the capacity by rounding it up. Could you provide a concrete
> >> reference to the code that does such a thing? I'm not ruling out that
> >> some crazy code could have gone upstream because some reviewers might
> >> not be doing their job right, but if that really happened, it's a
> >> kernel bug/regression and qemu should not do an equally-crazy thing to
> >> "fix" it.
> > 
> > I looked back at the original threads for details, but didn't
> > find any aside from this short message saying it broke Linux:
> > 
> >   https://www.mail-archive.com/qemu-devel@nongnu.org/msg720737.html
> > 
> > Philippe, do you have more details on the problem hit, or pointer
> > to where the power-of-2 restriction is in Linux ?
> 
> Sorry for not responding soon enough, too many things.
> 
> I wrote patches to address Tom's problem, but couldn't fix all
> the cases yet. So far the problem is not Linux but firmwares
> announcing pow2 to Linux without checking card layout.
> 
> It is hard to make everybody happy, security users and odd firmwares.
> 
> I came out with a larger series to be able to classify QEMU API /
> devices code as security sensible or not, and use of some unsafe
> API to taint some security mode (so far only displaying a warning).
> If the security mode is tainted (use of unsafe device, unsafe config,
> unsafe feature), then users shouldn't expect safety in the guest.
> 
> That way I could have classified the SD card model as unsafe and not
> bothered various users by restricting to pow2 card sizes.
> 
> >> No offense but what you claimed really sounds absurd and ridiculous.
> >> Although I don't have hundreds of SD cards in hand, I owned quite a
> >> few at least, like most people do, with capacities ranging from ~2G to
> >> ~128G, and I don't even recall seeing a single one that has the
> >> capacity being a power of 2. (Just like vendors of HDDs and SSDs, they
> >> literally never do that AFAICT, for whatever reasons.)
> > 
> > Yes, this does feel pretty odd to me too, based on the real physical
> > SD cards I've used with Linux non-power-2 sizes.
> > 
> > Also in general QEMU shouldn't be enforcing restrictions based on
> > guest behaviour, it should follow the hardware specs. If the
> > hardware spec doesn't mandate power-of-2 sizes, then QEMU shoud
> > not require that, even if some guest OS has added an artificial
> > restriction of its own.
> 
> The comment is misleading, the restriction was to answer CVE vuln.

Care to share the reference?

I would be really interested in the piece of software that relies on
power of two sized SD cards to be secure. Sounds like it's broken and
should be fixed rather than worked around in qemu.

It also means that on real hardware that lacks power of two sized SD
cards it is always insecure.

Thanks

Michal

Re: Regarding commit a9bcedd (SD card size has to be power of 2)

[Daniel P. Berrangé]

On Wed, Jun 23, 2021 at 12:59:45PM +0200, Philippe Mathieu-Daudé wrote:
> Hi,
> 
> On 6/23/21 11:28 AM, Daniel P. Berrangé wrote:
> > On Mon, Jun 07, 2021 at 04:29:54PM +0800, Tom Yan wrote:
> >> Hi philmd (and others),
> >>
> >> So I just noticed your commit of requiring the size of an emulated SD
> >> card to be a power of 2, when I was trying to emulate one for an
> >> actual one (well, it's a microSD, but still), as it errored out.
> >>
> >> You claim that the kernel will consider it to be a firmware bug and
> >> "correct" the capacity by rounding it up. Could you provide a concrete
> >> reference to the code that does such a thing? I'm not ruling out that
> >> some crazy code could have gone upstream because some reviewers might
> >> not be doing their job right, but if that really happened, it's a
> >> kernel bug/regression and qemu should not do an equally-crazy thing to
> >> "fix" it.
> > 
> > I looked back at the original threads for details, but didn't
> > find any aside from this short message saying it broke Linux:
> > 
> >   https://www.mail-archive.com/qemu-devel@nongnu.org/msg720737.html
> > 
> > Philippe, do you have more details on the problem hit, or pointer
> > to where the power-of-2 restriction is in Linux ?
> 
> Sorry for not responding soon enough, too many things.
> 
> I wrote patches to address Tom's problem, but couldn't fix all
> the cases yet. So far the problem is not Linux but firmwares
> announcing pow2 to Linux without checking card layout.
> 
> It is hard to make everybody happy, security users and odd firmwares.
> 
> I came out with a larger series to be able to classify QEMU API /
> devices code as security sensible or not, and use of some unsafe
> API to taint some security mode (so far only displaying a warning).
> If the security mode is tainted (use of unsafe device, unsafe config,
> unsafe feature), then users shouldn't expect safety in the guest.
> 
> That way I could have classified the SD card model as unsafe and not
> bothered various users by restricting to pow2 card sizes.

Ok, so QEMU has to be robust against guest OS, even if it is the
fault of the firmware for telling guest the wrong size info. I
don't think this means QEMU needs to restrict the sizes though.

If QEMU's CVE fix breaks guest when the firmware is giving wrong
info, then we should just pass that bug report onto the firmware
maintainers.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|