[PATCH] arch-powerpc: Return false instead of -EFAULT

Returning a negative value for a boolean function seem to have the
undesired effect of returning true. Replace -EINVAL by false in a
bool-returning function.

The diff of the .s file before and after the change (using cross
compilation) starts with:

440,441c440,441
< .L43:
< 	li 3,1	 # D.25775,
---

.L42:
	li 3,0	 # D.25775,

...

while if -EFAULT is replaced by true, the diff is empty.

There is only one call site, and it expects a boolean value:
arch/powerpc/kernel/ftrace.c:129:
        if (!is_module_trampoline(tramp)) {
                pr_err("Not a trampoline\n");
                return -EINVAL;
        }

This issue was found by the following Coccinelle semantic patch:
<smpl>
@@
identifier f;
constant C;
typedef bool;
@@
bool f (...){
<+...
* return -C;
...+>
}
</smpl>

Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
---
 arch/powerpc/kernel/module_64.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/module_64.c b/arch/powerpc/kernel/module_64.c
index 6838451..a94a5f1 100644
--- a/arch/powerpc/kernel/module_64.c
+++ b/arch/powerpc/kernel/module_64.c

@@ -160,7 +160,7 @@ bool is_module_trampoline(u32 *p)
 	BUILD_BUG_ON(sizeof(ppc64_stub_insns) != sizeof(ppc64_stub_mask));
 
 	if (probe_kernel_read(insns, p, sizeof(insns)))
-		return -EFAULT;
+		return false;
 
 	for (i = 0; i < ARRAY_SIZE(ppc64_stub_insns); i++) {
 		u32 insna = insns[i];

-- 
2.1.0

On Thu, 2015-09-17 at 11:18 +0200, Peter Senna Tschudin wrote:

Returning a negative value for a boolean function seem to have the
undesired effect of returning true. Replace -EINVAL by false in a
bool-returning function.

The diff of the .s file before and after the change (using cross
compilation) starts with:

440,441c440,441
< .L43:
< 	li 3,1	 # D.25775,
---

quoted

.L42:
	li 3,0	 # D.25775,

...

while if -EFAULT is replaced by true, the diff is empty.

Ah, that's rather unfortunate.

Can you post the full asm listing, for all three cases?

cheers

On Thu, Sep 17, 2015 at 11:47 AM, Michael Ellerman [off-list ref] wrote:

On Thu, 2015-09-17 at 11:18 +0200, Peter Senna Tschudin wrote:

quoted

Returning a negative value for a boolean function seem to have the
undesired effect of returning true. Replace -EINVAL by false in a
bool-returning function.

The diff of the .s file before and after the change (using cross
compilation) starts with:

440,441c440,441
< .L43:
<     li 3,1   # D.25775,
---

quoted

.L42:
    li 3,0   # D.25775,

...

while if -EFAULT is replaced by true, the diff is empty.

Ah, that's rather unfortunate.

Can you post the full asm listing, for all three cases?

Sure, but it would be 3 files of 70kb each. You can download them form:

http://petersenna.com/files/module_64.s.tar.xz

Let me know if this download method doesn't work for you.

module_64.s: Original file.
module_64-patched.s: With my patch
module_64-true.s: Changing -EFAULT by true

files were made with:
$ make arch/powerpc/kernel/module_64.s

using:
gcc-powerpc64-linux-gnu-4.9.2-5.fc21.x86_64

cheers

-- 
Peter