From patchwork Tue Nov 28 13:25:25 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arnd Bergmann X-Patchwork-Id: 10080313 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 CE7206056A for ; Tue, 28 Nov 2017 13:26:15 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id C16F528878 for ; Tue, 28 Nov 2017 13:26:15 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id B62E728975; Tue, 28 Nov 2017 13:26:15 +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=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI 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 7087C28878 for ; Tue, 28 Nov 2017 13:26:15 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752073AbdK1N0C (ORCPT ); Tue, 28 Nov 2017 08:26:02 -0500 Received: from mout.kundenserver.de ([217.72.192.75]:64428 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751807AbdK1N0B (ORCPT ); Tue, 28 Nov 2017 08:26:01 -0500 Received: from wuerfel.lan ([109.193.157.232]) by mrelayeu.kundenserver.de (mreue102 [212.227.15.145]) with ESMTPA (Nemesis) id 0Lm4KP-1esjWk0AYT-00ZgO0; Tue, 28 Nov 2017 14:25:39 +0100 From: Arnd Bergmann To: Adaptec OEM Raid Solutions , "James E.J. Bottomley" , "Martin K. Petersen" Cc: Meelis Roos , Arnd Bergmann , Dave Carroll , Raghava Aditya Renukunta , Johannes Thumshirn , Hannes Reinecke , linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH] scsi: aacraid: address UBSAN warning regression Date: Tue, 28 Nov 2017 14:25:25 +0100 Message-Id: <20171128132536.3037571-1-arnd@arndb.de> X-Mailer: git-send-email 2.9.0 X-Provags-ID: V03:K0:B6pX/qaYfs2AsmFlg+Nxl0wSKG+HvLIv42bfVaqFlsXEQrAs1+M j1mq2mGGjVa4eMu7ANcVeIccRsO40T1dwfmXINqpCB9YMNqulnYGuk/j31+6h13d2rZo2wg lcK+OJ1phTBX+e3xfl8YaNLSmotYQYxdIrCO3J+tp41AeljQS1yKOTPC9gEGafCN0Eyewol vFQcJjRzWx8nJPCbAusWw== X-UI-Out-Filterresults: notjunk:1; V01:K0:5sN72oMJGx8=:vHeyKTEKyn/wtjTSM9T7nO UPT724K+eZrwHTKbaEmhYwQXw56IiuQ3B2VVBro4SR9uhHlDeAcUsnvgyMqGJOrVSrIQJCMRK fuTu2LjDlLf8LCLV7QuZw7I21NsIYrknEtEio3v6wde/JwGBWmBl6E+Jjg3lKh/7B1vN+zboj yks60pifT1muya/U16mpGnjcHvu2+pRQo4ICrpk8a8ylRKo+jcErCBaEOGnMS1jlE2drqmgBm U1+9cG7V0WI2EvNNioXZhJQi7xPoUHRPPtDUGTwB5VNysLU7heZirHvugyvXXr9xGNZ+b5qXa 2qarnUyKq59ndppjnl4obDnFDvtob/lvYKr7VbHypsixll8QZTYT3cuKNnxuaqjII2yw9MVf3 2DMBMDCHzyprPKZb9Q+QzLxdBDnuROz2mAsgiqHeVsOth2KrBlndr1nWo8/366ha6fSmZ6WKa GPcH8FeFuiEEMFIZTGIE1ecUZ+VWs5Ea84ZYi/AEEipFo04WprZcelf3tqfkZfRhn6yIanjLJ 9xcYzsuP6pnGQm7hbHrzRUmier+lCAC/dgPUIXhJI2siERbe7IwD6brXRiVbR49hZ7A9CSIA7 lpR8jiANMm19Y00FqTkWGgVMc45AZL/wxOaDAaW3ejzYIHcqdoVJ4oGBSXZv6WKoNiMGO7w1+ OH7dqZyGu6zy/fH4r1/IfGlKFWkpwmwTUw2JB8Uo3Skc7HFKadxz4txRhlkpYJM+7upt6roR7 W6DeH74HBd1XDrK9n7RnL5j5K7lIvNxm30fyvg== Sender: linux-scsi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP As reported by Meelis Roos, my previous patch causes an incorrect calculation of the timeout, through an undefined signed integer overflow: [ 12.228155] UBSAN: Undefined behaviour in drivers/scsi/aacraid/commsup.c:2514:49 [ 12.228229] signed integer overflow: [ 12.228283] 964297611 * 250 cannot be represented in type 'long int' The problem is that doing a multiplication with HZ first and then dividing by USEC_PER_SEC worked correctly for 32-bit microseconds, but not for 32-bit nanoseconds, which would require up to 41 bits. This reworks the calculation to first convert the nanoseconds into jiffies, which should give us the same result as before and not overflow. Unfortunately I did not understand the exact intention of the algorithm, in particular the part where we add half a second, so it's possible that there is still a preexisting problem in this function. I added a comment that this would be handled more nicely using usleep_range(), which generally works better for waking up at a particular time than the current schedule_timeout() based implementation. I did not feel comfortable trying to implement that without being sure what the intent is here though. Fixes: 820f18865912 ("scsi: aacraid: use timespec64 instead of timeval") Tested-by: Meelis Roos Signed-off-by: Arnd Bergmann --- drivers/scsi/aacraid/commsup.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/aacraid/commsup.c b/drivers/scsi/aacraid/commsup.c index 525a652dab48..cc8fdefaebb6 100644 --- a/drivers/scsi/aacraid/commsup.c +++ b/drivers/scsi/aacraid/commsup.c @@ -2511,8 +2511,8 @@ int aac_command_thread(void *data) /* Synchronize our watches */ if (((NSEC_PER_SEC - (NSEC_PER_SEC / HZ)) > now.tv_nsec) && (now.tv_nsec > (NSEC_PER_SEC / HZ))) - difference = (((NSEC_PER_SEC - now.tv_nsec) * HZ) - + NSEC_PER_SEC / 2) / NSEC_PER_SEC; + difference = HZ + HZ / 2 - + now.tv_nsec / (NSEC_PER_SEC / HZ); else { if (now.tv_nsec > NSEC_PER_SEC / 2) ++now.tv_sec; @@ -2536,6 +2536,10 @@ int aac_command_thread(void *data) if (kthread_should_stop()) break; + /* + * we probably want usleep_range() here instead of the + * jiffies computation + */ schedule_timeout(difference); if (kthread_should_stop())