+-- On Thu, 25 Jan 2018, Gerd Hoffmann wrote --+
| Ok, finally queueed up v1 for merge.
Okay, cool. Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F
From: Prasad J Pandit
9p back-end first queries the size of an extended attribute,
allocates space for it via g_malloc() and then retrieves its
value into allocated buffer. Race between querying attribute
size and retrieving its could lead to memory bytes disclosure.
Use g_malloc0() to avoid it.
From: Prasad J Pandit
'dst' pointer could exceed vga vram size when writing to the
cirrus memory area; it'd lead to an OOB access issue. Add check
to avoid it.
Reported-by: Niu Guoxiang
Signed-off-by: Prasad J Pandit
---
hw/display/cirrus_vga.c | 10 --
1 file changed, 8 insertions(+)
From: Prasad J Pandit
'dst' pointer could exceed vga vram size when writing to the
cirrus memory area; it'd lead to an OOB access issue. Add check
to avoid it.
Reported-by: Niu Guoxiang
Signed-off-by: Prasad J Pandit
---
hw/display/cirrus_vga.c | 10 --
1 file changed, 8 insertions(+)
+-- On Wed, 11 Oct 2017, Gerd Hoffmann wrote --+
| Move dst calculation into the loop, so we apply the mask on each
s/into the loop/inside for loop
| interation and will not overflow vga memory.
s/interation/iteration
| diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c
| index afc
From: Prasad J Pandit
An uninitialised VirtQueue object or one with Vring.align field
set to zero(0) could lead to arithmetic exceptions. Add a unit
test to validate it.
Signed-off-by: Prasad J Pandit
---
tests/virtio-blk-test.c | 25 +
1 file changed, 25 insertions(+)
From: Prasad J Pandit
Hello,
A guest could attempt to use an uninitialised VirtQueue object
or set Vring object with undue values, raising an unexpected
exception in Qemu. This patch set fixes this issue and also adds
a unit test to the suite.
Thank you.
--
Prasad J Pandit (2):
virtio: check
From: Prasad J Pandit
A guest could attempt to use an uninitialised VirtQueue object
or unset Vring.align leading to a arithmetic exception. Add check
to avoid it.
Reported-by: Zhangboxian
Signed-off-by: Prasad J Pandit
---
hw/virtio/virtio.c | 14 +++---
1 file changed, 11 insertions
+-- On Wed, 29 Nov 2017, Cornelia Huck wrote --+
| I think the basic problem is still that you conflate two things:
| - vring.num, which cannot be flipped between 0 and !0 by the guest
| - vring.{desc,avail,used}, which can
|
| IOW, if vring.num == 0, the guest cannot manipulate the queue; if
| vr
+-- On Thu, 30 Nov 2017, Stefan Hajnoczi wrote --+
| Michael is the virtio maintainer. I have added him to this email
| thread so the patch series can be merged.
Thanks so much!
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F
From: Prasad J Pandit
During a dma access, SCSIRequest object 'current_req' could be
null, leading to a null pointer dereference. Add check to avoid
it.
Reported-by: Zhangboxian
Signed-off-by: Prasad J Pandit
---
hw/scsi/esp.c | 2 +-
hw/scsi/scsi-bus.c | 3 +++
2 files changed, 4 insert
+-- On Thu, 30 Nov 2017, P J P wrote --+
| +-- On Thu, 30 Nov 2017, Stefan Hajnoczi wrote --+
| | Michael is the virtio maintainer. I have added him to this email
| | thread so the patch series can be merged.
-> https://lists.gnu.org/archive/html/qemu-devel/2017-11/msg05473.html
@mst: t
From: Prasad J Pandit
The ctz32() routine could return value greater than
TC6393XB_GPIOS=16. This could lead to an OOB array access.
Add check to avoid it.
Reported-by: Moguofang
Signed-off-by: Prasad J Pandit
---
hw/display/tc6393xb.c | 4
1 file changed, 4 insertions(+)
diff --git a/h
From: Prasad J Pandit
The ctz32() routine could return value greater than
TC6393XB_GPIOS=16. This could lead to an OOB array access.
Mask 'level' to avoid it.
Reported-by: Moguofang
Signed-off-by: Prasad J Pandit
---
hw/display/tc6393xb.c | 1 +
1 file changed, 1 insertion(+)
Update: mask 'l
+-- On Mon, 11 Dec 2017, Peter Maydell wrote --+
| It would be more sensible to just mask off the top bits of
| 'level' before starting the loop, rather than checking every
| time around the loop:
|level &= MAKE_64BIT_MASK(0, TC6493XB_GPIOS);
Sent a revised patch v1. Thank you.
--
Prasad J Pan
From: Prasad J Pandit
During Qemu guest migration, a destination process invokes ps2
post_load function. In that, if 'rptr' and 'count' fields were
tampered, it could lead to OOB access or infinite loop issues.
Change their type to 'uint8_t' so they remain within bounds.
Reported-by: Cyrille Cha
From: Prasad J Pandit
During Qemu guest migration, a destination process invokes ps2
post_load function. In that, if 'rptr' and 'count' values were
invalid, it could lead to OOB access or infinite loop issue.
Add check to avoid it.
Reported-by: Cyrille Chatras
Signed-off-by: Prasad J Pandit
--
+-- On Wed, 15 Nov 2017, Dr. David Alan Gilbert wrote --+
| * Paolo Bonzini (pbonz...@redhat.com) wrote:
| > However, PJP's patch breaks migration by changing a 4-byte field to
| > 1-byte. The correct fix is to range-check the fields in
| > ps2_common_post_load.
|
| Agreed.
Sent revised patch v1
From: Prasad J Pandit
During Qemu guest migration, a destination process invokes ps2
post_load function. In that, if 'rptr' and 'count' values were
invalid, it could lead to OOB access issue. Add check to avoid
it.
Reported-by: Cyrille Chatras
Signed-off-by: Prasad J Pandit
---
hw/input/ps2.c
+-- On Thu, 16 Nov 2017, Paolo Bonzini wrote --+
| you don't need to change the invalid values to sane ones. Instead, make
| ps2_common_post_load return an int (just like the .post_load member of
| VMStateDescription). You can then detect out of range count/rptr/wptr
| and return -1 for bad indic
From: Prasad J Pandit
Hello,
An user could attempt to use an uninitialised VirtQueue object
or set Vring object with undue values, raising an unexpected
exception in Qemu. This patch set fixes this issue and also adds
a unit test to the suite.
Thank you.
--
Prasad J Pandit (2):
virtio: check
From: Prasad J Pandit
An uninitialised VirtQueue object or one with Vring.align field
set to zero(0) could lead to arithmetic exceptions. Add a unit
test to validate it.
Signed-off-by: Prasad J Pandit
---
tests/virtio-blk-test.c | 21 +
1 file changed, 21 insertions(+)
dif
From: Prasad J Pandit
An user could attempt to use an uninitialised VirtQueue object
or unset Vring.align leading to a arithmetic exception. Add check
to avoid it.
Reported-by: Zhangboxian
Signed-off-by: Prasad J Pandit
---
hw/virtio/virtio.c | 11 ---
1 file changed, 8 insertions(+),
From: Prasad J Pandit
An user could attempt to use an uninitialised VirtQueue object
or unset Vring.align leading to a arithmetic exception. Add check
to avoid it.
Reported-by: Zhangboxian
Signed-off-by: Prasad J Pandit
---
hw/virtio/virtio.c | 11 ---
1 file changed, 8 insertions(+),
From: Prasad J Pandit
Hello,
An user could attempt to use an uninitialised VirtQueue object
or set Vring object with undue values, raising an unexpected
exception in Qemu. This patch set fixes this issue and also adds
a unit test to the suite.
Thank you.
--
Prasad J Pandit (2):
virtio: check
From: Prasad J Pandit
An uninitialised VirtQueue object or one with Vring.align field
set to zero(0) could lead to arithmetic exceptions. Add a unit
test to validate it.
Signed-off-by: Prasad J Pandit
---
tests/virtio-blk-test.c | 21 +
1 file changed, 21 insertions(+)
dif
+-- On Fri, 24 Nov 2017, Paolo Bonzini wrote --+
| Why not check vring->num in virtio_queue_update_rings too?
Yes, sent a revised patch v1. These checks seem to repeat through sequence of
functions. I guess it'll help to do them in one place.
Thank you.
--
Prasad J Pandit / Red Hat Product Secur
From: Prasad J Pandit
Hello,
An user could attempt to use an uninitialised VirtQueue object
or set Vring object with undue values, raising an unexpected
exception in Qemu. This patch set fixes this issue and also adds
a unit test to the suite.
Thank you.
--
Prasad J Pandit (2):
virtio: check
From: Prasad J Pandit
An user could attempt to use an uninitialised VirtQueue object
or unset Vring.align leading to a arithmetic exception. Add check
to avoid it.
Reported-by: Zhangboxian
Signed-off-by: Prasad J Pandit
---
hw/virtio/virtio.c | 14 +++---
1 file changed, 11 insertions
From: Prasad J Pandit
An uninitialised VirtQueue object or one with Vring.align field
set to zero(0) could lead to arithmetic exceptions. Add a unit
test to validate it.
Signed-off-by: Prasad J Pandit
---
tests/virtio-blk-test.c | 25 +
1 file changed, 25 insertions(+)
From: Prasad J Pandit
An user could attempt to use an uninitialised VirtQueue object
or unset Vring.align leading to a arithmetic exception. Add check
to avoid it.
Reported-by: Zhangboxian
Signed-off-by: Prasad J Pandit
---
hw/virtio/virtio.c | 14 +++---
1 file changed, 11 insertions
From: Prasad J Pandit
An uninitialised VirtQueue object or one with Vring.align field
set to zero(0) could lead to arithmetic exceptions. Add a unit
test to validate it.
Signed-off-by: Prasad J Pandit
---
tests/virtio-blk-test.c | 25 +
1 file changed, 25 insertions(+)
From: Prasad J Pandit
Hello,
An user could attempt to use an uninitialised VirtQueue object
or set Vring object with undue values, raising an unexpected
exception in Qemu. This patch set fixes this issue and also adds
a unit test to the suite.
Thank you.
--
Prasad J Pandit (2):
virtio: check
+-- On Mon, 27 Nov 2017, Cornelia Huck wrote --+
|The check for align is not really needed, as virtio-1 disallows setting align
|anyway.
disallows...?
| Checking for !desc is wrong (why shouldn't a driver be able to unset a
| descriptor table?)
+-- On Mon, 27 Nov 2017, Stefan Hajnoczi wrote --
+-- On Tue, 28 Nov 2017, Stefan Hajnoczi wrote --+
| > This is conflating different things:
| > - vq does not exist (num == 0)
| > - vq is not setup by the guest (desc == 0)
| > - vq has no valid alignment (which is only relevant for legacy)
|
| I agree.
Either case, vq would be unfit for use, no
Hello Cornelia,
+-- On Tue, 28 Nov 2017, Cornelia Huck wrote --+
| What is "unfit for use"?
Unfit for use because we see checks like
if (!virtio_queue_get_num(vdev, n)) {
continue;
...
if (!vdev->vq[n].vring.num) {
return;
'virtio_queue_set_rings' sets 'vring.desc' as
From: Prasad J Pandit
While performing block transfer write in smb_ioport_writeb(),
'smb_index' is incremented and used to index smb_data[] array.
Check 'smb_index' value to avoid OOB access.
Reported-by: Michael Hanselmann
Signed-off-by: Prasad J Pandit
---
hw/i2c/pm_smbus.c | 3 +++
1 file
+-- On Thu, 6 Dec 2018, Igor Mammedov wrote --+
| > From: Prasad J Pandit
| >
| > While performing block transfer write in smb_ioport_writeb(),
| > 'smb_index' is incremented and used to index smb_data[] array.
| > Check 'smb_index' value to avoid OOB access.
| >
| > Reported-by: Michael Hanselm
+-- On Thu, 6 Dec 2018, Peter Maydell wrote --+
| > > Do we need patch v2, or it can be done while merging it?
| >
| > I can add in the Fixes line when I apply the patch to master.
|
| Oh, I think we should also add to the commit message something
| along the lines of:
|
| "Note that this bug is
+-- On Thu, 6 Dec 2018, P J P wrote --+
| | to clarify that this is a serious bug but also that it's
| | not one that will be affecting anybody's production systems.
|
| Okay, preparing patch v2...
Sent revised patch
[PATCH v1] i2c: pm_smbus: check smb_index before block transfer wr
From: Prasad J Pandit
While performing block transfer write in smb_ioport_writeb(),
'smb_index' is incremented and used to index smb_data[] array.
Check 'smb_index' value to avoid OOB access.
Note that this bug is exploitable by a guest to escape
from the virtual machine. However the commit whic
From: Prasad J Pandit
Hello,
Various issues OOB access, null dereference and possible infinite loop were
reported in the rdma/pvrdma backends. This patch set attempts to fix these.
Thank you.
---
Prasad J Pandit (5):
rdma: check that num_sge does not exceed MAX_SGE
pvrdma: add uar_read rout
From: Prasad J Pandit
rdma back-end has scatter/gather array ibv_sge[MAX_SGE=4] set
to have 4 elements. A guest could send a 'PvrdmaSqWqe' ring element
with 'num_sge' set to > MAX_SGE, which may lead to OOB access issue.
Add check to avoid it.
Reported-by: Saar Amar
Signed-off-by: Prasad J Pand
From: Prasad J Pandit
Define skeleton 'uar_read' routine. Avoid NULL dereference.
Reported-by: Li Qiang
Signed-off-by: Prasad J Pandit
---
hw/rdma/vmw/pvrdma_main.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/hw/rdma/vmw/pvrdma_main.c b/hw/rdma/vmw/pvrdma_main.c
index ca5fa8d981
From: Prasad J Pandit
When creating CQ/QP rings, an object can have up to
PVRDMA_MAX_FAST_REG_PAGES=128 pages. Check 'npages' parameter
to avoid excessive memory allocation or a null dereference.
Reported-by: Li Qiang
Signed-off-by: Prasad J Pandit
---
hw/rdma/vmw/pvrdma_cmd.c | 9 +
From: Prasad J Pandit
create_cq and create_qp routines allocate ring object, but it's
not released in case of an error, leading to memory leakage.
Reported-by: Li Qiang
Signed-off-by: Prasad J Pandit
---
hw/rdma/vmw/pvrdma_cmd.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
di
From: Prasad J Pandit
pvrdma_idx_ring_has_[data/space] routines also return invalid
index PVRDMA_INVALID_IDX[=-1], if ring has no data/space. Check
return value from these routines to avoid plausible infinite loops.
Reported-by: Li Qiang
Signed-off-by: Prasad J Pandit
---
hw/rdma/vmw/pvrdma_d
Hello Yuval,
+-- On Tue, 11 Dec 2018, Yuval Shaia wrote --+
| > Ditto, here send rind and recv rings stays mapped.
| > Look at how QP's ring is destroyed in destroy_qp.
| >
| > For both case suggesting to define a new static function that destroy rings
| > and call it from both error flow of cr
+-- On Wed, 12 Dec 2018, P J P wrote --+
| | Also, can you rebase this patch on top of the patchset i posted last week:
| | https://patchwork.kernel.org/patch/10705439/
|
| Okay, I'll send revised patch set. Thanks so much for the prompt review.
I tried to git apply above patch-set over v
From: Prasad J Pandit
rdma back-end has scatter/gather array ibv_sge[MAX_SGE=4] set
to have 4 elements. A guest could send a 'PvrdmaSqWqe' ring element
with 'num_sge' set to > MAX_SGE, which may lead to OOB access issue.
Add check to avoid it.
Reported-by: Saar Amar
Signed-off-by: Prasad J Pand
From: Prasad J Pandit
Define skeleton 'uar_read' routine. Avoid NULL dereference.
Reported-by: Li Qiang
Signed-off-by: Prasad J Pandit
---
hw/rdma/vmw/pvrdma_main.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/hw/rdma/vmw/pvrdma_main.c b/hw/rdma/vmw/pvrdma_main.c
index 23dc9926e3
From: Prasad J Pandit
When creating CQ/QP rings, an object can have up to
PVRDMA_MAX_FAST_REG_PAGES=128 pages. Check 'npages' parameter
to avoid excessive memory allocation or a null dereference.
Reported-by: Li Qiang
Signed-off-by: Prasad J Pandit
---
hw/rdma/vmw/pvrdma_cmd.c | 11 ++
From: Prasad J Pandit
create_cq and create_qp routines allocate ring object, but it's
not released in case of an error, leading to memory leakage.
Reported-by: Li Qiang
Signed-off-by: Prasad J Pandit
---
hw/rdma/vmw/pvrdma_cmd.c | 36 +---
1 file changed, 25 in
From: Prasad J Pandit
Replace VENDOR_ERR_NO_SGE macro with VENDOR_ERR_INV_NUM_SGE
to indicate invalid number of scatter/gather elements.
Signed-off-by: Prasad J Pandit
---
hw/rdma/rdma_backend.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/hw/rdma/rdma_backend.c b/hw/r
From: Prasad J Pandit
Hello,
This is a revised version v1 of the earlier patch set to fix issues
in the rdma/pvrdma backend.
Update to include review comments
-> https://lists.gnu.org/archive/html/qemu-devel/2018-12/msg02196.html
Please note, this patch is created after merging another patch
From: Prasad J Pandit
pvrdma_idx_ring_has_[data/space] routines also return invalid
index PVRDMA_INVALID_IDX[=-1], if ring has no data/space. Check
return value from these routines to avoid plausible infinite loops.
Reported-by: Li Qiang
Signed-off-by: Prasad J Pandit
---
hw/rdma/vmw/pvrdma_d
From: Prasad J Pandit
If during pvrdma device initialisation an error occurs,
pvrdma_realize() does not release memory resources, leading
to memory leakage.
Reported-by: Li Qiang
Signed-off-by: Prasad J Pandit
---
hw/rdma/vmw/pvrdma_main.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(
+-- On Wed, 12 Dec 2018, Yuval Shaia wrote --+
| Can you try master?
| I just did a rebase on top of master and had no conflicts.
Yes, I tried with master first, got the same errors, that's when I tried
earlier versions.
Preparing patch-set v2 with the suggested updates.
Thank you.
--
Prasad
From: Prasad J Pandit
Hello,
This is a revised version v2 of the earlier patch set to fix issues
in the rdma/pvrdma backend.
Update to include review comments from
-> https://lists.gnu.org/archive/html/qemu-devel/2018-12/msg02616.html
Thank you.
---
Prasad J Pandit (6):
rdma: check num_sge
From: Prasad J Pandit
When creating CQ/QP rings, an object can have up to
PVRDMA_MAX_FAST_REG_PAGES=128 pages. Check 'npages' parameter
to avoid excessive memory allocation or a null dereference.
Reported-by: Li Qiang
Signed-off-by: Prasad J Pandit
---
hw/rdma/vmw/pvrdma_cmd.c | 11 ++
From: Prasad J Pandit
create_cq and create_qp routines allocate ring object, but it's
not released in case of an error, leading to memory leakage.
Reported-by: Li Qiang
Signed-off-by: Prasad J Pandit
---
hw/rdma/vmw/pvrdma_cmd.c | 36 +---
1 file changed, 25 in
From: Prasad J Pandit
rdma back-end has scatter/gather array ibv_sge[MAX_SGE=4] set
to have 4 elements. A guest could send a 'PvrdmaSqWqe' ring element
with 'num_sge' set to > MAX_SGE, which may lead to OOB access issue.
Add check to avoid it.
Reported-by: Saar Amar
Signed-off-by: Prasad J Pand
From: Prasad J Pandit
pvrdma_idx_ring_has_[data/space] routines also return invalid
index PVRDMA_INVALID_IDX[=-1], if ring has no data/space. Check
return value from these routines to avoid plausible infinite loops.
Reported-by: Li Qiang
Signed-off-by: Prasad J Pandit
---
hw/rdma/vmw/pvrdma_d
From: Prasad J Pandit
Define skeleton 'uar_read' routine. Avoid NULL dereference.
Reported-by: Li Qiang
Signed-off-by: Prasad J Pandit
---
hw/rdma/vmw/pvrdma_main.c | 6 ++
1 file changed, 6 insertions(+)
Update: change return value from uar_read()
-> https://lists.gnu.org/archive/html
From: Prasad J Pandit
With commit 4481985c (rdma: check num_sge does not exceed MAX_SGE)
macro VENDOR_ERR_NO_SGE is no longer in use - delete it.
Signed-off-by: Prasad J Pandit
---
hw/rdma/rdma_backend.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
Update: change commit log message
Hello Gerd,
+-- On Thu, 13 Dec 2018, Markus Armbruster wrote --+
| Gerd Hoffmann writes:
| > Open files and directories with O_NOFOLLOW to avoid symlinks attacks.
| > While being at it also add O_CLOEXEC.
| >
| > usb-mtp only handles regular files and directories and ignores
| > everything else
From: Prasad J Pandit
When TIS request is done, set 'sts' data field across all localities.
Signed-off-by: Prasad J Pandit
---
hw/tpm/tpm_tis.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
index 20126dd838..58d90645bc 100644
--- a/hw/t
From: Prasad J Pandit
While performing mmio device r/w operations, guest could set 'addr'
parameter such that 'locty' index exceeds TPM_TIS_NUM_LOCALITIES=5.
Add check to avoid OOB access.
Reported-by: Cheng Feng
Signed-off-by: Prasad J Pandit
---
hw/tpm/tpm_tis.c | 10 --
1 file chan
From: Prasad J Pandit
While accessing script ram[2048] via 'lsi_ram_read/write' routines,
'addr' could exceed the ram range. Mask high order bits to avoid
OOB access.
Reported-by: Mark Kanda
Signed-off-by: Prasad J Pandit
---
hw/scsi/lsi53c895a.c | 2 ++
1 file changed, 2 insertions(+)
diff
From: Prasad J Pandit
The length parameter values are not negative, thus use an unsigned
type 'size_t' for them. Many routines pass 'len' values to memcpy(3)
calls. If it was negative, it could lead to memory corruption issues.
Add check to avoid it.
Reported-by: Arash TC
Signed-off-by: Prasad
+-- On Tue, 6 Nov 2018, Philippe Mathieu-Daudé wrote --+
| > @@ -113,6 +113,7 @@ static void vhci_host_send(void *opaque,
| > static uint8_t buf[4096];
| >
| > buf[0] = type;
| > +assert(len <= sizeof(buf) - 1);
|
| Why not simply "assert(len < sizeof(buf));"?
| > for (;;
.
|
| It turns out that the same can happen at several locations where
| v9fs_path_copy() is used to set the fid path. The fix is again to
| take the write lock.
|
| Cc: P J P
| Reported-by: zhibin hu
| Signed-off-by: Greg Kurz
| ---
| hw/9pfs/9p.c | 15 +++
| 1 file changed, 15
From: Prasad J Pandit
While performing mmio device r/w operations, guest could set 'addr'
parameter such that 'locty' index exceeds TPM_TIS_NUM_LOCALITIES=5
after setting new 'locty' via 'tpm_tis_new_active_locality'.
Add check to avoid OOB access.
Reported-by: Cheng Feng
Signed-off-by: Prasad
Hello Stefan,
+-- On Tue, 20 Nov 2018, Stefan Berger wrote --+
| On 11/20/18 2:22 AM, P J P wrote:
| > From: Prasad J Pandit
| >
| > While performing mmio device r/w operations, guest could set 'addr'
| > parameter such that 'locty' index exceeds TPM_TIS_NUM_LO
From: Prasad J Pandit
While reading virtual smart card data, if buffer 'size' is negative
it would lead to memory corruption errors. Add check to avoid it.
Reported-by: Arash TC
Signed-off-by: Prasad J Pandit
---
hw/usb/ccid-card-passthru.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/
+-- On Thu, 11 Oct 2018, Philippe Mathieu-Daudé wrote --+
| The IOReadHandler does not have documentation.
|
| typedef void IOReadHandler(void *opaque, const uint8_t *buf, int size);
|
| Why is the 'size' argument signed? Does it makes sens to call it with a
| negative value?
No, it doesn't I
+-- On Thu, 11 Oct 2018, Philippe Mathieu-Daudé wrote --+
| I started this change and already converted 40 files.
Wow, that's super swift! :) Will wait for the patch V2 from you then.
Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F
From: Prasad J Pandit
The length parameter values are not negative, thus use an unsigned
type 'size_t' for them. Many routines pass 'len' values to memcpy(3)
calls. If it was negative, it could lead to memory corruption issues.
Reported-by: Arash TC
Signed-off-by: Prasad J Pandit
---
bt-host.
+-- On Thu, 18 Oct 2018, Paolo Bonzini wrote --+
| So you have to first find out all places where something is subtracted
| from the length, and ensure it's okay or add assertions.
|
| Then you have to check a much more important issue: places that use a
| fixed-size buffer such as vhci_host_send
From: Prasad J Pandit
The length parameter values are not negative, thus use an unsigned
type 'size_t' for them. Many routines pass 'len' values to memcpy(3)
calls. If it was negative, it could lead to memory corruption issues.
Add check to avoid it.
Reported-by: Arash TC
Signed-off-by: Prasad
From: Prasad J Pandit
While performing gpio write via strongarm_gpio_handler_update
routine, the 'bit' index could access beyond s->handler[28] array.
Add check to avoid OOB access.
Reported-by: Moguofang
Signed-off-by: Prasad J Pandit
---
hw/arm/strongarm.c | 4 +++-
1 file changed, 3 insert
From: Prasad J Pandit
While in nvme_mmio_read, memcpy could read past the 'n->bar'
buffer, if addr offset was pointing towards its tail end.
Add check to avoid OOB access.
Reported-by: Caihongzhu
Signed-off-by: Prasad J Pandit
---
hw/block/nvme.c | 2 +-
1 file changed, 1 insertion(+), 1 dele
From: Prasad J Pandit
While performing PowerNV memory r/w operations, the access length
'sz' could exceed the data[4] buffer size. Add check to avoid OOB
access.
Reported-by: Moguofang
Signed-off-by: Prasad J Pandit
---
hw/ppc/pnv_lpc.c | 4
1 file changed, 4 insertions(+)
diff --git a/
From: Prasad J Pandit
While performing gpio write via strongarm_gpio_handler_update
routine, the 'bit' index could access beyond s->handler[28] array.
Add check to avoid OOB access.
Reported-by: Moguofang
Signed-off-by: Prasad J Pandit
---
hw/arm/strongarm.c | 4 +++-
1 file changed, 3 insert
+-- On Mon, 22 Oct 2018, liqsub1 wrote --+
| +if (bit < sizeof(s->handler) / sizeof(s->handler[0])) {
|
| Maybe you can use ARRAY_SIZE here.
Yes, sent patch v1.
Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F
+-- On Tue, 23 Oct 2018, Philippe Mathieu-Daudé wrote --+
| > From: Prasad J Pandit
| >
| > Update v1: use ARRAY_SIZE macro
| >-> https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg04826.html
| >
| > -qemu_set_irq(s->handler[bit], (level >> bit) & 1);
| > +if (bit < ARR
Hello Cedric,
+-- On Wed, 24 Oct 2018, Cédric Le Goater wrote --+
| I think using a data[8] would be more appropriate. It would make the
| pnv_lpc_do_eccb() routine a little more complex. I tried to rewrite it to
| have a common one with the P9 LPC model but could not find a common pattern.
|
+-- On Thu, 25 Oct 2018, Gerd Hoffmann wrote --+
| Simliar to deprecated machine types.
| Print a warning when creating a deprecated device.
| Add deprecation notice to -device help.
|
| TODO: add to intospection.
s/intospection/introspection ..?
| diff --git a/hw/core/qdev.c b/hw/core/qdev.c
|
+-- On Thu, 25 Oct 2018, Gerd Hoffmann wrote --+
| We have a lovely, guest-triggerable buffer overflow in opl2 emulation.
|
| Reproducer:
| outw(0xff60, 0x220);
| outw(0x1020, 0x220);
| outw(0xffb0, 0x220);
| Result:
| Will overflow FM_OPL->AR_TABLE[] (see hw/audio/fmopl.[ch])
+ R
+-- On Thu, 25 Oct 2018, Gerd Hoffmann wrote --+
| While being at it deprecate cirrus too.
|
| Reason (short version): use stdvga instead.
| Verbose version:
| https://www.kraxel.org/blog/2014/10/qemu-using-cirrus-considered-harmful
|
| Signed-off-by: Gerd Hoffmann
| ---
| hw/display/cirrus
From: Prasad J Pandit
While writing a message in 'lsi_do_msgin', message length value
in msg_len could be invalid, add check to avoid OOB access issue.
Reported-by: Ameya More
Signed-off-by: Prasad J Pandit
---
hw/scsi/lsi53c895a.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-
+-- On Thu, 25 Oct 2018, Peter Maydell wrote --+
| Hi; thanks for this patch. Looking at the SA1110 manual,
| it says that writes to the reserved bits [31:28] are
| ignored. So I think that rather than doing this check
| here, we should do what the strongarm_ppc_* code in the
| same file does -- ma
Hello Dan, all
+-- On Thu, 25 Oct 2018, Daniel P. Berrangé wrote --+
| On Thu, Oct 25, 2018 at 10:52:56AM +0200, Gerd Hoffmann wrote:
| > While being at it deprecate cirrus too.
| >
| > Reason (short version): use stdvga instead.
| > Verbose version:
| > https://www.kraxel.org/blog/2014/10/
+-- On Thu, 25 Oct 2018, Daniel P. Berrangé wrote --+
| On Thu, Oct 25, 2018 at 04:26:16PM +0530, P J P wrote:
| > +-- On Thu, 25 Oct 2018, Gerd Hoffmann wrote --+
| > | We have a lovely, guest-triggerable buffer overflow in opl2 emulation.
| > |
| > | Reproducer:
| > | out
From: Prasad J Pandit
The high[32:28] bits of 'direction' and 'state' registers of
SA-1100/SA-1110 device are reserved. Setting them may lead to
OOB 's->handler[]' array access issue. Mask off [32:28] bits to
avoid it.
Reported-by: Moguofang
Signed-off-by: Prasad J Pandit
---
hw/arm/strongarm
+-- On Thu, 25 Oct 2018, Ameya More wrote --+
| While Mark and I reported this issue to you, it was actually discovered by
| Dejvau Security and they should receive credit for reporting this issue.
| http://www.dejavusecurity.com
I see; Would it be possible to share email-id of the original repo
+-- On Fri, 26 Oct 2018, Paolo Bonzini wrote --+
| I am dumb and I don't understand. In set_ar_dr you get
|
| v = 0xff
| ar = 15
| dr = 15
|
| and OPL->AR_TABLE[60] is accessed. The size of the array is 75, which
| seems to be actually 14 more than required. Likewise OPL->DR_
+-- On Fri, 26 Oct 2018, Paolo Bonzini wrote --+
| > -int msg_len;
| > +uint8_t msg_len;
|
| Not wrong per se, but it's also not clear why it's needed. I understand
| that you want to switch from signed to unsigned, but it is not mentioned
| in the commit message.
Changed to uint8_t beca
+-- On Fri, 26 Oct 2018, Peter Maydell wrote --+
| > ===
| > diff --git a/hw/arm/strongarm.c b/hw/arm/strongarm.c
| > index ec2627374d..dd8c4b1f2e 100644
| > --- a/hw/arm/strongarm.c
| > +++ b/hw/arm/strongarm.c
| > @@ -587,12 +587,12 @@ static void strongarm_gpio_write(void *opaque, hwaddr
| > off
+-- On Fri, 26 Oct 2018, Cédric Le Goater wrote --+
| On 10/25/18 8:45 AM, P J P wrote:
| > - While we refactor the routine for better, a patch below seem okay to fix
| >the OOB access issue?
|
| I think it is fine. Please add something like :
|
| qemu_log_mask(LOG_GUEST
401 - 500 of 917 matches
Mail list logo
