+-- On Fri, 9 Aug 2019, P J P wrote --+
| From: Prasad J Pandit
|
| When executing script in lsi_execute_script(), the LSI scsi
| adapter emulator advances 's->dsp' index to read next opcode.
| This can lead to an infinite loop if the next opcode is empty.
| Exit such loop after reading
+-- On Thu, 8 Aug 2019, Philippe Mathieu-Daudé wrote --+
| >> trace_lsi_execute_script_tc_illegal();
| >> lsi_script_dma_interrupt(s, LSI_DSTAT_IID);
|
| So we agree using DSTAT.IID is the correct thing to do. Any volunteer to fix
| this? :)
Sent patch v3. Thank you.
--
Prasad J Pandit /
From: Prasad J Pandit
Use macro 'LSI_MAX_INSN' instead of a magic number 1.
Signed-off-by: Prasad J Pandit
---
hw/scsi/lsi53c895a.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
index e703ef4c9d..f6786607f8 100644
---
From: Prasad J Pandit
When executing script in lsi_execute_script(), the LSI scsi
adapter emulator advances 's->dsp' index to read next opcode.
This can lead to an infinite loop if the next opcode is empty.
Exit such loop after reading 10k empty opcodes.
Reported-by: Bugs SysSec
Signed-off-by:
From: Prasad J Pandit
Hello,
While executing script, the LSI SCSI Adapter emulator could run into an
infinite loop, if next instruction read by 's->dsp' index has an empty
opcode. Raise an illegal instruction interrupt and exit the loop after
10k iterations.
->
+-- On Thu, 8 Aug 2019, Paolo Bonzini wrote --+
| I suppose this one also blocks the monitor, but then "kill -9" is always
| your friend. :)
True. :)
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F
+-- On Thu, 8 Aug 2019, Philippe Mathieu-Daudé wrote --+
| >From user-mode? As unprivileged user?
No, needs privileges inside guest.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F
+-- On Thu, 8 Aug 2019, Paolo Bonzini wrote --+
| I am not sure this is worth a CVE.
True, it is a low one, as QEMU consumes cycles on the host.
| The kernel can cause QEMU to break, but is there a practical case in which
| an unprivileged user can do that?
QEMU does not break, it keeps
+-- On Thu, 8 Aug 2019, Stefano Garzarella wrote --+
| > +if (++insn_processed > 1) {
| ^
| Since we are using this "magic" number in several lines,
| should we define a macro?
Sent patch v2. Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF
From: Prasad J Pandit
When executing script in lsi_execute_script(), the LSI scsi
adapter emulator advances 's->dsp' index to read next opcode.
This can lead to an infinite loop if the next opcode is empty.
Exit such loop after reading 10k empty opcodes.
Reported-by: Bugs SysSec
Signed-off-by:
From: Prasad J Pandit
AHCI emulator while committing DMA buffer in ahci_commit_buf()
may do a NULL dereference if the command header 'ad->cur_cmd'
is null. Add check to avoid it.
Reported-by: Bugs SysSec
Signed-off-by: Prasad J Pandit
---
hw/ide/ahci.c | 6 --
1 file changed, 4
From: Prasad J Pandit
When executing script in lsi_execute_script(), the LSI scsi
adapter emulator advances 's->dsp' index to read next opcode.
This can lead to an infinite loop if the next opcode is empty.
Exit such loop after reading 10k empty opcodes.
Reported-by: Bugs SysSec
Signed-off-by:
+-- On Wed, 31 Jul 2019, Markus Armbruster wrote --+
| However, the code is still rather ugly, and I'd be tempted to use the
| opportunity to clean up some more. Untested sketch:
Patch v3 did a similar change
-> https://lists.gnu.org/archive/html/qemu-devel/2019-07/msg00578.html
Thank you.
+-- On Wed, 31 Jul 2019, Jason Wang wrote --+
| The series has been merged. Just need a patch on top and I can queue it for
| next release.
Sent patch v5. Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F
From: Prasad J Pandit
When invoking qemu-bridge-helper in 'net_bridge_run_helper',
instead of using fixed sized buffers, use dynamically allocated
ones initialised and returned by g_strdup_printf().
If bridge name 'br_buf' is undefined, pass empty string ("") to
g_strdup_printf() in its place,
+-- On Wed, 31 Jul 2019, Jason Wang wrote --+
| On 2019/7/29 下午11:04, Stefan Hajnoczi wrote:
| > This change isn't related to the topic of the patch. It's a separate bug
| > fix.
| >
| > Please either document it in the commit description so it's clear the
| > change is intentional, or send it
Hello Jason,
+-- On Thu, 25 Jul 2019, Jason Wang wrote --+
| > URL:https://patchew.org/QEMU/20190723104754.29324-1-ppan...@redhat.com/
|
| Prasad, this looks unrelated to the series? Please double check.
Yes, it is unrelated. Not sure how it gets triggered.
Thank you.
--
Prasad J Pandit /
+-- On Tue, 23 Jul 2019, Li Qiang wrote --+
| Stefan Hajnoczi 于2019年7月23日周二 下午9:03写道：
| > On Tue, Jul 23, 2019 at 04:17:54PM +0530, P J P wrote:
| > > -snprintf(helper_cmd, sizeof(helper_cmd), "%s %s %s %s",
| > > - helper, &qu
From: Prasad J Pandit
When invoking qemu-bridge-helper in 'net_bridge_run_helper',
instead of using fixed sized buffers, use dynamically allocated
ones initialised and returned by g_strdup_printf().
Signed-off-by: Prasad J Pandit
---
net/tap.c | 19 +++
1 file changed, 11
From: Prasad J Pandit
Move repeating error handling sequence in parse_acl_file routine
to an 'err' label.
Signed-off-by: Prasad J Pandit
---
qemu-bridge-helper.c | 19 +--
1 file changed, 9 insertions(+), 10 deletions(-)
Reviewed v3:
->
From: Prasad J Pandit
Hello,
Linux net_deivce defines network interface name to be of IFNAMSIZE(=16)
bytes, including the terminating null('\0') byte.
Qemu tap deivce, while invoking 'qemu-bridge-helper' tool to set up the
network bridge interface, supplies bridge name of 16 characters, thus
From: Prasad J Pandit
The network interface name in Linux is defined to be of size
IFNAMSIZ(=16), including the terminating null('\0') byte.
The same is applied to interface names read from 'bridge.conf'
file to form ACL rules. If user supplied '--br=bridge' name
is not restricted to the same
+-- On Tue, 16 Jul 2019, John Snow wrote --+
| I also feel that a privileged DOS by a guest of a legacy device is actually
| low priority security-wise, unless we can demonstrate that there are side
| effects that can be exploited.
Right, we are not treating this as a CVE issue as is.
+-- On Tue, 2 Jul 2019, P J P wrote --+
| |-netdev bridge,helper="/path/to/helper myarg otherarg"
| |
| | In theory any parts could contain shell meta characters, but even if
| | they don't we'll have slightly broken compat with this change.
|
| I wonder if anybody uses it like tha
+-- On Fri, 5 Jul 2019, Philippe Mathieu-Daudé wrote --+
| +static bool lqspi_accepts(void *opaque, hwaddr addr,
| + unsigned size, bool is_write,
| + MemTxAttrs attrs)
| +{
| +/*
| + * From UG1085, Chapter 24 (Quad-SPI controllers):
| +
+-- On Wed, 3 Jul 2019, no-re...@patchew.org wrote --+
| Patchew URL:
https://patchew.org/QEMU/20190703190615.31436-1-ppan...@redhat.com/
|
| This series failed the asan build test. Please find the testing commands and
| their output below. If you have Docker installed, you can probably
From: Prasad J Pandit
Define skeleton lqspi_write routine. Avoid NULL dereference.
Reported-by: Lei Sun
Signed-off-by: Prasad J Pandit
---
hw/ssi/xilinx_spips.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/hw/ssi/xilinx_spips.c b/hw/ssi/xilinx_spips.c
index
+-- On Wed, 3 Jul 2019, Daniel P. Berrangé wrote --+
| A supposed exploit of QEMU was recently announced as CVE-2019-12928
| claiming that the monitor console was insecure because the "migrate"
| command enabled arbitrary command execution for a remote attacker.
|
| To be a security risk the user
Hello Dan,
+-- On Tue, 2 Jul 2019, Daniel P. Berrangé wrote --+
| The original code was passing through to the shell to handle the case
| where the user requested
|
|-netdev bridge,helper="/path/to/helper myarg otherarg"
|
| In theory any parts could contain shell meta characters, but
Hello Li,
+-- On Mon, 1 Jul 2019, Li Qiang wrote --+
| You do two things here(avoid buffer formatting and get rid of calling
| shell), I would suggest you split these into split patch.
Both are related, 'helper_cmd' formatting was used with the shell invocation
as:
helper_cmd =
From: Prasad J Pandit
The network interface name in Linux is defined to be of size
IFNAMSIZ(=16), including the terminating null('\0') byte.
The same is applied to interface names read from 'bridge.conf'
file to form ACL rules. If user supplied '--br=bridge' name
is not restricted to the same
From: Prasad J Pandit
Refactor 'net_bridge_run_helper' routine to avoid buffer
formatting to prepare 'helper_cmd' and using shell to invoke
helper command. Instead directly execute helper program with
due arguments.
Signed-off-by: Prasad J Pandit
---
net/tap.c | 43
From: Prasad J Pandit
Move repeating error handling sequence in parse_acl_file routine
to an 'err' label.
Signed-off-by: Prasad J Pandit
---
qemu-bridge-helper.c | 19 +--
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/qemu-bridge-helper.c
From: Prasad J Pandit
Hello,
Linux net_deivce defines network interface name to be of IFNAMSIZE(=16)
bytes, including the terminating null('\0') byte.
Qemu tap deivce, while invoking 'qemu-bridge-helper' tool to set up the
network bridge interface, supplies bridge name of 16 characters, thus
+-- On Mon, 1 Jul 2019, Daniel P. Berrangé wrote --+
| > +if (strcmp(cmd, "include") && strlen(arg) >= IFNAMSIZ) {
| > +fprintf(stderr, "name `%s' too long: %lu\n", arg, strlen(arg));
|
| strlen returns size_t, which does not match %lu - it needs %zu - we can
| ignore the
+-- On Mon, 1 Jul 2019, Daniel P. Berrangé wrote --+
| Playing games with multiple "perfectly" sized static buffers & snprintf is
| madness. How about re-writing this method so that it just uses
| g_strdup_printf() to dynamically format the helper_cmd string.
|
| Alternatively we could get rid
From: Prasad J Pandit
The interface names in qemu-bridge-helper are defined to be
of size IFNAMSIZ(=16), including the terminating null('\0') byte.
The same is applied to interface names read from 'bridge.conf'
file to form ACLs rules. If user supplied '--br=bridge' name
is not restricted to the
From: Prasad J Pandit
The interface name in Linux interface request struct 'ifreq'
OR in qemu-bridge-helper is defined to be of size IFNAMSIZ(=16),
including the terminating null('\0') byte.
QEMU tap device, while invoking qemu-bridge-helper, supplies bridge
name of 16 characters, restrict it
From: Prasad J Pandit
Move repeating error handling sequence in parse_acl_file routine
to an 'err' label.
Signed-off-by: Prasad J Pandit
---
qemu-bridge-helper.c | 18 --
1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/qemu-bridge-helper.c b/qemu-bridge-helper.c
From: Prasad J Pandit
Hello,
Linux net_deivce defines network interface name to be of IFNAMSIZE(=16)
bytes, including the terminating null('\0') byte.
Qemu tap deivce, while invoking 'qemu-bridge-helper' tool to set up the
network bridge interface, supplies bridge name of 16 characters, thus
+-- On Fri, 28 Jun 2019, Daniel P. Berrangé wrote --+
| Ok, so we should explicitly report an error if the user supplied bridge name
| is too long, not silently truncate it.
|
| We should also report an error if config file has too long a bridge name.
Okay, ie. report error and exit?
+-- On
+-- On Fri, 28 Jun 2019, Daniel P. Berrangé wrote --+
| Can you elaborate on the way to exploit this as I'm not seeing
| any way that doesn't involve mis-configuration of the ACL
| config file data.
True, it depends on having an 'allow all' rule. If the bridge.conf had an
'allow all' rule below
From: Prasad J Pandit
The interface names in qemu-bridge-helper are defined to be
of size IFNAMSIZ(=16), including the terminating null('\0') byte.
The same is applied to interface names read from 'bridge.conf'
file to form ACLs rules. If user supplied '--br=bridge' name
is not restricted to the
+-- On Wed, 29 May 2019, Marc-André Lureau wrote --+
| The error is handled before guest_exec_get_args(), isn't it?
Yes, which is okay I think.
| The qga commands are only called through QMP, afaik.
I see, cool! Thanks much for the confirmation.
Thank you.
--
Prasad J Pandit / Red Hat
+-- On Wed, 29 May 2019, Marc-André Lureau wrote --+
| assert() is good if it's a programming error: that is if it should never
| happen at run-time. It's a decent way to document the code.
True; But terminating server because a user sent more input parameters does
not sound good.
Hello Marc,
+-- On Thu, 23 May 2019, Marc-André Lureau wrote --+
| I don't see how you could exploit this today.
|
| QMP parser has MAX_TOKEN_COUNT (2ULL << 20).
I see, didn't realise that. I tried to reproduce it and
{"error": {"class": "GenericError", "desc": "JSON token count limit
+-- On Wed, 22 May 2019, Marc-André Lureau wrote --+
| On Sun, May 19, 2019 at 10:55 AM P J P wrote:
| > Qemu guest agent while executing user commands does not seem to
| > check length of argument list and/or environment variables passed.
| > It may lead to integer overflow or infi
From: Prasad J Pandit
Qemu guest agent while executing user commands does not seem to
check length of argument list and/or environment variables passed.
It may lead to integer overflow or infinite loop issues. Add check
to avoid it.
Reported-by: Niu Guoxiang
Signed-off-by: Prasad J Pandit
---
+-- On Thu, 25 Apr 2019, P J P wrote --+
| When releasing spice resources in release_resource() routine,
| if release info object 'ext.info' is null, it leads to null
| pointer dereference. Add check to avoid it.
|
| diff --git a/hw/display/qxl.c b/hw/display/qxl.c
| index c8ce5781e0..632923add2
From: Prasad J Pandit
When releasing spice resources in release_resource() routine,
if release info object 'ext.info' is null, it leads to null
pointer dereference. Add check to avoid it.
Reported-by: Bugs SysSec
Signed-off-by: Prasad J Pandit
---
hw/display/qxl.c | 3 +++
1 file changed, 3
+-- On Mon, 25 Mar 2019, Peter Maydell wrote --+
| Noone has complained that it's too small because right now *we do not check
| against it* for the common case of "just load an external dtb".
|
| We should not be imposing an arbitrary limit within QEMU if we don't need
| to. Here, we do not
Hello David,
+-- On Mon, 25 Mar 2019, David Gibson wrote --+
| The only inherent limit to dtb size should be 2^31-1 bytes (the format
| uses signed 32-bit ints as offsets).
~2GB of dtb?! Seems quite big to specify the h/w that a kernel is
going to run/boot on.
| Indeed there shouldn't be
+-- On Fri, 22 Mar 2019, Peter Maydell wrote --+
| This document is specific to aarch64, but the part of
| QEMU's device tree code being modified here is
| architecture independent.
|
| Cc'ing David Gibson who will probably know if there is
| an architecture-independent limit on DTB size we
From: Prasad J Pandit
Device tree blob(dtb) file can not be larger than 2MB in size.[*]
Add check to avoid loading large dtb files in load_device_tree(),
and potential integer(dt_size) overflow.
[*] linux.git/tree/Documentation/arm64/booting.txt
Reported-by: Kurtis Miller
Signed-off-by:
+-- On Mon, 18 Feb 2019, Philippe Mathieu-Daudé wrote --+
| On 2/15/19 11:59 AM, Marc-André Lureau wrote:
| Arash or Prasad can you help us here? Do you have a reproducer?
No, we don't have a reproducer handy I'm afraid.
Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69
Hello Greg, Dan,
+-- On Mon, 18 Feb 2019, Greg Kurz wrote --+
| >>> +spapr->host_model = NULL;
| >>
| >> This isn't needed since object_initialize_with_type() already takes care
| >> of zeroing the instance for us.
| >>
| >>> +spapr->host_serial = NULL;
| >>
| >> Same here.
|
From: Prasad J Pandit
On ppc hosts, hypervisor shares following system attributes
- /proc/device-tree/system-id
- /proc/device-tree/model
with a guest. This could lead to information leakage and misuse.[*]
Add machine attributes to control such system information exposure
to a guest.
[*]
From: Prasad J Pandit
On ppc hosts, hypervisor shares following system attributes
- /proc/device-tree/system-id
- /proc/device-tree/model
with a guest. This could lead to information leakage and misuse.[*]
Add machine attributes to control such system information exposure
to a guest.
[*]
+-- On Wed, 13 Feb 2019, David Gibson wrote --+
| > +
| > +object_class_property_add_str(oc, "host-serial",
| > +machine_get_host_serial, machine_set_host_serial,
| > +_abort);
| > +object_class_property_set_description(oc, "host-serial",
| > +"Set host's system-id
From: Prasad J Pandit
On ppc hosts, hypervisor shares following system attributes
- /proc/device-tree/system-id
- /proc/device-tree/model
with a guest. This could lead to information leakage and misuse.[*]
Add machine attributes to control such system information exposure
to a guest.
[*]
+-- On Mon, 4 Feb 2019, David Gibson wrote --+
| On Mon, Feb 04, 2019 at 11:40:46AM +0530, P J P wrote:
| > Ie. make the default behaviour host-serial/host-model=NULL/none, instead of
| > 'passthrough' now?
|
| Yes.
|
Okay, I'll send a revised patch. Thank you.
--
Prasad J Pandit / R
+-- On Mon, 4 Feb 2019, David Gibson wrote --+
| I'm wondering if we can just ditch them entirely, or at least make
| them default to not present without regard to machine version.
Ie. make the default behaviour host-serial/host-model=NULL/none, instead of
'passthrough' now?
Thank you.
--
From: Prasad J Pandit
On ppc hosts, hypervisor shares following system attributes
- /proc/device-tree/system-id
- /proc/device-tree/model
with a guest. This could lead to information leakage and misuse.[*]
Add machine attributes to control such system information exposure
to a guest.
[*]
+-- On Thu, 24 Jan 2019, Michael Roth wrote --+
| I would call a helper function like get_args_max() or whatever and have
| the posix implementation in qga/commands-posix.c and a stub'd version
| in qga/commands-win32.c. There's an article here that might be useful
| for figuring out how we would
+-- On Fri, 11 Jan 2019, Paolo Bonzini wrote --+
| diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
| index 7237b4162e..42700e8897 100644
| --- a/hw/scsi/scsi-generic.c
| +++ b/hw/scsi/scsi-generic.c
| @@ -182,7 +182,7 @@ static void scsi_handle_inquiry_reply(SCSIGenericReq *r,
+-- On Fri, 11 Jan 2019, Marc-André Lureau wrote --+
| > | Check looks correct, it should probably return 1.
| >
| > Function comment says return 1 if 'm' is valid and should be appended via
| > sbappend(). Not sure if unprocessed 'm' should go to sbappend().
|
| If you look at the rest of the
From: Prasad J Pandit
While emulating identification protocol, tcp_emu() does not check
available space in the 'sc_rcv->sb_data' buffer. It could lead to
heap buffer overflow issue. Add check to avoid it.
Reported-by: Kira <864786...@qq.com>
Signed-off-by: Prasad J Pandit
---
slirp/tcp_subr.c
+-- On Fri, 11 Jan 2019, Daniel P. Berrangé wrote --+
| qga/commands.c already includes qemu/osdep.h which includs unistd.h.
|
| The build problem patchew reported was from *mingw* builds where
| sysconf does not exist.
I see; Not sure how to fix it. Maybe with conditional declaration?
#ifdef
+-- On Mon, 7 Jan 2019, P J P wrote --+
| Qemu guest agent while executing user commands does not seem to
| check length of argument list and/or environment variables passed.
| It may lead to integer overflow or infinite loop issues. Add check
| to avoid it.
|
| -size_t str_size = 1
+-- On Fri, 11 Jan 2019, Marc-André Lureau wrote --+
| > +if (m->m_len > so_rcv->sb_datalen
| > +- (so_rcv->sb_wptr - so_rcv->sb_data)) {
| > +m_free(m);
| > +return 0;
| > +}
|
| Check looks correct, it should
From: Prasad J Pandit
While emulating identification protocol, tcp_emu() does not check
available space in the 'sc_rcv->sb_data' buffer. It could lead to
heap buffer overflow issue. Add check to avoid it.
Reported-by: Kira <864786...@qq.com>
Signed-off-by: Prasad J Pandit
---
slirp/tcp_subr.c
From: Prasad J Pandit
Qemu guest agent while executing user commands does not seem to
check length of argument list and/or environment variables passed.
It may lead to integer overflow or infinite loop issues. Add check
to avoid it.
Reported-by: Niu Guoxiang
Signed-off-by: Prasad J Pandit
---
+-- On Fri, 4 Jan 2019, Mark Cave-Ayland wrote --+
| I asked someone with real Ultra-5 hardware to check this for me, and they've
| sent me back the following output:
|
| ok .properties
| address fffb8000
| button
| interrupts 0001
| reg
From: Prasad J Pandit
Define skeleton 'power_mem_read' routine. Avoid NULL dereference.
Reported-by: Fakhri Zulkifli
Signed-off-by: Prasad J Pandit
---
hw/sparc64/sun4u.c | 6 ++
1 file changed, 6 insertions(+)
Update v1: change return value to zero(0)
->
Hello Mark,
+-- On Thu, 3 Jan 2019, Mark Cave-Ayland wrote --+
| > /* Power */
| > +static uint64_t power_mem_read(void *opaque, hwaddr addr, unsigned size)
| > +{
| > +return 0x;
| > +}
| > +
| >
| > static const MemoryRegionOps power_mem_ops = {
| > +.read =
From: Prasad J Pandit
Define skeleton 'power_mem_read' routine. Avoid NULL dereference.
Reported-by: Fakhri Zulkifli
Signed-off-by: Prasad J Pandit
---
hw/sparc64/sun4u.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index
Hello Yuval,
+-- On Sun, 16 Dec 2018, Yuval Shaia wrote --+
| With this patch the last step fails, the guest OS hangs, trying to probably
| unload pvrdma driver and finally gave up after 3 minutes.
Strange...
| Anyways with debug turned on i have noticed that there is one case that
|
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
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
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()
->
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
---
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
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
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
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
+-- 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
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
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
---
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
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
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
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
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
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
+-- 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 v3.1.0
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
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
---
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(-)
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
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
301 - 400 of 912 matches
Mail list logo
