Re: [Qemu-devel] [PATCH] scsi: pvscsi: request descriptor data_length to 32 bit

+-- On Mon, 5 Sep 2016, Paolo Bonzini wrote --+ | Without a public spec it's hard, but I guess 2048 is more than enough. === diff --git a/hw/scsi/vmw_pvscsi.c b/hw/scsi/vmw_pvscsi.c index 4245c15..4823b9d 100644 --- a/hw/scsi/vmw_pvscsi.c +++ b/hw/scsi/vmw_pvscsi.c @@ -628,17 +628,16 @@

Re: [Qemu-devel] [PATCH] scsi: pvscsi: request descriptor data_length to 32 bit

+-- On Mon, 5 Sep 2016, Paolo Bonzini wrote --+ | No, that's not what happens. chunk_size is set to sg.resid, after which: | | sg.dataAddr += chunk_size; | data_length -= chunk_size; | sg.resid -= chunk_size; | | The loop is reentered with sg.resid == 0, it calls into |

Re: [Qemu-devel] [PATCH] scsi: pvscsi: request descriptor data_length to 32 bit

Hello Paolo, all +-- On Mon, 5 Sep 2016, Paolo Bonzini wrote --+ | > -uint64_t data_length = r->req.dataLen; | > +uint32_t data_length = r->req.dataLen; | | Why is this needed if you remove the cast in MIN, below? The outer while loop below is controlled by 'data_length'. The cast in

[Qemu-devel] [PATCH] scsi: pvscsi: request descriptor data_length to 32 bit

From: Prasad J Pandit In PVSCSI paravirtual SCSI bus, the request descriptor data length is defined to be 64 bit. While building SG list from a request descriptor, it gets truncated to 32bit in routine 'pvscsi_convert_sglist'. This could lead to an infinite loop situation

Re: [Qemu-devel] [PATCH v2] scsi: check page count while initialising descriptor rings

+-- On Thu, 1 Sep 2016, Dmitry Fleytman wrote --+ | In this case, please change pvscsi_ring_init_data() return value type to | void. Also I would suggest to do the new verification after | "trace_pvscsi_on_cmd_arrived("PVSCSI_CMD_SETUP_RINGS”)”. Done; I've sent a revised patch v3. Thank you.

[Qemu-devel] [PATCH v3] scsi: check page count while initialising descriptor rings

From: Prasad J Pandit Vmware Paravirtual SCSI emulation uses command descriptors to process SCSI commands. These descriptors come with their ring buffers. A guest could set the page count for these rings to an arbitrary value, leading to infinite loop or OOB access. Add

Re: [Qemu-devel] [PATCH v2] scsi: check page count while initialising descriptor rings

Hello Dmitry, +-- On Wed, 31 Aug 2016, Dmitry Fleytman wrote --+ | > -if ((ri->reqRingNumPages > PVSCSI_SETUP_RINGS_MAX_NUM_PAGES) | > -|| (ri->cmpRingNumPages > PVSCSI_SETUP_RINGS_MAX_NUM_PAGES)) { | > -return -1; | > -} | | Hello Prasad, | | Why did you decide to

[Qemu-devel] [PATCH 2/2] scsi: mptconfig: fix an assert expression

From: Prasad J Pandit When LSI SAS1068 Host Bus emulator builds configuration page headers, mptsas_config_pack() asserts to check returned size value is within limit of 256 bytes. Fix that assert expression. Suggested-by: Paolo Bonzini

[Qemu-devel] [PATCH 1/2] scsi: mptconfig: fix format string

From: Prasad J Pandit When LSI SAS1068 Host Bus emulator builds configuration page headers, the format string used in 'mptsas_config_manufacturing_1' was wrong. It could lead to an invalid memory access. Reported-by: Tom Victor Fix-suggested-by:

Re: [Qemu-devel] [PATCH] scsi: check page count while initialising descriptor rings

+-- On Wed, 31 Aug 2016, P J P wrote --+ | | -if ((ri->reqRingNumPages > PVSCSI_SETUP_RINGS_MAX_NUM_PAGES) | -|| (ri->cmpRingNumPages > PVSCSI_SETUP_RINGS_MAX_NUM_PAGES)) { | +if (!ri->reqRingNumPages | +|| ri->reqRingNumPages > PVSCSI_SETU

[Qemu-devel] [PATCH v2] scsi: check page count while initialising descriptor rings

From: Prasad J Pandit Vmware Paravirtual SCSI emulation uses command descriptors to process SCSI commands. These descriptors come with their ring buffers. A guest could set the page count for these rings to an arbitrary value, leading to infinite loop or OOB access. Add

[Qemu-devel] [PATCH] scsi: check page count while initialising descriptor rings

From: Prasad J Pandit Vmware Paravirtual SCSI emulation uses command descriptors to process SCSI commands. These descriptors come with their ring buffers. A guest could set the page count for these rings to be zero, leading to an infinite loop. Add check to avoid it.

Re: [Qemu-devel] [PATCH] 9pfs: add check for relative path

Hello Peter, all +-- On Mon, 22 Aug 2016, Peter Maydell wrote --+ | Opinions welcome on whether we need to hold 2.7 for this bug. I'm going through the VirtFS details to figure out a best fix for this issue. Nonetheless, IMO we need not hold 2.7 release for this bug. Thank you. -- Prasad J

[Qemu-devel] [PATCH v2] net: vmxnet: use g_new for pkt initialisation

From: Li Qiang When network transport abstraction layer initialises pkt, the maximum fragmentation count is not checked. This could lead to an integer overflow causing a NULL pointer dereference. Replace g_malloc() with g_new() to catch the multiplication overflow.

[Qemu-devel] [PATCH] 9pfs: add check for relative path

From: Prasad J Pandit At various places in 9pfs back-end, it creates full path by concatenating two path strings. It could lead to a path traversal issue if one of the parameter was a relative path. Add check to avoid it. Reported-by: Felix Wilhelm

[Qemu-devel] [PATCH] net: vmxnet: check fragments count at pkt initialisation

From: Li Qiang When net transport abstraction layer initialises the pkt, the maximum fragmentation count is not checked. This could lead to an integer overflow causing a NULL pointer dereference. Add check to avoid it. Reported-by: Li Qiang Signed-off-by:

[Qemu-devel] [PATCH] net: vmxnet: initialise local tx descriptor

From: Li Qiang In Vmxnet3 device emulator while processing transmit(tx) queue, when it reaches end of packet, it calls vmxnet3_complete_packet. In that local 'txcq_descr' object is not initialised, which could leak host memory bytes a guest. Reported-by: Li Qiang

[Qemu-devel] [PATCH] net: vmxnet3: check for device_active before write

From: Li Qiang Vmxnet3 device emulator does not check if the device is active, before using it for write. It leads to a use after free issue, if the vmxnet3_io_bar0_write routine is called after the device is deactivated. Add check to avoid it. Reported-by: Li Qiang

Re: [Qemu-devel] [PATCH] net: vmxnet: check fragment length during fragmentation

Hello Jason, +-- On Thu, 4 Aug 2016, Jason Wang wrote --+ | The patch doesn't apply cleanly on HEAD, we now move this logic to | hw/net/net_tx_pkt.c. Please resend on top of HEAD and cc Dmitry Fleytman | . I see, that explains why it did not show-up in search. I've sent

[Qemu-devel] [PATCH v2] net: check fragment length during fragmentation

From: Prasad J Pandit Network transport abstraction layer supports packet fragmentation. While fragmenting a packet, it checks for more fragments from packet length and current fragment length. It is susceptible to an infinite loop, if the current fragment length is zero.

[Qemu-devel] [PATCH] net: vmxnet: check fragment length during fragmentation

From: Prasad J Pandit VMware VMXNET* NIC emulator supports packet fragmentation. While fragmenting a packet, it checks for more fragments based on packet length and current fragment length. It is susceptible to an infinite loop, if the current fragment length is zero. Add

[Qemu-devel] [PATCH] virtio: check vring descriptor buffer length

From: Prasad J Pandit virtio back end uses set of buffers to facilitate I/O operations. An infinite loop unfolds in virtqueue_pop() if a buffer was of zero size. Add check to avoid it. Reported-by: Li Qiang Signed-off-by: Prasad J Pandit

Re: [Qemu-devel] [PATCH v2] scsi: esp: remove handling of SATN/STOP

+-- On Fri, 17 Jun 2016, Paolo Bonzini wrote --+ | The implementation of SATN/STOP is completely busted. The idea | would be that the next DMA read is for a SCSI message and after | that the adapter would transition to the command phase. | | The recent fix to SATN/STOP broke migration, which is

Re: [Qemu-devel] [PATCH] scsi: esp: remove handling of SATN/STOP

+-- On Fri, 17 Jun 2016, Paolo Bonzini wrote --+ | diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c | index baa0a2c..d11151f 100644 | --- a/hw/scsi/esp.c | +++ b/hw/scsi/esp.c | @@ -192,23 +192,6 @@ static void handle_s_without_atn(ESPState *s) | } | } | | -static void handle_satn_stop(ESPState

Re: [Qemu-devel] [PATCH v4] scsi: esp: check length before dma read

+-- On Fri, 17 Jun 2016, Amit Shah wrote --+ | This was flagged as an incompatibility in the vmstate by a nightly run | of the vmstate checker: | | Section "esp" Description "esp" Field "cmdbuf" size mismatch: 16 , 32 | Section "dc390" Description "esp" Field "cmdbuf" size mismatch: 16 , 32 |

[Qemu-devel] [PATCH v4] scsi: esp: check length before dma read

From: Prasad J Pandit While doing DMA read into ESP command buffer 's->cmdbuf', it could write past the 's->cmdbuf' area, if it was partially filled; ie. 's->cmdlen' wasn't set at the start of the buffer. Check 'len' to avoid OOB access. Also increase the command buffer

Re: [Qemu-devel] [PATCH v3] scsi: esp: check length before dma read

Hello Paolo, +-- On Wed, 15 Jun 2016, Paolo Bonzini wrote --+ | Actually, the commit message is wrong. The length parameter cannot | exceed the buffer size anymore. It wouldn't exceed after this patch, right? Is it possible 'esp_do_dma' is called via 'esp_transfer_data' with 's->do_cmd'

Re: [Qemu-devel] [PATCH] scsi: esp: check length before dma read

+-- On Wed, 15 Jun 2016, Paolo Bonzini wrote --+ | So a better fix is to change cmdbuf[] to 32 bytes in | include/hw/scsi/esp.h, and define a constant ESP_CMDBUF_SZ equal to 32 | that can be used in handle_ti and in the definition of cmdbuf. Sent a revised patch v3. Thank you. -- Prasad J Pandit

[Qemu-devel] [PATCH v3] scsi: esp: check length before dma read

From: Prasad J Pandit While doing DMA read into ESP command buffer 's->cmdbuf', the length parameter could exceed the buffer size. Add check to avoid OOB access. Also increase the command buffer size to 32, which is maximum when 's->do_cmd' is set. Reported-by: Li Qiang

Re: [Qemu-devel] [PATCH] scsi: esp: clean up handle_ti/esp_do_dma if s->do_cmd

+-- On Wed, 15 Jun 2016, Laszlo Ersek wrote --+ | And I guess Prasad will submit a new version of the buffer overflow fix, | on top of this patch, according to your previous message | . Yes, I'm preparing an update. -- Prasad

[Qemu-devel] [PATCH v2] scsi: esp: check length before dma read

From: Prasad J Pandit While doing DMA read into ESP command buffer 's->cmdbuf', the length parameter could exceed the buffer size. Add check to avoid OOB access. Reported-by: Li Qiang Signed-off-by: Prasad J Pandit ---

[Qemu-devel] [PATCH] scsi: esp: check length before dma read

From: Prasad J Pandit While doing DMA read into ESP command buffer 's->cmdbuf', the length parameter could exceed the buffer size. Add check to avoid OOB access. Reported-by: Li Qiang Signed-off-by: Prasad J Pandit ---

Re: [Qemu-devel] [PATCH v2] net: mipsnet: check transmit buffer size before sending

Hello Jason, +-- On Mon, 13 Jun 2016, Jason Wang wrote --+ | > case MIPSNET_TX_DATA_BUFFER: | > s->tx_buffer[s->tx_written++] = val; | | I believe we may still have a buffer overflow here, no? No, this is the overflow that the patch is meant to fix. | > -if

[Qemu-devel] [PATCH v2] net: mipsnet: check transmit buffer size before sending

From: Prasad J Pandit When processing MIPSnet I/O port write operation, it uses a transmit buffer tx_buffer[MAX_ETH_FRAME_SIZE=1514]. Two indices 's->tx_written' and 's->tx_count' are used to control data written to this buffer. If the two were to be equal before writing,

Re: [Qemu-devel] [PATCH] net: mipsnet: check transmit buffer size before sending

Hello Jason, +-- On Wed, 8 Jun 2016, Jason Wang wrote --+ | We need to fix this issue, but instead of changing the behavior, is it | better the add a check in MIPSNET_TX_DATA_BUFFER? Yes, the patch has that too. Thank you. -- Prasad J Pandit / Red Hat Product Security Team 47AF CE69 3A90

[Qemu-devel] [PATCH] scsi: megasas: null terminate bios version buffer

From: Prasad J Pandit While reading information via 'megasas_ctrl_get_info' routine, a local bios version buffer isn't null terminated. Add the terminating null byte to avoid any OOB access. Reported-by: Li Qiang Signed-off-by: Prasad J Pandit

Re: [Qemu-devel] [PATCH v2] scsi: megasas: initialise local configuration data buffer

+-- On Tue, 7 Jun 2016, Paolo Bonzini wrote --+ | They are in already. In particular this is commit | d37af740730dbbb93960cd318e040372d04d6dcf. Okay, thank you. (sorry to bother you) -- Prasad J Pandit / Red Hat Product Security Team 47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F

Re: [Qemu-devel] [PATCH] scsi: mptsas: infinite loop while fetching requests

+-- On Tue, 7 Jun 2016, Paolo Bonzini wrote --+ | > | +if (s->state != MPI_IOC_STATE_OPERATIONAL) { | > | +mptsas_set_fault(s, MPI_IOCSTATUS_INVALID_STATE); | > | +return; | > | +} | > | while (!MPTSAS_FIFO_EMPTY(s, request_post)) { | > |

Re: [Qemu-devel] [PATCH v2] scsi: megasas: initialise local configuration data buffer

+-- On Wed, 25 May 2016, P J P wrote --+ | Update as per | -> https://lists.gnu.org/archive/html/qemu-devel/2016-05/msg04402.html | | diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c | index dcbd3e1..bf642d4 100644 | --- a/hw/scsi/megasas.c | +++ b/hw/scsi/megasas.c | @@ -1293,7 +129

Re: [Qemu-devel] [PATCH] scsi: mptsas: infinite loop while fetching requests

+-- On Tue, 24 May 2016, P J P wrote --+ | diff --git a/hw/scsi/mptsas.c b/hw/scsi/mptsas.c | index 499c146..be88e16 100644 | --- a/hw/scsi/mptsas.c | +++ b/hw/scsi/mptsas.c | @@ -754,11 +754,6 @@ static void mptsas_fetch_request(MPTSASState *s) | hwaddr addr; | int size

Re: [Qemu-devel] [PATCH] net: mipsnet: check transmit buffer size before sending

+-- On Fri, 3 Jun 2016, P J P wrote --+ | +-- On Thu, 2 Jun 2016, Peter Maydell wrote --+ | | > case MIPSNET_TX_DATA_COUNT: | | > - s->tx_count = (val <= MAX_ETH_FRAME_SIZE) ? val : 0; | | > +s->tx_count = (val < MAX_ETH_FRAME_SIZE) ? val : MAX_ETH_FRAME_S

[Qemu-devel] [PATCH v3] scsi: esp: check TI buffer index before read/write

From: Prasad J Pandit The 53C9X Fast SCSI Controller(FSC) comes with internal 16-byte FIFO buffers. One is used to handle commands and other is for information transfer. Three control variables 'ti_rptr', 'ti_wptr' and 'ti_size' are used to control r/w access to the

Re: [Qemu-devel] [PATCH v2] scsi: esp: check TI buffer index before read/write

+-- On Tue, 31 May 2016, P J P wrote --+ | switch (saddr) { | case ESP_FIFO: | -if (s->ti_size > 0) { | +if ((s->rregs[ESP_RSTAT] & STAT_PIO_MASK) == 0) { | +/* Data out. */ | +qemu_log_mask(LOG_UNIMP, "esp: PIO data read

Re: [Qemu-devel] [PATCH] net: mipsnet: check transmit buffer size before sending

+-- On Thu, 2 Jun 2016, Peter Maydell wrote --+ | > case MIPSNET_TX_DATA_COUNT: | > - s->tx_count = (val <= MAX_ETH_FRAME_SIZE) ? val : 0; | > +s->tx_count = (val < MAX_ETH_FRAME_SIZE) ? val : MAX_ETH_FRAME_SIZE; | > s->tx_written = 0; | | This is a behaviour change

[Qemu-devel] [PATCH] net: mipsnet: check transmit buffer size before sending

From: Prasad J Pandit When processing MIPSnet I/O port write operation, it uses a transmit buffer tx_buffer[MAX_ETH_FRAME_SIZE=1514]. Two indices 's->tx_written' and 's->tx_count' are used to control data written to this buffer. If the two were to be equal before writing,

[Qemu-devel] [PATCH] scsi: check buffer length before reading scsi command

From: Prasad J Pandit The 53C9X Fast SCSI Controller(FSC) comes with an internal 16-byte FIFO buffer. It is used to handle command and data transfer. Routine get_cmd() in non-DMA mode, uses 'ti_size' to read scsi command into a buffer. Add check to validate command length

Re: [Qemu-devel] [PATCH] scsi: esp: check TI buffer index before read/write

+-- On Tue, 31 May 2016, Peter Maydell wrote --+ | This is a FIFO *read*, and the trace message is for the case when the FIFO | is too full, which can't happen for a read. Okay. | OK, so we need to fix these code paths so that they keep all of the read, | write and size values in sync (or

[Qemu-devel] [PATCH v2] scsi: esp: check TI buffer index before read/write

From: Prasad J Pandit The 53C9X Fast SCSI Controller(FSC) comes with internal 16-byte FIFO buffers. One is used to handle commands and other is for information transfer. While reading/writing to these buffers an index into 's->ti_buf[TI_BUFSZ=16]' could exceed its size.

Re: [Qemu-devel] [PATCH] scsi: esp: check TI buffer index before read/write

Hello Peter, +-- On Mon, 30 May 2016, Peter Maydell wrote --+ | > +} else if (s->ti_rptr < TI_BUFSZ) { | > s->rregs[ESP_FIFO] = s->ti_buf[s->ti_rptr++]; | > +} else { | > +trace_esp_error_fifo_overrun(); | | Isn't this an underrun, not

[Qemu-devel] [PATCH] scsi: esp: check TI buffer index before read/write

From: Prasad J Pandit The 53C9X Fast SCSI Controller(FSC) comes with internal 16-byte FIFO buffers. One is used to handle commands and other is for information transfer. While reading/writing to these buffers an index into 's->ti_buf[TI_BUFSZ=16]' could exceed its size,

[Qemu-devel] [PATCH v2] scsi: megasas: check 'read_queue_head' index value

From: Prasad J Pandit While doing MegaRAID SAS controller command frame lookup, routine 'megasas_lookup_frame' uses 'read_queue_head' value as an index into 'frames[MEGASAS_MAX_FRAMES=2048]' array. Limit its value within array bounds to avoid any OOB access. Reported-by:

Re: [Qemu-devel] [PATCH 1/3] scsi: megasas: use appropriate property buffer size

+-- On Wed, 25 May 2016, Alexander Graf wrote --+ | > http://git.qemu.org/?p=qemu.git;a=blob;f=hw/scsi/megasas.c;h=a63a581550a328d0326ddee4f7fe1c4ffdecc194;hb=HEAD#l1439 | > 'dcmd_size' is same as that of 'info' object. | | Ok, then this patch is definitely bogus. The guest may receive less

[Qemu-devel] [PATCH v2] scsi: megasas: initialise local configuration data buffer

From: Prasad J Pandit When reading MegaRAID SAS controller configuration via MegaRAID Firmware Interface(MFI) commands, routine megasas_dcmd_cfg_read uses an uninitialised local data buffer. Initialise this buffer to avoid stack information leakage. Reported-by: Li Qiang

Re: [Qemu-devel] [PATCH 1/3] scsi: megasas: use appropriate property buffer size

Hello Alex, +-- On Wed, 25 May 2016, Alexander Graf wrote --+ | > -dma_buf_write((uint8_t *), cmd->iov_size, >qsg); | > +dma_buf_write((uint8_t *), dcmd_size, >qsg); | | This looks odd - can dcmd_size be bigger than iov_size? Wouldn't we overwrite | guest memory then? And where does

[Qemu-devel] [PATCH 3/3] scsi: megasas: check 'read_queue_head' index value

From: Prasad J Pandit While doing MegaRAID SAS controller command frame lookup, routine 'megasas_lookup_frame' uses 'read_queue_head' value as an index into 'frames[MEGASAS_MAX_FRAMES=2048]' array. Limit its value within array bounds to avoid any OOB access. Reported-by:

[Qemu-devel] [PATCH 2/3] scsi: megasas: initialise local configuration data buffer

From: Prasad J Pandit When reading MegaRAID SAS controller configuration via MegaRAID Firmware Interface(MFI) commands, routine megasas_dcmd_cfg_read uses an uninitialised local data buffer. Initialise this buffer to avoid stack information leakage. Reported-by: Li Qiang

[Qemu-devel] [PATCH 1/3] scsi: megasas: use appropriate property buffer size

From: Prasad J Pandit When setting MegaRAID SAS controller properties via MegaRAID Firmware Interface(MFI) commands, a user supplied size parameter is used to set property value. Use appropriate size value to avoid OOB access issues. Reported-by: Li Qiang

[Qemu-devel] [PATCH 0/3] Qemu: scsi: megasas: various OOB r/w access checks

From: Prasad J Pandit Hello, Multiple OOB r/w access issues were reported by Li Qiang, in SCSI MegaRAID SAS Host bus Adapter emulation. Please see below proposed patches to fix these issues. Thank you. -- Prasad J Pandit (3): scsi: megasas: use appropriate property

[Qemu-devel] [PATCH] scsi: mptsas: infinite loop while fetching requests

From: Prasad J Pandit The LSI SAS1068 Host Bus Adapter emulator in Qemu, periodically looks for requests and fetches them. A loop doing that in mptsas_fetch_requests() could run infinitely if 's->state' was not operational. Move check to avoid such a loop. Reported-by:

Re: [Qemu-devel] [PATCH] scsi: pvscsi: check command descriptor ring buffer size

+-- On Mon, 23 May 2016, Paolo Bonzini wrote --+ | Is there a CVE number? Yes, CVE-2016-4952: -> https://bugzilla.redhat.com/show_bug.cgi?id=1334384 Thank you. -- Prasad J Pandit / Red Hat Product Security Team 47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F

[Qemu-devel] [PATCH] scsi: pvscsi: check command descriptor ring buffer size

From: Prasad J Pandit Vmware Paravirtual SCSI emulation uses command descriptors to process SCSI commands. These descriptors come with their ring buffers. A guest could set the ring buffer size to an arbitrary value leading to OOB access issue. Add check to avoid it.

[Qemu-devel] [PATCH 1/2] scsi: check command buffer length before write(CVE-2016-4439)

From: Prasad J Pandit The 53C9X Fast SCSI Controller(FSC) comes with an internal 16-byte FIFO buffer. It is used to handle command and data transfer. While writing to this command buffer 's->cmdbuf[TI_BUFSZ=16]', a check was missing to validate input length. Add check to

[Qemu-devel] [PATCH 2/2] scsi: check dma length before reading scsi command(CVE-2016-4441)

From: Prasad J Pandit The 53C9X Fast SCSI Controller(FSC) comes with an internal 16-byte FIFO buffer. It is used to handle command and data transfer. Routine get_cmd() uses DMA to read scsi commands into this buffer. Add check to validate DMA length against buffer size to

[Qemu-devel] [PATCH 0/2] Qemu: scsi: esp: check command buffer input length

From: Prasad J Pandit Hello, The ESP 53C9X Fast SCSI Controller(FSC) comes with an internal 16-byte FIFO buffer. It is used to handle command and data transfer between controller and the bus. Couple of OOB write access issues were found and reported in its emulation by

[Qemu-devel] [PATCH] display: vga: add check to limit display width

From: Prasad J Pandit In vga_draw_graphic, display width could exceed the maximum range of VBE_DISPI_MAX_XRES(16000). This could lead to possible integer overflows. Add check to avoid it. Reported-by: Zuozhi Fzz Signed-off-by: Prasad J Pandit

Re: [Qemu-devel] [PATCH 1/2] ehci: apply limit to itd/sidt descriptors

+-- On Mon, 18 Apr 2016, Gerd Hoffmann wrote --+ | Commit "156a2e4 ehci: make idt processing more robust" tries to avoid a | DoS by the guest (create a circular itd queue and let qemu ehci | emulation run in circles forever). Unfortunaly this has two problems: | First it misses the case of sitds,

Re: [Qemu-devel] [PATCH] net: mipsnet: check packet length against buffer

+-- On Mon, 11 Apr 2016, Jason Wang wrote --+ | Can't find either. Looking at kernel driver git logs, the driver was even | removed since 2012 because it was not longer supported by MIPS. Consider it | indeed fixes a memory corruption, I tend to apply this first for 2.6. Okay, thank you. --

Re: [Qemu-devel] [PATCH] net: mipsnet: check packet length against buffer

+-- On Thu, 7 Apr 2016, Markus Armbruster wrote --+ | P J P <ppan...@redhat.com> writes: | | > --- a/hw/net/mipsnet.c | > +++ b/hw/net/mipsnet.c | > @@ -82,6 +82,9 @@ static ssize_t mipsnet_receive(NetClientState *nc, const uint8_t *buf, size_t si | > if (!mips

Re: [Qemu-devel] [PATCH v2] net: stellaris_enet: check packet length against receive buffer

+-- On Thu, 7 Apr 2016, Peter Maydell wrote --+ | I think you could reasonably put a blank line before and after | the if() {} you've added here. Sent a revised patch v3. Thank you. -- Prasad J Pandit / Red Hat Product Security Team 47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F

[Qemu-devel] [PATCH v3] net: stellaris_enet: check packet length against receive buffer

From: Prasad J Pandit When receiving packets over Stellaris ethernet controller, it uses receive buffer of size 2048 bytes. In case the controller accepts large(MTU) packets, it could lead to memory corruption. Add check to avoid it. Reported-by: Oleksandr Bazhaniuk

Re: [Qemu-devel] [PATCH 1/2] net: stellaris_enet: check packet length against receive buffer

+-- On Thu, 7 Apr 2016, Peter Maydell wrote --+ | > n -= 31; | > s->np++; | | We should do this check before we increase s->np, because | if we're going to bail out then we won't be putting this | packet into the RX FIFO. Ah, right. | The datasheet for this chip says that we

[Qemu-devel] [PATCH v2] net: stellaris_enet: check packet length against receive buffer

From: Prasad J Pandit When receiving packets over Stellaris ethernet controller, it uses receive buffer of size 2048 bytes. In case the controller accepts large(MTU) packets, it could lead to memory corruption. Add check to avoid it. Reported by: Oleksandr Bazhaniuk

[Qemu-devel] [PATCH] net: mipsnet: check packet length against buffer

From: Prasad J Pandit When receiving packets over MIPSnet network device, it uses receive buffer of size 1514 bytes. In case the controller accepts large(MTU) packets, it could lead to memory corruption. Add check to avoid it. Reported by: Oleksandr Bazhaniuk

[Qemu-devel] [PATCH 1/2] net: stellaris_enet: check packet length against receive buffer

From: Prasad J Pandit When receiving packets over Stellaris ethernet controller, it uses receive buffer of size 2048 bytes. In case the controller accepts large(MTU) packets, it could lead to memory corruption. Add check to avoid it. Reported by: Oleksandr Bazhaniuk

[Qemu-devel] [PATCH] i386: kvmvapic: initialise imm32 variable

From: Prasad J Pandit When processing Task Priorty Register(TPR) access, it could leak automatic stack variable 'imm32' in patch_instruction(). Initialise the variable to avoid it. Reported by: Donghai Zdh Signed-off-by: Prasad J Pandit

Re: [Qemu-devel] [PATCH v2 1/2] net: check packet payload length

Hello Jason, +-- On Wed, 2 Mar 2016, Jason Wang wrote --+ | How about L4, since we will calculate L4 checksum I believe? And it | looks like the following check: | | plen + hlen >= length | only count L3 header plus payload? Yes, I've sent a revised patch v3. Thank you. -- Prasad J Pandit

[Qemu-devel] [PATCH v3] net: check packet payload length

From: Prasad J Pandit While computing IP checksum, 'net_checksum_calculate' reads payload length from the packet. It could exceed the given 'data' buffer size. Add a check to avoid it. Reported-by: Liu Ling Signed-off-by: Prasad J Pandit

Re: [Qemu-devel] [PATCH v2 1/2] net: check packet payload length

Hello Jason, +-- On Fri, 26 Feb 2016, Jason Wang wrote --+ | Should we count mac header here? Did "plen + hlen >= length" imply "14 + | hlen + csum_offset + 1" < length? | | Looks not. Consider a TCP packet can report evil plen (e.g 20) but just | have 10 bytes payload in fact. In this case: |

[Qemu-devel] [PATCH v2] net: ne2000: check ring buffer control registers

From: Prasad J Pandit Ne2000 NIC uses ring buffer of NE2000_MEM_SIZE(49152) bytes to process network packets. Registers PSTART & PSTOP define ring buffer size & location. Setting these registers to invalid values could lead to infinite loop or OOB r/w access issues. Add

Re: [Qemu-devel] [PATCH] net: ne2000: check ring buffer control registers

specifies such an interface for the driver. I did not find it mentioned anywhere; Even kernel drivers have it fixed. I'll send a revised patch for ne2000_buffer_full(). -- - P J P 47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F

Re: [Qemu-devel] [PATCH] net: check packet payload length

+-- On Wed, 24 Feb 2016, Jason Wang wrote --+ | > - hw/net/virtio-net.c | > if (size > 27 && size < 1500) && | > (buf[34] == 0 && buf[35] == 67)) { | | Are we sure size of buf is >= 35 here? No; I'm yet to see why it checks for > 27. If (27 < size < 34) then we have

[Qemu-devel] [PATCH v2 1/2] net: check packet payload length

From: Prasad J Pandit While computing IP checksum, 'net_checksum_calculate' reads payload length from the packet. It could exceed the given 'data' buffer size. Add a check to avoid it. Reported-by: Liu Ling Signed-off-by: Prasad J Pandit

[Qemu-devel] [PATCH v2 2/2] net: minor indentation updates

From: Prasad J Pandit Due indentation and braces were missing at places, added them. Signed-off-by: Prasad J Pandit --- net/checksum.c | 20 +++- 1 file changed, 11 insertions(+), 9 deletions(-) Update as per review: ->

[Qemu-devel] [PATCH v2 0/2] net: check payload length and minor updates

From: Prasad J Pandit The 'net_checksum_calculate' routine reads payload length from the packet. It could exceed the given data length value and lead to OOB memory access. While fixing that I also came across couple of minor coding style glitches. This series fixes both

Re: [Qemu-devel] [PATCH] net: check packet payload length

0x0f) * 4; plen = (data[16] << 8 | data[17]) - hlen; proto = data[23]; I'll send a revised patch with a check for minimum 'data' length to include complete layer-2(14) + layer-3(20) headers. +if (len < 14+20) +return; Thank you. -- - P J P 47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F

Re: [Qemu-devel] [PATCH] net: ne2000: check ring buffer control registers

> s->start' ... avail = (s->stop - s->start) - (index - boundary); Is there a case wherein drivers need to adjust ring buffer pointers? If not, I think it's better to convert EN0_STARTPG:, EN0_STOPPG:, EN0_BOUNDARY: and EN1_CURPAG: cases into no-ops. -- - P J P 47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F

Re: [Qemu-devel] [PATCH] net: check packet payload length

Hello Markus, +-- On Thu, 18 Feb 2016, Markus Armbruster wrote --+ | | if ((data[14] & 0xf0) != 0x40) | Buffer overrun when length <= 14. | | proto = data[23]; | Buffer overrun when length <= 23. | | I think we should check that we got at least an IPv4 header without |

Re: [Qemu-devel] [PATCH] net: check packet payload length

+-- On Thu, 18 Feb 2016, P J P wrote --+ | +-- On Wed, 17 Feb 2016, Markus Armbruster wrote --+ | | Is calling this function with a partial IPv4 TCP/UDP packet legitimate? | | If partial packet is okay, what about a partial header? | | Partial? That shouldn't harm I guess. For partial TCP

Re: [Qemu-devel] [PATCH] net: minor indentation updates

+-- On Wed, 17 Feb 2016, Eric Blake wrote --+ | Cleaning up existing code is best done as part of a series that is | otherwise touching the code; doing it in isolation makes 'git blame' | attribute the wrong author for no good reason. Ah, okay. I noticed it while patching

Re: [Qemu-devel] [PATCH] net: check packet payload length

+-- On Wed, 17 Feb 2016, Markus Armbruster wrote --+ | Is calling this function with a partial IPv4 TCP/UDP packet legitimate? | If partial packet is okay, what about a partial header? Partial? That shouldn't harm I guess. | If not, should we assert plen + hlen <= length? Or == length, even?

[Qemu-devel] [PATCH] net: minor indentation updates

From: Prasad J Pandit Due indentation and braces were missing at places, added them. Signed-off-by: Prasad J Pandit --- net/checksum.c | 20 +++- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/net/checksum.c

[Qemu-devel] [PATCH] net: check packet payload length

From: Prasad J Pandit While computing IP checksum, 'net_checksum_calculate' reads payload length from the packet. It could exceed the given 'data' buffer size. Add a check to avoid it. Reported-by: Liu Ling Signed-off-by: Prasad J Pandit

Re: [Qemu-devel] [PATCH] usb: check RNDIS buffer offsets & length

+-- On Tue, 16 Feb 2016, Gerd Hoffmann wrote --+ | > @@ -172,11 +172,18 @@ static void do_token_in(USBDevice *s, USBPacket *p) | > assert(p->ep->nr == 0); | > +if (s->setup_len > sizeof(s->data_buf)) { | > +fprintf(stderr, | > +"usb_generic_handle_packet: ctrl

Re: [Qemu-devel] [PATCH] usb: check RNDIS buffer offsets & length

Hello Gerd, +-- On Tue, 16 Feb 2016, Gerd Hoffmann wrote --+ | Moves up the check so it is done for every control xfer. Good. ... | Why this is needed? All control transfers go through do_token_setup | first, so with the check moved in do_token_setup we should never ever | trigger it here

[Qemu-devel] [PATCH 2/2] usb: check RNDIS buffer offsets & length

From: Prasad J Pandit When processing remote NDIS control message packets, the USB Net device emulator uses a fixed length(4096) data buffer. The incoming informationBufferOffset & Length combination could overflow and cross that range. Check control message buffer

[Qemu-devel] [PATCH 0/2] usb: check RNDIS offsets & length

From: Prasad J Pandit Hello, When processing remote NDIS control message packets, the USB Net device emulator uses a fixed length(4096) data buffer. The incoming packet length could exceed that OR informationBufferOffset & Length combination could overflow and cross that

[Qemu-devel] [PATCH 1/2] usb: check RNDIS message length

From: Prasad J Pandit When processing remote NDIS control message packets, the USB Net device emulator uses a fixed length(4096) data buffer. The incoming packet length could exceed this limit. Add a check to avoid it. Signed-off-by: Prasad J Pandit

Re: [Qemu-devel] [PATCH] usb: ohci avoid multiple eof timers

Hello Gerd, +-- On Tue, 16 Feb 2016, Gerd Hoffmann wrote --+ | Can you try the attached patch instead? Yes, this one works too. Thank you. -- Prasad J Pandit / Red Hat Product Security Team 47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F

[Qemu-devel] [PATCH] usb: ohci avoid multiple eof timers

From: Prasad J Pandit When transitioning an OHCI controller to the OHCI_USB_OPERATIONAL state, it creates an eof timer object in 'ohci_bus_start'. It does not check if one already exists. This results in memory leakage and null dereference issue. Add a check to avoid it.

Re: [Qemu-devel] [PATCH] usb: check RNDIS buffer offsets & length

+-- On Tue, 9 Feb 2016, P J P wrote --+ | +-- On Fri, 5 Feb 2016, P J P wrote --+ | | From: Prasad J Pandit <p...@fedoraproject.org> | | | | When processing remote NDIS control message packets, the USB Net | | device emulator uses a fixed length(4096) data buffer. The in

<    3   4   5   6   7   8   9   10   >