* [PATCH v2] scsi: virtio-scsi: Fix address translation failure of HighMem pages used by sg list
@ 2012-07-25 12:13 Wang Sen
2012-07-25 12:21 ` Ben Hutchings
2012-07-25 12:34 ` Paolo Bonzini
0 siblings, 2 replies; 6+ messages in thread
From: Wang Sen @ 2012-07-25 12:13 UTC (permalink / raw)
To: linux-scsi
Cc: JBottomley, pbonzini, stefanha, mc, linux-kernel, stable,
Wang Sen
When using the commands below to write some data to a virtio-scsi LUN of the
QEMU guest(32-bit) with 1G physical memory(qemu -m 1024), the qemu will crash.
# sudo mkfs.ext4 /dev/sdb (/dev/sdb is the virtio-scsi LUN.)
# sudo mount /dev/sdb /mnt
# dd if=/dev/zero of=/mnt/file bs=1M count=1024
In current implementation, sg_set_buf is called to add buffers to sg list which
is put into the virtqueue eventually. But if there are some HighMem pages in
table->sgl you can not get virtual address by sg_virt. So, sg_virt(sg_elem) may
return NULL value. This will cause QEMU exit when virtqueue_map_sg is called
in QEMU because an invalid GPA is passed by virtqueue.
I take Paolo's solution mentioned in last thread to avoid failure on handling
flag bits.
I have tested the patch on my workstation. QEMU would not crash any more.
Signed-off-by: Wang Sen <senwang@linux.vnet.ibm.com>
---
drivers/scsi/virtio_scsi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
index 1b38431..6661610 100644
--- a/drivers/scsi/virtio_scsi.c
+++ b/drivers/scsi/virtio_scsi.c
@@ -198,7 +198,7 @@ static void virtscsi_map_sgl(struct scatterlist *sg, unsigned int *p_idx,
int i;
for_each_sg(table->sgl, sg_elem, table->nents, i)
- sg_set_buf(&sg[idx++], sg_virt(sg_elem), sg_elem->length);
+ sg[idx++] = *sg_elem;
*p_idx = idx;
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] scsi: virtio-scsi: Fix address translation failure of HighMem pages used by sg list
2012-07-25 12:13 [PATCH v2] scsi: virtio-scsi: Fix address translation failure of HighMem pages used by sg list Wang Sen
@ 2012-07-25 12:21 ` Ben Hutchings
2012-07-26 9:20 ` Wang Sen
2012-07-25 12:34 ` Paolo Bonzini
1 sibling, 1 reply; 6+ messages in thread
From: Ben Hutchings @ 2012-07-25 12:21 UTC (permalink / raw)
To: Wang Sen
Cc: linux-scsi, JBottomley, pbonzini, stefanha, mc, linux-kernel,
stable
[-- Attachment #1: Type: text/plain, Size: 1231 bytes --]
On Wed, 2012-07-25 at 20:13 +0800, Wang Sen wrote:
> When using the commands below to write some data to a virtio-scsi LUN of the
> QEMU guest(32-bit) with 1G physical memory(qemu -m 1024), the qemu will crash.
>
> # sudo mkfs.ext4 /dev/sdb (/dev/sdb is the virtio-scsi LUN.)
> # sudo mount /dev/sdb /mnt
> # dd if=/dev/zero of=/mnt/file bs=1M count=1024
>
> In current implementation, sg_set_buf is called to add buffers to sg list which
> is put into the virtqueue eventually. But if there are some HighMem pages in
> table->sgl you can not get virtual address by sg_virt. So, sg_virt(sg_elem) may
> return NULL value. This will cause QEMU exit when virtqueue_map_sg is called
> in QEMU because an invalid GPA is passed by virtqueue.
>
> I take Paolo's solution mentioned in last thread to avoid failure on handling
> flag bits.
>
> I have tested the patch on my workstation. QEMU would not crash any more.
>
> Signed-off-by: Wang Sen <senwang@linux.vnet.ibm.com>
[...]
This is not the correct way to submit a change for stable. See
Documentation/stable_kernel_rules.txt.
Ben.
--
Ben Hutchings
If more than one person is responsible for a bug, no one is at fault.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] scsi: virtio-scsi: Fix address translation failure of HighMem pages used by sg list
2012-07-25 12:13 [PATCH v2] scsi: virtio-scsi: Fix address translation failure of HighMem pages used by sg list Wang Sen
2012-07-25 12:21 ` Ben Hutchings
@ 2012-07-25 12:34 ` Paolo Bonzini
2012-07-25 12:47 ` Boaz Harrosh
1 sibling, 1 reply; 6+ messages in thread
From: Paolo Bonzini @ 2012-07-25 12:34 UTC (permalink / raw)
To: Wang Sen; +Cc: linux-scsi, JBottomley, stefanha, mc, linux-kernel, stable
Il 25/07/2012 14:13, Wang Sen ha scritto:
> When using the commands below to write some data to a virtio-scsi LUN of the
> QEMU guest(32-bit) with 1G physical memory(qemu -m 1024), the qemu will crash.
>
> # sudo mkfs.ext4 /dev/sdb (/dev/sdb is the virtio-scsi LUN.)
> # sudo mount /dev/sdb /mnt
> # dd if=/dev/zero of=/mnt/file bs=1M count=1024
>
> In current implementation, sg_set_buf is called to add buffers to sg list which
> is put into the virtqueue eventually. But if there are some HighMem pages in
> table->sgl you can not get virtual address by sg_virt. So, sg_virt(sg_elem) may
> return NULL value. This will cause QEMU exit when virtqueue_map_sg is called
> in QEMU because an invalid GPA is passed by virtqueue.
>
> I take Paolo's solution mentioned in last thread to avoid failure on handling
> flag bits.
Please include an URL or (better) summarize the reason why sg_set_page
is not correct in the commit message. For example, replace this
paragraph with the following:
"To fix this, we can simply copy the original scatterlist entries into
virtio-scsi's; we need to copy the entries entirely, including the flag
bits, so using sg_set_page is not correct".
Please send v3 with this change and I'll add my Acked-by.
Paolo
>
> I have tested the patch on my workstation. QEMU would not crash any more.
>
> Signed-off-by: Wang Sen <senwang@linux.vnet.ibm.com>
> ---
> drivers/scsi/virtio_scsi.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
> index 1b38431..6661610 100644
> --- a/drivers/scsi/virtio_scsi.c
> +++ b/drivers/scsi/virtio_scsi.c
> @@ -198,7 +198,7 @@ static void virtscsi_map_sgl(struct scatterlist *sg, unsigned int *p_idx,
> int i;
>
> for_each_sg(table->sgl, sg_elem, table->nents, i)
> - sg_set_buf(&sg[idx++], sg_virt(sg_elem), sg_elem->length);
> + sg[idx++] = *sg_elem;
>
> *p_idx = idx;
> }
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] scsi: virtio-scsi: Fix address translation failure of HighMem pages used by sg list
2012-07-25 12:34 ` Paolo Bonzini
@ 2012-07-25 12:47 ` Boaz Harrosh
2012-07-25 12:50 ` Paolo Bonzini
0 siblings, 1 reply; 6+ messages in thread
From: Boaz Harrosh @ 2012-07-25 12:47 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Wang Sen, linux-scsi, JBottomley, stefanha, mc, linux-kernel,
stable
On 07/25/2012 03:34 PM, Paolo Bonzini wrote:
> Il 25/07/2012 14:13, Wang Sen ha scritto:
>> When using the commands below to write some data to a virtio-scsi LUN of the
>> QEMU guest(32-bit) with 1G physical memory(qemu -m 1024), the qemu will crash.
>>
>> # sudo mkfs.ext4 /dev/sdb (/dev/sdb is the virtio-scsi LUN.)
>> # sudo mount /dev/sdb /mnt
>> # dd if=/dev/zero of=/mnt/file bs=1M count=1024
>>
>> In current implementation, sg_set_buf is called to add buffers to sg list which
>> is put into the virtqueue eventually. But if there are some HighMem pages in
>> table->sgl you can not get virtual address by sg_virt. So, sg_virt(sg_elem) may
>> return NULL value. This will cause QEMU exit when virtqueue_map_sg is called
>> in QEMU because an invalid GPA is passed by virtqueue.
>>
>> I take Paolo's solution mentioned in last thread to avoid failure on handling
>> flag bits.
>
> Please include an URL or (better) summarize the reason why sg_set_page
> is not correct in the commit message. For example, replace this
> paragraph with the following:
>
> "To fix this, we can simply copy the original scatterlist entries into
> virtio-scsi's; we need to copy the entries entirely, including the flag
> bits, so using sg_set_page is not correct".
>
> Please send v3 with this change and I'll add my Acked-by.
>
NACK-by: Boaz Harrosh
Apart from the HighMem pages problem, where in previous sg_set_buf()
code was the marker copied? It was not because it is not needed because
the allocation of sg took care of that. For example in 64bit the is no
bugs, right?
If there was a destination sg_list termination bug, it should be fixed
as a separate patch from this "HighMem pages problem". But I bet if
you inspect the code carefully there isn't such a bug.
Cheers
Boaz
> Paolo
>
>>
>> I have tested the patch on my workstation. QEMU would not crash any more.
>>
>> Signed-off-by: Wang Sen <senwang@linux.vnet.ibm.com>
>> ---
>> drivers/scsi/virtio_scsi.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
>> index 1b38431..6661610 100644
>> --- a/drivers/scsi/virtio_scsi.c
>> +++ b/drivers/scsi/virtio_scsi.c
>> @@ -198,7 +198,7 @@ static void virtscsi_map_sgl(struct scatterlist *sg, unsigned int *p_idx,
>> int i;
>>
>> for_each_sg(table->sgl, sg_elem, table->nents, i)
>> - sg_set_buf(&sg[idx++], sg_virt(sg_elem), sg_elem->length);
>> + sg[idx++] = *sg_elem;
>>
>> *p_idx = idx;
>> }
>>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] scsi: virtio-scsi: Fix address translation failure of HighMem pages used by sg list
2012-07-25 12:47 ` Boaz Harrosh
@ 2012-07-25 12:50 ` Paolo Bonzini
0 siblings, 0 replies; 6+ messages in thread
From: Paolo Bonzini @ 2012-07-25 12:50 UTC (permalink / raw)
To: Boaz Harrosh
Cc: Wang Sen, linux-scsi, JBottomley, stefanha, mc, linux-kernel,
stable
Il 25/07/2012 14:47, Boaz Harrosh ha scritto:
>
> NACK-by: Boaz Harrosh
>
>
> Apart from the HighMem pages problem, where in previous sg_set_buf()
> code was the marker copied? It was not because it is not needed because
> the allocation of sg took care of that. For example in 64bit the is no
> bugs, right?
>
> If there was a destination sg_list termination bug, it should be fixed
> as a separate patch from this "HighMem pages problem". But I bet if
> you inspect the code carefully there isn't such a bug.
Hmm, we're talking past each other. Let's sort it out in the v1 thread.
Sen, please hold up the patch for a moment.
Paolo
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] scsi: virtio-scsi: Fix address translation failure of HighMem pages used by sg list
2012-07-25 12:21 ` Ben Hutchings
@ 2012-07-26 9:20 ` Wang Sen
0 siblings, 0 replies; 6+ messages in thread
From: Wang Sen @ 2012-07-26 9:20 UTC (permalink / raw)
To: Ben Hutchings
Cc: Wang Sen, linux-scsi, JBottomley, pbonzini, stefanha, mc,
linux-kernel, stable
2012/7/25 Ben Hutchings <ben@decadent.org.uk>:
> On Wed, 2012-07-25 at 20:13 +0800, Wang Sen wrote:
>> When using the commands below to write some data to a virtio-scsi LUN of the
>> QEMU guest(32-bit) with 1G physical memory(qemu -m 1024), the qemu will crash.
>>
>> # sudo mkfs.ext4 /dev/sdb (/dev/sdb is the virtio-scsi LUN.)
>> # sudo mount /dev/sdb /mnt
>> # dd if=/dev/zero of=/mnt/file bs=1M count=1024
>>
>> In current implementation, sg_set_buf is called to add buffers to sg list which
>> is put into the virtqueue eventually. But if there are some HighMem pages in
>> table->sgl you can not get virtual address by sg_virt. So, sg_virt(sg_elem) may
>> return NULL value. This will cause QEMU exit when virtqueue_map_sg is called
>> in QEMU because an invalid GPA is passed by virtqueue.
>>
>> I take Paolo's solution mentioned in last thread to avoid failure on handling
>> flag bits.
>>
>> I have tested the patch on my workstation. QEMU would not crash any more.
>>
>> Signed-off-by: Wang Sen <senwang@linux.vnet.ibm.com>
> [...]
>
> This is not the correct way to submit a change for stable. See
> Documentation/stable_kernel_rules.txt.
OK, thanks.
>
> Ben.
>
> --
> Ben Hutchings
> If more than one person is responsible for a bug, no one is at fault.
--
------------------------------------------
Wang Sen
Addr: XUPT,Xi'an,Shaanxi,China
Email: kelvin.xupt@gmail.com
------------------------------------------
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-07-26 9:20 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-07-25 12:13 [PATCH v2] scsi: virtio-scsi: Fix address translation failure of HighMem pages used by sg list Wang Sen
2012-07-25 12:21 ` Ben Hutchings
2012-07-26 9:20 ` Wang Sen
2012-07-25 12:34 ` Paolo Bonzini
2012-07-25 12:47 ` Boaz Harrosh
2012-07-25 12:50 ` Paolo Bonzini
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox