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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20131029004338.GB17038@bbox>
Date:	Tue, 29 Oct 2013 09:43:38 +0900
From:	Minchan Kim <minchan@...nel.org>
To:	Rashika Kheria <rashika.kheria@...il.com>
Cc:	opw-kernel@...glegroups.com,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Jiang Liu <jiang.liu@...wei.com>,
	Nitin Gupta <ngupta@...are.org>,
	Jerome Marchand <jmarchand@...hat.com>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v6 1/3] Staging: zram: Fix access of NULL pointer

Hello Rashika,

On Mon, Oct 28, 2013 at 05:51:38PM +0530, Rashika Kheria wrote:
> This patch fixes the bug in reset_store caused by accessing NULL pointer.
> Hence, It introduces a check for bdev. It also removes unnecessary check
> of bdev for fsync_bdev().

It's better than old but I still want it more better.
First of all, I hope patch description is more clear in case of bugfix patch
because other maintainers from stable/distro or other people could understand
well so they could judge once again whether they need to backport or not.
If maintainer judge that it's not severe, even they couldn't backport.

Patch description includes followin as, at least.

1. When this bug happens?
2. What's the result of user POV?
3. How to fix it.

You were good for 2 and 3 but 1

I can help.

If memory pressure is severe, inode could be reclaimed.
In this case, bdget_disk can return NULL because allocation of inode
in bdget could fail.

I hope you could do better massage.

In addition, this bug have been for a long time so it deserves backporting to
stable tree. In such case, you could Cc stable tree with stable mark.
Please refer Documentation/stable_kernel_rules.txt and others patches
in LKML.

After this year kernel summit, it seems to change something rule for
marking stable but I thinks it's not solid so old rule still would be
valid.

Thanks!

> 
> Signed-off-by: Rashika Kheria <rashika.kheria@...il.com>
> ---
> 
> This revision fixes the following issues of the previous revision-
> Seperating patches into Bug fix and Smatch fix 
> 
>  drivers/staging/zram/zram_drv.c |    6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/zram/zram_drv.c b/drivers/staging/zram/zram_drv.c
> index 2c4ed52..d640a8f 100644
> --- a/drivers/staging/zram/zram_drv.c
> +++ b/drivers/staging/zram/zram_drv.c
> @@ -648,6 +648,9 @@ static ssize_t reset_store(struct device *dev,
>  	zram = dev_to_zram(dev);
>  	bdev = bdget_disk(zram->disk, 0);
>  
> +	if (!bdev)
> +		return -EBUSY;
> +
>  	/* Do not reset an active device! */
>  	if (bdev->bd_holders)
>  		return -EBUSY;
> @@ -660,8 +663,7 @@ static ssize_t reset_store(struct device *dev,
>  		return -EINVAL;
>  
>  	/* Make sure all pending I/O is finished */
> -	if (bdev)
> -		fsync_bdev(bdev);
> +	fsync_bdev(bdev);
>  
>  	zram_reset_device(zram, true);
>  	return len;
> -- 
> 1.7.9.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

-- 
Kind regards,
Minchan Kim
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ