2014-02-25 10:15:02

by Jiri Slaby

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

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 <[email protected]>
Reported-and-debugged-by: Rui Hui Dian <[email protected]>
Cc: Tomas Cech <[email protected]>
Cc: Corey Minyard <[email protected]>
Cc: <[email protected]>
---
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


2014-02-25 21:50:33

by Corey Minyard

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

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 <[email protected]>
> Reported-and-debugged-by: Rui Hui Dian <[email protected]>
> Cc: Tomas Cech <[email protected]>
> Cc: Corey Minyard <[email protected]>
> Cc: <[email protected]>
> ---
> 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.

2014-04-17 13:21:47

by Torsten Duwe

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

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

2014-04-17 13:33:16

by Corey Minyard

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

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