2013-10-30 13:10:49

by Rashika Kheria

Subject: [PATCH v8 2/3] Staging: zram: Fix decrement of variable by calling bdput()

As suggested by Jerome Marchand "The code in reset_store get the block device
(bdget_disk()) but it does not put it (bdput()) when it's done using it.
The usage count is therefore incremented but never decremented."

This patch also puts bdput() for all error cases.

Cc: [email protected]
Signed-off-by: Rashika Kheria <[email protected]>
---

This revision fixes the following issues of the previous revision-
Proper error handling

drivers/staging/zram/zram_drv.c | 25 ++++++++++++++++++-------
1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/zram/zram_drv.c b/drivers/staging/zram/zram_drv.c
index 012ba15..0bc2835 100644
--- a/drivers/staging/zram/zram_drv.c
+++ b/drivers/staging/zram/zram_drv.c
@@ -648,25 +648,36 @@ static ssize_t reset_store(struct device *dev,
zram = dev_to_zram(dev);
bdev = bdget_disk(zram->disk, 0);

- if (!bdev)
- return -ENOMEM;
+ if (!bdev) {
+ ret = -ENOMEM;
+ goto out;
+ }

/* Do not reset an active device! */
- if (bdev->bd_holders)
- return -EBUSY;
+ if (bdev->bd_holders) {
+ ret = -EBUSY;
+ goto out;
+ }

ret = kstrtou16(buf, 10, &do_reset);
if (ret)
- return ret;
+ goto out;

- if (!do_reset)
- return -EINVAL;
+ if (!do_reset) {
+ ret = -EINVAL;
+ goto out;
+ }

/* Make sure all pending I/O is finished */
fsync_bdev(bdev);
+ bdput(bdev);

zram_reset_device(zram, true);
return len;
+
+out:
+ bdput(bdev);
+ return ret;
}

static void __zram_make_request(struct zram *zram, struct bio *bio, int rw)
--
1.7.9.5

2013-10-31 09:42:10

by Weijie Yang

Subject: Re: [PATCH v8 2/3] Staging: zram: Fix decrement of variable by calling bdput()

Hello, Rashika

On Wed, Oct 30, 2013 at 9:10 PM, Rashika Kheria
<[email protected]> wrote:
> As suggested by Jerome Marchand "The code in reset_store get the block device
> (bdget_disk()) but it does not put it (bdput()) when it's done using it.
> The usage count is therefore incremented but never decremented."
>
> This patch also puts bdput() for all error cases.
>
> Cc: [email protected]
> Signed-off-by: Rashika Kheria <[email protected]>
> ---
>
> This revision fixes the following issues of the previous revision-
> Proper error handling
>
> drivers/staging/zram/zram_drv.c | 25 ++++++++++++++++++-------
> 1 file changed, 18 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/staging/zram/zram_drv.c b/drivers/staging/zram/zram_drv.c
> index 012ba15..0bc2835 100644
> --- a/drivers/staging/zram/zram_drv.c
> +++ b/drivers/staging/zram/zram_drv.c
> @@ -648,25 +648,36 @@ static ssize_t reset_store(struct device *dev,
> zram = dev_to_zram(dev);
> bdev = bdget_disk(zram->disk, 0);
>
> - if (!bdev)
> - return -ENOMEM;
> + if (!bdev) {
> + ret = -ENOMEM;
> + goto out;
> + }

If bdev is NULL, just return -ENOMEM; DO NOT goto out;
or you will get a NULL point reference in bdput(bdev);

> /* Do not reset an active device! */
> - if (bdev->bd_holders)
> - return -EBUSY;
> + if (bdev->bd_holders) {
> + ret = -EBUSY;
> + goto out;
> + }
>
> ret = kstrtou16(buf, 10, &do_reset);
> if (ret)
> - return ret;
> + goto out;
>
> - if (!do_reset)
> - return -EINVAL;
> + if (!do_reset) {
> + ret = -EINVAL;
> + goto out;
> + }
>
> /* Make sure all pending I/O is finished */
> fsync_bdev(bdev);
> + bdput(bdev);
>
> zram_reset_device(zram, true);
> return len;
> +
> +out:
> + bdput(bdev);
> + return ret;
> }
>
> static void __zram_make_request(struct zram *zram, struct bio *bio, int rw)
> --
> 1.7.9.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/