| Message ID | 7878868c-bc54-5577-b808-ed096bbf3759@users.sourceforge.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Oct 13, 2016 at 06:24:46PM +0200, SF Markus Elfring wrote: > From: Markus Elfring <elfring@users.sourceforge.net> > Date: Thu, 13 Oct 2016 10:50:24 +0200 > > The kfree() function was called in one case by the > redrat3_transmit_ir() function during error handling > even if the passed variable contained a null pointer. > > * Adjust jump targets according to the Linux coding style convention. > > * Move the resetting for the data structure member "transmitting" > at the end. > > * Delete initialisations for the variables "irdata" and "sample_lens" > at the beginning which became unnecessary with this refactoring. > > Signed-off-by: Markus Elfring <elfring@users.sourceforge.net> > --- > drivers/media/rc/redrat3.c | 18 ++++++++---------- > 1 file changed, 8 insertions(+), 10 deletions(-) > > diff --git a/drivers/media/rc/redrat3.c b/drivers/media/rc/redrat3.c > index 7ae2ced..71e901d 100644 > --- a/drivers/media/rc/redrat3.c > +++ b/drivers/media/rc/redrat3.c > @@ -723,10 +723,10 @@ static int redrat3_transmit_ir(struct rc_dev *rcdev, unsigned *txbuf, > { > struct redrat3_dev *rr3 = rcdev->priv; > struct device *dev = rr3->dev; > - struct redrat3_irdata *irdata = NULL; > + struct redrat3_irdata *irdata; > int ret, ret_len; > int lencheck, cur_sample_len, pipe; > - int *sample_lens = NULL; > + int *sample_lens; > u8 curlencheck; > unsigned i, sendbuf_len; > > @@ -747,7 +747,7 @@ static int redrat3_transmit_ir(struct rc_dev *rcdev, unsigned *txbuf, > irdata = kzalloc(sizeof(*irdata), GFP_KERNEL); > if (!irdata) { > ret = -ENOMEM; > - goto out; > + goto free_sample; > } > > /* rr3 will disable rc detector on transmit */ > @@ -776,7 +776,7 @@ static int redrat3_transmit_ir(struct rc_dev *rcdev, unsigned *txbuf, > curlencheck++; > } else { > ret = -EINVAL; > - goto out; > + goto reset_member; > } > } > irdata->sigdata[i] = lencheck; > @@ -811,14 +811,12 @@ static int redrat3_transmit_ir(struct rc_dev *rcdev, unsigned *txbuf, > dev_err(dev, "Error: control msg send failed, rc %d\n", ret); > else > ret = count; > - > -out: > - kfree(irdata); > - kfree(sample_lens); > - > +reset_member: > rr3->transmitting = false; > /* rr3 re-enables rc detector because it was enabled before */ > - > + kfree(irdata); > +free_sample: > + kfree(sample_lens); In this error path, rr3->transmitting is not set to false so now the driver will never allow you transmit again. Also this patch does not apply against latest. Sean > return ret; > } > > -- > 2.10.1 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-media" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
>> diff --git a/drivers/media/rc/redrat3.c b/drivers/media/rc/redrat3.c >> index 7ae2ced..71e901d 100644 >> --- a/drivers/media/rc/redrat3.c >> +++ b/drivers/media/rc/redrat3.c >> @@ -723,10 +723,10 @@ static int redrat3_transmit_ir(struct rc_dev *rcdev, unsigned *txbuf, >> { >> struct redrat3_dev *rr3 = rcdev->priv; >> struct device *dev = rr3->dev; >> - struct redrat3_irdata *irdata = NULL; >> + struct redrat3_irdata *irdata; >> int ret, ret_len; >> int lencheck, cur_sample_len, pipe; >> - int *sample_lens = NULL; >> + int *sample_lens; >> u8 curlencheck; >> unsigned i, sendbuf_len; >> >> @@ -747,7 +747,7 @@ static int redrat3_transmit_ir(struct rc_dev *rcdev, unsigned *txbuf, >> irdata = kzalloc(sizeof(*irdata), GFP_KERNEL); >> if (!irdata) { >> ret = -ENOMEM; >> - goto out; >> + goto free_sample; >> } >> >> /* rr3 will disable rc detector on transmit */ >> @@ -776,7 +776,7 @@ static int redrat3_transmit_ir(struct rc_dev *rcdev, unsigned *txbuf, >> curlencheck++; >> } else { >> ret = -EINVAL; >> - goto out; >> + goto reset_member; >> } >> } >> irdata->sigdata[i] = lencheck; >> @@ -811,14 +811,12 @@ static int redrat3_transmit_ir(struct rc_dev *rcdev, unsigned *txbuf, >> dev_err(dev, "Error: control msg send failed, rc %d\n", ret); >> else >> ret = count; >> - >> -out: >> - kfree(irdata); >> - kfree(sample_lens); >> - >> +reset_member: >> rr3->transmitting = false; >> /* rr3 re-enables rc detector because it was enabled before */ >> - >> + kfree(irdata); >> +free_sample: >> + kfree(sample_lens); > > In this error path, rr3->transmitting is not set to false Can it be that this reset is not needed because it should have still got this value already in the software refactoring I proposed here? > so now the driver will never allow you transmit again. I have got an other impression. > Also this patch does not apply against latest. Do you want that I rebase my update suggestion for this software module on a published commit that is more recent than 2016-09-22 (d6ae162bd13998a6511e5efbc7c19ab542ba1555 for example)? Regards, Markus -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/media/rc/redrat3.c b/drivers/media/rc/redrat3.c index 7ae2ced..71e901d 100644 --- a/drivers/media/rc/redrat3.c +++ b/drivers/media/rc/redrat3.c @@ -723,10 +723,10 @@ static int redrat3_transmit_ir(struct rc_dev *rcdev, unsigned *txbuf, { struct redrat3_dev *rr3 = rcdev->priv; struct device *dev = rr3->dev; - struct redrat3_irdata *irdata = NULL; + struct redrat3_irdata *irdata; int ret, ret_len; int lencheck, cur_sample_len, pipe; - int *sample_lens = NULL; + int *sample_lens; u8 curlencheck; unsigned i, sendbuf_len; @@ -747,7 +747,7 @@ static int redrat3_transmit_ir(struct rc_dev *rcdev, unsigned *txbuf, irdata = kzalloc(sizeof(*irdata), GFP_KERNEL); if (!irdata) { ret = -ENOMEM; - goto out; + goto free_sample; } /* rr3 will disable rc detector on transmit */ @@ -776,7 +776,7 @@ static int redrat3_transmit_ir(struct rc_dev *rcdev, unsigned *txbuf, curlencheck++; } else { ret = -EINVAL; - goto out; + goto reset_member; } } irdata->sigdata[i] = lencheck; @@ -811,14 +811,12 @@ static int redrat3_transmit_ir(struct rc_dev *rcdev, unsigned *txbuf, dev_err(dev, "Error: control msg send failed, rc %d\n", ret); else ret = count; - -out: - kfree(irdata); - kfree(sample_lens); - +reset_member: rr3->transmitting = false; /* rr3 re-enables rc detector because it was enabled before */ - + kfree(irdata); +free_sample: + kfree(sample_lens); return ret; }