| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <55fac469-5265-5710-7b0e-17d00271bfc5@mida.se>
Date: Tue, 14 Dec 2021 03:44:19 +0100
From: rkardell <rkardell@...a.se>
To: linux-media@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: About 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@...a.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@...a.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 = ®, .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@...nel.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. */
>>
>
Powered by blists - more mailing lists