Skip to contentSkip to conversation

zram: fix stale scan bounds after reinitialization

PATCH Applied Longlong Xia 3 participants 8 messages Aug 4 – Aug 5

Subsystems (3)
Stable branch Block layer zram
lore
Share & tools
lore

Longlong Xia

author
#0 lore raw

cover zram: fix stale scan bounds after reinitialization

From: Longlong Xia <[email protected]>

Both writeback_store() and read_block_state() derive their table scan bounds from zram->disksize before acquiring dev_lock. If the device is reset and reinitialized with a smaller disksize between that read and lock acquisition, the bound can describe the old table while the scan operates on the new one. This can lead to out-of-bounds slot accesses.

Move both bound calculations under dev_lock so each bound remains consistent with the table throughout its scan. Keep the fixes separate because the affected interfaces originate from different commits and can be backported independently.

The series has been checked with:

  scripts/checkpatch.pl --strict <patches>
  make M=drivers/block/zram modules

Longlong Xia (2):
zram: fix out-of-bounds access in writeback_store()
zram: fix out-of-bounds access in read_block_state()

 drivers/block/zram/zram_drv.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

base-commit: 848acc8ffe1b7cd5f1bf427b93069becfebc2c9d

Signature · 1 line
2.43.0
author
#1 lore raw

PATCH 1/2 zram: fix out-of-bounds access in writeback_store()

From: Longlong Xia <[email protected]>

writeback_store() calculates the table scan bounds before taking dev_lock. A reset followed by reconfiguration with a smaller disksize can therefore replace zram->table while writeback_store() is waiting for the lock. Once it acquires the lock, it sees an initialized device but scans the new table using the old upper bound, resulting in an out-of-bounds access.

Calculate the number of pages while holding dev_lock so the scan bound matches the table protected by the lock.

Fixes a939888ec38b ("zram: support idle/huge page writeback")
Cc <[email protected]>
Assisted-by Codex:gpt-5.6-sol
Signed-off-by Longlong Xia <[email protected]>
---
 drivers/block/zram/zram_drv.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)
drivers/block/zram/zram_drv.c +5 -2
index ace65c586072..02fd64475a9a 100644
@@ -1244,8 +1244,8 @@ static ssize_t writeback_store(struct device *dev,
 			       const char *buf, size_t len)
 {
 	struct zram *zram = dev_to_zram(dev);
-	u64 nr_pages = zram->disksize >> PAGE_SHIFT;
-	unsigned long lo = 0, hi = nr_pages;
+	u64 nr_pages;
+	unsigned long lo = 0, hi;
 	struct zram_pp_ctl *pp_ctl = NULL;
 	struct zram_wb_ctl *wb_ctl = NULL;
 	char *args, *param, *val;

@@ -1259,6 +1259,9 @@ static ssize_t writeback_store(struct device *dev,
 	if (!zram->backing_dev)
 		return -ENODEV;
 
+	nr_pages = zram->disksize >> PAGE_SHIFT;
+	hi = nr_pages;
+
 	pp_ctl = init_pp_ctl();
 	if (!pp_ctl)
 		return -ENOMEM;
Signature · 1 line
2.43.0
maintainer Reviewed-by on v1 1/2
#2 lore raw

Re: [PATCH 1/2] zram: fix out-of-bounds access in writeback_store()

Quoted 7 lines · Longlong Xia

writeback_store() calculates the table scan bounds before taking dev_lock. A reset followed by reconfiguration with a smaller disksize can therefore replace zram->table while writeback_store() is waiting for the lock. Once it acquires the lock, it sees an initialized device but scans the new table using the old upper bound, resulting in an out-of-bounds access.

Quoted 7 lines

Calculate the number of pages while holding dev_lock so the scan bound matches the table protected by the lock.

Fixes a939888ec38b ("zram: support idle/huge page writeback")
Cc <[email protected]>
Assisted-by Codex:gpt-5.6-sol
Signed-off-by Longlong Xia <[email protected]>
Reviewed-by Sergey Senozhatsky <[email protected]>
author
#3 lore raw

PATCH 2/2 zram: fix out-of-bounds access in read_block_state()

In reply to Longlong Xia

From: Longlong Xia <[email protected]>

read_block_state() calculates nr_pages before taking dev_lock. If the device is reset and reinitialized with a smaller disksize before lock acquisition, nr_pages still describes the old table. The subsequent loop can then call slot_lock() past the end of the newly allocated table.

Read disksize after acquiring dev_lock and checking that the device is initialized. The read lock then keeps the table and its bound stable for the duration of the scan.

Fixes c0265342bff4 ("zram: introduce zram memory tracking")
Cc <[email protected]>
Assisted-by Codex:gpt-5.6-sol
Signed-off-by Longlong Xia <[email protected]>
---
 drivers/block/zram/zram_drv.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/block/zram/zram_drv.c +3 -1
index 02fd64475a9a..e5f42f56220d 100644
@@ -1552,7 +1552,7 @@ static ssize_t read_block_state(struct file *file, char __user *buf,
 	char *kbuf;
 	ssize_t index, written = 0;
 	struct zram *zram = file->private_data;
-	unsigned long nr_pages = zram->disksize >> PAGE_SHIFT;
+	unsigned long nr_pages;
 
 	kbuf = kvmalloc(count, GFP_KERNEL);
 	if (!kbuf)

@@ -1564,6 +1564,8 @@ static ssize_t read_block_state(struct file *file, char __user *buf,
 		return -EINVAL;
 	}
 
+	nr_pages = zram->disksize >> PAGE_SHIFT;
+
 	for (index = *ppos; index < nr_pages; index++) {
 		int copied;
Signature · 1 line
2.43.0
maintainer Reviewed-by on v1 2/2
#4 lore raw

Re: [PATCH 2/2] zram: fix out-of-bounds access in read_block_state()

Quoted 6 lines · Longlong Xia

read_block_state() calculates nr_pages before taking dev_lock. If the device is reset and reinitialized with a smaller disksize before lock acquisition, nr_pages still describes the old table. The subsequent loop can then call slot_lock() past the end of the newly allocated table.

Quoted 8 lines

Read disksize after acquiring dev_lock and checking that the device is initialized. The read lock then keeps the table and its bound stable for the duration of the scan.

Fixes c0265342bff4 ("zram: introduce zram memory tracking")
Cc <[email protected]>
Assisted-by Codex:gpt-5.6-sol
Signed-off-by Longlong Xia <[email protected]>
Reviewed-by Sergey Senozhatsky <[email protected]>
#5 lore raw
In reply to Longlong Xia
Quoted 10 lines · On Tue, 4 Aug 2026 14:59:17 +0800 Longlong Xia <[email protected]> wrote:

Both writeback_store() and read_block_state() derive their table scan bounds from zram->disksize before acquiring dev_lock. If the device is reset and reinitialized with a smaller disksize between that read and lock acquisition, the bound can describe the old table while the scan operates on the new one. This can lead to out-of-bounds slot accesses.

Move both bound calculations under dev_lock so each bound remains consistent with the table throughout its scan. Keep the fixes separate because the affected interfaces originate from different commits and can be backported independently.

Thanks. AI review found several things to be worried about, all are pre-existing.

	https://sashiko.dev/#/patchset/[email protected]
maintainer
#6 lore raw
Quoted 13 lines · Andrew Morton
Quoted 10 lines · On Tue, 4 Aug 2026 14:59:17 +0800 Longlong Xia <[email protected]> wrote:

Both writeback_store() and read_block_state() derive their table scan bounds from zram->disksize before acquiring dev_lock. If the device is reset and reinitialized with a smaller disksize between that read and lock acquisition, the bound can describe the old table while the scan operates on the new one. This can lead to out-of-bounds slot accesses.

Move both bound calculations under dev_lock so each bound remains consistent with the table throughout its scan. Keep the fixes separate because the affected interfaces originate from different commits and can be backported independently.

Quoted 4 lines

Thanks. AI review found several things to be worried about, all are pre-existing.

	https://sashiko.dev/#/patchset/[email protected]

- u32 disksize range check is fixed in a separate Longlong Xia's patch.

- read_block_state() read of zram->disksize outside of device lock is fixed in this series.

- read_block_state() tear-down is something to look into.

maintainer
#7 lore raw
Quoted 2 lines · Sergey Senozhatsky

- read_block_state() tear-down is something to look into.

Likely false report.