| Message ID | Y9JIGt0HT8mLkUXF@kili (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | ALSA: pci: lx6464es: fix a debug loop | expand |
On Thu, 26 Jan 2023 10:30:02 +0100, Dan Carpenter wrote: > > This loop accidentally reuses the "i" iterator for both the inside and > the outside loop. The value of MAX_STREAM_BUFFER is 5. I believe that > chip->rmh.stat_len is in the 2-12 range. If the value of .stat_len is > 4 or more then it will loop exactly one time, but if it's less then it > is a forever loop. > > Fixes: 8e6320064c33 ("ALSA: lx_core: Remove useless #if 0 .. #endif") > Signed-off-by: Dan Carpenter <error27@gmail.com> > --- > sound/pci/lx6464es/lx_core.c | 12 +++++------- > 1 file changed, 5 insertions(+), 7 deletions(-) > > diff --git a/sound/pci/lx6464es/lx_core.c b/sound/pci/lx6464es/lx_core.c > index d3f58a3d17fb..7c1b380a54c0 100644 > --- a/sound/pci/lx6464es/lx_core.c > +++ b/sound/pci/lx6464es/lx_core.c > @@ -493,13 +493,11 @@ int lx_buffer_ask(struct lx6464es *chip, u32 pipe, int is_capture, > dev_dbg(chip->card->dev, > "CMD_08_ASK_BUFFERS: needed %d, freed %d\n", > *r_needed, *r_freed); > - for (i = 0; i < MAX_STREAM_BUFFER; ++i) { > - for (i = 0; i != chip->rmh.stat_len; ++i) > - dev_dbg(chip->card->dev, > - " stat[%d]: %x, %x\n", i, > - chip->rmh.stat[i], > - chip->rmh.stat[i] & MASK_DATA_SIZE); > - } > + for (i = 0; i < chip->rmh.stat_len; ++i) Judging from the previous lines, the access over MAX_STREAM_BUFFER might be unsafe. So I guess a more safer change would be something like: for (i = 0; i < MAX_STREAM_BUFFER && chip->rmh.stat_len; ++i) Care to resubmit with it? Thanks! Takashi
On Thu, Jan 26, 2023 at 01:53:01PM +0100, Takashi Iwai wrote: > On Thu, 26 Jan 2023 10:30:02 +0100, > Dan Carpenter wrote: > > > > This loop accidentally reuses the "i" iterator for both the inside and > > the outside loop. The value of MAX_STREAM_BUFFER is 5. I believe that > > chip->rmh.stat_len is in the 2-12 range. If the value of .stat_len is > > 4 or more then it will loop exactly one time, but if it's less then it > > is a forever loop. > > > > Fixes: 8e6320064c33 ("ALSA: lx_core: Remove useless #if 0 .. #endif") > > Signed-off-by: Dan Carpenter <error27@gmail.com> > > --- > > sound/pci/lx6464es/lx_core.c | 12 +++++------- > > 1 file changed, 5 insertions(+), 7 deletions(-) > > > > diff --git a/sound/pci/lx6464es/lx_core.c b/sound/pci/lx6464es/lx_core.c > > index d3f58a3d17fb..7c1b380a54c0 100644 > > --- a/sound/pci/lx6464es/lx_core.c > > +++ b/sound/pci/lx6464es/lx_core.c > > @@ -493,13 +493,11 @@ int lx_buffer_ask(struct lx6464es *chip, u32 pipe, int is_capture, > > dev_dbg(chip->card->dev, > > "CMD_08_ASK_BUFFERS: needed %d, freed %d\n", > > *r_needed, *r_freed); > > - for (i = 0; i < MAX_STREAM_BUFFER; ++i) { > > - for (i = 0; i != chip->rmh.stat_len; ++i) > > - dev_dbg(chip->card->dev, > > - " stat[%d]: %x, %x\n", i, > > - chip->rmh.stat[i], > > - chip->rmh.stat[i] & MASK_DATA_SIZE); > > - } > > + for (i = 0; i < chip->rmh.stat_len; ++i) > > Judging from the previous lines, the access over MAX_STREAM_BUFFER > might be unsafe. So I guess a more safer change would be something > like: > > for (i = 0; i < MAX_STREAM_BUFFER && chip->rmh.stat_len; ++i) && i < chip->rmh.stat_len TBH, I'd prefer to just delete all this code since it used be ifdef 0. But I'll resend as you have suggested. regards, dan carpenter
diff --git a/sound/pci/lx6464es/lx_core.c b/sound/pci/lx6464es/lx_core.c index d3f58a3d17fb..7c1b380a54c0 100644 --- a/sound/pci/lx6464es/lx_core.c +++ b/sound/pci/lx6464es/lx_core.c @@ -493,13 +493,11 @@ int lx_buffer_ask(struct lx6464es *chip, u32 pipe, int is_capture, dev_dbg(chip->card->dev, "CMD_08_ASK_BUFFERS: needed %d, freed %d\n", *r_needed, *r_freed); - for (i = 0; i < MAX_STREAM_BUFFER; ++i) { - for (i = 0; i != chip->rmh.stat_len; ++i) - dev_dbg(chip->card->dev, - " stat[%d]: %x, %x\n", i, - chip->rmh.stat[i], - chip->rmh.stat[i] & MASK_DATA_SIZE); - } + for (i = 0; i < chip->rmh.stat_len; ++i) + dev_dbg(chip->card->dev, + " stat[%d]: %x, %x\n", i, + chip->rmh.stat[i], + chip->rmh.stat[i] & MASK_DATA_SIZE); } mutex_unlock(&chip->msg_lock);
This loop accidentally reuses the "i" iterator for both the inside and the outside loop. The value of MAX_STREAM_BUFFER is 5. I believe that chip->rmh.stat_len is in the 2-12 range. If the value of .stat_len is 4 or more then it will loop exactly one time, but if it's less then it is a forever loop. Fixes: 8e6320064c33 ("ALSA: lx_core: Remove useless #if 0 .. #endif") Signed-off-by: Dan Carpenter <error27@gmail.com> --- sound/pci/lx6464es/lx_core.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-)