Re: Understanding max_sectors

[Roland Dreier <roland@xxxxxxxxxxxxxxx>]

On Wed, Mar 21, 2012 at 4:18 AM, Stefan Gourguis <brockz@xxxxxxxxxxxx> wrote:
> And also my relevant demsg output:
> [ 1936.317399] Rounding down aligned max_sectors from 4294967295 to 8388600

Looks like a bug exposed with hch's change to iblock in c1775c389556:

-       limits->max_hw_sectors = queue_max_hw_sectors(q);
-       limits->max_sectors = queue_max_sectors(q);
+       limits->max_hw_sectors = UINT_MAX;
+       limits->max_sectors = UINT_MAX;

now the code in target_core_device.c is broken:

u32 se_dev_align_max_sectors(u32 max_sectors, u32 block_size)
{
        u32 tmp, aligned_max_sectors;
        /*
         * Limit max_sectors to a PAGE_SIZE aligned value for modern
         * transport_allocate_data_tasks() operation.
         */
        tmp = rounddown((max_sectors * block_size), PAGE_SIZE);
--> max_sectors is UINT_MAX, so max_sectors * block_size overflows.

        aligned_max_sectors = (tmp / block_size);
        if (max_sectors != aligned_max_sectors) {
                printk(KERN_INFO "Rounding down aligned max_sectors from %u"
                                " to %u\n", max_sectors, aligned_max_sectors);
                return aligned_max_sectors;
        }

        return max_sectors;
}

I think the attached patch (compile tested only, sorry...) should fix this.

From 52326b0b4dc09143ed8b79f6137c5bc27e597427 Mon Sep 17 00:00:00 2001
From: Roland Dreier <roland@xxxxxxxxxxxxxxx>
Date: Wed, 21 Mar 2012 10:08:21 -0700
Subject: [PATCH] target: Avoid integer overflow in se_dev_align_max_sectors()

Computing max_sectors * block_size might overflow 32 bits if
max_sectors is big.  Fix this by rounding down directly without
needing that intermediate value.

Signed-off-by: Roland Dreier <roland@xxxxxxxxxxxxxxx>
---
 drivers/target/target_core_device.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c
index 755035f..67fd6e4 100644
--- a/drivers/target/target_core_device.c
+++ b/drivers/target/target_core_device.c
@@ -827,13 +827,12 @@ int se_dev_check_shutdown(struct se_device *dev)
 
 u32 se_dev_align_max_sectors(u32 max_sectors, u32 block_size)
 {
-	u32 tmp, aligned_max_sectors;
+	u32 aligned_max_sectors;
 	/*
 	 * Limit max_sectors to a PAGE_SIZE aligned value for modern
 	 * transport_allocate_data_tasks() operation.
 	 */
-	tmp = rounddown((max_sectors * block_size), PAGE_SIZE);
-	aligned_max_sectors = (tmp / block_size);
+	aligned_max_sectors = rounddown(max_sectors, PAGE_SIZE / block_size);
 	if (max_sectors != aligned_max_sectors) {
 		printk(KERN_INFO "Rounding down aligned max_sectors from %u"
 				" to %u\n", max_sectors, aligned_max_sectors);
-- 
1.7.9.1