ryantimwilson / rpms / systemd

Forked from rpms/systemd a month ago
Clone
Zbigniew Jędrzejewski-Szmek 126222
From ba087745bca7eea8f62cc529c3e6c5eaf6bf2904 Mon Sep 17 00:00:00 2001
Zbigniew Jędrzejewski-Szmek 126222
From: Stefan Beller <stefanbeller@googlemail.com>
Zbigniew Jędrzejewski-Szmek 126222
Date: Mon, 30 Dec 2013 17:43:52 +0100
Zbigniew Jędrzejewski-Szmek 126222
Subject: [PATCH] sleep-config: Dereference pointer before check for NULL
Zbigniew Jędrzejewski-Szmek 126222
Zbigniew Jędrzejewski-Szmek 126222
This fixes a bug pointed out by http://css.csail.mit.edu/stack/
Zbigniew Jędrzejewski-Szmek 126222
(Optimization-unstable code)
Zbigniew Jędrzejewski-Szmek 126222
It is a similar fix as f146f5e159 (2013-12-30, core:
Zbigniew Jędrzejewski-Szmek 126222
Forgot to dereference pointer when checking for NULL)
Zbigniew Jędrzejewski-Szmek 126222
Zbigniew Jędrzejewski-Szmek 126222
To explain this bug consider the following similar, but simpler code:
Zbigniew Jędrzejewski-Szmek 126222
	if (!p)
Zbigniew Jędrzejewski-Szmek 126222
		free(*p)
Zbigniew Jędrzejewski-Szmek 126222
Zbigniew Jędrzejewski-Szmek 126222
Assume the if condition evaluates to true, then we will access *p,
Zbigniew Jędrzejewski-Szmek 126222
which means the compiler can assume p is a valid pointer, so it could
Zbigniew Jędrzejewski-Szmek 126222
dereference p and use the value *p.
Zbigniew Jędrzejewski-Szmek 126222
Assuming p as a valid pointer, !p will be false.
Zbigniew Jędrzejewski-Szmek 126222
But initally we assumed the condition evaluates to true.
Zbigniew Jędrzejewski-Szmek 126222
Zbigniew Jędrzejewski-Szmek 126222
By this reasoning the optimizing compiler can deduce, we have dead code.
Zbigniew Jędrzejewski-Szmek 126222
("The if will never be taken, as *p must be valid, because otherwise
Zbigniew Jędrzejewski-Szmek 126222
accessing *p inside the if would segfault")
Zbigniew Jędrzejewski-Szmek 126222
Zbigniew Jędrzejewski-Szmek 126222
This led to an error message of the static code checker, so I checked the
Zbigniew Jędrzejewski-Szmek 126222
code in question.
Zbigniew Jędrzejewski-Szmek 126222
Zbigniew Jędrzejewski-Szmek 126222
As we access *modes and *states before the check in the changed line of
Zbigniew Jędrzejewski-Szmek 126222
this patch, I assume the line to be wrong and we actually wanted to check
Zbigniew Jędrzejewski-Szmek 126222
for *modes and *states being both non null.
Zbigniew Jędrzejewski-Szmek 126222
---
Zbigniew Jędrzejewski-Szmek 126222
 src/shared/sleep-config.c | 2 +-
Zbigniew Jędrzejewski-Szmek 126222
 1 file changed, 1 insertion(+), 1 deletion(-)
Zbigniew Jędrzejewski-Szmek 126222
Zbigniew Jędrzejewski-Szmek 126222
diff --git a/src/shared/sleep-config.c b/src/shared/sleep-config.c
Zbigniew Jędrzejewski-Szmek 126222
index d76e3ad..b2a0787 100644
Zbigniew Jędrzejewski-Szmek 126222
--- a/src/shared/sleep-config.c
Zbigniew Jędrzejewski-Szmek 126222
+++ b/src/shared/sleep-config.c
Zbigniew Jędrzejewski-Szmek 126222
@@ -94,7 +94,7 @@ int parse_sleep_config(const char *verb, char ***modes, char ***states) {
Zbigniew Jędrzejewski-Szmek 126222
         } else
Zbigniew Jędrzejewski-Szmek 126222
                 assert_not_reached("what verb");
Zbigniew Jędrzejewski-Szmek 126222
 
Zbigniew Jędrzejewski-Szmek 126222
-        if (!modes || !states) {
Zbigniew Jędrzejewski-Szmek 126222
+        if (!*modes || !*states) {
Zbigniew Jędrzejewski-Szmek 126222
                 strv_free(*modes);
Zbigniew Jędrzejewski-Szmek 126222
                 strv_free(*states);
Zbigniew Jędrzejewski-Szmek 126222
                 return log_oom();