Re: [Qemu-devel] [PATCH] scsi: pvscsi: limit process IO loop to maximum page count

[P J P <ppandit@redhat.com>]

  Hello Dmitry,

+-- On Tue, 13 Sep 2016, Dmitry Fleytman wrote --+
| > On 13 Sep 2016, at 10:00 AM, P J P <ppandit@redhat.com> wrote:
| > 
| > +-- On Wed, 7 Sep 2016, P J P wrote --+
| > | From: Prasad J Pandit <pjp@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.

| > | Reported-by: Li Qiang <liqiang6-s@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?

--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F