diff mbox series

[PATCH-next,v4] loop: loop_set_status_from_info() check before assignment

Message ID 20230221095027.3656193-1-zhongjinghua@huaweicloud.com (mailing list archive)
State New, archived
Headers show
Series [PATCH-next,v4] loop: loop_set_status_from_info() check before assignment | expand

Commit Message

zhongjinghua Feb. 21, 2023, 9:50 a.m. UTC 
From: Zhong Jinghua <zhongjinghua@huawei.com>

In loop_set_status_from_info(), lo->lo_offset and lo->lo_sizelimit should
be checked before reassignment, because if an overflow error occurs, the
original correct value will be changed to the wrong value, and it will not
be changed back.

More, the original patch did not solve the problem, the value was set and
ioctl returned an error, but the subsequent io used the value in the loop
driver, which still caused an alarm:

loop_handle_cmd
 do_req_filebacked
  loff_t pos = ((loff_t) blk_rq_pos(rq) << 9) + lo->lo_offset;
  lo_rw_aio
   cmd->iocb.ki_pos = pos

Fixes: c490a0b5a4f3 ("loop: Check for overflow while configuring loop")
Signed-off-by: Zhong Jinghua <zhongjinghua@huawei.com>
---
 v2: Modify note: overflowing -> overflow
 v3: Modify commit message
 v4: Modify commit message
 drivers/block/loop.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Chaitanya Kulkarni Feb. 21, 2023, 5:59 p.m. UTC | #1 
On 2/21/2023 1:50 AM, Zhong Jinghua wrote:
> From: Zhong Jinghua <zhongjinghua@huawei.com>
> 
> In loop_set_status_from_info(), lo->lo_offset and lo->lo_sizelimit should
> be checked before reassignment, because if an overflow error occurs, the
> original correct value will be changed to the wrong value, and it will not
> be changed back.
> 
> More, the original patch did not solve the problem, the value was set and
> ioctl returned an error, but the subsequent io used the value in the loop
> driver, which still caused an alarm:
> 
> loop_handle_cmd
>   do_req_filebacked
>    loff_t pos = ((loff_t) blk_rq_pos(rq) << 9) + lo->lo_offset;
>    lo_rw_aio
>     cmd->iocb.ki_pos = pos
> 
> Fixes: c490a0b5a4f3 ("loop: Check for overflow while configuring loop")
> Signed-off-by: Zhong Jinghua <zhongjinghua@huawei.com>


Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>

-ck
Jens Axboe Feb. 23, 2023, 3:43 a.m. UTC | #2 
On Tue, 21 Feb 2023 17:50:27 +0800, Zhong Jinghua wrote:
> In loop_set_status_from_info(), lo->lo_offset and lo->lo_sizelimit should
> be checked before reassignment, because if an overflow error occurs, the
> original correct value will be changed to the wrong value, and it will not
> be changed back.
> 
> More, the original patch did not solve the problem, the value was set and
> ioctl returned an error, but the subsequent io used the value in the loop
> driver, which still caused an alarm:
> 
> [...]

Applied, thanks!

[1/1] loop: loop_set_status_from_info() check before assignment
      commit: 9f6ad5d533d1c71e51bdd06a5712c4fbc8768dfa

Best regards,
diff mbox series

Patch

diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 1518a6423279..1b35cbd029c7 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -977,13 +977,13 @@  loop_set_status_from_info(struct loop_device *lo,
 		return -EINVAL;
 	}
 
+	/* Avoid assigning overflow values */
+	if (info->lo_offset > LLONG_MAX || info->lo_sizelimit > LLONG_MAX)
+		return -EOVERFLOW;
+
 	lo->lo_offset = info->lo_offset;
 	lo->lo_sizelimit = info->lo_sizelimit;
 
-	/* loff_t vars have been assigned __u64 */
-	if (lo->lo_offset < 0 || lo->lo_sizelimit < 0)
-		return -EOVERFLOW;
-
 	memcpy(lo->lo_file_name, info->lo_file_name, LO_NAME_SIZE);
 	lo->lo_file_name[LO_NAME_SIZE-1] = 0;
 	lo->lo_flags = info->lo_flags;