From patchwork Tue Sep 24 17:47:28 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Nick Desaulniers X-Patchwork-Id: 174295 Delivered-To: patch@linaro.org Received: by 2002:a92:7e96:0:0:0:0:0 with SMTP id q22csp4459451ill; Tue, 24 Sep 2019 10:47:35 -0700 (PDT) X-Google-Smtp-Source: APXvYqwYS+YkHvG4SDbH9QAHXjwm4J0tJmum/WCUBeBmjoyh9U4LYI4M4u1kg6Ese5V41LyrXNNq X-Received: by 2002:a17:906:fd4:: with SMTP id c20mr3592044ejk.41.1569347255193; Tue, 24 Sep 2019 10:47:35 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1569347255; cv=none; d=google.com; s=arc-20160816; b=y4a+VyZqPMUW/iMRROKc5eYFwQm7gZOPlZ9+DR0Lgouieb7uxFSk+z3AhHNp0cgHZr U88gNRhKCIqhNqejXXwUV8XiALkUs8DfW0BciT16odi7oc3xFp/FloQvhwRmQzfFJpKm cpPLU+LDpq8gpSzkzjkJVNTprA6sJ6D/kAiEHpV3D1fIzv7he3UUuCN3qYvbg6r64Dgt ENPoweL7wmqMHpC6L8u9VyZGppf4esWMijh84yLhkM+HGgUOUGBjcDqBn+xhkdT6QoCy qVVQGeKF/uA223/MWJSLHF/QUiDmE4t24OUpSAHjnpwZnLzPbobsUldDWoVFCivSaWcK Eldg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding:cc:to:from :subject:references:mime-version:message-id:in-reply-to:date :dkim-signature; bh=DBG/wFyung5W8UWO8gFujad4LCgDfce9k3vKq6AZOHc=; b=hI87SiUFyN7+MUezDiSiPpqf99xajk14hfhQcAx6prLTBR7Ef8uLby0CHK11fq9CLU hawlvbonum9bt4Qmqz9QfbOfg8rSFFSVIVfMGP9N+bfaJRupRon1xN4+Sz4BON2xSyy0 T7Aqx6yL+0NZGJwkryJZlf4nLP5QsqgHg3SW3LYQbpHCRcdV9AkNC4DwZD4jsBJ8BwL/ cvRNcKGZXF5EEnGk1axHG9encfn3lgjuvxAvCszDRn7Fnk5RIrdInE46UEPgnRVbhQC/ f8NEIaPp17UEuHOAnMANjmiRmroceFMWkexAFtbgM2JfmoGvDdGU8SLG9KCAuOGCvJVx qcDg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@google.com header.s=20161025 header.b=seXQRnG4; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id b3si1308497ejq.45.2019.09.24.10.47.34; Tue, 24 Sep 2019 10:47:35 -0700 (PDT) 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; Authentication-Results: mx.google.com; dkim=pass header.i=@google.com header.s=20161025 header.b=seXQRnG4; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2407858AbfIXRre (ORCPT + 26 others); Tue, 24 Sep 2019 13:47:34 -0400 Received: from mail-qt1-f201.google.com ([209.85.160.201]:43610 "EHLO mail-qt1-f201.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2407478AbfIXRrd (ORCPT ); Tue, 24 Sep 2019 13:47:33 -0400 Received: by mail-qt1-f201.google.com with SMTP id j5so2836870qtn.10 for ; Tue, 24 Sep 2019 10:47:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:in-reply-to:message-id:mime-version:references:subject:from:to :cc:content-transfer-encoding; bh=DBG/wFyung5W8UWO8gFujad4LCgDfce9k3vKq6AZOHc=; b=seXQRnG46I6Ci9HSw67oM4f9WU5RkjmQX2F1eTZvaF10NZJKm/ILV0ZrtVikdF+Sl/ sgcA8DGRARP+NC27ixqtahqQ0TrX25UIlrcGpQs+2UU0fZx/OfPejIjIWx60N/VQw8pA e6XQMVBYmrhl/IdQnJ/tnYp0/5QdvCbanCSHTfeiKlV8eqmTujwDoJyjjFzHm1blLi+t msRPBmg70+21BGQqQWSLUWVo2HCcjgEJGsFkj9eqJZAFg95KvDAUA/DbgOzTiEpwFmr8 q378DkEh/bKx4RYUt6EgDvf68S88RRjlndPnuornArPd7L1ZTGixqCCQwb94iO4mGkve hfeA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:in-reply-to:message-id:mime-version :references:subject:from:to:cc:content-transfer-encoding; bh=DBG/wFyung5W8UWO8gFujad4LCgDfce9k3vKq6AZOHc=; b=UUGKLzOH5huzFFppvYBcOGrujVnVI/IhZjkFns06+u3eicBGyy3GnULnDQRZBEk5u5 BXqOXbcZhLAcXRSsC3AnuFtj2xArqt0ozOFnjNivX0lPLIDLOkQcShYitVNaiUNF6f2l Kp95OHBd6puzmoidatz6FjROHzDbDocmuxUUvdxoROpYVcZmn3M/uvo9vGApdJ39y1fB de2FG0YiwOZNAX5fK8NQXxIixMuiwWRm4EA4vjkrvK/r5cseMAVKT/jovu+bKEu2qQIK dg+cs0q74Cf2kKSNYYPSmO69wM9YaD/8GASRevyh6teVqsnySpqLlrOed7vWwbgXOooV vIFA== X-Gm-Message-State: APjAAAWMirltaGvURAPYA6buiOxpSxFfSM8B9qdhIB6lcUv4OnENlIxK w2LtsUtsDU90x2pB7rOashx2PDILijykRUi8kqs= X-Received: by 2002:ae9:f404:: with SMTP id y4mr3717758qkl.225.1569347252597; Tue, 24 Sep 2019 10:47:32 -0700 (PDT) Date: Tue, 24 Sep 2019 10:47:28 -0700 In-Reply-To: Message-Id: <20190924174728.201464-1-ndesaulniers@google.com> Mime-Version: 1.0 References: X-Mailer: git-send-email 2.23.0.351.gc4317032e6-goog Subject: [PATCH v2] hwmon: (applesmc) fix UB and udelay overflow From: Nick Desaulniers To: linux@roeck-us.net Cc: clang-built-linux@googlegroups.com, jdelvare@suse.com, Nick Desaulniers , "=?UTF-8?q?Tomasz=20Pawe=C5=82=20Gajc?=" , Nathan Chancellor , Henrik Rydberg , linux-hwmon@vger.kernel.org, linux-kernel@vger.kernel.org Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Fixes the following 2 issues in the driver: 1. Left shifting a signed integer is undefined behavior. Unsigned integral types should be used for bitwise operations. 2. The delay scales from 0x0010 to 0x20000 by powers of 2, but udelay will result in a linkage failure when given a constant that's greater than 20000 (0x4E20). Agressive loop unrolling can fully unroll the loop, resulting in later iterations overflowing the call to udelay. 2 is fixed via splitting the loop in two, iterating the first up to the point where udelay would overflow, then switching to mdelay, as suggested in Documentation/timers/timers-howto.rst. Reported-by: Tomasz Paweł Gajc Link: https://github.com/ClangBuiltLinux/linux/issues/678 Debugged-by: Nathan Chancellor Signed-off-by: Nick Desaulniers --- Changes V1 -> V2: * The first loop in send_byte() needs to break out on the same condition now. Technically, the loop condition could even be removed. The diff looks funny because of the duplicated logic between existing and newly added for loops. drivers/hwmon/applesmc.c | 35 +++++++++++++++++++++++++++++++---- 1 file changed, 31 insertions(+), 4 deletions(-) -- 2.23.0.351.gc4317032e6-goog Reported-by: Tomasz Paweł Gajc Signed-off-by: Nick Desaulniers Reviewed-by: Cengiz Can diff --git a/drivers/hwmon/applesmc.c b/drivers/hwmon/applesmc.c index 183ff3d25129..c76adb504dff 100644 --- a/drivers/hwmon/applesmc.c +++ b/drivers/hwmon/applesmc.c @@ -46,6 +46,7 @@ #define APPLESMC_MIN_WAIT 0x0010 #define APPLESMC_RETRY_WAIT 0x0100 #define APPLESMC_MAX_WAIT 0x20000 +#define APPLESMC_UDELAY_MAX 20000 #define APPLESMC_READ_CMD 0x10 #define APPLESMC_WRITE_CMD 0x11 @@ -157,14 +158,23 @@ static struct workqueue_struct *applesmc_led_wq; static int wait_read(void) { u8 status; - int us; - for (us = APPLESMC_MIN_WAIT; us < APPLESMC_MAX_WAIT; us <<= 1) { + unsigned int us; + + for (us = APPLESMC_MIN_WAIT; us < APPLESMC_UDELAY_MAX; us <<= 1) { udelay(us); status = inb(APPLESMC_CMD_PORT); /* read: wait for smc to settle */ if (status & 0x01) return 0; } + /* switch to mdelay for longer sleeps */ + for (; us < APPLESMC_MAX_WAIT; us <<= 1) { + mdelay(us); + status = inb(APPLESMC_CMD_PORT); + /* read: wait for smc to settle */ + if (status & 0x01) + return 0; + } pr_warn("wait_read() fail: 0x%02x\n", status); return -EIO; @@ -177,10 +187,10 @@ static int wait_read(void) static int send_byte(u8 cmd, u16 port) { u8 status; - int us; + unsigned int us; outb(cmd, port); - for (us = APPLESMC_MIN_WAIT; us < APPLESMC_MAX_WAIT; us <<= 1) { + for (us = APPLESMC_MIN_WAIT; us < APPLESMC_UDELAY_MAX; us <<= 1) { udelay(us); status = inb(APPLESMC_CMD_PORT); /* write: wait for smc to settle */ @@ -190,6 +200,23 @@ static int send_byte(u8 cmd, u16 port) if (status & 0x04) return 0; /* timeout: give up */ + if (us << 1 == APPLESMC_UDELAY_MAX) + break; + /* busy: long wait and resend */ + udelay(APPLESMC_RETRY_WAIT); + outb(cmd, port); + } + /* switch to mdelay for longer sleeps */ + for (; us < APPLESMC_MAX_WAIT; us <<= 1) { + mdelay(us); + status = inb(APPLESMC_CMD_PORT); + /* write: wait for smc to settle */ + if (status & 0x02) + continue; + /* ready: cmd accepted, return */ + if (status & 0x04) + return 0; + /* timeout: give up */ if (us << 1 == APPLESMC_MAX_WAIT) break; /* busy: long wait and resend */