From patchwork Mon Jan 4 15:24:49 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ard Biesheuvel X-Patchwork-Id: 59138 Delivered-To: patch@linaro.org Received: by 10.112.130.2 with SMTP id oa2csp5421093lbb; Mon, 4 Jan 2016 07:26:50 -0800 (PST) X-Received: by 10.98.7.4 with SMTP id b4mr79301776pfd.152.1451921210658; Mon, 04 Jan 2016 07:26:50 -0800 (PST) Return-Path: Received: from bombadil.infradead.org (bombadil.infradead.org. [2001:1868:205::9]) by mx.google.com with ESMTPS id ei11si28819228pac.83.2016.01.04.07.26.50 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 04 Jan 2016 07:26:50 -0800 (PST) Received-SPF: pass (google.com: domain of linux-arm-kernel-bounces+patch=linaro.org@lists.infradead.org designates 2001:1868:205::9 as permitted sender) client-ip=2001:1868:205::9; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-arm-kernel-bounces+patch=linaro.org@lists.infradead.org designates 2001:1868:205::9 as permitted sender) smtp.mailfrom=linux-arm-kernel-bounces+patch=linaro.org@lists.infradead.org; dkim=neutral (body hash did not verify) header.i=@linaro.org Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1aG70V-0004jz-Fw; Mon, 04 Jan 2016 15:25:15 +0000 Received: from mail-io0-x22f.google.com ([2607:f8b0:4001:c06::22f]) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1aG70Q-0003V3-W0 for linux-arm-kernel@lists.infradead.org; Mon, 04 Jan 2016 15:25:12 +0000 Received: by mail-io0-x22f.google.com with SMTP id 1so111364982ion.1 for ; Mon, 04 Jan 2016 07:24:50 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=+7NBNWOLjHEPiC9O+iNUKfCV3BIgBqxFSD9w5GZXbk8=; b=ghFQGt2nEGKq+xGGpu8Ni0ACZtKQJs9ET8gPggAAoB7QOBfUqE5px1QCqw+0pnYzHG pKLHf0LYjtutd26eW0RjYJmblaOpfpj+3lzoILfwp6V/mXwOl6fnCxevSg9XdPwJhzJZ 2l4IeBLz9lNOyJh5YF8DNdVLZbBFNYpzT2f5A= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=+7NBNWOLjHEPiC9O+iNUKfCV3BIgBqxFSD9w5GZXbk8=; b=XZBVLvpO3HIfVlMEZ3zKieCqktdoDcWfESvTo+gRd+QlMvnZQ8SiBJkOakwwmwkdGr +7WmcZHcNhL/JjSc5BXo32QPGQEEaEw+gXY672F/NgcDgkGb8bvXbLipIV78dMQKxPRI fosdzKzR0nUfGZx5lpWLKjmgWay54jc/HB4Xd0Rqk4M/erorDsTYCZ9QykrK4x1KjkxA DslxMHJwqmVKgpbdyrLfhIreHboQQLwG//5cLPy0T9lIvo9JLfM+WDEwWKUm62bwQ4Ix OHH8Y/YBp5qXSGvHya8jd32heaGCnvZ7Ab8L6tbt7fUw5SI1WKup90m3REYxmmnRaE2h PX3w== X-Gm-Message-State: ALoCoQnotNrOkPgtBMqkxqmCF9zLNabwUnl05tHUROtHT9dcu0n5oudq1aDlvoL4hM9kv63/oRbsJ1qQD/cg7PrwxCpBHgVtVpG0o2DjESdoAH0bPbe63+g= MIME-Version: 1.0 X-Received: by 10.107.18.219 with SMTP id 88mr76633366ios.130.1451921089681; Mon, 04 Jan 2016 07:24:49 -0800 (PST) Received: by 10.36.29.6 with HTTP; Mon, 4 Jan 2016 07:24:49 -0800 (PST) In-Reply-To: References: <20160104141657.GC1616@arm.com> Date: Mon, 4 Jan 2016 16:24:49 +0100 Message-ID: Subject: Re: linux-4.4-rc8/arch/arm64/kernel/module.c:78: 32/64 bit problem ? From: Ard Biesheuvel To: Will Deacon X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20160104_072511_344945_547819F1 X-CRM114-Status: GOOD ( 16.92 ) X-Spam-Score: -2.7 (--) X-Spam-Report: SpamAssassin version 3.4.0 on bombadil.infradead.org summary: Content analysis details: (-2.7 points) pts rule name description ---- ---------------------- -------------------------------------------------- -0.7 RCVD_IN_DNSWL_LOW RBL: Sender listed at http://www.dnswl.org/, low trust [2607:f8b0:4001:c06:0:0:0:22f listed in] [list.dnswl.org] -0.0 SPF_PASS SPF: sender matches SPF record -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] -0.1 DKIM_VALID_AU Message has a valid DKIM or DK signature from author's domain 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid -0.1 DKIM_VALID Message has at least one valid DKIM or DK signature X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Catalin Marinas , "linux-arm-kernel@lists.infradead.org" , David Binderman Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patch=linaro.org@lists.infradead.org On 4 January 2016 at 15:32, Ard Biesheuvel wrote: > On 4 January 2016 at 15:16, Will Deacon wrote: >> On Mon, Jan 04, 2016 at 08:25:30AM +0000, David Binderman wrote: >>> Hello there, >> >> Hi David, >> >>> [linux-4.4-rc8/arch/arm64/kernel/module.c:78] -> >>> [linux-4.4-rc8/arch/arm64/kernel/module.c:88]: (warning) Shifting 32-bit >>> value by 64 bits is undefined behaviour. See condition at line 88. >> >> Curious, but how are you seeing this warning? GCC is silent for me... >> >>> Source code is >>> >>> u64 imm_mask = (1 << len) - 1; >>> s64 sval = do_reloc(op, place, val); >>> >>> switch (len) { >>> case 16: >>> *(s16 *)place = sval; >>> break; >>> case 32: >>> *(s32 *)place = sval; >>> break; >>> case 64: >>> >>> So it seems that len can be 64. Suggest new code >>> >>> u64 imm_mask = (1UL << len) - 1; >> >> That still ends up shifting by the width of the type when len == 64, >> which is potentially still broken. We're better off using GENMASK. >> > > Can't we simply return from the function rather than break from the > switch statement if len == 64? > The range check does not make any sense in that case anyway. > Or perhaps: ------------8<----------------- case 64: @@ -92,21 +95,6 @@ static int reloc_data(enum aarch64_reloc_op op, void *place, u64 val, int len) pr_err("Invalid length (%d) for data relocation\n", len); return 0; } - - /* - * Extract the upper value bits (including the sign bit) and - * shift them to bit 0. - */ - sval = (s64)(sval & ~(imm_mask >> 1)) >> (len - 1); - - /* - * Overflow has occurred if the value is not representable in - * len bits (i.e the bottom len bits are not sign-extended and - * the top bits are not all zero). - */ - if ((u64)(sval + 1) > 2) - return -ERANGE; - return 0; } _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel diff --git a/arch/arm64/kernel/module.c b/arch/arm64/kernel/module.c index f4bc779e62e8..fd1f4e678655 100644 --- a/arch/arm64/kernel/module.c +++ b/arch/arm64/kernel/module.c @@ -75,14 +75,17 @@ static u64 do_reloc(enum aarch64_reloc_op reloc_op, void *place, u64 val) static int reloc_data(enum aarch64_reloc_op op, void *place, u64 val, int len) { - u64 imm_mask = (1 << len) - 1; s64 sval = do_reloc(op, place, val); switch (len) { case 16: + if (sval < S16_MIN || sval > U16_MAX) + return -ERANGE; *(s16 *)place = sval; break; case 32: + if (sval < S32_MIN || sval > U32_MAX) + return -ERANGE; *(s32 *)place = sval; break;