Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1751178AbdHXBD6 (ORCPT <rfc822;w@1wt.eu>);
        Wed, 23 Aug 2017 21:03:58 -0400
Received: from mail-pg0-f41.google.com ([74.125.83.41]:37801 "EHLO
        mail-pg0-f41.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1750962AbdHXBD4 (ORCPT
        <rfc822;linux-kernel@vger.kernel.org>);
        Wed, 23 Aug 2017 21:03:56 -0400
From: Bjorn Andersson <bjorn.andersson@linaro.org>
To: Rob Herring <robh+dt@kernel.org>,
        Frank Rowand <frowand.list@gmail.com>
Cc: devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
        Rob Herring <robh@kernel.org>
Subject: [PATCH] of/device: Fix of_device_get_modalias() buffer handling
Date: Wed, 23 Aug 2017 18:03:52 -0700
Message-Id: <20170824010352.9085-1-bjorn.andersson@linaro.org>
X-Mailer: git-send-email 2.12.0
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 2734
Lines: 78

of_device_request_module() calls of_device_get_modalias() with "len" 0,
to calculate the size of the buffer needed to store the result, but due
to integer promotion the ssize_t "len" will be compared as unsigned with
strlen(compat) and the loop will generally never break. This results in
a call to snprintf() with a negative len, which triggers below warning,
followed by a dereference of a invalid pointer:

  [    3.060067] WARNING: CPU: 0 PID: 51 at lib/vsprintf.c:2122 vsnprintf+0x348/0x6d8
  ...
  [    3.060301] [<ffffff800891ede8>] vsnprintf+0x348/0x6d8
  [    3.060308] [<ffffff800891f248>] snprintf+0x48/0x50
  [    3.060316] [<ffffff80086a7c80>] of_device_get_modalias+0x108/0x160
  [    3.060322] [<ffffff80086a7cf8>] of_device_request_module+0x20/0x88
  ...

Further more of_device_get_modalias() is supposed to return the number
of bytes needed to store the entire modalias, so the loop needs to
continue accumulate the total size even though the buffer is full.

Finally the function is not expected to ensure space for the NUL, nor
include it in the returned size, so only 1 should be added to the length
of "compat" in the loop (to account for the character 'C').

Fixes: bc575064d688 ("of/device: use of_property_for_each_string to parse compatible strings")
Cc: Rob Herring <robh@kernel.org>
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
 drivers/of/device.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/of/device.c b/drivers/of/device.c
index 6f33a0e0d351..7cff599a9c6a 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -195,10 +195,11 @@ EXPORT_SYMBOL(of_device_get_match_data);
 
 static ssize_t of_device_get_modalias(struct device *dev, char *str, ssize_t len)
 {
-	const char *compat, *start = str;
+	const char *compat;
 	char *c;
 	struct property *p;
 	ssize_t csize;
+	ssize_t tsize;
 
 	if ((!dev) || (!dev->of_node))
 		return -ENODEV;
@@ -206,12 +207,16 @@ static ssize_t of_device_get_modalias(struct device *dev, char *str, ssize_t len
 	/* Name & Type */
 	csize = snprintf(str, len, "of:N%sT%s", dev->of_node->name,
 			 dev->of_node->type);
+	tsize = csize;
 	len -= csize;
-	str += csize;
+	if (str)
+		str += csize;
 
 	of_property_for_each_string(dev->of_node, "compatible", p, compat) {
-		if (strlen(compat) + 2 > len)
-			break;
+		csize = strlen(compat) + 1;
+		tsize += csize;
+		if (csize > len)
+			continue;
 
 		csize = snprintf(str, len, "C%s", compat);
 		for (c = str; c; ) {
@@ -223,7 +228,7 @@ static ssize_t of_device_get_modalias(struct device *dev, char *str, ssize_t len
 		str += csize;
 	}
 
-	return str - start;
+	return tsize;
 }
 
 int of_device_request_module(struct device *dev)
-- 
2.12.0