* [Qemu-devel] [PATCH v1 1/1] sdhci.c: Limit the maximum block size
@ 2015-10-06 17:40 Alistair Francis
2015-10-06 18:34 ` Peter Crosthwaite
2015-10-08 9:50 ` Stefan Hajnoczi
0 siblings, 2 replies; 5+ messages in thread
From: Alistair Francis @ 2015-10-06 17:40 UTC (permalink / raw)
To: qemu-devel
Cc: oleksandr.bazhaniuk, peter.maydell, i.mitsyanko, stefanha,
james.l.walter, armbru, alistair.francis, crosthwaitepeter, kevin,
wehuang, jsnow, secure
It is possible for the guest to set an invalid block
size which is larger then the fifo_buffer[] array. This
could cause a buffer overflow.
To avoid this limit the maximum size of the blksize variable.
Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
Suggested-by: Igor Mitsyanko <i.mitsyanko@gmail.com>
Reported-by: Intel Security ATR <secure@intel.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
hw/sd/sdhci.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 65304cf..1d47f5c 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -1006,6 +1006,16 @@ sdhci_write(void *opaque, hwaddr offset, uint64_t val, unsigned size)
MASKED_WRITE(s->blksize, mask, value);
MASKED_WRITE(s->blkcnt, mask >> 16, value >> 16);
}
+
+ /* Limit block size to the maximum buffer size */
+ if (extract32(s->blksize, 0, 12) > s->buf_maxsz) {
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: Size 0x%x is larger than " \
+ "the maximum buffer 0x%x", __func__, s->blksize,
+ s->buf_maxsz);
+
+ s->blksize = deposit32(s->blksize, 0, 12, s->buf_maxsz);
+ }
+
break;
case SDHC_ARGUMENT:
MASKED_WRITE(s->argument, mask, value);
--
2.1.4
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [Qemu-devel] [PATCH v1 1/1] sdhci.c: Limit the maximum block size
2015-10-06 17:40 [Qemu-devel] [PATCH v1 1/1] sdhci.c: Limit the maximum block size Alistair Francis
@ 2015-10-06 18:34 ` Peter Crosthwaite
2015-10-08 9:49 ` Stefan Hajnoczi
2015-10-08 9:50 ` Stefan Hajnoczi
1 sibling, 1 reply; 5+ messages in thread
From: Peter Crosthwaite @ 2015-10-06 18:34 UTC (permalink / raw)
To: Alistair Francis
Cc: Bazhaniuk Oleksandr, Peter Maydell, Igor Mitsyanko,
Stefan Hajnoczi, Walter James L, qemu-devel@nongnu.org Developers,
Markus Armbruster, Sai Pavan Boddu, Kevin OConnor, wehuang,
John Snow, secure
On Tue, Oct 6, 2015 at 10:40 AM, Alistair Francis
<alistair.francis@xilinx.com> wrote:
> It is possible for the guest to set an invalid block
> size which is larger then the fifo_buffer[] array. This
> could cause a buffer overflow.
>
> To avoid this limit the maximum size of the blksize variable.
>
> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
> Suggested-by: Igor Mitsyanko <i.mitsyanko@gmail.com>
> Reported-by: Intel Security ATR <secure@intel.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
With Pavan's patches and now this, the SD patches are starting to pile
up on list. What queue do they target? target-arm (as lead/major user)
or something block-related?
Regards,
Peter
> ---
>
> hw/sd/sdhci.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
> index 65304cf..1d47f5c 100644
> --- a/hw/sd/sdhci.c
> +++ b/hw/sd/sdhci.c
> @@ -1006,6 +1006,16 @@ sdhci_write(void *opaque, hwaddr offset, uint64_t val, unsigned size)
> MASKED_WRITE(s->blksize, mask, value);
> MASKED_WRITE(s->blkcnt, mask >> 16, value >> 16);
> }
> +
> + /* Limit block size to the maximum buffer size */
> + if (extract32(s->blksize, 0, 12) > s->buf_maxsz) {
> + qemu_log_mask(LOG_GUEST_ERROR, "%s: Size 0x%x is larger than " \
> + "the maximum buffer 0x%x", __func__, s->blksize,
> + s->buf_maxsz);
> +
> + s->blksize = deposit32(s->blksize, 0, 12, s->buf_maxsz);
> + }
> +
> break;
> case SDHC_ARGUMENT:
> MASKED_WRITE(s->argument, mask, value);
> --
> 2.1.4
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [Qemu-devel] [PATCH v1 1/1] sdhci.c: Limit the maximum block size
2015-10-06 18:34 ` Peter Crosthwaite
@ 2015-10-08 9:49 ` Stefan Hajnoczi
2015-10-08 15:46 ` Alistair Francis
0 siblings, 1 reply; 5+ messages in thread
From: Stefan Hajnoczi @ 2015-10-08 9:49 UTC (permalink / raw)
To: Peter Crosthwaite
Cc: Bazhaniuk Oleksandr, Peter Maydell, Igor Mitsyanko,
Markus Armbruster, Walter James L,
qemu-devel@nongnu.org Developers, Alistair Francis,
Sai Pavan Boddu, Kevin OConnor, wehuang, John Snow, secure
On Tue, Oct 06, 2015 at 11:34:46AM -0700, Peter Crosthwaite wrote:
> On Tue, Oct 6, 2015 at 10:40 AM, Alistair Francis
> <alistair.francis@xilinx.com> wrote:
> > It is possible for the guest to set an invalid block
> > size which is larger then the fifo_buffer[] array. This
> > could cause a buffer overflow.
> >
> > To avoid this limit the maximum size of the blksize variable.
> >
> > Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
> > Suggested-by: Igor Mitsyanko <i.mitsyanko@gmail.com>
> > Reported-by: Intel Security ATR <secure@intel.com>
> > Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
>
> Reviewed-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
>
> With Pavan's patches and now this, the SD patches are starting to pile
> up on list. What queue do they target? target-arm (as lead/major user)
> or something block-related?
I can pick them up for now in my block pull requests. Note that I'm not
an SD expert so I can't review/maintain the code.
Stefan
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH v1 1/1] sdhci.c: Limit the maximum block size
2015-10-08 9:49 ` Stefan Hajnoczi
@ 2015-10-08 15:46 ` Alistair Francis
0 siblings, 0 replies; 5+ messages in thread
From: Alistair Francis @ 2015-10-08 15:46 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: Bazhaniuk Oleksandr, Peter Maydell, Igor Mitsyanko,
Walter James L, Markus Armbruster,
qemu-devel@nongnu.org Developers, Sai Pavan Boddu,
Peter Crosthwaite, Kevin OConnor, Wei Huang, Alistair Francis,
John Snow, secure
On Thu, Oct 8, 2015 at 2:49 AM, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> On Tue, Oct 06, 2015 at 11:34:46AM -0700, Peter Crosthwaite wrote:
>> On Tue, Oct 6, 2015 at 10:40 AM, Alistair Francis
>> <alistair.francis@xilinx.com> wrote:
>> > It is possible for the guest to set an invalid block
>> > size which is larger then the fifo_buffer[] array. This
>> > could cause a buffer overflow.
>> >
>> > To avoid this limit the maximum size of the blksize variable.
>> >
>> > Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
>> > Suggested-by: Igor Mitsyanko <i.mitsyanko@gmail.com>
>> > Reported-by: Intel Security ATR <secure@intel.com>
>> > Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
>>
>> Reviewed-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
>>
>> With Pavan's patches and now this, the SD patches are starting to pile
>> up on list. What queue do they target? target-arm (as lead/major user)
>> or something block-related?
>
> I can pick them up for now in my block pull requests. Note that I'm not
> an SD expert so I can't review/maintain the code.
Thanks :) Applying them should be enough
Alistair
>
> Stefan
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH v1 1/1] sdhci.c: Limit the maximum block size
2015-10-06 17:40 [Qemu-devel] [PATCH v1 1/1] sdhci.c: Limit the maximum block size Alistair Francis
2015-10-06 18:34 ` Peter Crosthwaite
@ 2015-10-08 9:50 ` Stefan Hajnoczi
1 sibling, 0 replies; 5+ messages in thread
From: Stefan Hajnoczi @ 2015-10-08 9:50 UTC (permalink / raw)
To: Alistair Francis
Cc: oleksandr.bazhaniuk, peter.maydell, i.mitsyanko, james.l.walter,
qemu-devel, armbru, crosthwaitepeter, kevin, wehuang, jsnow,
secure
On Tue, Oct 06, 2015 at 10:40:41AM -0700, Alistair Francis wrote:
> It is possible for the guest to set an invalid block
> size which is larger then the fifo_buffer[] array. This
> could cause a buffer overflow.
>
> To avoid this limit the maximum size of the blksize variable.
>
> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
> Suggested-by: Igor Mitsyanko <i.mitsyanko@gmail.com>
> Reported-by: Intel Security ATR <secure@intel.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>
> hw/sd/sdhci.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block
Stefan
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-10-08 15:47 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-06 17:40 [Qemu-devel] [PATCH v1 1/1] sdhci.c: Limit the maximum block size Alistair Francis
2015-10-06 18:34 ` Peter Crosthwaite
2015-10-08 9:49 ` Stefan Hajnoczi
2015-10-08 15:46 ` Alistair Francis
2015-10-08 9:50 ` Stefan Hajnoczi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).