From: Dan Carpenter <dan.carpenter@oracle.com>
To: Ananth N Mavinakayanahalli <ananth@linux.vnet.ibm.com>,
Masami Hiramatsu <mhiramat@kernel.org>
Cc: Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>,
"David S. Miller" <davem@davemloft.net>,
linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org
Subject: [PATCH v2] kprobes: cleanup _kprobe_addr()
Date: Fri, 24 Mar 2017 12:50:34 +0300 [thread overview]
Message-ID: <20170324095033.GA15884@mwanda> (raw)
In-Reply-To: <20170323091516.1a41f01703629a0c756d0f94@kernel.org>
The important bit here is that at the end of the function we know that
since "addr" isn't be NULL, it means we don't need to test
"addr + offset" for NULL.
The NULL test generates a static checker warning because often something
else is intended. For example, a common bug looks like:
addr = strchr(buf, ':') + 1;
if (addr) {
In that example, we intended to test the return of strchr() instead of
"strchr() + 1".
Techinically, with these static checker warnings you could worry about
the addition wrapping but the NULL check wouldn't prevent "addr" from
pointing to some other invalid memory outside the text area. Also
pointer wrapping is undefined behavior in C.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
v2: more cleanups and changelog
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index d733479a10ee..ad30b5e22bda 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -1395,7 +1395,7 @@ static kprobe_opcode_t *_kprobe_addr(kprobe_opcode_t *addr,
const char *symbol_name, unsigned int offset)
{
if ((symbol_name && addr) || (!symbol_name && !addr))
- goto invalid;
+ return ERR_PTR(-EINVAL);
if (symbol_name) {
kprobe_lookup_name(symbol_name, addr);
@@ -1403,12 +1403,7 @@ static kprobe_opcode_t *_kprobe_addr(kprobe_opcode_t *addr,
return ERR_PTR(-ENOENT);
}
- addr = (kprobe_opcode_t *)(((char *)addr) + offset);
- if (addr)
- return addr;
-
-invalid:
- return ERR_PTR(-EINVAL);
+ return (kprobe_opcode_t *)(((char *)addr) + offset);
}
static kprobe_opcode_t *kprobe_addr(struct kprobe *p)
next parent reply other threads:[~2017-03-24 9:51 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20170323091516.1a41f01703629a0c756d0f94@kernel.org>
2017-03-24 9:50 ` Dan Carpenter [this message]
2017-03-27 2:19 ` [PATCH v2] kprobes: cleanup _kprobe_addr() Masami Hiramatsu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170324095033.GA15884@mwanda \
--to=dan.carpenter@oracle.com \
--cc=ananth@linux.vnet.ibm.com \
--cc=anil.s.keshavamurthy@intel.com \
--cc=davem@davemloft.net \
--cc=kernel-janitors@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mhiramat@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox