> On 13 Sep 2016, at 13:48 PM, P J P <ppan...@redhat.com> wrote:
>
> Hello Dmitry,
>
> +-- On Tue, 13 Sep 2016, Dmitry Fleytman wrote --+
> | > On 13 Sep 2016, at 10:00 AM, P J P <ppan...@redhat.com> wrote:
> | >
> | > +-- On Wed, 7 Sep 2016, P J P wrote --+
> | > | From: Prasad J Pandit <p...@fedoraproject.org>
> | > |
> | > | Vmware Paravirtual SCSI emulator while processing IO requests
> | > | could run into an infinite loop if 'pvscsi_ring_pop_req_descr'
> | > | always returned positive value. Limit IO loop to the maximum
> |
> | Do you see any specific scenario why this might happen?
>
> A guest user could set the 'ready_ptr' and 'PVSCSIRingInfo *mgr' parameter
> in 'pvscsi_ring_pop_req_descr', such that it always returns true.
I see. The problematic code is if (ready_ptr != mgr->consumed_ptr) {…}
mgr->consumed_ptr is managed by device and not visible to the driver,
but ready_ptr is managed by driver and may be set to some “big” number.
In this case it may take a lot of iterations for consumed_ptr
to become equal to ready_ptr and additionally some requests will be send
multiple times.
The most straightforward way to fix this issue will be to
check that ready_ptr - consumed_ptr is less than ring size.
>
> | > | Reported-by: Li Qiang <liqiang...@360.cn>
> | > | pvscsi_process_io(PVSCSIState *s)
> | > | {
> | > | + int descr_pa_cnt = 0;
> | > | PVSCSIRingReqDesc descr;
> | > | hwaddr next_descr_pa;
> | > |
> | > | assert(s->rings_info_valid);
> | > | - while ((next_descr_pa = pvscsi_ring_pop_req_descr(&s->rings)) !=
> 0) {
> | > | + while (((next_descr_pa = pvscsi_ring_pop_req_descr(&s->rings)) !=
> 0)
> | > | + && descr_pa_cnt++ < PVSCSI_SETUP_RINGS_MAX_NUM_PAGES) {
> |
> | Why do you limit number of processed descriptors by maximal number of pages
> | in data exchange ring? What will happen to requests still waiting in the
> | ring after this function exits?
>
> I limit it to maximum page count thinking that the descriptor value returned
> by pvscsi_ring_pop_req_descr() is derived from the mgr->req_ring_pages_pa[]
> array, which is of size 'PVSCSI_SETUP_RINGS_MAX_NUM_PAGES'. If
> pvscsi_process_io() was to go into an infinite loop, it'd continue processing
> the same set of req_ring_pages?
I think you’re mixing concepts of number of
pages in the ring and number of requests in the ring.
Each page contains (much) more than one request.
>
> --
> Prasad J Pandit / Red Hat Product Security Team
> 47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F
