| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20250809093507.372430-3-abinashsinghlalotra@gmail.com>
Date: Sat, 9 Aug 2025 15:05:07 +0530
From: Abinash Singh <abinashsinghlalotra@...il.com>
To: abinashsinghlalotra@...il.com
Cc: James.Bottomley@...senPartnership.com,
bvanassche@....org,
dlemoal@...nel.org,
linux-kernel@...r.kernel.org,
linux-scsi@...r.kernel.org,
martin.petersen@...cle.com
Subject: [PATCH v6 2/2] scsi: sd: Fix build warning in sd_revalidate_disk()
A build warning was triggered due to excessive stack usage in
sd_revalidate_disk():
drivers/scsi/sd.c: In function ‘sd_revalidate_disk.isra’:
drivers/scsi/sd.c:3824:1: warning: the frame size of 1160 bytes is larger than 1024 bytes [-Wframe-larger-than=]
This is caused by a large local struct queue_limits (~400B) allocated
on the stack. Replacing it with a heap allocation using kmalloc()
significantly reduces frame usage. Kernel stack is limited (~8 KB),
and allocating large structs on the stack is discouraged.
As the function already performs heap allocations (e.g. for buffer),
this change fits well.
Signed-off-by: Abinash Singh <abinashsinghlalotra@...il.com>
---
drivers/scsi/sd.c | 48 +++++++++++++++++++++++++++--------------------
1 file changed, 28 insertions(+), 20 deletions(-)
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 9d5963de8c82..bc54884e910a 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -3696,8 +3696,8 @@ static void sd_revalidate_disk(struct gendisk *disk)
struct scsi_disk *sdkp = scsi_disk(disk);
struct scsi_device *sdp = sdkp->device;
sector_t old_capacity = sdkp->capacity;
- struct queue_limits lim;
- unsigned char *buffer;
+ struct queue_limits *lim = NULL;
+ unsigned char *buffer = NULL;
unsigned int dev_max;
int err;
@@ -3711,6 +3711,13 @@ static void sd_revalidate_disk(struct gendisk *disk)
if (!scsi_device_online(sdp))
goto out;
+ lim = kmalloc(sizeof(*lim), GFP_KERNEL);
+ if (!lim) {
+ sd_printk(KERN_WARNING, sdkp,
+ "sd_revalidate_disk: Disk limit allocation failure.\n");
+ goto out;
+ }
+
buffer = kmalloc(SD_BUF_SIZE, GFP_KERNEL);
if (!buffer) {
sd_printk(KERN_WARNING, sdkp, "sd_revalidate_disk: Memory "
@@ -3720,14 +3727,14 @@ static void sd_revalidate_disk(struct gendisk *disk)
sd_spinup_disk(sdkp);
- lim = queue_limits_start_update(sdkp->disk->queue);
+ *lim = queue_limits_start_update(sdkp->disk->queue);
/*
* Without media there is no reason to ask; moreover, some devices
* react badly if we do.
*/
if (sdkp->media_present) {
- sd_read_capacity(sdkp, &lim, buffer);
+ sd_read_capacity(sdkp, lim, buffer);
/*
* Some USB/UAS devices return generic values for mode pages
* until the media has been accessed. Trigger a READ operation
@@ -3741,17 +3748,17 @@ static void sd_revalidate_disk(struct gendisk *disk)
* cause this to be updated correctly and any device which
* doesn't support it should be treated as rotational.
*/
- lim.features |= (BLK_FEAT_ROTATIONAL | BLK_FEAT_ADD_RANDOM);
+ lim->features |= (BLK_FEAT_ROTATIONAL | BLK_FEAT_ADD_RANDOM);
if (scsi_device_supports_vpd(sdp)) {
sd_read_block_provisioning(sdkp);
- sd_read_block_limits(sdkp, &lim);
+ sd_read_block_limits(sdkp, lim);
sd_read_block_limits_ext(sdkp);
- sd_read_block_characteristics(sdkp, &lim);
- sd_zbc_read_zones(sdkp, &lim, buffer);
+ sd_read_block_characteristics(sdkp, lim);
+ sd_zbc_read_zones(sdkp, lim, buffer);
}
- sd_config_discard(sdkp, &lim, sd_discard_mode(sdkp));
+ sd_config_discard(sdkp, lim, sd_discard_mode(sdkp));
sd_print_capacity(sdkp, old_capacity);
@@ -3761,45 +3768,44 @@ static void sd_revalidate_disk(struct gendisk *disk)
sd_read_app_tag_own(sdkp, buffer);
sd_read_write_same(sdkp, buffer);
sd_read_security(sdkp, buffer);
- sd_config_protection(sdkp, &lim);
+ sd_config_protection(sdkp, lim);
}
/*
* We now have all cache related info, determine how we deal
* with flush requests.
*/
- sd_set_flush_flag(sdkp, &lim);
+ sd_set_flush_flag(sdkp, lim);
/* Initial block count limit based on CDB TRANSFER LENGTH field size. */
dev_max = sdp->use_16_for_rw ? SD_MAX_XFER_BLOCKS : SD_DEF_XFER_BLOCKS;
/* Some devices report a maximum block count for READ/WRITE requests. */
dev_max = min_not_zero(dev_max, sdkp->max_xfer_blocks);
- lim.max_dev_sectors = logical_to_sectors(sdp, dev_max);
+ lim->max_dev_sectors = logical_to_sectors(sdp, dev_max);
if (sd_validate_min_xfer_size(sdkp))
- lim.io_min = logical_to_bytes(sdp, sdkp->min_xfer_blocks);
+ lim->io_min = logical_to_bytes(sdp, sdkp->min_xfer_blocks);
else
- lim.io_min = 0;
+ lim->io_min = 0;
/*
* Limit default to SCSI host optimal sector limit if set. There may be
* an impact on performance for when the size of a request exceeds this
* host limit.
*/
- lim.io_opt = sdp->host->opt_sectors << SECTOR_SHIFT;
+ lim->io_opt = sdp->host->opt_sectors << SECTOR_SHIFT;
if (sd_validate_opt_xfer_size(sdkp, dev_max)) {
- lim.io_opt = min_not_zero(lim.io_opt,
+ lim->io_opt = min_not_zero(lim->io_opt,
logical_to_bytes(sdp, sdkp->opt_xfer_blocks));
}
sdkp->first_scan = 0;
set_capacity_and_notify(disk, logical_to_sectors(sdp, sdkp->capacity));
- sd_config_write_same(sdkp, &lim);
- kfree(buffer);
+ sd_config_write_same(sdkp, lim);
- err = queue_limits_commit_update_frozen(sdkp->disk->queue, &lim);
+ err = queue_limits_commit_update_frozen(sdkp->disk->queue, lim);
if (err)
goto out;
@@ -3820,7 +3826,9 @@ static void sd_revalidate_disk(struct gendisk *disk)
set_capacity_and_notify(disk, 0);
out:
- /* Placeholder for future cleanup */
+ kfree(lim);
+ kfree(buffer);
+
}
/**
--
2.50.1
Powered by blists - more mailing lists