| Message ID | abe4c51f513290bbb85d1ee271cb1a3d463d7561.1444067470.git.alistair.francis@xilinx.com |
|---|---|
| State | New |
| Headers | show |
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 >
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
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
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 >
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);