lkml.org 
[lkml]   [2022]   [Jan]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH AUTOSEL 4.4 11/29] media: m920x: don't use stack on USB reads
    Hi!

    > From: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
    >
    > [ Upstream commit a2ab06d7c4d6bfd0b545a768247a70463e977e27 ]
    >
    > Using stack-allocated pointers for USB message data don't work.
    > This driver is almost OK with that, except for the I2C read
    > logic.
    >
    > Fix it by using a temporary read buffer, just like on all other
    > calls to m920x_read().

    This introduces memory leak... and I don't believe it really fixes the
    problem.

    > index eafc5c82467f4..5b806779e2106 100644
    > --- a/drivers/media/usb/dvb-usb/m920x.c
    > +++ b/drivers/media/usb/dvb-usb/m920x.c
    > @@ -284,6 +284,13 @@ static int m920x_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msg[], int nu
    > /* Should check for ack here, if we knew how. */
    > }
    > if (msg[i].flags & I2C_M_RD) {
    > + char *read = kmalloc(1, GFP_KERNEL);
    > + if (!read) {
    > + ret = -ENOMEM;
    > + kfree(read);
    > + goto unlock;
    > + }

    kfree(NULL);

    > for (j = 0; j < msg[i].len; j++) {
    > /* Last byte of transaction?
    > * Send STOP, otherwise send ACK. */
    > @@ -291,9 +298,12 @@ static int m920x_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msg[], int nu
    >
    > if ((ret = m920x_read(d->udev, M9206_I2C, 0x0,
    > 0x20 | stop,
    > - &msg[i].buf[j], 1)) != 0)
    > + read, 1)) != 0)
    > goto unlock;

    Memory leak of read.

    > + msg[i].buf[j] = read[0];
    > }
    > +
    > + kfree(read);
    > } else {
    > for (j = 0; j < msg[i].len; j++) {
    > /* Last byte of transaction? Then send STOP. */

    But more importantly, do we have exact copy of the read problem just
    below, during write?

    Best regards,
    Pavel

    diff --git a/drivers/media/usb/dvb-usb/m920x.c b/drivers/media/usb/dvb-usb/m920x.c
    index 691e05833db1..e5ee54324a28 100644
    --- a/drivers/media/usb/dvb-usb/m920x.c
    +++ b/drivers/media/usb/dvb-usb/m920x.c
    @@ -250,7 +250,7 @@ static int m920x_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msg[], int nu
    {
    struct dvb_usb_device *d = i2c_get_adapdata(adap);
    int i, j;
    - int ret = 0;
    + int ret;

    if (mutex_lock_interruptible(&d->i2c_mutex) < 0)
    return -EAGAIN;
    @@ -277,7 +277,6 @@ static int m920x_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msg[], int nu
    char *read = kmalloc(1, GFP_KERNEL);
    if (!read) {
    ret = -ENOMEM;
    - kfree(read);
    goto unlock;
    }

    @@ -288,8 +287,10 @@ static int m920x_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msg[], int nu

    if ((ret = m920x_read(d->udev, M9206_I2C, 0x0,
    0x20 | stop,
    - read, 1)) != 0)
    + read, 1)) != 0) {
    + kfree(read);
    goto unlock;
    + }
    msg[i].buf[j] = read[0];
    }


    --
    DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
    HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
    [unhandled content-type:application/pgp-signature]
    \
     
     \ /
      Last update: 2022-01-20 11:26    [W:3.810 / U:0.312 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site