linux-kernel - [media-pci-cx25821] question about value overwrite

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 18 May 2017 17:07:09 -0500
From:   "Gustavo A. R. Silva" <garsilva@...eddedor.com>
To:     Mauro Carvalho Chehab <mchehab@...nel.org>
Cc:     linux-media@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: [media-pci-cx25821] question about value overwrite

Hello everybody,

While looking into Coverity ID 1226903 I ran into the following piece  
of code at drivers/media/pci/cx25821/cx25821-medusa-video.c:393:

393int medusa_set_videostandard(struct cx25821_dev *dev)
394{
395        int status = 0;
396        u32 value = 0, tmp = 0;
397
398        if (dev->tvnorm & V4L2_STD_PAL_BG || dev->tvnorm & V4L2_STD_PAL_DK)
399                status = medusa_initialize_pal(dev);
400        else
401                status = medusa_initialize_ntsc(dev);
402
403        /* Enable DENC_A output */
404        value = cx25821_i2c_read(&dev->i2c_bus[0], DENC_A_REG_4, &tmp);
405        value = setBitAtPos(value, 4);
406        status = cx25821_i2c_write(&dev->i2c_bus[0], DENC_A_REG_4, value);
407
408        /* Enable DENC_B output */
409        value = cx25821_i2c_read(&dev->i2c_bus[0], DENC_B_REG_4, &tmp);
410        value = setBitAtPos(value, 4);
411        status = cx25821_i2c_write(&dev->i2c_bus[0], DENC_B_REG_4, value);
412
413        return status;
414}

The issue is that the value stored in variable _status_ at lines 399  
and 401 is overwritten by the one stored at line 406 and then at line  
411, before it can be used.

My question is if the original intention was to ORed the return  
values, something like in the following patch:

index 0a9db05..226d14f 100644
--- a/drivers/media/pci/cx25821/cx25821-medusa-video.c
+++ b/drivers/media/pci/cx25821/cx25821-medusa-video.c
@@ -403,12 +403,12 @@ int medusa_set_videostandard(struct cx25821_dev *dev)
         /* Enable DENC_A output */
         value = cx25821_i2c_read(&dev->i2c_bus[0], DENC_A_REG_4, &tmp);
         value = setBitAtPos(value, 4);
-       status = cx25821_i2c_write(&dev->i2c_bus[0], DENC_A_REG_4, value);
+       status |= cx25821_i2c_write(&dev->i2c_bus[0], DENC_A_REG_4, value);

         /* Enable DENC_B output */
         value = cx25821_i2c_read(&dev->i2c_bus[0], DENC_B_REG_4, &tmp);
         value = setBitAtPos(value, 4);
-       status = cx25821_i2c_write(&dev->i2c_bus[0], DENC_B_REG_4, value);
+       status |= cx25821_i2c_write(&dev->i2c_bus[0], DENC_B_REG_4, value);

         return status;
  }

What do you think?

I'd really appreciate any comment on this.

Thank you!
--
Gustavo A. R. Silva