From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752653AbdHGImv (ORCPT ); Mon, 7 Aug 2017 04:42:51 -0400 Received: from szxga05-in.huawei.com ([45.249.212.191]:2551 "EHLO szxga05-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752409AbdHGImu (ORCPT ); Mon, 7 Aug 2017 04:42:50 -0400 Subject: Re: [PATCH] ipmi: fix unsigned long underflow To: References: <1501296161-74544-1-git-send-email-chenweilong@huawei.com> <1501381228-30958-1-git-send-email-minyard@acm.org> CC: , , Corey Minyard From: Weilong Chen Message-ID: Date: Mon, 7 Aug 2017 16:41:09 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.0 MIME-Version: 1.0 In-Reply-To: <1501381228-30958-1-git-send-email-minyard@acm.org> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [10.177.216.125] X-CFilter-Loop: Reflected X-Mirapoint-Virus-RAPID-Raw: score=unknown(0), refid=str=0001.0A020205.598827E7.012E,ss=1,re=0.000,recu=0.000,reip=0.000,cl=1,cld=1,fgs=0, ip=0.0.0.0, so=2014-11-16 11:51:01, dmn=2013-03-21 17:37:32 X-Mirapoint-Loop-Id: 8a96286eff4f49c733064c5fd833135d Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Minyard, I test this patch, it works. Thanks. On 2017/7/30 10:20, minyard@acm.org wrote: > From: Corey Minyard > > When I set the timeout to a specific value such as 500ms, the timeout > event will not happen in time due to the overflow in function > check_msg_timeout: > ... > ent->timeout -= timeout_period; > if (ent->timeout > 0) > return; > ... > > The type of timeout_period is long, but ent->timeout is unsigned long. > This patch makes the type consistent. > > Reported-by: Weilong Chen > Signed-off-by: Corey Minyard > --- > I like to keep things consistent (though I obviously messed up here) > and keep variables that should be positive unsigned. > > But you are right, there is a bug here and some inconsistency. > This patch changes timeout_period to be unsigned and fixes the > check. Can you try this out? > > drivers/char/ipmi/ipmi_msghandler.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c > index 810b138..c82d9fd 100644 > --- a/drivers/char/ipmi/ipmi_msghandler.c > +++ b/drivers/char/ipmi/ipmi_msghandler.c > @@ -4030,7 +4030,8 @@ smi_from_recv_msg(ipmi_smi_t intf, struct ipmi_recv_msg *recv_msg, > } > > static void check_msg_timeout(ipmi_smi_t intf, struct seq_table *ent, > - struct list_head *timeouts, long timeout_period, > + struct list_head *timeouts, > + unsigned long timeout_period, > int slot, unsigned long *flags, > unsigned int *waiting_msgs) > { > @@ -4043,8 +4044,8 @@ static void check_msg_timeout(ipmi_smi_t intf, struct seq_table *ent, > if (!ent->inuse) > return; > > - ent->timeout -= timeout_period; > - if (ent->timeout > 0) { > + if (timeout_period < ent->timeout) { > + ent->timeout -= timeout_period; > (*waiting_msgs)++; > return; > } > @@ -4110,7 +4111,8 @@ static void check_msg_timeout(ipmi_smi_t intf, struct seq_table *ent, > } > } > > -static unsigned int ipmi_timeout_handler(ipmi_smi_t intf, long timeout_period) > +static unsigned int ipmi_timeout_handler(ipmi_smi_t intf, > + unsigned long timeout_period) > { > struct list_head timeouts; > struct ipmi_recv_msg *msg, *msg2; >