[PATCH 3/3] powerpc/kprobes: Check return value of patch_instruction()

[Steven Rostedt]

On Thu, 23 Apr 2020 17:41:52 +0200
Christophe Leroy <christophe.leroy at c-s.fr> wrote:
  
> > diff --git a/arch/powerpc/kernel/optprobes.c b/arch/powerpc/kernel/optprobes.c
> > index 024f7aad1952..046485bb0a52 100644
> > --- a/arch/powerpc/kernel/optprobes.c
> > +++ b/arch/powerpc/kernel/optprobes.c
> > @@ -139,52 +139,67 @@ void arch_remove_optimized_kprobe(struct optimized_kprobe *op)
> >   	}
> >   }
> >   
> > +#define PATCH_INSN(addr, instr)						     \
> > +do {									     \
> > +	int rc = patch_instruction((unsigned int *)(addr), instr);	     \
> > +	if (rc) {							     \
> > +		pr_err("%s:%d Error patching instruction at 0x%pK (%pS): %d\n", \
> > +				__func__, __LINE__,			     \
> > +				(void *)(addr), (void *)(addr), rc);	     \
> > +		return rc;						     \
> > +	}								     \
> > +} while (0)
> > +  
> 
> I hate this kind of macro which hides the "return".
> 
> What about keeping the return action in the caller ?
> 
> Otherwise, what about implementing something based on the use of goto, 
> on the same model as unsafe_put_user() for instance ?

#define PATCH_INSN(addr, instr) \
({
	int rc = patch_instruction((unsigned int *)(addr), instr);	     \
	if (rc)								     \
		pr_err("%s:%d Error patching instruction at 0x%pK (%pS): %d\n", \
				__func__, __LINE__,			     \
				(void *)(addr), (void *)(addr), rc);	     \
	rc;								     \
})


Then you can just do:

	ret = PATCH_INSN(...);
	if (ret)
		return ret;

in the code.

-- Steve