Re: [Qemu-devel] [PATCH] spapr-vscsi: Adding VSCSI capabilities

2013-08-27 Thread Nikunj A Dadhania
Alexander Graf writes: >> +static int vscsi_send_capabilities(VSCSIState *s, vscsi_req *req) >> +{ >> +struct viosrp_capabilities *vcap; >> +struct capabilities cap = { }; >> +uint16_t len, req_len; >> +uint64_t buffer; >> +int rc; >> + >> +vcap = &req->iu.mad.capabilities;

Re: [Qemu-devel] [PATCH] spapr-vscsi: Adding VSCSI capabilities

2013-08-27 Thread Alexander Graf
On 27.08.2013, at 07:43, Nikunj A Dadhania wrote: > Alexander Graf writes: > >> On 26.08.2013, at 13:46, Nikunj A Dadhania wrote: >> >>> Alexander Graf writes: >>> On 26.08.2013, at 12:58, Nikunj A Dadhania wrote: > This implements capabilities exchange between host and client

Re: [Qemu-devel] [PATCH] spapr-vscsi: Adding VSCSI capabilities

2013-08-26 Thread Nikunj A Dadhania
Alexander Graf writes: > On 26.08.2013, at 13:46, Nikunj A Dadhania wrote: > >> Alexander Graf writes: >> >>> On 26.08.2013, at 12:58, Nikunj A Dadhania wrote: >>> This implements capabilities exchange between host and client. As at the moment no capability is supported, put zero fla

Re: [Qemu-devel] [PATCH] spapr-vscsi: Adding VSCSI capabilities

2013-08-26 Thread Nikunj A Dadhania
Alexander Graf writes: +rc = spapr_vio_dma_read(&s->vdev, buffer, &cap, len); +if (rc) { +fprintf(stderr, "vscsi_send_capabilities: DMA read failure !\n"); >>> >>> At this point cap contains random host data, no? >> >> Yes, and we zero it out in this case. > > The

Re: [Qemu-devel] [PATCH] spapr-vscsi: Adding VSCSI capabilities

2013-08-26 Thread Nikunj A Dadhania
Paolo Bonzini writes: > Il 26/08/2013 11:06, Nikunj A Dadhania ha scritto: +fprintf(stderr, "vscsi_send_capabilities: size out of bound !\n"); +goto error_out; +} >>> >>> I am not 100% familiar with the protocol, could it be that we should >>> just read sizeof(c

Re: [Qemu-devel] [PATCH] spapr-vscsi: Adding VSCSI capabilities

2013-08-26 Thread Benjamin Herrenschmidt
On Mon, 2013-08-26 at 15:37 +0200, Paolo Bonzini wrote: > There are certainly cases where time-of-check-to-time-of-use > vulnerability could make QEMU access uninitialized memory (or worse, > out-of-bounds arrays). For example, you could try racing the host on > the length of a scatter/gather list

Re: [Qemu-devel] [PATCH] spapr-vscsi: Adding VSCSI capabilities

2013-08-26 Thread Paolo Bonzini
Il 26/08/2013 11:06, Nikunj A Dadhania ha scritto: >>> +fprintf(stderr, "vscsi_send_capabilities: size out of bound !\n"); >>> +goto error_out; >>> +} >> >> I am not 100% familiar with the protocol, could it be that we should >> just read sizeof(cap) instead of erroring out or i

Re: [Qemu-devel] [PATCH] spapr-vscsi: Adding VSCSI capabilities

2013-08-26 Thread Paolo Bonzini
Il 25/08/2013 22:51, Benjamin Herrenschmidt ha scritto: > On Sun, 2013-08-25 at 17:41 +0100, Alexander Graf wrote: >> >> While I don't think any harm could happen from it, this could lead to >> a potential timing attack where we read and write from different >> locations in memory if the guest swiz

Re: [Qemu-devel] [PATCH] spapr-vscsi: Adding VSCSI capabilities

2013-08-26 Thread Alexander Graf
On 26.08.2013, at 13:46, Nikunj A Dadhania wrote: > Alexander Graf writes: > >> On 26.08.2013, at 12:58, Nikunj A Dadhania wrote: >> >>> This implements capabilities exchange between host and client. >>> As at the moment no capability is supported, put zero flags everywhere >>> and return. >>>

Re: [Qemu-devel] [PATCH] spapr-vscsi: Adding VSCSI capabilities

2013-08-26 Thread Nikunj A Dadhania
Alexander Graf writes: > On 26.08.2013, at 12:58, Nikunj A Dadhania wrote: > >> This implements capabilities exchange between host and client. >> As at the moment no capability is supported, put zero flags everywhere >> and return. >> >> Signed-off-by: Nikunj A Dadhania >> --- >> +rc = spap

Re: [Qemu-devel] [PATCH] spapr-vscsi: Adding VSCSI capabilities

2013-08-26 Thread Alexander Graf
On 26.08.2013, at 12:58, Nikunj A Dadhania wrote: > Alexander Graf writes: >> Am 26.08.2013 um 05:32 schrieb Nikunj A Dadhania : >> >>> Benjamin Herrenschmidt writes: >>> On Sun, 2013-08-25 at 17:41 +0100, Alexander Graf wrote: >> +vcap = &req->iu.mad.capabilities; >> +rc

Re: [Qemu-devel] [PATCH] spapr-vscsi: Adding VSCSI capabilities

2013-08-26 Thread Nikunj A Dadhania
Alexander Graf writes: > Am 26.08.2013 um 05:32 schrieb Nikunj A Dadhania : > >> Benjamin Herrenschmidt writes: >> >>> On Sun, 2013-08-25 at 17:41 +0100, Alexander Graf wrote: > +vcap = &req->iu.mad.capabilities; > +rc = spapr_vio_dma_read(&s->vdev, be64_to_cpu(vcap->buffer), >>>

Re: [Qemu-devel] [PATCH] spapr-vscsi: Adding VSCSI capabilities

2013-08-26 Thread Nikunj A Dadhania
Alexander Graf writes: > Am 26.08.2013 um 05:32 schrieb Nikunj A Dadhania : > >> Benjamin Herrenschmidt writes: >> From: Nikunj A Dadhania >> >> This implements capabilities exchange between host and client. > > Client? vscsi host implemented by VIOS and vscsi client running inside the guest.

Re: [Qemu-devel] [PATCH] spapr-vscsi: Adding VSCSI capabilities

2013-08-26 Thread Benjamin Herrenschmidt
On Mon, 2013-08-26 at 10:43 +0200, Alexander Graf wrote: > Do we ever need to preserve random fields in that struct? Or would it > be clearer to just set the whole thing to 0 and then go from there? Yeah sort of, we set/clear bits more/less based on what the guest put in the first place, it's a bi

Re: [Qemu-devel] [PATCH] spapr-vscsi: Adding VSCSI capabilities

2013-08-26 Thread Alexander Graf
On 26.08.2013, at 11:08, Nikunj A Dadhania wrote: > Alexander Graf writes: > >> Am 26.08.2013 um 08:22 schrieb Benjamin Herrenschmidt >> : >> >>> On Mon, 2013-08-26 at 06:44 +0100, Alexander Graf wrote: > +cap.flags = 0; > +cap.migration.ecl = 0; > +cap.reserve.type =

Re: [Qemu-devel] [PATCH] spapr-vscsi: Adding VSCSI capabilities

2013-08-26 Thread Nikunj A Dadhania
Alexander Graf writes: > Am 26.08.2013 um 08:22 schrieb Benjamin Herrenschmidt > : > >> On Mon, 2013-08-26 at 06:44 +0100, Alexander Graf wrote: +cap.flags = 0; +cap.migration.ecl = 0; +cap.reserve.type = 0; +cap.migration.common.server_support = 0; +

Re: [Qemu-devel] [PATCH] spapr-vscsi: Adding VSCSI capabilities

2013-08-26 Thread Nikunj A Dadhania
Benjamin Herrenschmidt writes: > On Mon, 2013-08-26 at 10:02 +0530, Nikunj A Dadhania wrote: > >> >> From: Nikunj A Dadhania >> >> This implements capabilities exchange between host and client. >> As at the moment no capability is supported, put zero flags everywhere >> and return. >> >> Sign

Re: [Qemu-devel] [PATCH] spapr-vscsi: Adding VSCSI capabilities

2013-08-26 Thread Alexander Graf
Am 26.08.2013 um 08:22 schrieb Benjamin Herrenschmidt : > On Mon, 2013-08-26 at 06:44 +0100, Alexander Graf wrote: >>> +cap.flags = 0; >>> +cap.migration.ecl = 0; >>> +cap.reserve.type = 0; >>> +cap.migration.common.server_support = 0; >>> +cap.reserve.common.server_support

Re: [Qemu-devel] [PATCH] spapr-vscsi: Adding VSCSI capabilities

2013-08-25 Thread Benjamin Herrenschmidt
On Mon, 2013-08-26 at 06:44 +0100, Alexander Graf wrote: > > +cap.flags = 0; > > +cap.migration.ecl = 0; > > +cap.reserve.type = 0; > > +cap.migration.common.server_support = 0; > > +cap.reserve.common.server_support = 0; > > My question stands unanswered. Is this just memset(0

Re: [Qemu-devel] [PATCH] spapr-vscsi: Adding VSCSI capabilities

2013-08-25 Thread Benjamin Herrenschmidt
On Mon, 2013-08-26 at 10:02 +0530, Nikunj A Dadhania wrote: > > From: Nikunj A Dadhania > > This implements capabilities exchange between host and client. > As at the moment no capability is supported, put zero flags everywhere > and return. > > Signed-off-by: Nikunj A Dadhania > --- > hw/sc

Re: [Qemu-devel] [PATCH] spapr-vscsi: Adding VSCSI capabilities

2013-08-25 Thread Alexander Graf
Am 26.08.2013 um 05:32 schrieb Nikunj A Dadhania : > Benjamin Herrenschmidt writes: > >> On Sun, 2013-08-25 at 17:41 +0100, Alexander Graf wrote: +vcap = &req->iu.mad.capabilities; +rc = spapr_vio_dma_read(&s->vdev, be64_to_cpu(vcap->buffer), +

Re: [Qemu-devel] [PATCH] spapr-vscsi: Adding VSCSI capabilities

2013-08-25 Thread Nikunj A Dadhania
Benjamin Herrenschmidt writes: > On Sun, 2013-08-25 at 17:41 +0100, Alexander Graf wrote: >> > +vcap = &req->iu.mad.capabilities; >> > +rc = spapr_vio_dma_read(&s->vdev, be64_to_cpu(vcap->buffer), >> > +&cap, >> be16_to_cpu(vcap->common.length)); >> >> While I

Re: [Qemu-devel] [PATCH] spapr-vscsi: Adding VSCSI capabilities

2013-08-25 Thread Benjamin Herrenschmidt
On Sun, 2013-08-25 at 17:41 +0100, Alexander Graf wrote: > > +vcap = &req->iu.mad.capabilities; > > +rc = spapr_vio_dma_read(&s->vdev, be64_to_cpu(vcap->buffer), > > +&cap, > be16_to_cpu(vcap->common.length)); > > While I don't think any harm could happen from i

Re: [Qemu-devel] [PATCH] spapr-vscsi: Adding VSCSI capabilities

2013-08-25 Thread Benjamin Herrenschmidt
On Sun, 2013-08-25 at 17:41 +0100, Alexander Graf wrote: > > While I don't think any harm could happen from it, this could lead to > a potential timing attack where we read and write from different > locations in memory if the guest swizzles the request while we're > processing it. > > It's certa

Re: [Qemu-devel] [PATCH] spapr-vscsi: Adding VSCSI capabilities

2013-08-25 Thread Alexander Graf
On 23.08.2013, at 10:22, Alexey Kardashevskiy wrote: > From: Nikunj A Dadhania > > This implements capabilities exchange between host and client. > As at the moment no capability is supported, put zero flags everywhere > and return. > > Signed-off-by: Nikunj A Dadhania > Signed-off-by: Alexey

[Qemu-devel] [PATCH] spapr-vscsi: Adding VSCSI capabilities

2013-08-23 Thread Alexey Kardashevskiy
From: Nikunj A Dadhania This implements capabilities exchange between host and client. As at the moment no capability is supported, put zero flags everywhere and return. Signed-off-by: Nikunj A Dadhania Signed-off-by: Alexey Kardashevskiy --- hw/scsi/spapr_vscsi.c | 31 +++