[PATCH 1/1] Char: ipmi_bt_sm, fix infinite loop

[PATCH 1/1] Char: ipmi_bt_sm, fix infinite loop

[Jiri Slaby]

In read_all_bytes, we do
  unsigned char i;
...
  bt->read_data[0] = BMC2HOST;
  bt->read_count = bt->read_data[0];
...
  for (i = 1; i <= bt->read_count; i++)
    bt->read_data[i] = BMC2HOST;

If bt->read_data[0] == bt->read_count == 255, we loop infinitely in
the 'for' loop. Make 'i' an 'int' instead of 'char' to get rid of the
overflow and finish the loop after 255 iterations every time.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Reported-and-debugged-by: Rui Hui Dian <rhdian@novell.com>
Cc: Tomas Cech <tcech@suse.cz>
Cc: Corey Minyard <minyard@acm.org>
Cc: <openipmi-developer@lists.sourceforge.net>
---
 drivers/char/ipmi/ipmi_bt_sm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/char/ipmi/ipmi_bt_sm.c b/drivers/char/ipmi/ipmi_bt_sm.c
index f5e4cd7617f6..61e71616689b 100644
--- a/drivers/char/ipmi/ipmi_bt_sm.c
+++ b/drivers/char/ipmi/ipmi_bt_sm.c
@@ -352,7 +352,7 @@ static inline void write_all_bytes(struct si_sm_data *bt)
 
 static inline int read_all_bytes(struct si_sm_data *bt)
 {
-	unsigned char i;
+	unsigned int i;
 
 	/*
 	 * length is "framing info", minimum = 4: NetFn, Seq, Cmd, cCode.
-- 
1.9.0

Re: [PATCH 1/1] Char: ipmi_bt_sm, fix infinite loop

[Corey Minyard]

Looks right to me.  Rocky, copying you in case there's an issue with this.

Thanks,

-corey

On 02/25/2014 04:14 AM, Jiri Slaby wrote:
> In read_all_bytes, we do
>   unsigned char i;
> ...
>   bt->read_data[0] = BMC2HOST;
>   bt->read_count = bt->read_data[0];
> ...
>   for (i = 1; i <= bt->read_count; i++)
>     bt->read_data[i] = BMC2HOST;
>
> If bt->read_data[0] == bt->read_count == 255, we loop infinitely in
> the 'for' loop. Make 'i' an 'int' instead of 'char' to get rid of the
> overflow and finish the loop after 255 iterations every time.
>
> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> Reported-and-debugged-by: Rui Hui Dian <rhdian@novell.com>
> Cc: Tomas Cech <tcech@suse.cz>
> Cc: Corey Minyard <minyard@acm.org>
> Cc: <openipmi-developer@lists.sourceforge.net>
> ---
>  drivers/char/ipmi/ipmi_bt_sm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/char/ipmi/ipmi_bt_sm.c b/drivers/char/ipmi/ipmi_bt_sm.c
> index f5e4cd7617f6..61e71616689b 100644
> --- a/drivers/char/ipmi/ipmi_bt_sm.c
> +++ b/drivers/char/ipmi/ipmi_bt_sm.c
> @@ -352,7 +352,7 @@ static inline void write_all_bytes(struct si_sm_data *bt)
>  
>  static inline int read_all_bytes(struct si_sm_data *bt)
>  {
> -	unsigned char i;
> +	unsigned int i;
>  
>  	/*
>  	 * length is "framing info", minimum = 4: NetFn, Seq, Cmd, cCode.

Re: [PATCH 1/1] Char: ipmi_bt_sm, fix infinite loop

[Jiri Slaby]

On 02/25/2014 10:50 PM, Corey Minyard wrote:
> Looks right to me.  Rocky, copying you in case there's an issue with this.

Hi,

any updates here, I don't see it in the -next tree yet?

Thanks.

> On 02/25/2014 04:14 AM, Jiri Slaby wrote:
>> In read_all_bytes, we do
>>   unsigned char i;
>> ...
>>   bt->read_data[0] = BMC2HOST;
>>   bt->read_count = bt->read_data[0];
>> ...
>>   for (i = 1; i <= bt->read_count; i++)
>>     bt->read_data[i] = BMC2HOST;
>>
>> If bt->read_data[0] == bt->read_count == 255, we loop infinitely in
>> the 'for' loop. Make 'i' an 'int' instead of 'char' to get rid of the
>> overflow and finish the loop after 255 iterations every time.
>>
>> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
>> Reported-and-debugged-by: Rui Hui Dian <rhdian@novell.com>
>> Cc: Tomas Cech <tcech@suse.cz>
>> Cc: Corey Minyard <minyard@acm.org>
>> Cc: <openipmi-developer@lists.sourceforge.net>
>> ---
>>  drivers/char/ipmi/ipmi_bt_sm.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/char/ipmi/ipmi_bt_sm.c b/drivers/char/ipmi/ipmi_bt_sm.c
>> index f5e4cd7617f6..61e71616689b 100644
>> --- a/drivers/char/ipmi/ipmi_bt_sm.c
>> +++ b/drivers/char/ipmi/ipmi_bt_sm.c
>> @@ -352,7 +352,7 @@ static inline void write_all_bytes(struct si_sm_data *bt)
>>  
>>  static inline int read_all_bytes(struct si_sm_data *bt)
>>  {
>> -	unsigned char i;
>> +	unsigned int i;
>>  
>>  	/*
>>  	 * length is "framing info", minimum = 4: NetFn, Seq, Cmd, cCode.
> 


-- 
js
suse labs

Re: [PATCH 1/1] Char: ipmi_bt_sm, fix infinite loop

[Corey Minyard]

On 03/24/2014 07:24 AM, Jiri Slaby wrote:
> On 02/25/2014 10:50 PM, Corey Minyard wrote:
>> Looks right to me.  Rocky, copying you in case there's an issue with this.
> Hi,
>
> any updates here, I don't see it in the -next tree yet?
>
> Thanks.

I normally don't submit to the -next tree because there is little to
integrate with in the IPMI driver.  Maybe it would be a good idea to
start, though.  If you want to, I can.

Thanks,

-corey

>> On 02/25/2014 04:14 AM, Jiri Slaby wrote:
>>> In read_all_bytes, we do
>>>   unsigned char i;
>>> ...
>>>   bt->read_data[0] = BMC2HOST;
>>>   bt->read_count = bt->read_data[0];
>>> ...
>>>   for (i = 1; i <= bt->read_count; i++)
>>>     bt->read_data[i] = BMC2HOST;
>>>
>>> If bt->read_data[0] == bt->read_count == 255, we loop infinitely in
>>> the 'for' loop. Make 'i' an 'int' instead of 'char' to get rid of the
>>> overflow and finish the loop after 255 iterations every time.
>>>
>>> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
>>> Reported-and-debugged-by: Rui Hui Dian <rhdian@novell.com>
>>> Cc: Tomas Cech <tcech@suse.cz>
>>> Cc: Corey Minyard <minyard@acm.org>
>>> Cc: <openipmi-developer@lists.sourceforge.net>
>>> ---
>>>  drivers/char/ipmi/ipmi_bt_sm.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/char/ipmi/ipmi_bt_sm.c b/drivers/char/ipmi/ipmi_bt_sm.c
>>> index f5e4cd7617f6..61e71616689b 100644
>>> --- a/drivers/char/ipmi/ipmi_bt_sm.c
>>> +++ b/drivers/char/ipmi/ipmi_bt_sm.c
>>> @@ -352,7 +352,7 @@ static inline void write_all_bytes(struct si_sm_data *bt)
>>>  
>>>  static inline int read_all_bytes(struct si_sm_data *bt)
>>>  {
>>> -	unsigned char i;
>>> +	unsigned int i;
>>>  
>>>  	/*
>>>  	 * length is "framing info", minimum = 4: NetFn, Seq, Cmd, cCode.
>

Re: [PATCH 1/1] Char: ipmi_bt_sm, fix infinite loop

[Torsten Duwe]

On Mon, Mar 24, 2014 at 01:59:55PM -0500, Corey Minyard wrote:
> On 03/24/2014 07:24 AM, Jiri Slaby wrote:
> > On 02/25/2014 10:50 PM, Corey Minyard wrote:
> >> Looks right to me.  Rocky, copying you in case there's an issue with this.
> > Hi,
> >
> > any updates here, I don't see it in the -next tree yet?
> >
> > Thanks.
> 
> I normally don't submit to the -next tree because there is little to
> integrate with in the IPMI driver.  Maybe it would be a good idea to
> start, though.  If you want to, I can.

Yes, please! :)
And you may want to send bugfixes towards -stable.

Thanks,

	Torsten

Re: [PATCH 1/1] Char: ipmi_bt_sm, fix infinite loop

[Corey Minyard]

On 04/17/2014 08:21 AM, Torsten Duwe wrote:
> On Mon, Mar 24, 2014 at 01:59:55PM -0500, Corey Minyard wrote:
>> On 03/24/2014 07:24 AM, Jiri Slaby wrote:
>>> On 02/25/2014 10:50 PM, Corey Minyard wrote:
>>>> Looks right to me.  Rocky, copying you in case there's an issue with this.
>>> Hi,
>>>
>>> any updates here, I don't see it in the -next tree yet?
>>>
>>> Thanks.
>> I normally don't submit to the -next tree because there is little to
>> integrate with in the IPMI driver.  Maybe it would be a good idea to
>> start, though.  If you want to, I can.
> Yes, please! :)
> And you may want to send bugfixes towards -stable.

I will definitely do that.

Thanks,

-corey