From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752236AbcFTDG4 (ORCPT ); Sun, 19 Jun 2016 23:06:56 -0400 Received: from mailout1.samsung.com ([203.254.224.24]:48872 "EHLO mailout1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751466AbcFTDGr (ORCPT ); Sun, 19 Jun 2016 23:06:47 -0400 X-AuditID: cbfee68e-f79266d000001428-af-57675635400f Message-id: <57675635.20808@samsung.com> Date: Mon, 20 Jun 2016 11:34:29 +0900 From: Jaehoon Chung User-Agent: Mozilla/5.0 (X11; Linux i686; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 MIME-version: 1.0 To: Seung-Woo Kim , ulf.hansson@linaro.org, linux-mmc@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2] mmc: dw_mmc: remove UBSAN warning in dw_mci_setup_bus() References: <1465358840-22826-1-git-send-email-sw0312.kim@samsung.com> <1466140609-5773-1-git-send-email-sw0312.kim@samsung.com> In-reply-to: <1466140609-5773-1-git-send-email-sw0312.kim@samsung.com> Content-type: text/plain; charset=windows-1252 Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFvrDLMWRmVeSWpSXmKPExsWyRsSkUNcsLD3cYF0Hs8XlXXPYLI7872e0 mDH5JZvF8bXhDiwed67tYfPo27KK0ePzJrkA5igum5TUnMyy1CJ9uwSujCnXOpkKNstXzL/4 lbmBcY1kFyMnh4SAiUR75xZGCFtM4sK99WxdjFwcQgIrGCWWPp7JDlN0ofEcWJGQwCxGiUVz pSGKHjBKnFzzAyzBK6Ah0fJzOTOIzSKgKvH90nswm01AR2L7t+NMILaoQJjEg3V7WSHqBSV+ TL7HAmKLCNRKvDmxFCwuLOAv8XFWGzPEgmuMEn03msCu4BRwk1jy7DPQMg4OZgE9ifsXtUDC zALyEpvXvAWrlxCYxy5xdv4xdogjBCS+TT7EAlIvISArsekAM8QzkhIHV9xgmcAoOgvJGbMQ ps5CMnUBI/MqRtHUguSC4qT0IiO94sTc4tK8dL3k/NxNjMBIOf3vWd8OxpsHrA8xCnAwKvHw WpxNCxdiTSwrrsw9xGgKdMREZinR5HxgPOaVxBsamxlZmJqYGhuZW5opifMmSP0MFhJITyxJ zU5NLUgtii8qzUktPsTIxMEp1cCYn/z9DPvqRq2PxlGLj4hfrpnRIR5+qE1ROqX/+555TTLL 2SrWdyXMu5B81uqR9VpdY6fgu4dedG408ytnWSoTkOdu3Nx9Ia7Q7WF7vsBrK/edubODb28N Pqp+RuEf27bDUra8X7anqMzRmZnQ5bZlYnPOvKlLLfsP67V95WlXVtz0NfLx7F9KLMUZiYZa zEXFiQCt2Rb6jwIAAA== X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFjrJIsWRmVeSWpSXmKPExsVy+t9jQV3TsPRwg6cN1haXd81hszjyv5/R Ysbkl2wWx9eGO7B43Lm2h82jb8sqRo/Pm+QCmKMaGG0yUhNTUosUUvOS81My89JtlbyD453j Tc0MDHUNLS3MlRTyEnNTbZVcfAJ03TJzgLYpKZQl5pQChQISi4uV9O0wTQgNcdO1gGmM0PUN CYLrMTJAAwlrGDOmXOtkKtgsXzH/4lfmBsY1kl2MnBwSAiYSFxrPMULYYhIX7q1nA7GFBGYx SiyaK93FyAVkP2CUOLnmB1gRr4CGRMvP5cwgNouAqsT3S+/BbDYBHYnt344zgdiiAmESD9bt ZYWoF5T4MfkeC4gtIlAr8ebEUrC4sIC/xMdZbcwQC64xSvTdaGIHSXAKuEksefYZaBkHB7OA nsT9i1ogYWYBeYnNa94yT2Dkn4Vk7CyEqllIqhYwMq9ilEgtSC4oTkrPNcxLLdcrTswtLs1L 10vOz93ECI7GZ1I7GA/ucj/EKMDBqMTDK+CQHi7EmlhWXJl7iFGCg1lJhHd6IFCINyWxsiq1 KD++qDQntfgQoynQ3xOZpUST84GJIq8k3tDYxMzI0sjc0MLI2FxJnPfx/3VhQgLpiSWp2amp BalFMH1MHJxSDYwM3dWiQlPTQ672GfCzKn+aqHMoqz9PImf137bkzwxp6sysZysV1xQxOaiF GK7yTP77b2H7FK8VtXlNHOZJO11ncL9gyWP0uu3OLPTtxRpN6a3e9eUBrjkmkgv35prsn7Tu 3h67z256O1b2fHjVuSY0uV6T9fikhD6eV1KtUTzB004GmFzlVGIpzkg01GIuKk4EAGMh9dXc AgAA DLP-Filter: Pass X-MTR: 20000000000000000@CPGS X-CFilter-Loop: Reflected Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi SeungWoo, On 06/17/2016 02:16 PM, Seung-Woo Kim wrote: > This patch removes following UBSAN warnings in dw_mci_setup_bus(). > > UBSAN: Undefined behaviour in drivers/mmc/host/dw_mmc.c:1102:14 > shift exponent 250 is too large for 32-bit type 'unsigned int' > Call trace: > [] dump_backtrace+0x0/0x380 > [] show_stack+0x14/0x20 > [] dump_stack+0xe0/0x120 > [] ubsan_epilogue+0x18/0x68 > [] __ubsan_handle_shift_out_of_bounds+0x18c/0x1bc > [] dw_mci_setup_bus+0x3a0/0x438 > [...] > > UBSAN: Undefined behaviour in drivers/mmc/host/dw_mmc.c:1132:27 > shift exponent 250 is too large for 32-bit type 'unsigned int' > Call trace: > [] dump_backtrace+0x0/0x380 > [] show_stack+0x14/0x20 > [] dump_stack+0xe0/0x120 > [] ubsan_epilogue+0x18/0x68 > [] __ubsan_handle_shift_out_of_bounds+0x18c/0x1bc > [] dw_mci_setup_bus+0x384/0x438 > [...] > > The warnings are caused because of shift with more than 31 on 32 > bit variable, so this patch fixes to keep both clock and divider > instead of shift. > > Signed-off-by: Seung-Woo Kim > --- > drivers/mmc/host/dw_mmc.c | 8 +++++--- > drivers/mmc/host/dw_mmc.h | 8 +++++--- > 2 files changed, 10 insertions(+), 6 deletions(-) > > diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c > index 2cc6123..d05c8cc 100644 > --- a/drivers/mmc/host/dw_mmc.c > +++ b/drivers/mmc/host/dw_mmc.c > @@ -1099,7 +1099,8 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot, bool force_clkinit) > > div = (host->bus_hz != clock) ? DIV_ROUND_UP(div, 2) : 0; > > - if ((clock << div) != slot->__clk_old || force_clkinit) > + if (clock != slot->__clk_old || div != slot->__div_old || > + force_clkinit) I have realized why we put this condition..after checking your below codes. This was patch to avoid the spamming for CONFIG_MMC_CLKGATE.. but CONFIG_MMC_CLKAGET didn't use anymore.. I think we can remove this condition. How about? Best Regards, Jaehoon Chung > dev_info(&slot->mmc->class_dev, > "Bus speed (slot %d) = %dHz (slot req %dHz, actual %dHZ div = %d)\n", > slot->id, host->bus_hz, clock, > @@ -1128,8 +1129,9 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot, bool force_clkinit) > /* inform CIU */ > mci_send_cmd(slot, sdmmc_cmd_bits, 0); > > - /* keep the clock with reflecting clock dividor */ > - slot->__clk_old = clock << div; > + /* keep the clock and clock divider */ > + slot->__clk_old = clock; > + slot->__div_old = div; > } > > host->current_speed = clock; > diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h > index 1e8d838..fdfc3f5 100644 > --- a/drivers/mmc/host/dw_mmc.h > +++ b/drivers/mmc/host/dw_mmc.h > @@ -245,9 +245,10 @@ extern int dw_mci_resume(struct dw_mci *host); > * @queue_node: List node for placing this node in the @queue list of > * &struct dw_mci. > * @clock: Clock rate configured by set_ios(). Protected by host->lock. > - * @__clk_old: The last updated clock with reflecting clock divider. > - * Keeping track of this helps us to avoid spamming the console > - * with CONFIG_MMC_CLKGATE. > + * @__clk_old: The last updated clock. > + * @__div_old: The last updated clock divider. > + * Keeping track of clock and clock divider helps us to avoid spamming > + * the console with CONFIG_MMC_CLKGATE. > * @flags: Random state bits associated with the slot. > * @id: Number of this slot. > * @sdio_id: Number of this slot in the SDIO interrupt registers. > @@ -263,6 +264,7 @@ struct dw_mci_slot { > > unsigned int clock; > unsigned int __clk_old; > + unsigned int __div_old; > > unsigned long flags; > #define DW_MMC_CARD_PRESENT 0 >