diff mbox series

[4.19,228/264] PM: hibernate: remove the bogus call to get_gendisk() in software_resume()

Message ID 20201027135441.360964081@linuxfoundation.org
State Superseded
Headers show
Series None | expand

Commit Message

Greg KH Oct. 27, 2020, 1:54 p.m. UTC
From: Christoph Hellwig <hch@lst.de>

[ Upstream commit 428805c0c5e76ef643b1fbc893edfb636b3d8aef ]

get_gendisk grabs a reference on the disk and file operation, so this
code will leak both of them while having absolutely no use for the
gendisk itself.

This effectively reverts commit 2df83fa4bce421f ("PM / Hibernate: Use
get_gendisk to verify partition if resume_file is integer format")

Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 kernel/power/hibernate.c | 11 -----------
 1 file changed, 11 deletions(-)

Comments

Pavel Machek Oct. 28, 2020, 7:10 a.m. UTC | #1
On Tue 2020-10-27 14:54:46, Greg Kroah-Hartman wrote:
> From: Christoph Hellwig <hch@lst.de>

> 

> [ Upstream commit 428805c0c5e76ef643b1fbc893edfb636b3d8aef ]

> 

> get_gendisk grabs a reference on the disk and file operation, so this

> code will leak both of them while having absolutely no use for the

> gendisk itself.

> 

> This effectively reverts commit 2df83fa4bce421f ("PM / Hibernate: Use

> get_gendisk to verify partition if resume_file is integer format")


This does not fix anything in 4.19, and should not be there.

It will break resume for people using resumewait option and using
numeric values, as original changelog explains. Eventually, someone
will cry regression and we'll have to fix it in the mainline, but no
need to bring this to -stable, too.

								Pavel
> +++ b/kernel/power/hibernate.c

> @@ -842,17 +842,6 @@ static int software_resume(void)

>  

>  	/* Check if the device is there */

>  	swsusp_resume_device = name_to_dev_t(resume_file);

> -

> -	/*

> -	 * name_to_dev_t is ineffective to verify parition if resume_file is in

> -	 * integer format. (e.g. major:minor)

> -	 */

> -	if (isdigit(resume_file[0]) && resume_wait) {

> -		int partno;

> -		while (!get_gendisk(swsusp_resume_device, &partno))

> -			msleep(10);

> -	}

> -

>  	if (!swsusp_resume_device) {

>  		/*

>  		 * Some device discovery might still be in progress; we need


-- 
http://www.livejournal.com/~pavelmachek
Christoph Hellwig Oct. 28, 2020, 7:12 a.m. UTC | #2
On Wed, Oct 28, 2020 at 08:10:57AM +0100, Pavel Machek wrote:
> This does not fix anything in 4.19, and should not be there.

> 

> It will break resume for people using resumewait option and using

> numeric values, as original changelog explains. Eventually, someone

> will cry regression and we'll have to fix it in the mainline, but no

> need to bring this to -stable, too.


If this code ever gets hit this patch fixes a reference count leak.
diff mbox series

Patch

diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
index 537a2a3c1dea2..28db51274ed0e 100644
--- a/kernel/power/hibernate.c
+++ b/kernel/power/hibernate.c
@@ -842,17 +842,6 @@  static int software_resume(void)
 
 	/* Check if the device is there */
 	swsusp_resume_device = name_to_dev_t(resume_file);
-
-	/*
-	 * name_to_dev_t is ineffective to verify parition if resume_file is in
-	 * integer format. (e.g. major:minor)
-	 */
-	if (isdigit(resume_file[0]) && resume_wait) {
-		int partno;
-		while (!get_gendisk(swsusp_resume_device, &partno))
-			msleep(10);
-	}
-
 	if (!swsusp_resume_device) {
 		/*
 		 * Some device discovery might still be in progress; we need