[PATCH] scsi: reject negative SG_SET_TIMEOUT values

[PATCH] scsi: reject negative SG_SET_TIMEOUT values

[Wang Yingjie]

From: Yingjie Wang <x-professor@outlook.com>

SG_SET_TIMEOUT reads its argument as a signed int in USER_HZ units. The
sg character device rejects negative values, while the block SCSI ioctl
and BSG paths pass them to clock_t_to_jiffies(), which takes an unsigned
long.

On a 64-bit system with HZ=1000, passing -5 first converts it to a large
unsigned value, causing clock_t_to_jiffies() to return ULONG_MAX.
Storing that result in sdev->sg_timeout or bd->timeout truncates it to
UINT_MAX. SG_GET_TIMEOUT then reports 429496729 ticks, and an SG_IO
request with a zero per-request timeout inherits a timeout of about
49.7 days.

Reject negative values in both paths with -EIO, matching the
longstanding behavior of the sg character device.

The block SCSI path predates Git history. The BSG path has exposed the
same behavior since BSG was introduced.

Fixes: 3d6392cfbd7d ("bsg: support for full generic block layer SG v3")
Cc: stable@vger.kernel.org
Signed-off-by: Yingjie Wang <x-professor@outlook.com>
---
 block/bsg.c               | 2 ++
 drivers/scsi/scsi_ioctl.c | 9 ++++++---
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/block/bsg.c b/block/bsg.c
index 82aaf3cee582..681da79a5ba2 100644
--- a/block/bsg.c
+++ b/block/bsg.c
@@ -132,6 +132,8 @@ static long bsg_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 	case SG_SET_TIMEOUT:
 		if (get_user(val, intp))
 			return -EFAULT;
+		if (val < 0)
+			return -EIO;
 		bd->timeout = clock_t_to_jiffies(val);
 		return 0;
 	case SG_GET_TIMEOUT:
diff --git a/drivers/scsi/scsi_ioctl.c b/drivers/scsi/scsi_ioctl.c
index c14f81403a09..162a21112d49 100644
--- a/drivers/scsi/scsi_ioctl.c
+++ b/drivers/scsi/scsi_ioctl.c
@@ -195,10 +195,13 @@ static int sg_set_timeout(struct scsi_device *sdev, int __user *p)
 {
 	int timeout, err = get_user(timeout, p);
 
-	if (!err)
-		sdev->sg_timeout = clock_t_to_jiffies(timeout);
+	if (err)
+		return err;
+	if (timeout < 0)
+		return -EIO;
 
-	return err;
+	sdev->sg_timeout = clock_t_to_jiffies(timeout);
+	return 0;
 }
 
 static int sg_get_reserved_size(struct scsi_device *sdev, int __user *p)

base-commit: 58161ead29e50d70a317ea26b472e0c7508e69da
-- 
2.43.0

Re: [PATCH] scsi: reject negative SG_SET_TIMEOUT values

[Bart Van Assche]

On 10/17/26 1:45 AM, Wang Yingjie wrote:
> The block SCSI path predates Git history. The BSG path has exposed the
> same behavior since BSG was introduced.
> 
> Fixes: 3d6392cfbd7d ("bsg: support for full generic block layer SG v3")

As mentioned above this issue is older than when Linus started using git
so I think it would be appropriate to add the following:

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")

> diff --git a/block/bsg.c b/block/bsg.c
> index 82aaf3cee582..681da79a5ba2 100644
> --- a/block/bsg.c
> +++ b/block/bsg.c
> @@ -132,6 +132,8 @@ static long bsg_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
>   	case SG_SET_TIMEOUT:
>   		if (get_user(val, intp))
>   			return -EFAULT;
> +		if (val < 0)
> +			return -EIO;
>   		bd->timeout = clock_t_to_jiffies(val);
>   		return 0;
>   	case SG_GET_TIMEOUT:
> diff --git a/drivers/scsi/scsi_ioctl.c b/drivers/scsi/scsi_ioctl.c
> index c14f81403a09..162a21112d49 100644
> --- a/drivers/scsi/scsi_ioctl.c
> +++ b/drivers/scsi/scsi_ioctl.c
> @@ -195,10 +195,13 @@ static int sg_set_timeout(struct scsi_device *sdev, int __user *p)
>   {
>   	int timeout, err = get_user(timeout, p);
>   
> -	if (!err)
> -		sdev->sg_timeout = clock_t_to_jiffies(timeout);
> +	if (err)
> +		return err;
> +	if (timeout < 0)
> +		return -EIO;
>   
> -	return err;
> +	sdev->sg_timeout = clock_t_to_jiffies(timeout);
> +	return 0;
>   }

Rejecting negative values with -EIO matches drivers/scsi/sg.c, which
looks good. However, large positive int values still overflow
bd->timeout and sdev->sg_timeout: both fields are 32-bit unsigned int 
while clock_t_to_jiffies() returns unsigned long (clamping only at
ULONG_MAX).

Bart.

[PATCH v2] scsi: handle out-of-range SG_SET_TIMEOUT values

[Wang Yingjie]

SG_SET_TIMEOUT accepts a signed int in USER_HZ units. The sg character
device rejects negative values and limits positive values so the converted
timeout fits its destination field. The block SCSI and BSG paths instead
pass the value to clock_t_to_jiffies(), which accepts an unsigned long,
and store the result in an unsigned int.

Negative values are converted into large unsigned timeouts. On 64-bit
systems, large positive values can also exceed UINT_MAX after conversion
and are truncated on assignment. With HZ=1000, for example, 429496730
clock ticks become 4294967300 jiffies and are stored as 4 jiffies. On
32-bit systems, an overflowing conversion becomes ULONG_MAX, which is an
unsafe relative interval for the block timer comparisons.

Reject negative values with -EIO, matching drivers/scsi/sg.c. Saturate the
converted value at the smaller of the destination field limit and
MAX_JIFFY_OFFSET. This preserves successful ioctl behavior while avoiding
both narrowing wraparound and an unsafe 32-bit timer interval.

The block SCSI behavior predates Git history. BSG has exposed the same
behavior since it was introduced.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Fixes: 3d6392cfbd7d ("bsg: support for full generic block layer SG v3")
Cc: stable@vger.kernel.org
Reported-by: Bart Van Assche <bvanassche@acm.org>
Link: https://lore.kernel.org/r/9321a59c-c560-4742-8b71-5b0ec1f30c2f@acm.org
Signed-off-by: Yingjie Wang <x-professor@outlook.com>
---
Changes in v2:
- Add the pre-Git Fixes tag for the block SCSI path.
- Saturate large positive conversions at the smaller of UINT_MAX and
  MAX_JIFFY_OFFSET instead of allowing narrowing wraparound or an unsafe
  32-bit timer interval.
- Expand the commit message and boundary testing for the positive and 32-bit
  cases.

Tested on x86-64 with HZ=1000 through both /dev/sr0 and BSG. Values 0,
1, and 429496729 round-trip unchanged; 429496730 and INT_MAX saturate to
429496729 USER_HZ ticks; -5 returns EIO. Zero-timeout SG_IO requests still
complete after setting the timeout to 1.

Also tested on an actual i386 HZ=1000 QEMU kernel through both /dev/sr0 and
BSG. Values 0, 1, and 107374182 round-trip unchanged; 107374183, 429496729,
and INT_MAX saturate to 107374182 USER_HZ ticks; -5 returns EIO.

v1: https://lore.kernel.org/r/20261017084547.2880694-1-x-professor@outlook.com

 block/bsg.c               |  6 +++++-
 drivers/scsi/scsi_ioctl.c | 12 +++++++++---
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/block/bsg.c b/block/bsg.c
index 82aaf3cee5825..632b3f6d98b29 100644
--- a/block/bsg.c
+++ b/block/bsg.c
@@ -132,7 +132,11 @@ static long bsg_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 	case SG_SET_TIMEOUT:
 		if (get_user(val, intp))
 			return -EFAULT;
-		bd->timeout = clock_t_to_jiffies(val);
+		if (val < 0)
+			return -EIO;
+		bd->timeout = min_t(unsigned long, clock_t_to_jiffies(val),
+				    min_t(unsigned long, UINT_MAX,
+					  MAX_JIFFY_OFFSET));
 		return 0;
 	case SG_GET_TIMEOUT:
 		return jiffies_to_clock_t(bd->timeout);
diff --git a/drivers/scsi/scsi_ioctl.c b/drivers/scsi/scsi_ioctl.c
index c14f81403a09e..8277e6adf497e 100644
--- a/drivers/scsi/scsi_ioctl.c
+++ b/drivers/scsi/scsi_ioctl.c
@@ -9,6 +9,7 @@
 #include <linux/blkdev.h>
 #include <linux/interrupt.h>
 #include <linux/errno.h>
+#include <linux/jiffies.h>
 #include <linux/kernel.h>
 #include <linux/sched.h>
 #include <linux/mm.h>
@@ -195,10 +196,15 @@ static int sg_set_timeout(struct scsi_device *sdev, int __user *p)
 {
 	int timeout, err = get_user(timeout, p);
 
-	if (!err)
-		sdev->sg_timeout = clock_t_to_jiffies(timeout);
+	if (err)
+		return err;
+	if (timeout < 0)
+		return -EIO;
 
-	return err;
+	sdev->sg_timeout =
+		min_t(unsigned long, clock_t_to_jiffies(timeout),
+		      min_t(unsigned long, UINT_MAX, MAX_JIFFY_OFFSET));
+	return 0;
 }
 
 static int sg_get_reserved_size(struct scsi_device *sdev, int __user *p)

base-commit: 58161ead29e50d70a317ea26b472e0c7508e69da
-- 
2.43.0

Re: [PATCH v2] scsi: handle out-of-range SG_SET_TIMEOUT values

[Bart Van Assche]

On 10/21/26 11:18 AM, Wang Yingjie wrote:
> diff --git a/block/bsg.c b/block/bsg.c
> index 82aaf3cee5825..632b3f6d98b29 100644
> --- a/block/bsg.c
> +++ b/block/bsg.c
> @@ -132,7 +132,11 @@ static long bsg_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
>   	case SG_SET_TIMEOUT:
>   		if (get_user(val, intp))
>   			return -EFAULT;
> -		bd->timeout = clock_t_to_jiffies(val);
> +		if (val < 0)
> +			return -EIO;
> +		bd->timeout = min_t(unsigned long, clock_t_to_jiffies(val),
> +				    min_t(unsigned long, UINT_MAX,
> +					  MAX_JIFFY_OFFSET));

MAX_JIFFY_OFFSET is guaranteed to be larger than UINT_MAX so the second
min_t() expression can be changed into UINT_MAX. Additionally, please
use min() instead of min_t(). min() has been modified recently such that
it allows some type promotions.

Thanks,

Bart.

[PATCH v3] scsi: handle out-of-range SG_SET_TIMEOUT values

[Wang Yingjie]

SG_SET_TIMEOUT accepts a signed int in USER_HZ units. The sg character
device rejects negative values and limits positive values so the converted
timeout fits its destination field. The block SCSI and BSG paths instead
pass the value to clock_t_to_jiffies(), which accepts an unsigned long,
and store the result in an unsigned int.

Negative values are converted into large unsigned timeouts. On 64-bit
systems, large positive values can also exceed UINT_MAX after conversion
and are truncated on assignment. With HZ=1000 and USER_HZ=100, for
example, 429496730 clock ticks become 4294967300 jiffies and are stored
as 4 jiffies. On 32-bit systems, an overflowing conversion becomes
ULONG_MAX, which is an unsafe relative interval for the block timer
comparisons.

Reject negative values with -EIO, matching drivers/scsi/sg.c. Saturate the
converted value at the smaller of the destination field limit and
MAX_JIFFY_OFFSET. This preserves successful ioctl behavior while avoiding
both narrowing wraparound and an unsafe 32-bit timer interval.

The block SCSI behavior predates Git history. BSG has exposed the same
behavior since it was introduced.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Fixes: 3d6392cfbd7d ("bsg: support for full generic block layer SG v3")
Cc: stable@vger.kernel.org
Reported-by: Bart Van Assche <bvanassche@acm.org>
Link: https://lore.kernel.org/r/9321a59c-c560-4742-8b71-5b0ec1f30c2f@acm.org
Signed-off-by: Yingjie Wang <x-professor@outlook.com>
---
Changes in v3:
- Use the type-safe min() macro instead of min_t(), as requested in review.
- Keep MAX_JIFFY_OFFSET in the effective limit. It is defined as
  (LONG_MAX >> 1) - 1 and therefore is 0x3ffffffe on 32-bit kernels, which is
  smaller than UINT_MAX; the i386 HZ=1000 boundary test exercises this case.

Changes in v2:
- Add the pre-Git Fixes tag for the block SCSI path.
- Saturate large positive conversions at the smaller of UINT_MAX and
  MAX_JIFFY_OFFSET instead of allowing narrowing wraparound or an unsafe
  32-bit timer interval.
- Expand the commit message and boundary testing for the positive and 32-bit
  cases.

Tested v3 on x86-64 with HZ=1000 through both /dev/sr0 and BSG. Values 0,
1, and 429496729 round-trip unchanged; 429496730 and INT_MAX saturate to
429496729 USER_HZ ticks; -5 returns EIO. Zero-timeout SG_IO requests still
complete after setting the timeout to 1.

Also tested v3 on an actual i386 HZ=1000 QEMU kernel through both /dev/sr0
and BSG. Values 0, 1, and 107374182 round-trip unchanged; 107374183,
429496729, and INT_MAX saturate to 107374182 USER_HZ ticks; -5 returns EIO.

v2: https://lore.kernel.org/r/20261021181804.136336-1-x-professor@outlook.com
v1: https://lore.kernel.org/r/20261017084547.2880694-1-x-professor@outlook.com
 block/bsg.c               |  5 ++++-
 drivers/scsi/scsi_ioctl.c | 11 ++++++++---
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/block/bsg.c b/block/bsg.c
index 82aaf3cee5825..b296fa3732924 100644
--- a/block/bsg.c
+++ b/block/bsg.c
@@ -132,7 +132,10 @@ static long bsg_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 	case SG_SET_TIMEOUT:
 		if (get_user(val, intp))
 			return -EFAULT;
-		bd->timeout = clock_t_to_jiffies(val);
+		if (val < 0)
+			return -EIO;
+		bd->timeout = min(clock_t_to_jiffies(val),
+				  min(UINT_MAX, MAX_JIFFY_OFFSET));
 		return 0;
 	case SG_GET_TIMEOUT:
 		return jiffies_to_clock_t(bd->timeout);
diff --git a/drivers/scsi/scsi_ioctl.c b/drivers/scsi/scsi_ioctl.c
index c14f81403a09e..81a0519a2984b 100644
--- a/drivers/scsi/scsi_ioctl.c
+++ b/drivers/scsi/scsi_ioctl.c
@@ -9,6 +9,7 @@
 #include <linux/blkdev.h>
 #include <linux/interrupt.h>
 #include <linux/errno.h>
+#include <linux/jiffies.h>
 #include <linux/kernel.h>
 #include <linux/sched.h>
 #include <linux/mm.h>
@@ -195,10 +196,14 @@ static int sg_set_timeout(struct scsi_device *sdev, int __user *p)
 {
 	int timeout, err = get_user(timeout, p);
 
-	if (!err)
-		sdev->sg_timeout = clock_t_to_jiffies(timeout);
+	if (err)
+		return err;
+	if (timeout < 0)
+		return -EIO;
 
-	return err;
+	sdev->sg_timeout = min(clock_t_to_jiffies(timeout),
+			       min(UINT_MAX, MAX_JIFFY_OFFSET));
+	return 0;
 }
 
 static int sg_get_reserved_size(struct scsi_device *sdev, int __user *p)

base-commit: 58161ead29e50d70a317ea26b472e0c7508e69da
-- 
2.43.0

Re: [PATCH v3] scsi: handle out-of-range SG_SET_TIMEOUT values

[Bart Van Assche]

On 10/21/26 7:55 PM, Wang Yingjie wrote:
> SG_SET_TIMEOUT accepts a signed int in USER_HZ units. The sg character
> device rejects negative values and limits positive values so the converted
> timeout fits its destination field. The block SCSI and BSG paths instead
> pass the value to clock_t_to_jiffies(), which accepts an unsigned long,
> and store the result in an unsigned int.

Reported-by: Bart Van Assche <bvanassche@acm.org>