1. Patchew
  2. QEMU
  3. View series

[PATCH v2] hw/scsi/scsi-disk: Fix out of bounds access in mode_sense_page()

Philippe Mathieu-Daudé posted 1 patch 3 years, 3 months ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20210204225041.1822673-1-philmd@redhat.com
hw/scsi/scsi-disk.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH v2] hw/scsi/scsi-disk: Fix out of bounds access in mode_sense_page()
Posted by Philippe Mathieu-Daudé 3 years, 3 months ago 
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
Re: [PATCH v2] hw/scsi/scsi-disk: Fix out of bounds access in mode_sense_page()
Posted by Li Qiang 3 years, 3 months ago 
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
>
Re: [PATCH v2] hw/scsi/scsi-disk: Fix out of bounds access in mode_sense_page()
Posted by Paolo Bonzini 3 years, 3 months ago 
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
Re: [PATCH v2] hw/scsi/scsi-disk: Fix out of bounds access in mode_sense_page()
Posted by Alexander Bulekov 3 years, 1 month ago 
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
>
Re: [PATCH-for-6.2 v2] hw/scsi/scsi-disk: Fix out of bounds access in mode_sense_page()
Posted by Philippe Mathieu-Daudé 2 years, 6 months ago 
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),
>
Re: [PATCH v2] hw/scsi/scsi-disk: Fix out of bounds access in mode_sense_page()
Posted by Alexander Bulekov 3 years, 3 months ago 
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
>

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.