Re: [PATCH] Input: hideep - fix the uninitialized use in hideep_nvm_unlock()

Started by Dmitry Torokhov on Jun 20, 2021 · 4 messages from 2 authors · Last reply Jun 20, 2021

open the first message on its own page

Dmitry Torokhov

Message details and related content
Open message details and related content
On Wed, Jun 16, 2021 at 03:48:51PM -0700, Yizhuo Zhai wrote:
Inside function hideep_nvm_unlock(), variable "unmask_code" could
be uninitialized if hideep_pgm_r_reg() returns error, however, it
is used in the later if statement after an "and" operation, which
is potentially unsafe.
I think this is pretty sensible, but let's see if the original author
has some comments...
quoted hunk
Signed-off-by: Yizhuo <redacted>
---
 drivers/input/touchscreen/hideep.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/input/touchscreen/hideep.c
b/drivers/input/touchscreen/hideep.c
index ddad4a82a5e5..f860a815b603 100644
--- a/drivers/input/touchscreen/hideep.c
+++ b/drivers/input/touchscreen/hideep.c
@@ -364,9 +364,13 @@ static int hideep_enter_pgm(struct hideep_ts *ts)
 static void hideep_nvm_unlock(struct hideep_ts *ts)
 {
        u32 unmask_code;
+       int ret;

        hideep_pgm_w_reg(ts, HIDEEP_FLASH_CFG, HIDEEP_NVM_SFR_RPAGE);
-       hideep_pgm_r_reg(ts, 0x0000000C, &unmask_code);
+       ret = hideep_pgm_r_reg(ts, 0x0000000C, &unmask_code);
+       if (ret)
+               return ret;
+
        hideep_pgm_w_reg(ts, HIDEEP_FLASH_CFG, HIDEEP_NVM_DEFAULT_PAGE);

        /* make it unprotected code */
@@ -462,7 +466,9 @@ static int hideep_program_nvm(struct hideep_ts *ts,
        u32 addr = 0;
        int error;

-       hideep_nvm_unlock(ts);
+       error = hideep_nvm_unlock(ts);
+       if (error)
+               return error;

        while (ucode_len > 0) {
                xfer_len = min_t(size_t, ucode_len, HIDEEP_NVM_PAGE_SIZE);
-- 
2.17.1
-- 
Dmitry

Dmitry Torokhov replied to Dmitry Torokhov

Message details and related content
Open message details and related content
On Sat, Jun 19, 2021 at 10:10:37PM -0700, Dmitry Torokhov wrote:
On Wed, Jun 16, 2021 at 03:48:51PM -0700, Yizhuo Zhai wrote:
quoted
Inside function hideep_nvm_unlock(), variable "unmask_code" could
be uninitialized if hideep_pgm_r_reg() returns error, however, it
is used in the later if statement after an "and" operation, which
is potentially unsafe.
I think this is pretty sensible, but let's see if the original author
has some comments...
I guess not. Oh well...

Applied, thank you.

-- 
Dmitry

Dmitry Torokhov replied to Dmitry Torokhov

Message details and related content
Open message details and related content
On Sat, Jun 19, 2021 at 10:26:40PM -0700, Dmitry Torokhov wrote:
On Sat, Jun 19, 2021 at 10:10:37PM -0700, Dmitry Torokhov wrote:
quoted
On Wed, Jun 16, 2021 at 03:48:51PM -0700, Yizhuo Zhai wrote:
quoted
Inside function hideep_nvm_unlock(), variable "unmask_code" could
be uninitialized if hideep_pgm_r_reg() returns error, however, it
is used in the later if statement after an "and" operation, which
is potentially unsafe.
I think this is pretty sensible, but let's see if the original author
has some comments...
I guess not. Oh well...

Applied, thank you.
Note that I had to make some changes to make it compile. Please next
time try building your changes before posting them,

Thanks.

-- 
Dmitry

Yizhuo Zhai replied to Dmitry Torokhov

Message details and related content
Open message details and related content
Dimitry:

Sorry for the inconvenience, I would build the changes next time.
Thanks for your help : )

On Sat, Jun 19, 2021 at 10:35 PM Dmitry Torokhov
[off-list ref] wrote:
On Sat, Jun 19, 2021 at 10:26:40PM -0700, Dmitry Torokhov wrote:
quoted
On Sat, Jun 19, 2021 at 10:10:37PM -0700, Dmitry Torokhov wrote:
quoted
On Wed, Jun 16, 2021 at 03:48:51PM -0700, Yizhuo Zhai wrote:
quoted
Inside function hideep_nvm_unlock(), variable "unmask_code" could
be uninitialized if hideep_pgm_r_reg() returns error, however, it
is used in the later if statement after an "and" operation, which
is potentially unsafe.
I think this is pretty sensible, but let's see if the original author
has some comments...
I guess not. Oh well...

Applied, thank you.
Note that I had to make some changes to make it compile. Please next
time try building your changes before posting them,

Thanks.

--
Dmitry


-- 
Kind Regards,

Yizhuo Zhai

Computer Science, Graduate Student
University of California, Riverside
Keyboard shortcuts
hback out one level
jnext message in thread / next series-diff position
kprevious message in thread / previous series-diff position
ldrill in
Escclose help / fold thread tree
?toggle this help