diff mbox series

sparc64: Properly range check DAX completion index

Message ID 1522558381-31281-1-git-send-email-rob.gardner@oracle.com
State Accepted
Delegated to: David Miller
Headers show
Series sparc64: Properly range check DAX completion index | expand

Commit Message

Rob Gardner April 1, 2018, 4:53 a.m. UTC 
Each Oracle DAX CCB has a corresponding completion area, and the required
number of areas must fit within a previously allocated array of completion
areas beginning at the requested index.  Since the completion area index
is specified by a file offset, a user can pass arbitrary values, including
negative numbers. So the index must be thoroughly range checked to prevent
access to addresses outside the bounds of the allocated completion
area array.  The index cannot be negative, and it cannot exceed the
total array size, less the number of CCBs requested. The old code did
not check for negative values and was off by one on the upper bound.

Signed-off-by: Rob Gardner <rob.gardner@oracle.com>
Signed-off-by: Jonathan Helman <jonathan.helman@oracle.com>
Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
---
 drivers/sbus/char/oradax.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Comments

Linus Torvalds April 1, 2018, 5:11 p.m. UTC | #1 
On Sat, Mar 31, 2018 at 9:53 PM, Rob Gardner <rob.gardner@oracle.com> wrote:
>
> Signed-off-by: Rob Gardner <rob.gardner@oracle.com>
> Signed-off-by: Jonathan Helman <jonathan.helman@oracle.com>
> Reported-by: Linus Torvalds <torvalds@linux-foundation.org>

That Reported-by: should be "oguard <oguard@protonmail.com>"

I was just the messenger.

                Linus
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rob Gardner April 1, 2018, 6:42 p.m. UTC | #2 
On 04/01/2018 11:11 AM, Linus Torvalds wrote:
> On Sat, Mar 31, 2018 at 9:53 PM, Rob Gardner <rob.gardner@oracle.com> wrote:
>> Signed-off-by: Rob Gardner <rob.gardner@oracle.com>
>> Signed-off-by: Jonathan Helman <jonathan.helman@oracle.com>
>> Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
> That Reported-by: should be "oguard <oguard@protonmail.com>"
>
> I was just the messenger.
>
>                  Linus


oguard observed "lack of size check on the copy_from_user", but that 
wasn't really a bug since 'count' actually is checked in dax_write().

But you noticed that idx could be negative and idx + nccbs could 
overflow, and this is a genuine bug that nobody else saw.

Rob

--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller April 2, 2018, 12:08 a.m. UTC | #3 
From: Rob Gardner <rob.gardner@oracle.com>
Date: Sun, 1 Apr 2018 12:42:46 -0600

> On 04/01/2018 11:11 AM, Linus Torvalds wrote:
>> On Sat, Mar 31, 2018 at 9:53 PM, Rob Gardner <rob.gardner@oracle.com>
>> wrote:
>>> Signed-off-by: Rob Gardner <rob.gardner@oracle.com>
>>> Signed-off-by: Jonathan Helman <jonathan.helman@oracle.com>
>>> Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
>> That Reported-by: should be "oguard <oguard@protonmail.com>"
>>
>> I was just the messenger.
>>
>>                  Linus
> 
> 
> oguard observed "lack of size check on the copy_from_user", but that
> wasn't really a bug since 'count' actually is checked in dax_write().
> 
> But you noticed that idx could be negative and idx + nccbs could
> overflow, and this is a genuine bug that nobody else saw.

Agreed, Linus's as the reporter is correct.

Applied and queued up for -stable, thank you.
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox series

Patch

diff --git a/drivers/sbus/char/oradax.c b/drivers/sbus/char/oradax.c
index d8597d5..96b4ad7 100644
--- a/drivers/sbus/char/oradax.c
+++ b/drivers/sbus/char/oradax.c
@@ -880,7 +880,7 @@  static int dax_ccb_exec(struct dax_ctx *ctx, const char __user *buf,
 	dax_dbg("args: ccb_buf_len=%ld, idx=%d", count, idx);
 
 	/* for given index and length, verify ca_buf range exists */
-	if (idx + nccbs >= DAX_CA_ELEMS) {
+	if (idx < 0 || idx > (DAX_CA_ELEMS - nccbs)) {
 		ctx->result.exec.status = DAX_SUBMIT_ERR_NO_CA_AVAIL;
 		return 0;
 	}