From: Dan Carpenter <dan.carpenter@oracle.com>
To: Martyn Welch <martyn@welchs.me.uk>,
Dmitry Kalinkin <dmitry.kalinkin@gmail.com>
Cc: Manohar Vanga <manohar.vanga@gmail.com>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org,
kernel-janitors@vger.kernel.org
Subject: [PATCH] vme: Fix integer overflow checking in vme_check_window()
Date: Sat, 30 Sep 2017 14:27:41 +0300 [thread overview]
Message-ID: <20170930112741.6wmswvk25j7o44ui@mwanda> (raw)
The controversial part of this patch is that I've changed it so we now
prevent integer overflows for VME_USER types and before we didn't. I
view it as kernel-hardening. I looked at a couple places that used
VME_USER types and they seemed pretty suspicious so I'm pretty sure
preventing overflows here is a good idea.
The most common problem which this function is for cases like VME_A16
where we don't put an upper bound on "size" so you could have "size" set
to U64_MAX and a valid vme_base would overflow the "vme_base + size"
into the valid range as well.
In the VME_A64 case, the integer overflow checking doesn't work because
"U64_MAX + 1" has an integer overflow and it's just a complicated way of
saying zero. That VME_A64 case is sort of interesting as well because
there is a VME_A64_MAX define which is set to "U64_MAX + 1". The
compiler will never let anyone use it since it can't be stored in a u64
variable... With my patch it's now limited to just U64_MAX.
Anyway, I put one integer overflow check at the start of the function
and deleted all existing checks.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/drivers/vme/vme.c b/drivers/vme/vme.c
index 6a3ead42aba8..5b4c898d7509 100644
--- a/drivers/vme/vme.c
+++ b/drivers/vme/vme.c
@@ -208,29 +208,27 @@ int vme_check_window(u32 aspace, unsigned long long vme_base,
{
int retval = 0;
+ if (vme_base + size < size)
+ return -EINVAL;
+
switch (aspace) {
case VME_A16:
- if (((vme_base + size) > VME_A16_MAX) ||
- (vme_base > VME_A16_MAX))
+ if (vme_base + size > VME_A16_MAX)
retval = -EFAULT;
break;
case VME_A24:
- if (((vme_base + size) > VME_A24_MAX) ||
- (vme_base > VME_A24_MAX))
+ if (vme_base + size > VME_A24_MAX)
retval = -EFAULT;
break;
case VME_A32:
- if (((vme_base + size) > VME_A32_MAX) ||
- (vme_base > VME_A32_MAX))
+ if (vme_base + size > VME_A32_MAX)
retval = -EFAULT;
break;
case VME_A64:
- if ((size != 0) && (vme_base > U64_MAX + 1 - size))
- retval = -EFAULT;
+ /* The VME_A64_MAX limit is actually U64_MAX + 1 */
break;
case VME_CRCSR:
- if (((vme_base + size) > VME_CRCSR_MAX) ||
- (vme_base > VME_CRCSR_MAX))
+ if (vme_base + size > VME_CRCSR_MAX)
retval = -EFAULT;
break;
case VME_USER1:
next reply other threads:[~2017-09-30 11:27 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-09-30 11:27 Dan Carpenter [this message]
2017-10-01 15:19 ` Dmitry Kalinkin
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=20170930112741.6wmswvk25j7o44ui@mwanda \
--to=dan.carpenter@oracle.com \
--cc=devel@driverdev.osuosl.org \
--cc=dmitry.kalinkin@gmail.com \
--cc=gregkh@linuxfoundation.org \
--cc=kernel-janitors@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=manohar.vanga@gmail.com \
--cc=martyn@welchs.me.uk \
/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
all inboxes | Powered by JetHome®