lkml.org 
[lkml]   [2021]   [Dec]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectAbout patches
I explicit asked if the patches were ok when I sent them and I got no 
response. Now I received this “This patch is broken on so many ways:”
with comments. Responses like this will certainly not encourage new
members, especially since the comments and changes of the code was not
of high quality. And it certainly discourage me!

I include a copy of my comments sent to the responsible person and
de-register from the group.




Den 2021-12-07 kl. 09:29, skrev rkardell:
>
> Sorry if I created problems for linux-media by pointing to a solution
to a problem that has been in the kernel since the introduction of USB
EHCI. The only PC I have that works with this DVB device is from 2005.
>
> I first asked for comments since C/C++ is not my preferred language
and Linux kernel is not my normal working environment, but I got no
response. I submitted the patches to the best of my knowledge.
>
>
> 1. Your documented way of submitting patches did not work, so I had
to retrieve the patch and send it as an ordinary mail.
>
> 2. If you cant get 1 byte buffer in the kernel, then you wont reach
this code, but yes I could have tested.
>
> 3. I didn’t change the write code, because it was not a problem.
>
> 4. Sorry for that
>
> It is obvious that its the USB transfer that is the problem since USB
is used for the i2c transfer!
>
> I have no way of knowing what combination of chips/drivers are used
and possible side effects of changes. That’s way I preferred to change
the the code this way
>
> From the documentation I found, it’s not the use of pointers on the
stack that is the problem, its the use of buffer on the stack.
>
>
> About your code: you don’t have to free memory that you could not
allocate.
>
> My pc is updated with my patch, so I cant test your solution.
>
>
> I don’t need this type of comment after my effort to point out the
problem that has been in the kernel for such a long time, so I will
de-register from the group.
>
>
>
> Den 2021-12-06 kl. 16:01, skrev Mauro Carvalho Chehab:
>> Hi,
>>
>> This patch is broken on so many ways:
>>
>> 1. your e-mailer converted spaces and tabs into UTF spaces (0xa0);
>> 2. it is not checking if allocation failed;
>> 3. it doesn't fix the write function, which also uses stack;
>> 4. you need your real name on your from and SoB.
>>
>> Besides that, there's no issue on using the stack for I2C transfers.
>> The issue, is actually to use stack for USB transfers.
>>
>> So, the right fix should be, instead, at the bridge driver, and not
>> on tuner. It seems that Mega Sky 580 USB DVB is supportd by m920x
driver.
>> Right?
>>
>> If so, the enclosed patch should fix the issue.
>>
>> Please test.
>>
>> Regards,
>> Mauro
>>
>> Em Thu, 7 Oct 2021 14:29:00 +0200
>> rkardell <rkardell@mida.se> escreveu:
>>
>>> Solve problem with initialization of Mega Sky 580 USB DVB (and other
>>> using mt352), error when reading i2c id.
>>>
>>>
>>> Signed-off-by: rkl099 <rkardell@mida.se>
>>> ---
>>>   drivers/media/tuners/qt1010.c | 6 +++++-
>>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/media/tuners/qt1010.c
b/drivers/media/tuners/qt1010.c
>>> index 3853a3d43..1bc0756f7 100644
>>> --- a/drivers/media/tuners/qt1010.c
>>> +++ b/drivers/media/tuners/qt1010.c
>>> @@ -11,18 +11,22 @@
>>>   /* read single register */
>>>   static int qt1010_readreg(struct qt1010_priv *priv, u8 reg, u8 *val)
>>>   {
>>> +    u8 *b1=kmalloc(1,GFP_KERNEL);
>>>          struct i2c_msg msg[2] = {
>>>                  { .addr = priv->cfg->i2c_address,
>>>                    .flags = 0, .buf = &reg, .len = 1 },
>>>                  { .addr = priv->cfg->i2c_address,
>>> -                 .flags = I2C_M_RD, .buf = val, .len = 1 },
>>> +                 .flags = I2C_M_RD, .buf = b1, .len = 1 },
>>>          };
>>>
>>>          if (i2c_transfer(priv->i2c, msg, 2) != 2) {
>>>                  dev_warn(&priv->i2c->dev, "%s: i2c rd failed
reg=%02x\n",
>>>                                  KBUILD_MODNAME, reg);
>>> +           kfree(b1);
>>>                  return -EREMOTEIO;
>>>          }
>>> +       *val=b1[0];
>>> +       kfree(b1);
>>>          return 0;
>>>   }
>>>
>> Thanks,
>> Mauro
>>
>> [PATCH] media: m920x: don't use stack on USB reads
>>
>> 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().
>>
>> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
>>
>> diff --git a/drivers/media/usb/dvb-usb/m920x.c
b/drivers/media/usb/dvb-usb/m920x.c
>> index 4bb5b82599a7..691e05833db1 100644
>> --- a/drivers/media/usb/dvb-usb/m920x.c
>> +++ b/drivers/media/usb/dvb-usb/m920x.c
>> @@ -274,6 +274,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;
>> +            }
>> +
>>              for (j = 0; j < msg[i].len; j++) {
>>                  /* Last byte of transaction?
>>                   * Send STOP, otherwise send ACK. */
>> @@ -281,9 +288,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;
>> +                msg[i].buf[j] = read[0];
>>              }
>> +
>> +            kfree(read);
>>          } else {
>>              for (j = 0; j < msg[i].len; j++) {
>>                  /* Last byte of transaction? Then send STOP. */
>>
>


\
 
 \ /
  Last update: 2021-12-14 03:45    [W:0.030 / U:0.964 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site