Skip to contentSkip to conversation

PM / hibernate: memory corruption in resumedelay_setup()

PATCH Superseded Dan Carpenter 3 participants 5 messages May 14, 2014 – May 19, 2014

Subsystems (3)
Kernel janitors Suspend to RAM Hibernation
lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

PATCH 1/1 PM / hibernate: memory corruption in resumedelay_setup()

"resume_delay" is not an unsigned long, it's an int, so this will corrupt memory on 64 bit systems.

Fixes 317cf7e5e85e ('PM / hibernate: convert simple_strtoul to kstrtoul')
Signed-off-by Dan Carpenter <[email protected]>
kernel/power/hibernate.c +2 -1
index 2377ff7..3659b26 100644
@@ -1115,8 +1115,9 @@ static int __init resumewait_setup(char *str)
 
 static int __init resumedelay_setup(char *str)
 {
-	int rc = kstrtoul(str, 0, (unsigned long *)&resume_delay);
+	int rc;
 
+	rc = kstrtoint(str, 0, &resume_delay);
 	if (rc)
 		return rc;
 	return 1;
#1 lore raw

Hi!

Quoted 2 lines

"resume_delay" is not an unsigned long, it's an int, so this will corrupt memory on 64 bit systems.

ssleep takes unsigned int.

Variable is int.

We parse it using kstrtoul.

So I'd suggest: switch resume_delay variable to unsigned int, and use kstrtouint().

Quoted 8 lines
Fixes 317cf7e5e85e ('PM / hibernate: convert simple_strtoul to kstrtoul')
Signed-off-by Dan Carpenter <[email protected]>
kernel/power/hibernate.c +0 -0
index 2377ff7..3659b26 100644

kernel/power/hibernate.c static int __init resumewait_setup(char *str) · line 1120

Quoted 7 lines
kernel/power/hibernate.c +2 -1
@@ -1115,8 +1115,9 @@ static int __init resumewait_setup(char *str)
 
 static int __init resumedelay_setup(char *str)
 {
-	int rc = kstrtoul(str, 0, (unsigned long *)&resume_delay);
+	int rc;
 
+	rc = kstrtoint(str, 0, &resume_delay);

...and if possible, keep it on one line.

Thanks,
Pavel

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
author
#2 lore raw

PATCH v2 1/1 PM / hibernate: memory corruption in resumedelay_setup()

In the original code "resume_delay" is an int so on 64 bits, the call to kstrtoul() will cause memory corruption. We may as well fix a style issue here as well and make "resume_delay" unsigned int, since that's what we pass to ssleep().

Fixes 317cf7e5e85e ('PM / hibernate: convert simple_strtoul to kstrtoul')
Signed-off-by Dan Carpenter <[email protected]>

---
v2: style differences

kernel/power/hibernate.c +2 -2
index 2377ff7..df88d55 100644
@@ -35,7 +35,7 @@
 static int nocompress;
 static int noresume;
 static int resume_wait;
-static int resume_delay;
+static unsigned int resume_delay;
 static char resume_file[256] = CONFIG_PM_STD_PARTITION;
 dev_t swsusp_resume_device;
 sector_t swsusp_resume_block;

@@ -1115,7 +1115,7 @@ static int __init resumewait_setup(char *str)
 
 static int __init resumedelay_setup(char *str)
 {
-	int rc = kstrtoul(str, 0, (unsigned long *)&resume_delay);
+	int rc = kstrtouint(str, 0, &resume_delay);
 
 	if (rc)
 		return rc;
Acked-by on v2 1/1
#3 lore raw
Quoted 7 lines · Dan Carpenter

In the original code "resume_delay" is an int so on 64 bits, the call to kstrtoul() will cause memory corruption. We may as well fix a style issue here as well and make "resume_delay" unsigned int, since that's what we pass to ssleep().

Fixes 317cf7e5e85e ('PM / hibernate: convert simple_strtoul to kstrtoul')
Signed-off-by Dan Carpenter <[email protected]>
Acked-by Pavel Machek <[email protected]>

And thanks :-).
Pavel

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Applied
#4 lore raw
Quoted 10 lines · Pavel Machek
Quoted 7 lines · Dan Carpenter

In the original code "resume_delay" is an int so on 64 bits, the call to kstrtoul() will cause memory corruption. We may as well fix a style issue here as well and make "resume_delay" unsigned int, since that's what we pass to ssleep().

Fixes 317cf7e5e85e ('PM / hibernate: convert simple_strtoul to kstrtoul')
Signed-off-by Dan Carpenter <[email protected]>
Acked-by Pavel Machek <[email protected]>

Queued up for 3.16, thanks!

Signature · 2 lines
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.