From patchwork Tue Jun 26 13:28:29 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "H. Nikolaus Schaller" X-Patchwork-Id: 10489071 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 5401260386 for ; Tue, 26 Jun 2018 13:34:51 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 482072892C for ; Tue, 26 Jun 2018 13:34:51 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 3C96528939; Tue, 26 Jun 2018 13:34:51 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.8 required=2.0 tests=BAYES_00,DKIM_SIGNED, MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI, T_DKIM_INVALID autolearn=unavailable version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id DBB032892C for ; Tue, 26 Jun 2018 13:34:50 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933522AbeFZNet (ORCPT ); Tue, 26 Jun 2018 09:34:49 -0400 Received: from mo4-p02-ob.smtp.rzone.de ([85.215.255.84]:35979 "EHLO mo4-p02-ob.smtp.rzone.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S935643AbeFZNer (ORCPT ); Tue, 26 Jun 2018 09:34:47 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; t=1530020086; s=strato-dkim-0002; d=goldelico.com; h=References:In-Reply-To:References:In-Reply-To:Message-Id:Date: Subject:Cc:To:From:X-RZG-CLASS-ID:X-RZG-AUTH:From:Subject:Sender; bh=i85IAHhXB2TQOgIrgkmYW7f1yDKqvdXJ9uudlA8U//0=; b=JHLaTw6fD0Z2o7cKMba+2YylzDFOs+S299hpKyp6q9FAIjW4wLtersVccy4MsukrOg lxC0BBg/WQrnZScWh1SjMyscBwxlQsfLsZ1gl/o8uP571pw0zQIpahdm9Xi2E9dd6Ilu BsMaf6X+P52kgX/ooBvCdoVBDWvW3nJAtNbI1tLy52TZjXakkF/2xy7Sk3To5LfwwHSu swrNvVb/4cS3zz3N62X60jophy08551iw4RNvSfg/pNsev/kehBPqK7cdmZVJqBcAaO6 8AVoKKl/VkVZ0l9epFyHo10BWvRvyl9mArcVwrzWtAco+jr9ofgOLfWAjiZAA3R0uRc0 8AGw== X-RZG-AUTH: ":JGIXVUS7cutRB/49FwqZ7WcJeFKiMhflhwDubTJ9o12DNO4Ij0JfyqA2nA==" X-RZG-CLASS-ID: mo00 Received: from iMac.fritz.box by smtp.strato.de (RZmta 43.10 DYNA|AUTH) with ESMTPSA id L06bd3u5QDSVNPv (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (curve secp521r1 with 521 ECDH bits, eq. 15360 bits RSA)) (Client did not present a certificate); Tue, 26 Jun 2018 15:28:31 +0200 (CEST) From: "H. Nikolaus Schaller" To: Sebastian Reichel , anish kumar , Krzysztof Kozlowski , Anton Vorontsov , Jonathan Cameron Cc: linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org, letux-kernel@openphoenux.org, kernel@pyra-handheld.com, "H. Nikolaus Schaller" , stable@vger.kernel.org Subject: [PATCH 1/2] power: generic-adc-battery: fix out-of-bounds write when copying channel properties Date: Tue, 26 Jun 2018 15:28:29 +0200 Message-Id: <06e010990476a1ab0d3f3d4006c7623a77aa4041.1530019688.git.hns@goldelico.com> X-Mailer: git-send-email 2.12.2 In-Reply-To: References: In-Reply-To: References: Sender: linux-pm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pm@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP We did have sporadic problems in the pinctrl framework during boot where a pin group name unexpectedly became NULL leading to a NULL dereference in strcmp. Detailled analysis of the failing cases did reveal that there were two devm allocated objects close to each other. The second one was the affected group_desc in pinmux and the first one was the psy_desc->properties buffer of the gab driver. Review of the gab code showed that the address calculation for one memcpy() is wrong. It does properties + sizeof(type) * index but C is defined to do the index multiplication already for pointer + integer additions. Hence the factor was applied twice and the memcpy() does write outside of the properties buffer. Sometimes it happened to be the pinctrl and triggered the strcmp(NULL). Anyways, it is overkill to use a memcpy() here instead of a simple assignment, which is easier to read and has less risk for wrong address calculations. So we change code to a simple assignment. If we initialize the index to the first free location, we can even remove the local variable 'properties'. This bug seems to exist right from the beginning in 3.7-rc1 in commit e60fea794e6e ("power: battery: Generic battery driver using IIO") Signed-off-by: H. Nikolaus Schaller Cc: stable@vger.kernel.org --- drivers/power/supply/generic-adc-battery.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/drivers/power/supply/generic-adc-battery.c b/drivers/power/supply/generic-adc-battery.c index 28dc056eaafa..11f59275bff4 100644 --- a/drivers/power/supply/generic-adc-battery.c +++ b/drivers/power/supply/generic-adc-battery.c @@ -241,10 +241,9 @@ static int gab_probe(struct platform_device *pdev) struct power_supply_desc *psy_desc; struct power_supply_config psy_cfg = {}; struct gab_platform_data *pdata = pdev->dev.platform_data; - enum power_supply_property *properties; int ret = 0; int chan; - int index = 0; + int index = ARRAY_SIZE(gab_props); adc_bat = devm_kzalloc(&pdev->dev, sizeof(*adc_bat), GFP_KERNEL); if (!adc_bat) { @@ -278,8 +277,6 @@ static int gab_probe(struct platform_device *pdev) } memcpy(psy_desc->properties, gab_props, sizeof(gab_props)); - properties = (enum power_supply_property *) - ((char *)psy_desc->properties + sizeof(gab_props)); /* * getting channel from iio and copying the battery properties @@ -293,15 +290,12 @@ static int gab_probe(struct platform_device *pdev) adc_bat->channel[chan] = NULL; } else { /* copying properties for supported channels only */ - memcpy(properties + sizeof(*(psy_desc->properties)) * index, - &gab_dyn_props[chan], - sizeof(gab_dyn_props[chan])); - index++; + psy_desc->properties[index++] = gab_dyn_props[chan]; } } /* none of the channels are supported so let's bail out */ - if (index == 0) { + if (index == ARRAY_SIZE(gab_props)) { ret = -ENODEV; goto second_mem_fail; } @@ -312,7 +306,7 @@ static int gab_probe(struct platform_device *pdev) * as come channels may be not be supported by the device.So * we need to take care of that. */ - psy_desc->num_properties = ARRAY_SIZE(gab_props) + index; + psy_desc->num_properties = index; adc_bat->psy = power_supply_register(&pdev->dev, psy_desc, &psy_cfg); if (IS_ERR(adc_bat->psy)) {