* Re: [PATCH v2] hw/scsi/scsi-disk: Fix out of bounds access in mode_sense_page()
2021-02-04 22:50 [PATCH v2] hw/scsi/scsi-disk: Fix out of bounds access in mode_sense_page() Philippe Mathieu-Daudé
@ 2021-02-05 4:59 ` Li Qiang
2021-02-05 16:02 ` Alexander Bulekov
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Li Qiang @ 2021-02-05 4:59 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Fam Zheng, Alexander Bulekov, qemu-stable, Qemu Developers,
Darren Kenny, Paolo Bonzini
Philippe Mathieu-Daudé <philmd@redhat.com> 于2021年2月5日周五 上午6:50写道：
>
> Per the "SCSI Commands Reference Manual" (Rev. J) chapter 5.3
> "Mode parameters" and table 359 "Mode page codes and subpage
> codes", the last page code is 0x3f. When using it as array index,
> the array must have 0x40 elements. Replace the magic 0x3f value
> by its definition and increase the size of the mode_sense_valid[]
> to avoid an out of bound access (reproducer available in [Buglink]):
>
> hw/scsi/scsi-disk.c:1104:10: runtime error: index 63 out of bounds for type 'const int [63]'
> SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior hw/scsi/scsi-disk.c:1104:10 in
> =================================================================
> ==1813911==ERROR: AddressSanitizer: global-buffer-overflow
> READ of size 4 at 0x5624b84aff1c thread T0
> #0 0x5624b63004b3 in mode_sense_page hw/scsi/scsi-disk.c:1104:10
> #1 0x5624b630f798 in scsi_disk_check_mode_select hw/scsi/scsi-disk.c:1447:11
> #2 0x5624b630efb8 in mode_select_pages hw/scsi/scsi-disk.c:1515:17
> #3 0x5624b630988e in scsi_disk_emulate_mode_select hw/scsi/scsi-disk.c:1570:13
> #4 0x5624b62f08e7 in scsi_disk_emulate_write_data hw/scsi/scsi-disk.c:1861:9
> #5 0x5624b62b171b in scsi_req_continue hw/scsi/scsi-bus.c:1391:9
> #6 0x5624b62b2d4c in scsi_req_data hw/scsi/scsi-bus.c:1427:5
> #7 0x5624b62f05f6 in scsi_disk_emulate_write_data hw/scsi/scsi-disk.c:1853:9
> #8 0x5624b62b171b in scsi_req_continue hw/scsi/scsi-bus.c:1391:9
> #9 0x5624b63e47ed in megasas_enqueue_req hw/scsi/megasas.c:1660:9
> #10 0x5624b63b9cfa in megasas_handle_scsi hw/scsi/megasas.c:1735:5
> #11 0x5624b63acf91 in megasas_handle_frame hw/scsi/megasas.c:1974:24
> #12 0x5624b63aa200 in megasas_mmio_write hw/scsi/megasas.c:2131:9
> #13 0x5624b63ebed3 in megasas_port_write hw/scsi/megasas.c:2182:5
> #14 0x5624b6f43568 in memory_region_write_accessor softmmu/memory.c:491:5
>
> Cc: qemu-stable@nongnu.org
> Reported-by: OSS-Fuzz
> Reported-by: Alexander Bulekov <alxndr@bu.edu>
> Buglink: https://bugs.launchpad.net/qemu/+bug/1914638
> Fixes: a8f4bbe2900 ("scsi-disk: store valid mode pages in a table")
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Li Qiang <liq3ea@gmail.com>
> ---
> v2: Mention reproducer link
> ---
> hw/scsi/scsi-disk.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
> index ed52fcd49ff..93aec483e88 100644
> --- a/hw/scsi/scsi-disk.c
> +++ b/hw/scsi/scsi-disk.c
> @@ -1089,7 +1089,7 @@ static int scsi_emulate_mechanism_status(SCSIDiskState *s, uint8_t *outbuf)
> static int mode_sense_page(SCSIDiskState *s, int page, uint8_t **p_outbuf,
> int page_control)
> {
> - static const int mode_sense_valid[0x3f] = {
> + static const int mode_sense_valid[MODE_PAGE_ALLS + 1] = {
> [MODE_PAGE_HD_GEOMETRY] = (1 << TYPE_DISK),
> [MODE_PAGE_FLEXIBLE_DISK_GEOMETRY] = (1 << TYPE_DISK),
> [MODE_PAGE_CACHING] = (1 << TYPE_DISK) | (1 << TYPE_ROM),
> --
> 2.26.2
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] hw/scsi/scsi-disk: Fix out of bounds access in mode_sense_page()
2021-02-04 22:50 [PATCH v2] hw/scsi/scsi-disk: Fix out of bounds access in mode_sense_page() Philippe Mathieu-Daudé
2021-02-05 4:59 ` Li Qiang
@ 2021-02-05 16:02 ` Alexander Bulekov
2021-02-05 17:27 ` Paolo Bonzini
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Alexander Bulekov @ 2021-02-05 16:02 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Fam Zheng, Li Qiang, qemu-stable, qemu-devel, Darren Kenny,
Paolo Bonzini
On 210204 2350, Philippe Mathieu-Daudé wrote:
> Per the "SCSI Commands Reference Manual" (Rev. J) chapter 5.3
> "Mode parameters" and table 359 "Mode page codes and subpage
> codes", the last page code is 0x3f. When using it as array index,
> the array must have 0x40 elements. Replace the magic 0x3f value
> by its definition and increase the size of the mode_sense_valid[]
> to avoid an out of bound access (reproducer available in [Buglink]):
>
> hw/scsi/scsi-disk.c:1104:10: runtime error: index 63 out of bounds for type 'const int [63]'
> SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior hw/scsi/scsi-disk.c:1104:10 in
> =================================================================
> ==1813911==ERROR: AddressSanitizer: global-buffer-overflow
> READ of size 4 at 0x5624b84aff1c thread T0
> #0 0x5624b63004b3 in mode_sense_page hw/scsi/scsi-disk.c:1104:10
> #1 0x5624b630f798 in scsi_disk_check_mode_select hw/scsi/scsi-disk.c:1447:11
> #2 0x5624b630efb8 in mode_select_pages hw/scsi/scsi-disk.c:1515:17
> #3 0x5624b630988e in scsi_disk_emulate_mode_select hw/scsi/scsi-disk.c:1570:13
> #4 0x5624b62f08e7 in scsi_disk_emulate_write_data hw/scsi/scsi-disk.c:1861:9
> #5 0x5624b62b171b in scsi_req_continue hw/scsi/scsi-bus.c:1391:9
> #6 0x5624b62b2d4c in scsi_req_data hw/scsi/scsi-bus.c:1427:5
> #7 0x5624b62f05f6 in scsi_disk_emulate_write_data hw/scsi/scsi-disk.c:1853:9
> #8 0x5624b62b171b in scsi_req_continue hw/scsi/scsi-bus.c:1391:9
> #9 0x5624b63e47ed in megasas_enqueue_req hw/scsi/megasas.c:1660:9
> #10 0x5624b63b9cfa in megasas_handle_scsi hw/scsi/megasas.c:1735:5
> #11 0x5624b63acf91 in megasas_handle_frame hw/scsi/megasas.c:1974:24
> #12 0x5624b63aa200 in megasas_mmio_write hw/scsi/megasas.c:2131:9
> #13 0x5624b63ebed3 in megasas_port_write hw/scsi/megasas.c:2182:5
> #14 0x5624b6f43568 in memory_region_write_accessor softmmu/memory.c:491:5
>
> Cc: qemu-stable@nongnu.org
> Reported-by: OSS-Fuzz
> Reported-by: Alexander Bulekov <alxndr@bu.edu>
> Buglink: https://bugs.launchpad.net/qemu/+bug/1914638
> Fixes: a8f4bbe2900 ("scsi-disk: store valid mode pages in a table")
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Alexander Bulekov <alxndr@bu.edu>
> ---
> v2: Mention reproducer link
> ---
> hw/scsi/scsi-disk.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
> index ed52fcd49ff..93aec483e88 100644
> --- a/hw/scsi/scsi-disk.c
> +++ b/hw/scsi/scsi-disk.c
> @@ -1089,7 +1089,7 @@ static int scsi_emulate_mechanism_status(SCSIDiskState *s, uint8_t *outbuf)
> static int mode_sense_page(SCSIDiskState *s, int page, uint8_t **p_outbuf,
> int page_control)
> {
> - static const int mode_sense_valid[0x3f] = {
> + static const int mode_sense_valid[MODE_PAGE_ALLS + 1] = {
> [MODE_PAGE_HD_GEOMETRY] = (1 << TYPE_DISK),
> [MODE_PAGE_FLEXIBLE_DISK_GEOMETRY] = (1 << TYPE_DISK),
> [MODE_PAGE_CACHING] = (1 << TYPE_DISK) | (1 << TYPE_ROM),
> --
> 2.26.2
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] hw/scsi/scsi-disk: Fix out of bounds access in mode_sense_page()
2021-02-04 22:50 [PATCH v2] hw/scsi/scsi-disk: Fix out of bounds access in mode_sense_page() Philippe Mathieu-Daudé
2021-02-05 4:59 ` Li Qiang
2021-02-05 16:02 ` Alexander Bulekov
@ 2021-02-05 17:27 ` Paolo Bonzini
2021-04-01 23:50 ` Alexander Bulekov
2021-11-05 11:35 ` [PATCH-for-6.2 " Philippe Mathieu-Daudé
4 siblings, 0 replies; 6+ messages in thread
From: Paolo Bonzini @ 2021-02-05 17:27 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Fam Zheng, Darren Kenny, Li Qiang, qemu-stable, Alexander Bulekov
On 04/02/21 23:50, Philippe Mathieu-Daudé wrote:
> diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
> index ed52fcd49ff..93aec483e88 100644
> --- a/hw/scsi/scsi-disk.c
> +++ b/hw/scsi/scsi-disk.c
> @@ -1089,7 +1089,7 @@ static int scsi_emulate_mechanism_status(SCSIDiskState *s, uint8_t *outbuf)
> static int mode_sense_page(SCSIDiskState *s, int page, uint8_t **p_outbuf,
> int page_control)
> {
> - static const int mode_sense_valid[0x3f] = {
> + static const int mode_sense_valid[MODE_PAGE_ALLS + 1] = {
> [MODE_PAGE_HD_GEOMETRY] = (1 << TYPE_DISK),
> [MODE_PAGE_FLEXIBLE_DISK_GEOMETRY] = (1 << TYPE_DISK),
> [MODE_PAGE_CACHING] = (1 << TYPE_DISK) | (1 << TYPE_ROM),
>
The bug is really that mode select with page 0x3f should fail, but it's
okay too. Can you also write a testcase along the lines of
test_unaligned_write_same?
Paolo
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] hw/scsi/scsi-disk: Fix out of bounds access in mode_sense_page()
2021-02-04 22:50 [PATCH v2] hw/scsi/scsi-disk: Fix out of bounds access in mode_sense_page() Philippe Mathieu-Daudé
` (2 preceding siblings ...)
2021-02-05 17:27 ` Paolo Bonzini
@ 2021-04-01 23:50 ` Alexander Bulekov
2021-11-05 11:35 ` [PATCH-for-6.2 " Philippe Mathieu-Daudé
4 siblings, 0 replies; 6+ messages in thread
From: Alexander Bulekov @ 2021-04-01 23:50 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Fam Zheng, Li Qiang, qemu-stable, qemu-devel, Darren Kenny,
Paolo Bonzini
By the way, this is also triggerable from the am53c974 device..
Should I provide a reproducer patch for this one?
On 210204 2350, Philippe Mathieu-Daudé wrote:
> Per the "SCSI Commands Reference Manual" (Rev. J) chapter 5.3
> "Mode parameters" and table 359 "Mode page codes and subpage
> codes", the last page code is 0x3f. When using it as array index,
> the array must have 0x40 elements. Replace the magic 0x3f value
> by its definition and increase the size of the mode_sense_valid[]
> to avoid an out of bound access (reproducer available in [Buglink]):
>
> hw/scsi/scsi-disk.c:1104:10: runtime error: index 63 out of bounds for type 'const int [63]'
> SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior hw/scsi/scsi-disk.c:1104:10 in
> =================================================================
> ==1813911==ERROR: AddressSanitizer: global-buffer-overflow
> READ of size 4 at 0x5624b84aff1c thread T0
> #0 0x5624b63004b3 in mode_sense_page hw/scsi/scsi-disk.c:1104:10
> #1 0x5624b630f798 in scsi_disk_check_mode_select hw/scsi/scsi-disk.c:1447:11
> #2 0x5624b630efb8 in mode_select_pages hw/scsi/scsi-disk.c:1515:17
> #3 0x5624b630988e in scsi_disk_emulate_mode_select hw/scsi/scsi-disk.c:1570:13
> #4 0x5624b62f08e7 in scsi_disk_emulate_write_data hw/scsi/scsi-disk.c:1861:9
> #5 0x5624b62b171b in scsi_req_continue hw/scsi/scsi-bus.c:1391:9
> #6 0x5624b62b2d4c in scsi_req_data hw/scsi/scsi-bus.c:1427:5
> #7 0x5624b62f05f6 in scsi_disk_emulate_write_data hw/scsi/scsi-disk.c:1853:9
> #8 0x5624b62b171b in scsi_req_continue hw/scsi/scsi-bus.c:1391:9
> #9 0x5624b63e47ed in megasas_enqueue_req hw/scsi/megasas.c:1660:9
> #10 0x5624b63b9cfa in megasas_handle_scsi hw/scsi/megasas.c:1735:5
> #11 0x5624b63acf91 in megasas_handle_frame hw/scsi/megasas.c:1974:24
> #12 0x5624b63aa200 in megasas_mmio_write hw/scsi/megasas.c:2131:9
> #13 0x5624b63ebed3 in megasas_port_write hw/scsi/megasas.c:2182:5
> #14 0x5624b6f43568 in memory_region_write_accessor softmmu/memory.c:491:5
>
> Cc: qemu-stable@nongnu.org
> Reported-by: OSS-Fuzz
> Reported-by: Alexander Bulekov <alxndr@bu.edu>
> Buglink: https://bugs.launchpad.net/qemu/+bug/1914638
> Fixes: a8f4bbe2900 ("scsi-disk: store valid mode pages in a table")
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> v2: Mention reproducer link
> ---
> hw/scsi/scsi-disk.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
> index ed52fcd49ff..93aec483e88 100644
> --- a/hw/scsi/scsi-disk.c
> +++ b/hw/scsi/scsi-disk.c
> @@ -1089,7 +1089,7 @@ static int scsi_emulate_mechanism_status(SCSIDiskState *s, uint8_t *outbuf)
> static int mode_sense_page(SCSIDiskState *s, int page, uint8_t **p_outbuf,
> int page_control)
> {
> - static const int mode_sense_valid[0x3f] = {
> + static const int mode_sense_valid[MODE_PAGE_ALLS + 1] = {
> [MODE_PAGE_HD_GEOMETRY] = (1 << TYPE_DISK),
> [MODE_PAGE_FLEXIBLE_DISK_GEOMETRY] = (1 << TYPE_DISK),
> [MODE_PAGE_CACHING] = (1 << TYPE_DISK) | (1 << TYPE_ROM),
> --
> 2.26.2
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH-for-6.2 v2] hw/scsi/scsi-disk: Fix out of bounds access in mode_sense_page()
2021-02-04 22:50 [PATCH v2] hw/scsi/scsi-disk: Fix out of bounds access in mode_sense_page() Philippe Mathieu-Daudé
` (3 preceding siblings ...)
2021-04-01 23:50 ` Alexander Bulekov
@ 2021-11-05 11:35 ` Philippe Mathieu-Daudé
4 siblings, 0 replies; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-11-05 11:35 UTC (permalink / raw)
To: Paolo Bonzini, Mauro Matteo Cascella, Alexander Bulekov
Cc: Fam Zheng, Darren Kenny, Li Qiang, qemu-stable, qemu-devel
It seems this became CVE-2021-3930.
https://bugzilla.redhat.com/show_bug.cgi?id=2020588#c0
On 2/4/21 23:50, Philippe Mathieu-Daudé wrote:
> Per the "SCSI Commands Reference Manual" (Rev. J) chapter 5.3
> "Mode parameters" and table 359 "Mode page codes and subpage
> codes", the last page code is 0x3f. When using it as array index,
> the array must have 0x40 elements. Replace the magic 0x3f value
> by its definition and increase the size of the mode_sense_valid[]
> to avoid an out of bound access (reproducer available in [Buglink]):
>
> hw/scsi/scsi-disk.c:1104:10: runtime error: index 63 out of bounds for type 'const int [63]'
> SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior hw/scsi/scsi-disk.c:1104:10 in
> =================================================================
> ==1813911==ERROR: AddressSanitizer: global-buffer-overflow
> READ of size 4 at 0x5624b84aff1c thread T0
> #0 0x5624b63004b3 in mode_sense_page hw/scsi/scsi-disk.c:1104:10
> #1 0x5624b630f798 in scsi_disk_check_mode_select hw/scsi/scsi-disk.c:1447:11
> #2 0x5624b630efb8 in mode_select_pages hw/scsi/scsi-disk.c:1515:17
> #3 0x5624b630988e in scsi_disk_emulate_mode_select hw/scsi/scsi-disk.c:1570:13
> #4 0x5624b62f08e7 in scsi_disk_emulate_write_data hw/scsi/scsi-disk.c:1861:9
> #5 0x5624b62b171b in scsi_req_continue hw/scsi/scsi-bus.c:1391:9
> #6 0x5624b62b2d4c in scsi_req_data hw/scsi/scsi-bus.c:1427:5
> #7 0x5624b62f05f6 in scsi_disk_emulate_write_data hw/scsi/scsi-disk.c:1853:9
> #8 0x5624b62b171b in scsi_req_continue hw/scsi/scsi-bus.c:1391:9
> #9 0x5624b63e47ed in megasas_enqueue_req hw/scsi/megasas.c:1660:9
> #10 0x5624b63b9cfa in megasas_handle_scsi hw/scsi/megasas.c:1735:5
> #11 0x5624b63acf91 in megasas_handle_frame hw/scsi/megasas.c:1974:24
> #12 0x5624b63aa200 in megasas_mmio_write hw/scsi/megasas.c:2131:9
> #13 0x5624b63ebed3 in megasas_port_write hw/scsi/megasas.c:2182:5
> #14 0x5624b6f43568 in memory_region_write_accessor softmmu/memory.c:491:5
>
> Cc: qemu-stable@nongnu.org
> Reported-by: OSS-Fuzz
> Reported-by: Alexander Bulekov <alxndr@bu.edu>
> Buglink: https://bugs.launchpad.net/qemu/+bug/1914638
> Fixes: a8f4bbe2900 ("scsi-disk: store valid mode pages in a table")
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> v2: Mention reproducer link
> ---
> hw/scsi/scsi-disk.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
> index ed52fcd49ff..93aec483e88 100644
> --- a/hw/scsi/scsi-disk.c
> +++ b/hw/scsi/scsi-disk.c
> @@ -1089,7 +1089,7 @@ static int scsi_emulate_mechanism_status(SCSIDiskState *s, uint8_t *outbuf)
> static int mode_sense_page(SCSIDiskState *s, int page, uint8_t **p_outbuf,
> int page_control)
> {
> - static const int mode_sense_valid[0x3f] = {
> + static const int mode_sense_valid[MODE_PAGE_ALLS + 1] = {
> [MODE_PAGE_HD_GEOMETRY] = (1 << TYPE_DISK),
> [MODE_PAGE_FLEXIBLE_DISK_GEOMETRY] = (1 << TYPE_DISK),
> [MODE_PAGE_CACHING] = (1 << TYPE_DISK) | (1 << TYPE_ROM),
>
^ permalink raw reply [flat|nested] 6+ messages in thread