[PATCH 0/4] zram: fix four error-handling and input-validation bugs

[PATCH 0/4] zram: fix four error-handling and input-validation bugs

[Andrew Stellman]

Four independent fixes for zram_drv.c:

1. read_from_bdev_async() silently drops the parent bio on allocation
   failure, leaving I/O hanging. Complete it with BLK_STS_IOERR.

2. zram_bio_discard() returns without bio_endio() on the unaligned
   single-fragment early-exit path.

3. recompress_store() accepts unrecognized type= values silently,
   running recompression with no slot filter (mode == 0).

4. algorithm_params_store() allows mutation of compression parameters
   after device initialization, unlike all peer sysfs store functions.

Andrew Stellman (4):
  zram: complete parent bio on allocation failure in
    read_from_bdev_async()
  zram: call bio_endio() on early return in zram_bio_discard()
  zram: reject unrecognized type= values in recompress_store()
  zram: add init_done() guard to algorithm_params_store()

 drivers/block/zram/zram_drv.c | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

-- 
2.34.1

[PATCH 1/4] zram: complete parent bio on allocation failure in read_from_bdev_async()

[Andrew Stellman]

read_from_bdev_async() silently returns without completing the parent
bio when either kmalloc or bio_alloc fails. The caller has already
submitted the parent bio for asynchronous completion, so returning
without calling bio_endio(parent) leaves the I/O hanging indefinitely.

Set parent->bi_status to BLK_STS_IOERR and call bio_endio(parent) on
both allocation failure paths, matching the error-completion pattern
used in zram_async_read_endio().

Signed-off-by: Andrew Stellman <astellman@stellman-greene.com>
---
 drivers/block/zram/zram_drv.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index af67937..f41f1ca 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -1437,12 +1437,17 @@ static void read_from_bdev_async(struct zram *zram, struct page *page,
 	struct bio *bio;
 
 	req = kmalloc_obj(*req, GFP_NOIO);
-	if (!req)
+	if (!req) {
+		parent->bi_status = BLK_STS_IOERR;
+		bio_endio(parent);
 		return;
+	}
 
 	bio = bio_alloc(zram->bdev, 1, parent->bi_opf, GFP_NOIO);
 	if (!bio) {
 		kfree(req);
+		parent->bi_status = BLK_STS_IOERR;
+		bio_endio(parent);
 		return;
 	}
 
-- 
2.34.1

Re: [PATCH 1/4] zram: complete parent bio on allocation failure in read_from_bdev_async()

[Sergey Senozhatsky]

On (26/04/07 08:38), Andrew Stellman wrote:
>  drivers/block/zram/zram_drv.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> index af67937..f41f1ca 100644
> --- a/drivers/block/zram/zram_drv.c
> +++ b/drivers/block/zram/zram_drv.c
> @@ -1437,12 +1437,17 @@ static void read_from_bdev_async(struct zram *zram, struct page *page,
>  	struct bio *bio;
>  
>  	req = kmalloc_obj(*req, GFP_NOIO);
> -	if (!req)
> +	if (!req) {
> +		parent->bi_status = BLK_STS_IOERR;
> +		bio_endio(parent);
>  		return;
> +	}
>  
>  	bio = bio_alloc(zram->bdev, 1, parent->bi_opf, GFP_NOIO);
>  	if (!bio) {
>  		kfree(req);
> +		parent->bi_status = BLK_STS_IOERR;
> +		bio_endio(parent);
>  		return;
>  	}

This is already fixed.

[PATCH 2/4] zram: call bio_endio() on early return in zram_bio_discard()

[Andrew Stellman]

When a discard request fits entirely within the unaligned head
fragment, zram_bio_discard() returns early without calling
bio_endio(bio). The bio is never completed, leaving the caller waiting
indefinitely. The normal exit path at the end of the function does call
bio_endio(), but the early return bypasses it.

Add bio_endio(bio) before the early return so the bio is properly
completed in all paths.

Signed-off-by: Andrew Stellman <astellman@stellman-greene.com>
---
 drivers/block/zram/zram_drv.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index f41f1ca..8ea2a12 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -2701,8 +2701,10 @@ static void zram_bio_discard(struct zram *zram, struct bio *bio)
 	 * skipping this logical block is appropriate here.
 	 */
 	if (offset) {
-		if (n <= (PAGE_SIZE - offset))
+		if (n <= (PAGE_SIZE - offset)) {
+			bio_endio(bio);
 			return;
+		}
 
 		n -= (PAGE_SIZE - offset);
 		index++;
-- 
2.34.1

Re: [PATCH 2/4] zram: call bio_endio() on early return in zram_bio_discard()

[Sergey Senozhatsky]

On (26/04/07 08:38), Andrew Stellman wrote:
>  drivers/block/zram/zram_drv.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> index f41f1ca..8ea2a12 100644
> --- a/drivers/block/zram/zram_drv.c
> +++ b/drivers/block/zram/zram_drv.c
> @@ -2701,8 +2701,10 @@ static void zram_bio_discard(struct zram *zram, struct bio *bio)
>  	 * skipping this logical block is appropriate here.
>  	 */
>  	if (offset) {
> -		if (n <= (PAGE_SIZE - offset))
> +		if (n <= (PAGE_SIZE - offset)) {
> +			bio_endio(bio);
>  			return;
> +		}
>  
>  		n -= (PAGE_SIZE - offset);
>  		index++;

This is already fixed.

[PATCH 3/4] zram: reject unrecognized type= values in recompress_store()

[Andrew Stellman]

recompress_store() parses the type= parameter with three independent if
statements checking for "idle", "huge", and "huge_idle". An
unrecognized value silently falls through with mode left at 0, causing
the recompression pass to run with no slot filter — processing all
slots instead of the intended subset.

Convert the if chain to if/else if/else and return -EINVAL for
unrecognized values, consistent with the function's other parameter
validation.

Signed-off-by: Andrew Stellman <astellman@stellman-greene.com>
---
 drivers/block/zram/zram_drv.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 8ea2a12..67dea80 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -2555,10 +2555,12 @@ static ssize_t recompress_store(struct device *dev,
 		if (!strcmp(param, "type")) {
 			if (!strcmp(val, "idle"))
 				mode = RECOMPRESS_IDLE;
-			if (!strcmp(val, "huge"))
+			else if (!strcmp(val, "huge"))
 				mode = RECOMPRESS_HUGE;
-			if (!strcmp(val, "huge_idle"))
+			else if (!strcmp(val, "huge_idle"))
 				mode = RECOMPRESS_IDLE | RECOMPRESS_HUGE;
+			else
+				return -EINVAL;
 			continue;
 		}
 
-- 
2.34.1

Re: [PATCH 3/4] zram: reject unrecognized type= values in recompress_store()

[Sergey Senozhatsky]

On (26/04/07 08:38), Andrew Stellman wrote:
> diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> index 8ea2a12..67dea80 100644
> --- a/drivers/block/zram/zram_drv.c
> +++ b/drivers/block/zram/zram_drv.c
> @@ -2555,10 +2555,12 @@ static ssize_t recompress_store(struct device *dev,
>  		if (!strcmp(param, "type")) {
>  			if (!strcmp(val, "idle"))
>  				mode = RECOMPRESS_IDLE;
> -			if (!strcmp(val, "huge"))
> +			else if (!strcmp(val, "huge"))
>  				mode = RECOMPRESS_HUGE;
> -			if (!strcmp(val, "huge_idle"))
> +			else if (!strcmp(val, "huge_idle"))
>  				mode = RECOMPRESS_IDLE | RECOMPRESS_HUGE;
> +			else
> +				return -EINVAL;
>  			continue;
>  		}

Let's do it like this:

---

@@ -2552,6 +2552,9 @@ static ssize_t recompress_store(struct device *dev,
                                mode = RECOMPRESS_HUGE;
                        if (!strcmp(val, "huge_idle"))
                                mode = RECOMPRESS_IDLE | RECOMPRESS_HUGE;
+
+                       if (!mode)
+                               return -EINVAL;
                        continue;
                }

[PATCH 4/4] zram: add init_done() guard to algorithm_params_store()

[Andrew Stellman]

algorithm_params_store() allows writing compression parameters after
the zram device has been initialized. Peer sysfs store functions
(comp_algorithm_store, disksize_store, mem_limit_store) all check
init_done(zram) and return -EBUSY when the device is already in use.
algorithm_params_store() uniquely lacks this guard, allowing runtime
mutation of compression parameters on an active device.

Acquire dev_lock and check init_done() immediately before calling
comp_params_store(), after argument parsing and priority resolution
complete. This keeps the lock scope narrow and matches the pattern in
__comp_algorithm_store().

Signed-off-by: Andrew Stellman <astellman@stellman-greene.com>
---
 drivers/block/zram/zram_drv.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 67dea80..d270964 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -1772,6 +1772,12 @@ static ssize_t algorithm_params_store(struct device *dev,
 	if (prio < ZRAM_PRIMARY_COMP || prio >= ZRAM_MAX_COMPS)
 		return -EINVAL;
 
+	guard(rwsem_write)(&zram->dev_lock);
+	if (init_done(zram)) {
+		pr_info("Can't change algorithm params for initialized device\n");
+		return -EBUSY;
+	}
+
 	ret = comp_params_store(zram, prio, level, dict_path, &deflate_params);
 	return ret ? ret : len;
 }
-- 
2.34.1

Re: [PATCH 4/4] zram: add init_done() guard to algorithm_params_store()

[Sergey Senozhatsky]

On (26/04/07 08:38), Andrew Stellman wrote:
> @@ -1772,6 +1772,12 @@ static ssize_t algorithm_params_store(struct device *dev,
>  	if (prio < ZRAM_PRIMARY_COMP || prio >= ZRAM_MAX_COMPS)
>  		return -EINVAL;
>  
> +	guard(rwsem_write)(&zram->dev_lock);
> +	if (init_done(zram)) {
> +		pr_info("Can't change algorithm params for initialized device\n");
> +		return -EBUSY;
> +	}
> +
>  	ret = comp_params_store(zram, prio, level, dict_path, &deflate_params);
>  	return ret ? ret : len;
>  }

This is already fixed.