Re: [PATCH] remove variable length array tuner-xc2028
From: ye janboe
Date: Fri Sep 11 2009 - 05:41:37 EST
thank you for your comments, Jesper
I will update the patch later after I fix the issue gmail wrap my patch:-(
2009年9月11日 Jesper Juhl <jj@xxxxxxxxxxxxx>:
>
On 2009年9月11日, ye janboe wrote:
>
>
> hi
>
>
>
> using variable length array on kernel stack is not safe, so this patch
>
> using kmalloc instead variable length array in tuner-xc2028
>
>
>
> Signed-off-by: janboe <janboe.ye@xxxxxxxxx>
>
> ---
>
> drivers/media/common/tuners/tuner-xc2028.c | 26 ++++++++++++++++----------
>
> 1 files changed, 16 insertions(+), 10 deletions(-)
>
>
>
> diff --git a/drivers/media/common/tuners/tuner-xc2028.c
>
> b/drivers/media/common/tuners/tuner-xc2028.c
>
> index 1adce9f..96259c5 100644
>
> --- a/drivers/media/common/tuners/tuner-xc2028.c
>
> +++ b/drivers/media/common/tuners/tuner-xc2028.c
>
> @@ -516,8 +516,8 @@ static int load_firmware(struct dvb_frontend *fe,
>
> unsigned int type,
>
> v4l2_std_id *id)
>
> {
>
> struct xc2028_data *priv = fe->tuner_priv;
>
> - int pos, rc;
>
> - unsigned char *p, *endp, buf[priv->ctrl.max_len];
>
> + int pos, rc, ret = -EINVAL;
>
> + unsigned char *p, *endp, *buf;
>
>
>
> tuner_dbg("%s called\n", __func__);
>
>
>
> @@ -532,6 +532,7 @@ static int load_firmware(struct dvb_frontend *fe,
>
> unsigned int type,
>
>
>
> p = priv->firm[pos].ptr;
>
> endp = p + priv->firm[pos].size;
>
> + buf = kmalloc(priv->ctrl.max_len, GFP_KERNEL);
>
>
>
>
The call to kmalloc() could fail, so shouldn't we make this
>
>
buf = kmalloc(priv->ctrl.max_len, GFP_KERNEL);
>
if (!buf)
>
return -ENOMEM;
>
>
or?
>
>
>
> while (p < endp) {
>
> __u16 size;
>
> @@ -539,14 +540,16 @@ static int load_firmware(struct dvb_frontend
>
> *fe, unsigned int type,
>
> /* Checks if there's enough bytes to read */
>
> if (p + sizeof(size) > endp) {
>
> tuner_err("Firmware chunk size is wrong\n");
>
> - return -EINVAL;
>
> + goto fail;
>
> }
>
>
>
> size = le16_to_cpu(*(__u16 *) p);
>
> p += sizeof(size);
>
>
>
> - if (size == 0xffff)
>
> - return 0;
>
> + if (size == 0xffff) {
>
> + ret = 0;
>
> + goto fail;
>
> + }
>
>
>
> if (!size) {
>
> /* Special callback command received */
>
> @@ -554,7 +557,7 @@ static int load_firmware(struct dvb_frontend *fe,
>
> unsigned int type,
>
> if (rc < 0) {
>
> tuner_err("Error at RESET code %d\n",
>
> (*p) & 0x7f);
>
> - return -EINVAL;
>
> + goto fail;
>
> }
>
> continue;
>
> }
>
> @@ -565,13 +568,13 @@ static int load_firmware(struct dvb_frontend
>
> *fe, unsigned int type,
>
> if (rc < 0) {
>
> tuner_err("Error at RESET code %d\n",
>
> (*p) & 0x7f);
>
> - return -EINVAL;
>
> + goto fail;
>
> }
>
> break;
>
> default:
>
> tuner_info("Invalid RESET code %d\n",
>
> size & 0x7f);
>
> - return -EINVAL;
>
> + goto fail;
>
>
>
> }
>
> continue;
>
> @@ -586,7 +589,7 @@ static int load_firmware(struct dvb_frontend *fe,
>
> unsigned int type,
>
> if ((size + p > endp)) {
>
> tuner_err("missing bytes: need %d, have %d\n",
>
> size, (int)(endp - p));
>
> - return -EINVAL;
>
> + goto fail;
>
> }
>
>
>
> buf[0] = *p;
>
> @@ -603,7 +606,7 @@ static int load_firmware(struct dvb_frontend *fe,
>
> unsigned int type,
>
> rc = i2c_send(priv, buf, len + 1);
>
> if (rc < 0) {
>
> tuner_err("%d returned from send\n", rc);
>
> - return -EINVAL;
>
> + goto fail;
>
> }
>
>
>
> p += len;
>
> @@ -611,6 +614,9 @@ static int load_firmware(struct dvb_frontend *fe,
>
> unsigned int type,
>
> }
>
> }
>
> return 0;
>
>
Why are we not freeing 'buf' here?
>
Previously it was an array on the stack and would be gone now and I don't
>
see what has changed so that it should live on after we return here.
>
>
>
> +fail:
>
> + kfree(buf);
>
> + return ret;
>
> }
>
>
>
> static int load_scode(struct dvb_frontend *fe, unsigned int type,
>
>
>
--
>
Jesper Juhl <jj@xxxxxxxxxxxxx> http://www.chaosbits.net/
>
Plain text mails only, please http://www.expita.com/nomime.html
>
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at
http://vger.kernel.org/majordomo-info.html
Please read the FAQ at
http://www.tux.org/lkml/