Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Received-SPF: Pass (protection.outlook.com: domain of unisys.com designates
 192.61.61.104 as permitted sender) receiver=protection.outlook.com;
 client-ip=192.61.61.104; helo=USEA-NAEDGE1.unisys.com;
From:   David Kershner <david.kershner@unisys.com>
To:     <david.kershner@unisys.com>, <gregkh@linuxfoundation.org>,
        <jes.sorensen@gmail.com>, <linux-kernel@vger.kernel.org>,
        <driverdev-devel@linuxdriverproject.org>,
        <sparmaintainer@unisys.com>, <erik.arfvidson@gmail.com>,
        <wadgaonkarsam@gmail.com>
CC:     Tim Sell <Timothy.Sell@unisys.com>
Subject: [PATCH 1/2] staging: unisys: visorbus: address theoretical int overflows
Date:   Fri, 17 Nov 2017 12:27:38 -0500
Message-ID: <1510939659-15039-2-git-send-email-david.kershner@unisys.com>
In-Reply-To: <1510939659-15039-1-git-send-email-david.kershner@unisys.com>
References: <1510939659-15039-1-git-send-email-david.kershner@unisys.com>
MIME-Version: 1.0
Content-Type: text/plain
SpamDiagnosticOutput: 1:99
SpamDiagnosticMetadata: NSPM
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 17 Nov 2017 17:28:04.8825
 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: f937a068-e5f4-4ee9-5405-08d52de08fc6
X-MS-Exchange-CrossTenant-Id: 8d894c2b-238f-490b-8dd1-d93898c5bf83
X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=8d894c2b-238f-490b-8dd1-d93898c5bf83;Ip=[192.61.61.104];Helo=[USEA-NAEDGE1.unisys.com]
X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY2PR07MB695
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

From: Tim Sell <Timothy.Sell@unisys.com>

Add necessary casting to several places where we were doing 32-bit
arithmetic (unsigned) to produce a 64-bit (unsigned long) result, to
prevent the theoretical possibility of a 32-bit overflow during the
arithmetic.

FYI, these are unsigned long:

	ctx->param_bytes
	ctx->allocbytes

These are unsigned int:

	bytes
	phdr->name_offset
	phdr->name_length

Here is the test program demonstrating why we really need the casts:

void main()
{
	unsigned int i;
	unsigned long il;

	printf("sizeof(int) =%dn",sizeof(i));
	printf("sizeof(long)=%dn",sizeof(il));

	i = (unsigned int)((((unsigned long)(1)) << 32) - 1);
	printf("i                     = %un", i);
	il = i+1;
	printf("adding 1 withOUT cast = %lun", il);
	il = (unsigned long)i+1;
	printf("adding 1 WITH    cast = %lun", il);
}
[selltc@mac tmp]$ gcc x.c -o x.out
[selltc@mac tmp]$ ./x.out
sizeof(int) =4
sizeof(long)=8
i                     = 4294967295
adding 1 withOUT cast = 0
adding 1 WITH    cast = 4294967296

Signed-off-by: Tim Sell <Timothy.Sell@unisys.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: David Kershner <david.kershner@unisys.com>
Reviewed-by: David Binder <david.binder@unisys.com>
---
 drivers/staging/unisys/visorbus/visorchipset.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/unisys/visorbus/visorchipset.c b/drivers/staging/unisys/visorbus/visorchipset.c
index fed554a4..ef2823a 100644
--- a/drivers/staging/unisys/visorbus/visorchipset.c
+++ b/drivers/staging/unisys/visorbus/visorchipset.c
@@ -590,7 +590,8 @@ static void *parser_name_get(struct parser_context *ctx)
 	struct visor_controlvm_parameters_header *phdr;
 
 	phdr = &ctx->data;
-	if (phdr->name_offset + phdr->name_length > ctx->param_bytes)
+	if ((unsigned long)phdr->name_offset +
+	    (unsigned long)phdr->name_length > ctx->param_bytes)
 		return NULL;
 	ctx->curr = (char *)&phdr + phdr->name_offset;
 	ctx->bytes_remaining = phdr->name_length;
@@ -1317,13 +1318,13 @@ static void parser_done(struct parser_context *ctx)
 static struct parser_context *parser_init_stream(u64 addr, u32 bytes,
 						 bool *retry)
 {
-	int allocbytes;
+	unsigned long allocbytes;
 	struct parser_context *ctx;
 	void *mapping;
 
 	*retry = false;
 	/* alloc an extra byte to ensure payload is \0 terminated */
-	allocbytes = bytes + 1 + (sizeof(struct parser_context) -
+	allocbytes = (unsigned long)bytes + 1 + (sizeof(struct parser_context) -
 		     sizeof(struct visor_controlvm_parameters_header));
 	if ((chipset_dev->controlvm_payload_bytes_buffered + bytes) >
 	     MAX_CONTROLVM_PAYLOAD_BYTES) {
-- 
1.9.1


From 1585391144074054560@xxx Wed Nov 29 09:13:35 +0000 2017
X-GM-THRID: 1585281151451498932
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread