[PULL 0/1] ufs queue

[PULL 0/1] ufs queue

[Jeuk Kim]

From: Jeuk Kim <jeuk20.kim@samsung.com>

The following changes since commit fd87be1dada5672f877e03c2ca8504458292c479:

  Merge tag 'accel-20240426' of https://github.com/philmd/qemu into staging (2024-04-26 15:28:13 -0700)

are available in the Git repository at:

  https://gitlab.com/jeuk20.kim/qemu.git tags/pull-ufs-20240429

for you to fetch changes up to f2c8aeb1afefcda92054c448b21fc59cdd99db30:

  hw/ufs: Fix buffer overflow bug (2024-04-29 12:13:35 +0900)

----------------------------------------------------------------
ufs queue

- Fix ufs sanitizer vulnerability

----------------------------------------------------------------
Jeuk Kim (1):
      hw/ufs: Fix buffer overflow bug

 hw/ufs/ufs.c | 8 ++++++++
 1 file changed, 8 insertions(+)

[PULL 1/1] hw/ufs: Fix buffer overflow bug

[Jeuk Kim]

From: Jeuk Kim <jeuk20.kim@samsung.com>

It fixes the buffer overflow vulnerability in the ufs device.
The bug was detected by sanitizers.

You can reproduce it by:

cat << EOF |\
qemu-system-x86_64 \
-display none -machine accel=qtest -m 512M -M q35 -nodefaults -drive \
file=null-co://,if=none,id=disk0 -device ufs,id=ufs_bus -device \
ufs-lu,drive=disk0,bus=ufs_bus -qtest stdio
outl 0xcf8 0x80000810
outl 0xcfc 0xe0000000
outl 0xcf8 0x80000804
outw 0xcfc 0x06
write 0xe0000058 0x1 0xa7
write 0xa 0x1 0x50
EOF

Resolves: #2299
Fixes: 329f16624499 ("hw/ufs: Support for Query Transfer Requests")
Reported-by: Zheyu Ma <zheyuma97@gmail.com>
Signed-off-by: Jeuk Kim <jeuk20.kim@samsung.com>
---
 hw/ufs/ufs.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/hw/ufs/ufs.c b/hw/ufs/ufs.c
index eccdb852a0..bac78a32bb 100644
--- a/hw/ufs/ufs.c
+++ b/hw/ufs/ufs.c
@@ -126,6 +126,10 @@ static MemTxResult ufs_dma_read_req_upiu(UfsRequest *req)
     copy_size = sizeof(UtpUpiuHeader) + UFS_TRANSACTION_SPECIFIC_FIELD_SIZE +
                 data_segment_length;
 
+    if (copy_size > sizeof(req->req_upiu)) {
+        copy_size = sizeof(req->req_upiu);
+    }
+
     ret = ufs_addr_read(u, req_upiu_base_addr, &req->req_upiu, copy_size);
     if (ret) {
         trace_ufs_err_dma_read_req_upiu(req->slot, req_upiu_base_addr);
@@ -225,6 +229,10 @@ static MemTxResult ufs_dma_write_rsp_upiu(UfsRequest *req)
         copy_size = rsp_upiu_byte_len;
     }
 
+    if (copy_size > sizeof(req->rsp_upiu)) {
+        copy_size = sizeof(req->rsp_upiu);
+    }
+
     ret = ufs_addr_write(u, rsp_upiu_base_addr, &req->rsp_upiu, copy_size);
     if (ret) {
         trace_ufs_err_dma_write_rsp_upiu(req->slot, rsp_upiu_base_addr);
-- 
2.34.1

Re: [PULL 1/1] hw/ufs: Fix buffer overflow bug

[Michael Tokarev]

29.04.2024 06:25, Jeuk Kim wrote:
> From: Jeuk Kim <jeuk20.kim@samsung.com>
> 
> It fixes the buffer overflow vulnerability in the ufs device.
> The bug was detected by sanitizers.
> 
...
> Resolves: #2299
> Fixes: 329f16624499 ("hw/ufs: Support for Query Transfer Requests")
> Reported-by: Zheyu Ma <zheyuma97@gmail.com>
> Signed-off-by: Jeuk Kim <jeuk20.kim@samsung.com>

Cc: qemu-stable@ for 8.2 and 9.0 series.

Please do not forget to Cc qemu-stable@ for relevant changes.

Thanks,

/mjt

Re: [PULL 0/1] ufs queue

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1241 bytes --]

On Mon, Apr 29, 2024 at 12:25:37PM +0900, Jeuk Kim wrote:
> From: Jeuk Kim <jeuk20.kim@samsung.com>
> 
> The following changes since commit fd87be1dada5672f877e03c2ca8504458292c479:
> 
>   Merge tag 'accel-20240426' of https://github.com/philmd/qemu into staging (2024-04-26 15:28:13 -0700)
> 
> are available in the Git repository at:
> 
>   https://gitlab.com/jeuk20.kim/qemu.git tags/pull-ufs-20240429
> 
> for you to fetch changes up to f2c8aeb1afefcda92054c448b21fc59cdd99db30:
> 
>   hw/ufs: Fix buffer overflow bug (2024-04-29 12:13:35 +0900)
> 
> ----------------------------------------------------------------
> ufs queue
> 
> - Fix ufs sanitizer vulnerability
> 
> ----------------------------------------------------------------
> Jeuk Kim (1):
>       hw/ufs: Fix buffer overflow bug
> 
>  hw/ufs/ufs.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 

Thanks, applied to my block tree:
https://gitlab.com/stefanha/qemu/commits/block

It will be included in my next block pull request.

You are welcome to send pull requests directly to the qemu.git/master
maintainer (Richard Henderson is on duty for this release cycle). If you
do that, make sure to GPG sign your pull request.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PULL 1/1] hw/ufs: Fix buffer overflow bug

[Stefan Hajnoczi]

From: Jeuk Kim <jeuk20.kim@samsung.com>

It fixes the buffer overflow vulnerability in the ufs device.
The bug was detected by sanitizers.

You can reproduce it by:

cat << EOF |\
qemu-system-x86_64 \
-display none -machine accel=qtest -m 512M -M q35 -nodefaults -drive \
file=null-co://,if=none,id=disk0 -device ufs,id=ufs_bus -device \
ufs-lu,drive=disk0,bus=ufs_bus -qtest stdio
outl 0xcf8 0x80000810
outl 0xcfc 0xe0000000
outl 0xcf8 0x80000804
outw 0xcfc 0x06
write 0xe0000058 0x1 0xa7
write 0xa 0x1 0x50
EOF

Resolves: #2299
Fixes: 329f16624499 ("hw/ufs: Support for Query Transfer Requests")
Reported-by: Zheyu Ma <zheyuma97@gmail.com>
Signed-off-by: Jeuk Kim <jeuk20.kim@samsung.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-ID: <f2c8aeb1afefcda92054c448b21fc59cdd99db30.1714360640.git.jeuk20.kim@samsung.com>
---
 hw/ufs/ufs.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/hw/ufs/ufs.c b/hw/ufs/ufs.c
index eccdb852a0..bac78a32bb 100644
--- a/hw/ufs/ufs.c
+++ b/hw/ufs/ufs.c
@@ -126,6 +126,10 @@ static MemTxResult ufs_dma_read_req_upiu(UfsRequest *req)
     copy_size = sizeof(UtpUpiuHeader) + UFS_TRANSACTION_SPECIFIC_FIELD_SIZE +
                 data_segment_length;
 
+    if (copy_size > sizeof(req->req_upiu)) {
+        copy_size = sizeof(req->req_upiu);
+    }
+
     ret = ufs_addr_read(u, req_upiu_base_addr, &req->req_upiu, copy_size);
     if (ret) {
         trace_ufs_err_dma_read_req_upiu(req->slot, req_upiu_base_addr);
@@ -225,6 +229,10 @@ static MemTxResult ufs_dma_write_rsp_upiu(UfsRequest *req)
         copy_size = rsp_upiu_byte_len;
     }
 
+    if (copy_size > sizeof(req->rsp_upiu)) {
+        copy_size = sizeof(req->rsp_upiu);
+    }
+
     ret = ufs_addr_write(u, rsp_upiu_base_addr, &req->rsp_upiu, copy_size);
     if (ret) {
         trace_ufs_err_dma_write_rsp_upiu(req->slot, rsp_upiu_base_addr);
-- 
2.44.0

Re: [PULL 0/1] ufs queue

[Richard Henderson]

On 4/29/24 06:41, Stefan Hajnoczi wrote:
> On Mon, Apr 29, 2024 at 12:25:37PM +0900, Jeuk Kim wrote:
>> From: Jeuk Kim <jeuk20.kim@samsung.com>
>>
>> The following changes since commit fd87be1dada5672f877e03c2ca8504458292c479:
>>
>>    Merge tag 'accel-20240426' of https://github.com/philmd/qemu into staging (2024-04-26 15:28:13 -0700)
>>
>> are available in the Git repository at:
>>
>>    https://gitlab.com/jeuk20.kim/qemu.git tags/pull-ufs-20240429
>>
>> for you to fetch changes up to f2c8aeb1afefcda92054c448b21fc59cdd99db30:
>>
>>    hw/ufs: Fix buffer overflow bug (2024-04-29 12:13:35 +0900)
>>
>> ----------------------------------------------------------------
>> ufs queue
>>
>> - Fix ufs sanitizer vulnerability
>>
>> ----------------------------------------------------------------
>> Jeuk Kim (1):
>>        hw/ufs: Fix buffer overflow bug
>>
>>   hw/ufs/ufs.c | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
> 
> Thanks, applied to my block tree:
> https://gitlab.com/stefanha/qemu/commits/block
> 
> It will be included in my next block pull request.
> 
> You are welcome to send pull requests directly to the qemu.git/master
> maintainer (Richard Henderson is on duty for this release cycle). If you
> do that, make sure to GPG sign your pull request.

He did. I have

     Merge tag 'pull-ufs-20240429' of https://gitlab.com/jeuk20.kim/qemu into staging

     ufs queue

     # -----BEGIN PGP SIGNATURE-----
     #
     # iQIzBAABCgAdFiEEUBfYMVl8eKPZB+73EuIgTA5dtgIFAmYvEScACgkQEuIgTA5d
     # tgL3Qg//R3IcISQqqDaJ/ySzKGmkyohJSc6ySLYvla4Aki7PV+um2Dx/XNS7uG2b
     # d3Qz4m6QaOKsocLfldRTn2FxVK238Rp5HNny5vc0kGRdwpR514B7aU0FhpT7qObS
     # wbbgRdDddIBIiCFLhtXtg5/TK2h32VxGrVI6llX4gmd2VzqM0e4xeG1Oj8rZseOY
     # SAgvDv68s1YwlO1p1vPvst/H+mUKYkqtPN1mjfCIn5tM6ss8kCLUnKjqGAg1BnSN
     # xwaGrqqOlzQK2+aV02eiItiow8evU/h+c9eiTnBo/EvBwjoBn6flNXABWXFENnmP
     # JjVIFeiNzSFhBPDzO23GXviuEt96j5lrcGYR48HYMZfEbJNpblXzWvEGMZWnXNgx
     # Q3cpcarZ4vSWIflR9OnCSQaGLA0Ny6YqLbmrM/oD+v67EITafKKc+flmiF7DBASB
     # fUoEsdffdA37LDtygJb7hfUhvPQWWAujmGzZ1cDP8Oa0MhT7aiD0Z/WqhhjVQbM0
     # iLiCDDD0cc0pmT3vw3EnEjKjnSkY3H62Q7pnYHiQgij4Ls/Rdd/P7OkSd0aI82t0
     # TooWGZJnyf8rjAzY2cEB1Twrhmhuyt9NnGxip9W8JsQBZMLabD2CahOm83zsk7jZ
     # 3fOONz6XrW2ttFkLZcRd4x4YjKONjEXsSX2ZrXTZ5t3USz/VNvY=
     # =Vwyi
     # -----END PGP SIGNATURE-----
     # gpg: Signature made Sun 28 Apr 2024 08:16:55 PM PDT
     # gpg:                using RSA key 5017D831597C78A3D907EEF712E2204C0E5DB602
     # gpg: Good signature from "Jeuk Kim <jeuk20.kim@samsung.com>" [unknown]
     # gpg:                 aka "Jeuk Kim <jeuk20.kim@gmail.com>" [unknown]
     # gpg: WARNING: This key is not certified with a trusted signature!
     # gpg:          There is no indication that the signature belongs to the owner.
     # Primary key fingerprint: 5017 D831 597C 78A3 D907  EEF7 12E2 204C 0E5D B602

queued for the next merge.


r~

Re: [PULL 1/1] hw/ufs: Fix buffer overflow bug

[Richard Henderson]

On 4/28/24 20:25, Jeuk Kim wrote:
> From: Jeuk Kim <jeuk20.kim@samsung.com>
> 
> It fixes the buffer overflow vulnerability in the ufs device.
> The bug was detected by sanitizers.
> 
> You can reproduce it by:
> 
> cat << EOF |\
> qemu-system-x86_64 \
> -display none -machine accel=qtest -m 512M -M q35 -nodefaults -drive \
> file=null-co://,if=none,id=disk0 -device ufs,id=ufs_bus -device \
> ufs-lu,drive=disk0,bus=ufs_bus -qtest stdio
> outl 0xcf8 0x80000810
> outl 0xcfc 0xe0000000
> outl 0xcf8 0x80000804
> outw 0xcfc 0x06
> write 0xe0000058 0x1 0xa7
> write 0xa 0x1 0x50
> EOF
> 
> Resolves: #2299
> Fixes: 329f16624499 ("hw/ufs: Support for Query Transfer Requests")
> Reported-by: Zheyu Ma <zheyuma97@gmail.com>
> Signed-off-by: Jeuk Kim <jeuk20.kim@samsung.com>
> ---
>   hw/ufs/ufs.c | 8 ++++++++
>   1 file changed, 8 insertions(+)

For some reason this appears to cause failures on s390x:

   https://gitlab.com/qemu-project/qemu/-/jobs/6740883283

All of the timeouts are new with this patch alone applied,
and go away when reverted.

I wasn't aware that these tests used ufs, but I have no
other explanation...


r~

Re: [PULL 1/1] hw/ufs: Fix buffer overflow bug

[Thomas Huth]

On 30/04/2024 02.17, Richard Henderson wrote:
> On 4/28/24 20:25, Jeuk Kim wrote:
>> From: Jeuk Kim <jeuk20.kim@samsung.com>
>>
>> It fixes the buffer overflow vulnerability in the ufs device.
>> The bug was detected by sanitizers.
>>
>> You can reproduce it by:
>>
>> cat << EOF |\
>> qemu-system-x86_64 \
>> -display none -machine accel=qtest -m 512M -M q35 -nodefaults -drive \
>> file=null-co://,if=none,id=disk0 -device ufs,id=ufs_bus -device \
>> ufs-lu,drive=disk0,bus=ufs_bus -qtest stdio
>> outl 0xcf8 0x80000810
>> outl 0xcfc 0xe0000000
>> outl 0xcf8 0x80000804
>> outw 0xcfc 0x06
>> write 0xe0000058 0x1 0xa7
>> write 0xa 0x1 0x50
>> EOF
>>
>> Resolves: #2299
>> Fixes: 329f16624499 ("hw/ufs: Support for Query Transfer Requests")
>> Reported-by: Zheyu Ma <zheyuma97@gmail.com>
>> Signed-off-by: Jeuk Kim <jeuk20.kim@samsung.com>
>> ---
>>   hw/ufs/ufs.c | 8 ++++++++
>>   1 file changed, 8 insertions(+)
> 
> For some reason this appears to cause failures on s390x:
> 
>    https://gitlab.com/qemu-project/qemu/-/jobs/6740883283
> 
> All of the timeouts are new with this patch alone applied,
> and go away when reverted.
> 
> I wasn't aware that these tests used ufs, but I have no
> other explanation...

I don't know for sure, but the test failure might instead be related to the 
problem that gets fixed by 
https://lore.kernel.org/qemu-devel/20240429075908.36302-1-thuth@redhat.com/ 
... I'm preparing a pull request for that fix right now, so maybe you could 
try this ufs pull request afterwards again to see whether the problem is fixed?

  Thomas

Re: [PULL 1/1] hw/ufs: Fix buffer overflow bug

[Thomas Huth]

On 30/04/2024 06.32, Thomas Huth wrote:
> On 30/04/2024 02.17, Richard Henderson wrote:
>> On 4/28/24 20:25, Jeuk Kim wrote:
>>> From: Jeuk Kim <jeuk20.kim@samsung.com>
>>>
>>> It fixes the buffer overflow vulnerability in the ufs device.
>>> The bug was detected by sanitizers.
>>>
>>> You can reproduce it by:
>>>
>>> cat << EOF |\
>>> qemu-system-x86_64 \
>>> -display none -machine accel=qtest -m 512M -M q35 -nodefaults -drive \
>>> file=null-co://,if=none,id=disk0 -device ufs,id=ufs_bus -device \
>>> ufs-lu,drive=disk0,bus=ufs_bus -qtest stdio
>>> outl 0xcf8 0x80000810
>>> outl 0xcfc 0xe0000000
>>> outl 0xcf8 0x80000804
>>> outw 0xcfc 0x06
>>> write 0xe0000058 0x1 0xa7
>>> write 0xa 0x1 0x50
>>> EOF
>>>
>>> Resolves: #2299
>>> Fixes: 329f16624499 ("hw/ufs: Support for Query Transfer Requests")
>>> Reported-by: Zheyu Ma <zheyuma97@gmail.com>
>>> Signed-off-by: Jeuk Kim <jeuk20.kim@samsung.com>
>>> ---
>>>   hw/ufs/ufs.c | 8 ++++++++
>>>   1 file changed, 8 insertions(+)
>>
>> For some reason this appears to cause failures on s390x:
>>
>>    https://gitlab.com/qemu-project/qemu/-/jobs/6740883283
>>
>> All of the timeouts are new with this patch alone applied,
>> and go away when reverted.
>>
>> I wasn't aware that these tests used ufs, but I have no
>> other explanation...
> 
> I don't know for sure, but the test failure might instead be related to the 
> problem that gets fixed by 
> https://lore.kernel.org/qemu-devel/20240429075908.36302-1-thuth@redhat.com/ 
> ... I'm preparing a pull request for that fix right now, so maybe you could 
> try this ufs pull request afterwards again to see whether the problem is fixed?

Hmm, thinking about it twice, it cannot be the reason: That bug affects 
aarch64/arm only, and in above CI run, some other targets were failing. So 
the problem must be something else, indeed.

  Thomas

Re: [PULL 0/1] ufs queue

[Richard Henderson]

On 4/28/24 20:25, Jeuk Kim wrote:
> From: Jeuk Kim<jeuk20.kim@samsung.com>
> 
> The following changes since commit fd87be1dada5672f877e03c2ca8504458292c479:
> 
>    Merge tag 'accel-20240426' ofhttps://github.com/philmd/qemu  into staging (2024-04-26 15:28:13 -0700)
> 
> are available in the Git repository at:
> 
>    https://gitlab.com/jeuk20.kim/qemu.git  tags/pull-ufs-20240429
> 
> for you to fetch changes up to f2c8aeb1afefcda92054c448b21fc59cdd99db30:
> 
>    hw/ufs: Fix buffer overflow bug (2024-04-29 12:13:35 +0900)
> 
> ----------------------------------------------------------------
> ufs queue
> 
> - Fix ufs sanitizer vulnerability

Applied, thanks.  Please update https://wiki.qemu.org/ChangeLog/9.1 as appropriate.


r~