[prev in list] [next in list] [prev in thread] [next in thread] 

List:       haiku-commits
Subject:    [haiku-commits] haiku: hrev56949 - in src/add-ons/kernel/drivers/disk: scsi/scsi_cd nvme virtual/vir
From:       waddlesplash <waddlesplash () gmail ! com>
Date:       2023-04-27 19:40:18
Message-ID: 20230427194018.A30DC3F9E1 () turing ! freelists ! org
[Download RAW message or body]

hrev56949 adds 3 changesets to branch 'master'
old head: fc12af740f936f90ab1dd2296caecd66f6bad349
new head: e0f07d3ce0c3d5608f19c57be7bbf296079a6134
overview: https://git.haiku-os.org/haiku/log/?qt=range&q=e0f07d3ce0c3+%5Efc12af740f93

----------------------------------------------------------------------------

8d2c997da265: nvme_disk: Add missing bounds check and adjust clamping.

f64b0991911a: scsi & virtio: Clean up IORequest usage.
  
   * Use TransferredBytes() instead of assuming length.
   * Consolidate checks and invoke io hook instead of scheduler directly.

e0f07d3ce0c3: IORequest: Add an assertion in SetTransferredBytes.

                              [ Augustin Cavalier <waddlesplash@gmail.com> ]

----------------------------------------------------------------------------

5 files changed, 43 insertions(+), 63 deletions(-)
.../kernel/drivers/disk/nvme/nvme_disk.cpp       | 20 ++++---
.../kernel/drivers/disk/scsi/scsi_cd/scsi_cd.cpp | 56 ++++++--------------
.../drivers/disk/scsi/scsi_disk/scsi_disk.cpp    | 14 +++--
.../disk/virtual/virtio_block/virtio_block.cpp   | 14 +++--
src/system/kernel/device_manager/IORequest.cpp   |  2 +

############################################################################

Commit:      8d2c997da2654166c00e74f2dd0ab98b9c5630b1
URL:         https://git.haiku-os.org/haiku/commit/?id=8d2c997da265
Author:      Augustin Cavalier <waddlesplash@gmail.com>
Date:        Thu Apr 27 19:12:14 2023 UTC

nvme_disk: Add missing bounds check and adjust clamping.

----------------------------------------------------------------------------

diff --git a/src/add-ons/kernel/drivers/disk/nvme/nvme_disk.cpp \
b/src/add-ons/kernel/drivers/disk/nvme/nvme_disk.cpp index 94ebe68507..dca857e91b \
                100644
--- a/src/add-ons/kernel/drivers/disk/nvme/nvme_disk.cpp
+++ b/src/add-ons/kernel/drivers/disk/nvme/nvme_disk.cpp
@@ -624,6 +624,10 @@ nvme_disk_io(void* cookie, io_request* request)
 
 	nvme_disk_handle* handle = (nvme_disk_handle*)cookie;
 
+	const off_t ns_end = (handle->info->capacity * handle->info->block_size);
+	if ((request->Offset() + (off_t)request->Length()) > ns_end)
+		return ERANGE;
+
 	nvme_io_request nvme_request;
 	memset(&nvme_request, 0, sizeof(nvme_io_request));
 
@@ -797,11 +801,11 @@ nvme_disk_read(void* cookie, off_t pos, void* buffer, size_t* \
length)  CALLED();
 	nvme_disk_handle* handle = (nvme_disk_handle*)cookie;
 
-	const off_t end = (handle->info->capacity * handle->info->block_size);
-	if (pos >= end)
+	const off_t ns_end = (handle->info->capacity * handle->info->block_size);
+	if (pos >= ns_end)
 		return B_BAD_VALUE;
-	if (pos + (off_t)*length > end)
-		*length = end - pos;
+	if ((pos + (off_t)*length) > ns_end)
+		*length = ns_end - pos;
 
 	IORequest request;
 	status_t status = request.Init(pos, (addr_t)buffer, *length, false, 0);
@@ -820,11 +824,11 @@ nvme_disk_write(void* cookie, off_t pos, const void* buffer, \
size_t* length)  CALLED();
 	nvme_disk_handle* handle = (nvme_disk_handle*)cookie;
 
-	const off_t end = (handle->info->capacity * handle->info->block_size);
-	if (pos >= end)
+	const off_t ns_end = (handle->info->capacity * handle->info->block_size);
+	if (pos >= ns_end)
 		return B_BAD_VALUE;
-	if (pos + (off_t)*length > end)
-		*length = end - pos;
+	if ((pos + (off_t)*length) > ns_end)
+		*length = ns_end - pos;
 
 	IORequest request;
 	status_t status = request.Init(pos, (addr_t)buffer, *length, true, 0);

############################################################################

Commit:      f64b0991911aa10b842a20054db137a481c07d12
URL:         https://git.haiku-os.org/haiku/commit/?id=f64b0991911a
Author:      Augustin Cavalier <waddlesplash@gmail.com>
Date:        Thu Apr 27 19:19:01 2023 UTC

scsi & virtio: Clean up IORequest usage.

 * Use TransferredBytes() instead of assuming length.
 * Consolidate checks and invoke io hook instead of scheduler directly.

----------------------------------------------------------------------------

diff --git a/src/add-ons/kernel/drivers/disk/scsi/scsi_cd/scsi_cd.cpp \
b/src/add-ons/kernel/drivers/disk/scsi/scsi_cd/scsi_cd.cpp index \
                1070f633f0..982e55d3ab 100644
--- a/src/add-ons/kernel/drivers/disk/scsi/scsi_cd/scsi_cd.cpp
+++ b/src/add-ons/kernel/drivers/disk/scsi/scsi_cd/scsi_cd.cpp
@@ -748,32 +748,35 @@ cd_free(void* cookie)
 
 
 static status_t
-cd_read(void* cookie, off_t pos, void* buffer, size_t* _length)
+cd_io(void* cookie, io_request* request)
 {
 	cd_handle* handle = (cd_handle*)cookie;
-	size_t length = *_length;
 
-	if (handle->info->capacity == 0)
+	if (handle->info->capacity == 0 || handle->info->io_scheduler == NULL) {
+		notify_io_request(request, B_DEV_NO_MEDIA);
 		return B_DEV_NO_MEDIA;
+	}
+
+	return handle->info->io_scheduler->ScheduleRequest(request);
+}
+
+
+static status_t
+cd_read(void* cookie, off_t pos, void* buffer, size_t* _length)
+{
+	size_t length = *_length;
 
 	IORequest request;
 	status_t status = request.Init(pos, (addr_t)buffer, length, false, 0);
 	if (status != B_OK)
 		return status;
 
-	if (handle->info->io_scheduler == NULL)
-		return B_DEV_NO_MEDIA;
-
-	status = handle->info->io_scheduler->ScheduleRequest(&request);
+	status = cd_io(cookie, &request);
 	if (status != B_OK)
 		return status;
 
 	status = request.Wait(0, 0);
-	if (status == B_OK)
-		*_length = length;
-	else
-		dprintf("cd_read(): request.Wait() returned: %s\n", strerror(status));
-
+	*_length = request.TransferredBytes();
 	return status;
 }
 
@@ -781,48 +784,23 @@ cd_read(void* cookie, off_t pos, void* buffer, size_t* _length)
 static status_t
 cd_write(void* cookie, off_t pos, const void* buffer, size_t* _length)
 {
-	cd_handle* handle = (cd_handle*)cookie;
 	size_t length = *_length;
 
-	if (handle->info->capacity == 0)
-		return B_DEV_NO_MEDIA;
-
 	IORequest request;
 	status_t status = request.Init(pos, (addr_t)buffer, length, true, 0);
 	if (status != B_OK)
 		return status;
 
-	if (handle->info->io_scheduler == NULL)
-		return B_DEV_NO_MEDIA;
-
-	status = handle->info->io_scheduler->ScheduleRequest(&request);
+	status = cd_io(cookie, &request);
 	if (status != B_OK)
 		return status;
 
 	status = request.Wait(0, 0);
-	if (status == B_OK)
-		*_length = length;
-	else
-		dprintf("cd_write(): request.Wait() returned: %s\n", strerror(status));
-
+	*_length = request.TransferredBytes();
 	return status;
 }
 
 
-static status_t
-cd_io(void* cookie, io_request* request)
-{
-	cd_handle* handle = (cd_handle*)cookie;
-
-	if (handle->info->capacity == 0) {
-		notify_io_request(request, B_DEV_NO_MEDIA);
-		return B_DEV_NO_MEDIA;
-	}
-
-	return handle->info->io_scheduler->ScheduleRequest(request);
-}
-
-
 static status_t
 cd_ioctl(void* cookie, uint32 op, void* buffer, size_t length)
 {
diff --git a/src/add-ons/kernel/drivers/disk/scsi/scsi_disk/scsi_disk.cpp \
b/src/add-ons/kernel/drivers/disk/scsi/scsi_disk/scsi_disk.cpp index \
                d79a68e9ec..09dfc03c59 100644
--- a/src/add-ons/kernel/drivers/disk/scsi/scsi_disk/scsi_disk.cpp
+++ b/src/add-ons/kernel/drivers/disk/scsi/scsi_disk/scsi_disk.cpp
@@ -341,10 +341,9 @@ das_read(void* cookie, off_t pos, void* buffer, size_t* _length)
 		return status;
 
 	status = request.Wait(0, 0);
-	if (status == B_OK)
-		*_length = length;
-	else
-		dprintf("das_read(): request.Wait() returned: %s\n", strerror(status));
+	*_length = request.TransferredBytes();
+	if (status != B_OK)
+		dprintf("das_read: request.Wait() returned: %s\n", strerror(status));
 
 	return status;
 }
@@ -366,10 +365,9 @@ das_write(void* cookie, off_t pos, const void* buffer, size_t* \
_length)  return status;
 
 	status = request.Wait(0, 0);
-	if (status == B_OK)
-		*_length = length;
-	else
-		dprintf("das_write(): request.Wait() returned: %s\n", strerror(status));
+	*_length = request.TransferredBytes();
+	if (status != B_OK)
+		dprintf("das_write: request.Wait() returned: %s\n", strerror(status));
 
 	return status;
 }
diff --git a/src/add-ons/kernel/drivers/disk/virtual/virtio_block/virtio_block.cpp \
b/src/add-ons/kernel/drivers/disk/virtual/virtio_block/virtio_block.cpp index \
                f42f1d1dea..802318850a 100644
--- a/src/add-ons/kernel/drivers/disk/virtual/virtio_block/virtio_block.cpp
+++ b/src/add-ons/kernel/drivers/disk/virtual/virtio_block/virtio_block.cpp
@@ -352,10 +352,9 @@ virtio_block_read(void* cookie, off_t pos, void* buffer, size_t* \
_length)  return status;
 
 	status = request.Wait(0, 0);
-	if (status == B_OK)
-		*_length = length;
-	else
-		dprintf("read(): request.Wait() returned: %s\n", strerror(status));
+	*_length = request.TransferredBytes();
+	if (status != B_OK)
+		dprintf("virtio_block_read: request.Wait() returned: %s\n", strerror(status));
 
 	return status;
 }
@@ -379,10 +378,9 @@ virtio_block_write(void* cookie, off_t pos, const void* buffer,
 		return status;
 
 	status = request.Wait(0, 0);
-	if (status == B_OK)
-		*_length = length;
-	else
-		dprintf("write(): request.Wait() returned: %s\n", strerror(status));
+	*_length = request.TransferredBytes();
+	if (status != B_OK)
+		dprintf("virtio_block_write: request.Wait() returned: %s\n", strerror(status));
 
 	return status;
 }

############################################################################

Revision:    hrev56949
Commit:      e0f07d3ce0c3d5608f19c57be7bbf296079a6134
URL:         https://git.haiku-os.org/haiku/commit/?id=e0f07d3ce0c3
Author:      Augustin Cavalier <waddlesplash@gmail.com>
Date:        Thu Apr 27 19:20:17 2023 UTC

IORequest: Add an assertion in SetTransferredBytes.

----------------------------------------------------------------------------

diff --git a/src/system/kernel/device_manager/IORequest.cpp \
b/src/system/kernel/device_manager/IORequest.cpp index 9c38a0a104..eb93c1c5c8 100644
--- a/src/system/kernel/device_manager/IORequest.cpp
+++ b/src/system/kernel/device_manager/IORequest.cpp
@@ -1093,6 +1093,8 @@ IORequest::SetTransferredBytes(bool partialTransfer,
 
 	MutexLocker _(fLock);
 
+	ASSERT(transferredBytes <= fLength);
+
 	fPartialTransfer = partialTransfer;
 	fTransferSize = transferredBytes;
 }


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic