linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/11] Address some smatch warnings
@ 2021-06-16 12:28 Mauro Carvalho Chehab
  2021-06-16 12:28 ` [PATCH 01/11] media: dvb_ca_en50221: avoid speculation from CA slot Mauro Carvalho Chehab
                   ` (10 more replies)
  0 siblings, 11 replies; 16+ messages in thread
From: Mauro Carvalho Chehab @ 2021-06-16 12:28 UTC (permalink / raw)
  Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, Chen-Yu Tsai,
	Hans Verkuil, Jernej Skrabec, Mauro Carvalho Chehab,
	Maxime Ripard, Yong Deng, linux-arm-kernel, linux-kernel,
	linux-media, linux-sunxi

There are currently a couple of smatch warnings at the media subsystem.

This series fix several of them. The end goal is to reduce smatch warnings
to be close to zero, but there are still some work to be done. I'll likely
submit another round along this week.

Mauro Carvalho Chehab (11):
  media: dvb_ca_en50221: avoid speculation from CA slot
  media: dvb_net: avoid speculation from net slot
  media: dvbdev: fix error logic at dvb_register_device()
  media: sun6i-csi: add a missing return code
  media: saa7134: use more meaninful goto labels
  media: saa7134: fix saa7134_initdev error handling logic
  media: siano: fix device register error path
  media: adv7842: better document EDID block size
  media: ttusb-dec: cleanup an error handling logic
  media: dvb-core: frontend: make GET/SET safer
  media: xilinx: simplify get fourcc logic

 drivers/media/common/siano/smsdvb-main.c      |   4 +
 drivers/media/dvb-core/dvb_ca_en50221.c       |   1 +
 drivers/media/dvb-core/dvb_frontend.c         | 213 ++++++++++--------
 drivers/media/dvb-core/dvb_net.c              |  25 +-
 drivers/media/dvb-core/dvbdev.c               |   3 +
 drivers/media/i2c/adv7842.c                   |  33 ++-
 drivers/media/pci/saa7134/saa7134-core.c      |  39 ++--
 .../platform/sunxi/sun6i-csi/sun6i_video.c    |   4 +-
 drivers/media/platform/xilinx/xilinx-dma.c    |   5 +-
 drivers/media/platform/xilinx/xilinx-vip.c    |   6 +-
 drivers/media/usb/ttusb-dec/ttusb_dec.c       |  25 +-
 11 files changed, 201 insertions(+), 157 deletions(-)

-- 
2.31.1



^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 01/11] media: dvb_ca_en50221: avoid speculation from CA slot
  2021-06-16 12:28 [PATCH 00/11] Address some smatch warnings Mauro Carvalho Chehab
@ 2021-06-16 12:28 ` Mauro Carvalho Chehab
  2021-06-16 12:28 ` [PATCH 02/11] media: dvb_net: avoid speculation from net slot Mauro Carvalho Chehab
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: Mauro Carvalho Chehab @ 2021-06-16 12:28 UTC (permalink / raw)
  Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, Hans Verkuil,
	Mauro Carvalho Chehab, linux-kernel, linux-media

As warned by smatch:
	drivers/media/dvb-core/dvb_ca_en50221.c:1392 dvb_ca_en50221_io_do_ioctl() warn: potential spectre issue 'ca->slot_info' [r] (local cap)

There's a potential of using a CAM ioctl for speculation.

The risk here is minimum, as only a small subset of DVB
boards have CI, with a CAM module installed. Also, exploiting
it would require an user capable of starting a DVB application.

There are probably a lot of easier ways to try to exploit.

Yet, it doesn't harm addressing it.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 drivers/media/dvb-core/dvb_ca_en50221.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/media/dvb-core/dvb_ca_en50221.c b/drivers/media/dvb-core/dvb_ca_en50221.c
index b7e4a3371176..15a08d8c69ef 100644
--- a/drivers/media/dvb-core/dvb_ca_en50221.c
+++ b/drivers/media/dvb-core/dvb_ca_en50221.c
@@ -1386,6 +1386,7 @@ static int dvb_ca_en50221_io_do_ioctl(struct file *file,
 			err = -EINVAL;
 			goto out_unlock;
 		}
+		slot = array_index_nospec(slot, ca->slot_count);
 
 		info->type = CA_CI_LINK;
 		info->flags = 0;
-- 
2.31.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 02/11] media: dvb_net: avoid speculation from net slot
  2021-06-16 12:28 [PATCH 00/11] Address some smatch warnings Mauro Carvalho Chehab
  2021-06-16 12:28 ` [PATCH 01/11] media: dvb_ca_en50221: avoid speculation from CA slot Mauro Carvalho Chehab
@ 2021-06-16 12:28 ` Mauro Carvalho Chehab
  2021-06-16 12:28 ` [PATCH 03/11] media: dvbdev: fix error logic at dvb_register_device() Mauro Carvalho Chehab
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: Mauro Carvalho Chehab @ 2021-06-16 12:28 UTC (permalink / raw)
  Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab,
	Gustavo A. R. Silva, Mauro Carvalho Chehab, linux-kernel,
	linux-media

The risk of especulation is actually almost-non-existing here,
as there are very few users of TCP/IP using the DVB stack,
as, this is mainly used with DVB-S/S2 cards, and only by people
that receives TCP/IP from satellite connections, which limits
a lot the number of users of such feature(*).

(*) In thesis, DVB-C cards could also benefit from it, but I'm
yet to see a hardware that supports it.

Yet, fixing it is trivial.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 drivers/media/dvb-core/dvb_net.c | 25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/drivers/media/dvb-core/dvb_net.c b/drivers/media/dvb-core/dvb_net.c
index 89620da983ba..dddebea644bb 100644
--- a/drivers/media/dvb-core/dvb_net.c
+++ b/drivers/media/dvb-core/dvb_net.c
@@ -45,6 +45,7 @@
 #include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/netdevice.h>
+#include <linux/nospec.h>
 #include <linux/etherdevice.h>
 #include <linux/dvb/net.h>
 #include <linux/uio.h>
@@ -1462,14 +1463,20 @@ static int dvb_net_do_ioctl(struct file *file,
 		struct net_device *netdev;
 		struct dvb_net_priv *priv_data;
 		struct dvb_net_if *dvbnetif = parg;
+		int if_num = dvbnetif->if_num;
 
-		if (dvbnetif->if_num >= DVB_NET_DEVICES_MAX ||
-		    !dvbnet->state[dvbnetif->if_num]) {
+		if (if_num >= DVB_NET_DEVICES_MAX) {
 			ret = -EINVAL;
 			goto ioctl_error;
 		}
+		if_num = array_index_nospec(if_num, DVB_NET_DEVICES_MAX);
 
-		netdev = dvbnet->device[dvbnetif->if_num];
+		if (!dvbnet->state[if_num]) {
+			ret = -EINVAL;
+			goto ioctl_error;
+		}
+
+		netdev = dvbnet->device[if_num];
 
 		priv_data = netdev_priv(netdev);
 		dvbnetif->pid=priv_data->pid;
@@ -1522,14 +1529,20 @@ static int dvb_net_do_ioctl(struct file *file,
 		struct net_device *netdev;
 		struct dvb_net_priv *priv_data;
 		struct __dvb_net_if_old *dvbnetif = parg;
+		int if_num = dvbnetif->if_num;
 
-		if (dvbnetif->if_num >= DVB_NET_DEVICES_MAX ||
-		    !dvbnet->state[dvbnetif->if_num]) {
+		if (if_num >= DVB_NET_DEVICES_MAX) {
 			ret = -EINVAL;
 			goto ioctl_error;
 		}
+		if_num = array_index_nospec(if_num, DVB_NET_DEVICES_MAX);
 
-		netdev = dvbnet->device[dvbnetif->if_num];
+		if (!dvbnet->state[if_num]) {
+			ret = -EINVAL;
+			goto ioctl_error;
+		}
+
+		netdev = dvbnet->device[if_num];
 
 		priv_data = netdev_priv(netdev);
 		dvbnetif->pid=priv_data->pid;
-- 
2.31.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 03/11] media: dvbdev: fix error logic at dvb_register_device()
  2021-06-16 12:28 [PATCH 00/11] Address some smatch warnings Mauro Carvalho Chehab
  2021-06-16 12:28 ` [PATCH 01/11] media: dvb_ca_en50221: avoid speculation from CA slot Mauro Carvalho Chehab
  2021-06-16 12:28 ` [PATCH 02/11] media: dvb_net: avoid speculation from net slot Mauro Carvalho Chehab
@ 2021-06-16 12:28 ` Mauro Carvalho Chehab
  2021-06-16 12:28 ` [PATCH 04/11] media: sun6i-csi: add a missing return code Mauro Carvalho Chehab
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: Mauro Carvalho Chehab @ 2021-06-16 12:28 UTC (permalink / raw)
  Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, Dinghao Liu,
	Mauro Carvalho Chehab, Peilin Ye, Sean Young, linux-kernel,
	linux-media

As reported by smatch:

	drivers/media/dvb-core/dvbdev.c: drivers/media/dvb-core/dvbdev.c:510 dvb_register_device() warn: '&dvbdev->list_head' not removed from list
	drivers/media/dvb-core/dvbdev.c: drivers/media/dvb-core/dvbdev.c:530 dvb_register_device() warn: '&dvbdev->list_head' not removed from list
	drivers/media/dvb-core/dvbdev.c: drivers/media/dvb-core/dvbdev.c:545 dvb_register_device() warn: '&dvbdev->list_head' not removed from list

The error logic inside dvb_register_device() doesn't remove
devices from the dvb_adapter_list in case of errors.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 drivers/media/dvb-core/dvbdev.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/media/dvb-core/dvbdev.c b/drivers/media/dvb-core/dvbdev.c
index 3862ddc86ec4..795d9bfaba5c 100644
--- a/drivers/media/dvb-core/dvbdev.c
+++ b/drivers/media/dvb-core/dvbdev.c
@@ -506,6 +506,7 @@ int dvb_register_device(struct dvb_adapter *adap, struct dvb_device **pdvbdev,
 			break;
 
 	if (minor == MAX_DVB_MINORS) {
+		list_del (&dvbdev->list_head);
 		kfree(dvbdevfops);
 		kfree(dvbdev);
 		up_write(&minor_rwsem);
@@ -526,6 +527,7 @@ int dvb_register_device(struct dvb_adapter *adap, struct dvb_device **pdvbdev,
 		      __func__);
 
 		dvb_media_device_free(dvbdev);
+		list_del (&dvbdev->list_head);
 		kfree(dvbdevfops);
 		kfree(dvbdev);
 		mutex_unlock(&dvbdev_register_lock);
@@ -541,6 +543,7 @@ int dvb_register_device(struct dvb_adapter *adap, struct dvb_device **pdvbdev,
 		pr_err("%s: failed to create device dvb%d.%s%d (%ld)\n",
 		       __func__, adap->num, dnames[type], id, PTR_ERR(clsdev));
 		dvb_media_device_free(dvbdev);
+		list_del (&dvbdev->list_head);
 		kfree(dvbdevfops);
 		kfree(dvbdev);
 		return PTR_ERR(clsdev);
-- 
2.31.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 04/11] media: sun6i-csi: add a missing return code
  2021-06-16 12:28 [PATCH 00/11] Address some smatch warnings Mauro Carvalho Chehab
                   ` (2 preceding siblings ...)
  2021-06-16 12:28 ` [PATCH 03/11] media: dvbdev: fix error logic at dvb_register_device() Mauro Carvalho Chehab
@ 2021-06-16 12:28 ` Mauro Carvalho Chehab
  2021-06-17  5:37   ` yong
  2021-06-16 12:28 ` [PATCH 05/11] media: saa7134: use more meaninful goto labels Mauro Carvalho Chehab
                   ` (6 subsequent siblings)
  10 siblings, 1 reply; 16+ messages in thread
From: Mauro Carvalho Chehab @ 2021-06-16 12:28 UTC (permalink / raw)
  Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, Chen-Yu Tsai,
	Jernej Skrabec, Mauro Carvalho Chehab, Maxime Ripard, Yong Deng,
	linux-arm-kernel, linux-kernel, linux-media, linux-sunxi

As pointed by smatch, there's a missing return code:

	drivers/media/platform/sunxi/sun6i-csi/sun6i_video.c:485 sun6i_video_open() warn: missing error code 'ret'

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 drivers/media/platform/sunxi/sun6i-csi/sun6i_video.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_video.c b/drivers/media/platform/sunxi/sun6i-csi/sun6i_video.c
index 3181d0781b61..07b2161392d2 100644
--- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_video.c
+++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_video.c
@@ -481,8 +481,10 @@ static int sun6i_video_open(struct file *file)
 		goto fh_release;
 
 	/* check if already powered */
-	if (!v4l2_fh_is_singular_file(file))
+	if (!v4l2_fh_is_singular_file(file)) {
+		ret = -EBUSY;
 		goto unlock;
+	}
 
 	ret = sun6i_csi_set_power(video->csi, true);
 	if (ret < 0)
-- 
2.31.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 05/11] media: saa7134: use more meaninful goto labels
  2021-06-16 12:28 [PATCH 00/11] Address some smatch warnings Mauro Carvalho Chehab
                   ` (3 preceding siblings ...)
  2021-06-16 12:28 ` [PATCH 04/11] media: sun6i-csi: add a missing return code Mauro Carvalho Chehab
@ 2021-06-16 12:28 ` Mauro Carvalho Chehab
  2021-06-16 12:28 ` [PATCH 06/11] media: saa7134: fix saa7134_initdev error handling logic Mauro Carvalho Chehab
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: Mauro Carvalho Chehab @ 2021-06-16 12:28 UTC (permalink / raw)
  Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, Hans Verkuil,
	Julia Lawall, Mauro Carvalho Chehab, Tasos Sahanidis,
	Vaibhav Gupta, Yang Yingliang, linux-kernel, linux-media

Instead of just numbering fail0 to fail4, use more meaninful
goto labels.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 drivers/media/pci/saa7134/saa7134-core.c | 34 ++++++++++++------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/media/pci/saa7134/saa7134-core.c b/drivers/media/pci/saa7134/saa7134-core.c
index ec8dd41f9ebb..97b1767f1fff 100644
--- a/drivers/media/pci/saa7134/saa7134-core.c
+++ b/drivers/media/pci/saa7134/saa7134-core.c
@@ -1031,7 +1031,7 @@ static int saa7134_initdev(struct pci_dev *pci_dev,
 	dev->media_dev = kzalloc(sizeof(*dev->media_dev), GFP_KERNEL);
 	if (!dev->media_dev) {
 		err = -ENOMEM;
-		goto fail0;
+		goto err_free_dev;
 	}
 	media_device_pci_init(dev->media_dev, pci_dev, dev->name);
 	dev->v4l2_dev.mdev = dev->media_dev;
@@ -1039,13 +1039,13 @@ static int saa7134_initdev(struct pci_dev *pci_dev,
 
 	err = v4l2_device_register(&pci_dev->dev, &dev->v4l2_dev);
 	if (err)
-		goto fail0;
+		goto err_free_dev;
 
 	/* pci init */
 	dev->pci = pci_dev;
 	if (pci_enable_device(pci_dev)) {
 		err = -EIO;
-		goto fail1;
+		goto err_v4l2_unregister;
 	}
 
 	/* pci quirks */
@@ -1095,7 +1095,7 @@ static int saa7134_initdev(struct pci_dev *pci_dev,
 	err = pci_set_dma_mask(pci_dev, DMA_BIT_MASK(32));
 	if (err) {
 		pr_warn("%s: Oops: no 32bit PCI DMA ???\n", dev->name);
-		goto fail1;
+		goto err_v4l2_unregister;
 	}
 
 	/* board config */
@@ -1129,7 +1129,7 @@ static int saa7134_initdev(struct pci_dev *pci_dev,
 		err = -EBUSY;
 		pr_err("%s: can't get MMIO memory @ 0x%llx\n",
 		       dev->name,(unsigned long long)pci_resource_start(pci_dev,0));
-		goto fail1;
+		goto err_v4l2_unregister;
 	}
 	dev->lmmio = ioremap(pci_resource_start(pci_dev, 0),
 			     pci_resource_len(pci_dev, 0));
@@ -1138,7 +1138,7 @@ static int saa7134_initdev(struct pci_dev *pci_dev,
 		err = -EIO;
 		pr_err("%s: can't ioremap() MMIO memory\n",
 		       dev->name);
-		goto fail2;
+		goto err_release_mem_reg;
 	}
 
 	/* initialize hardware #1 */
@@ -1151,7 +1151,7 @@ static int saa7134_initdev(struct pci_dev *pci_dev,
 	if (err < 0) {
 		pr_err("%s: can't get IRQ %d\n",
 		       dev->name,pci_dev->irq);
-		goto fail3;
+		goto err_iounmap;
 	}
 
 	/* wait a bit, register i2c bus */
@@ -1217,7 +1217,7 @@ static int saa7134_initdev(struct pci_dev *pci_dev,
 	if (err < 0) {
 		pr_info("%s: can't register video device\n",
 		       dev->name);
-		goto fail4;
+		goto err_unregister_video;
 	}
 	pr_info("%s: registered device %s [v4l2]\n",
 	       dev->name, video_device_node_name(dev->video_dev));
@@ -1234,7 +1234,7 @@ static int saa7134_initdev(struct pci_dev *pci_dev,
 	err = video_register_device(dev->vbi_dev,VFL_TYPE_VBI,
 				    vbi_nr[dev->nr]);
 	if (err < 0)
-		goto fail4;
+		goto err_unregister_video;
 	pr_info("%s: registered device %s\n",
 	       dev->name, video_device_node_name(dev->vbi_dev));
 
@@ -1248,7 +1248,7 @@ static int saa7134_initdev(struct pci_dev *pci_dev,
 		err = video_register_device(dev->radio_dev,VFL_TYPE_RADIO,
 					    radio_nr[dev->nr]);
 		if (err < 0)
-			goto fail4;
+			goto err_unregister_video;
 		pr_info("%s: registered device %s\n",
 		       dev->name, video_device_node_name(dev->radio_dev));
 	}
@@ -1259,7 +1259,7 @@ static int saa7134_initdev(struct pci_dev *pci_dev,
 	err = v4l2_mc_create_media_graph(dev->media_dev);
 	if (err) {
 		pr_err("failed to create media graph\n");
-		goto fail4;
+		goto err_unregister_video;
 	}
 #endif
 	/* everything worked */
@@ -1278,24 +1278,24 @@ static int saa7134_initdev(struct pci_dev *pci_dev,
 #ifdef CONFIG_MEDIA_CONTROLLER
 	err = media_device_register(dev->media_dev);
 	if (err)
-		goto fail4;
+		goto err_unregister_video;
 #endif
 
 	return 0;
 
- fail4:
+err_unregister_video:
 	saa7134_unregister_video(dev);
 	saa7134_i2c_unregister(dev);
 	free_irq(pci_dev->irq, dev);
- fail3:
+err_iounmap:
 	saa7134_hwfini(dev);
 	iounmap(dev->lmmio);
- fail2:
+err_release_mem_reg:
 	release_mem_region(pci_resource_start(pci_dev,0),
 			   pci_resource_len(pci_dev,0));
- fail1:
+err_v4l2_unregister:
 	v4l2_device_unregister(&dev->v4l2_dev);
- fail0:
+err_free_dev:
 #ifdef CONFIG_MEDIA_CONTROLLER
 	kfree(dev->media_dev);
 #endif
-- 
2.31.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 06/11] media: saa7134: fix saa7134_initdev error handling logic
  2021-06-16 12:28 [PATCH 00/11] Address some smatch warnings Mauro Carvalho Chehab
                   ` (4 preceding siblings ...)
  2021-06-16 12:28 ` [PATCH 05/11] media: saa7134: use more meaninful goto labels Mauro Carvalho Chehab
@ 2021-06-16 12:28 ` Mauro Carvalho Chehab
  2021-06-16 12:28 ` [PATCH 07/11] media: siano: fix device register error path Mauro Carvalho Chehab
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: Mauro Carvalho Chehab @ 2021-06-16 12:28 UTC (permalink / raw)
  Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, Hans Verkuil,
	Julia Lawall, Mauro Carvalho Chehab, Tasos Sahanidis,
	Vaibhav Gupta, Yang Yingliang, linux-kernel, linux-media

Smatch reported an issue there:
	drivers/media/pci/saa7134/saa7134-core.c:1302 saa7134_initdev() warn: '&dev->devlist' not removed from list

But besides freeing the list, the media controller graph also
needs to be cleaned up on errors. Address those issues.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 drivers/media/pci/saa7134/saa7134-core.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/media/pci/saa7134/saa7134-core.c b/drivers/media/pci/saa7134/saa7134-core.c
index 97b1767f1fff..47158ab3956b 100644
--- a/drivers/media/pci/saa7134/saa7134-core.c
+++ b/drivers/media/pci/saa7134/saa7134-core.c
@@ -1277,14 +1277,17 @@ static int saa7134_initdev(struct pci_dev *pci_dev,
 	 */
 #ifdef CONFIG_MEDIA_CONTROLLER
 	err = media_device_register(dev->media_dev);
-	if (err)
+	if (err) {
+		media_device_cleanup(dev->media_dev);
 		goto err_unregister_video;
+	}
 #endif
 
 	return 0;
 
 err_unregister_video:
 	saa7134_unregister_video(dev);
+	list_del(&dev->devlist);
 	saa7134_i2c_unregister(dev);
 	free_irq(pci_dev->irq, dev);
 err_iounmap:
-- 
2.31.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 07/11] media: siano: fix device register error path
  2021-06-16 12:28 [PATCH 00/11] Address some smatch warnings Mauro Carvalho Chehab
                   ` (5 preceding siblings ...)
  2021-06-16 12:28 ` [PATCH 06/11] media: saa7134: fix saa7134_initdev error handling logic Mauro Carvalho Chehab
@ 2021-06-16 12:28 ` Mauro Carvalho Chehab
  2021-06-16 12:28 ` [PATCH 08/11] media: adv7842: better document EDID block size Mauro Carvalho Chehab
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: Mauro Carvalho Chehab @ 2021-06-16 12:28 UTC (permalink / raw)
  Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, Davidlohr Bueso,
	Keita Suzuki, Mauro Carvalho Chehab, Nicolas Stuardo Diaz,
	Sean Young, Ye Bin, linux-kernel, linux-media

As reported by smatch:
	drivers/media/common/siano/smsdvb-main.c:1231 smsdvb_hotplug() warn: '&client->entry' not removed from list

If an error occur at the end of the registration logic, it won't
drop the device from the list.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 drivers/media/common/siano/smsdvb-main.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/media/common/siano/smsdvb-main.c b/drivers/media/common/siano/smsdvb-main.c
index b8a163a47d09..f80caaa333da 100644
--- a/drivers/media/common/siano/smsdvb-main.c
+++ b/drivers/media/common/siano/smsdvb-main.c
@@ -1212,6 +1212,10 @@ static int smsdvb_hotplug(struct smscore_device_t *coredev,
 	return 0;
 
 media_graph_error:
+	mutex_lock(&g_smsdvb_clientslock);
+	list_del(&client->entry);
+	mutex_unlock(&g_smsdvb_clientslock);
+
 	smsdvb_debugfs_release(client);
 
 client_error:
-- 
2.31.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 08/11] media: adv7842: better document EDID block size
  2021-06-16 12:28 [PATCH 00/11] Address some smatch warnings Mauro Carvalho Chehab
                   ` (6 preceding siblings ...)
  2021-06-16 12:28 ` [PATCH 07/11] media: siano: fix device register error path Mauro Carvalho Chehab
@ 2021-06-16 12:28 ` Mauro Carvalho Chehab
  2021-06-16 12:32   ` Hans Verkuil
                     ` (2 more replies)
  2021-06-16 12:28 ` [PATCH 09/11] media: ttusb-dec: cleanup an error handling logic Mauro Carvalho Chehab
                   ` (2 subsequent siblings)
  10 siblings, 3 replies; 16+ messages in thread
From: Mauro Carvalho Chehab @ 2021-06-16 12:28 UTC (permalink / raw)
  Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, Hans Verkuil,
	Mauro Carvalho Chehab, linux-kernel, linux-media

While the logic there is correct, it leads to smatch warnings:
	/home/hans/work/build/media-git/drivers/media/i2c/adv7842.c:2538 adv7842_set_edid() error: memcpy() '&state->vga_edid.edid' too small (128 vs 512)

Because the code tricks static analyzers by doing:
	memcpy(&state->hdmi_edid.edid, e->edid, 128 * e->blocks);

for ADV7842_EDID_PORT_VGA, where a logic before that makes
e->blocks being either 0 or 1.

Yet, it is ugly to see the "128" magic number all spread about the
EDID code. So, while here, replace 128 (and 4 x 128) by macros:

And ensure that the logic which copy into the VGA block
will use EDID_MAX_VGA_BLOCKS.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 drivers/media/i2c/adv7842.c | 33 ++++++++++++++++++++++-----------
 1 file changed, 22 insertions(+), 11 deletions(-)

diff --git a/drivers/media/i2c/adv7842.c b/drivers/media/i2c/adv7842.c
index 78e61fe6f2f0..30bddab320b9 100644
--- a/drivers/media/i2c/adv7842.c
+++ b/drivers/media/i2c/adv7842.c
@@ -85,6 +85,10 @@ struct adv7842_format_info {
 	u8 op_format_sel;
 };
 
+#define EDID_BLOCK_SIZE		128
+#define EDID_MAX_HDMI_BLOCKS	4
+#define EDID_MAX_VGA_BLOCKS	1
+
 struct adv7842_state {
 	struct adv7842_platform_data pdata;
 	struct v4l2_subdev sd;
@@ -98,12 +102,12 @@ struct adv7842_state {
 
 	v4l2_std_id norm;
 	struct {
-		u8 edid[512];
+		u8 edid[EDID_BLOCK_SIZE * EDID_MAX_HDMI_BLOCKS];
 		u32 blocks;
 		u32 present;
 	} hdmi_edid;
 	struct {
-		u8 edid[128];
+		u8 edid[EDID_MAX_VGA_BLOCKS * EDID_MAX_VGA_BLOCKS];
 		u32 blocks;
 		u32 present;
 	} vga_edid;
@@ -732,12 +736,13 @@ static int edid_write_vga_segment(struct v4l2_subdev *sd)
 	/* edid segment pointer '1' for VGA port */
 	rep_write_and_or(sd, 0x77, 0xef, 0x10);
 
-	for (i = 0; !err && i < blocks * 128; i += I2C_SMBUS_BLOCK_MAX)
+	for (i = 0; && i < blocks * EDID_BLOCK_SIZE; i += I2C_SMBUS_BLOCK_MAX) {
 		err = i2c_smbus_write_i2c_block_data(state->i2c_edid, i,
 						     I2C_SMBUS_BLOCK_MAX,
 						     edid + i);
-	if (err)
-		return err;
+		if (err)
+			return err;
+	}
 
 	/* Calculates the checksums and enables I2C access
 	 * to internal EDID ram from VGA DDC port.
@@ -785,7 +790,7 @@ static int edid_write_hdmi_segment(struct v4l2_subdev *sd, u8 port)
 		return 0;
 	}
 
-	pa = v4l2_get_edid_phys_addr(edid, blocks * 128, &spa_loc);
+	pa = v4l2_get_edid_phys_addr(edid, blocks * EDID_BLOCK_SIZE, &spa_loc);
 	err = v4l2_phys_addr_validate(pa, &parent_pa, NULL);
 	if (err)
 		return err;
@@ -800,7 +805,7 @@ static int edid_write_hdmi_segment(struct v4l2_subdev *sd, u8 port)
 	}
 
 
-	for (i = 0; !err && i < blocks * 128; i += I2C_SMBUS_BLOCK_MAX) {
+	for (i = 0; !err && i < blocks * EDID_BLOCK_SIZE; i += I2C_SMBUS_BLOCK_MAX) {
 		/* set edid segment pointer for HDMI ports */
 		if (i % 256 == 0)
 			rep_write_and_or(sd, 0x77, 0xef, i >= 256 ? 0x10 : 0x00);
@@ -2491,7 +2496,9 @@ static int adv7842_get_edid(struct v4l2_subdev *sd, struct v4l2_edid *edid)
 	if (edid->start_block + edid->blocks > blocks)
 		edid->blocks = blocks - edid->start_block;
 
-	memcpy(edid->edid, data + edid->start_block * 128, edid->blocks * 128);
+	memcpy(edid->edid,
+	       data + edid->start_block * EDID_BLOCK_SIZE,
+	       edid->blocks * EDID_BLOCK_SIZE);
 
 	return 0;
 }
@@ -2506,9 +2513,12 @@ static int adv7842_get_edid(struct v4l2_subdev *sd, struct v4l2_edid *edid)
 static int adv7842_set_edid(struct v4l2_subdev *sd, struct v4l2_edid *e)
 {
 	struct adv7842_state *state = to_state(sd);
-	unsigned int max_blocks = e->pad == ADV7842_EDID_PORT_VGA ? 1 : 4;
+	unsigned int max_blocks;
 	int err = 0;
 
+	max_blocks = e->pad == ADV7842_EDID_PORT_VGA ?
+		     EDID_MAX_VGA_BLOCKS  : EDID_MAX_HDMI_BLOCKS;
+
 	memset(e->reserved, 0, sizeof(e->reserved));
 
 	if (e->pad > ADV7842_EDID_PORT_VGA)
@@ -2535,7 +2545,7 @@ static int adv7842_set_edid(struct v4l2_subdev *sd, struct v4l2_edid *e)
 		state->vga_edid.blocks = e->blocks;
 		state->vga_edid.present = e->blocks ? 0x1 : 0x0;
 		if (e->blocks)
-			memcpy(&state->vga_edid.edid, e->edid, 128 * e->blocks);
+			memcpy(&state->vga_edid.edid, e->edid, EDID_BLOCK_SIZE);
 		err = edid_write_vga_segment(sd);
 		break;
 	case ADV7842_EDID_PORT_A:
@@ -2544,7 +2554,8 @@ static int adv7842_set_edid(struct v4l2_subdev *sd, struct v4l2_edid *e)
 		state->hdmi_edid.blocks = e->blocks;
 		if (e->blocks) {
 			state->hdmi_edid.present |= 0x04 << e->pad;
-			memcpy(&state->hdmi_edid.edid, e->edid, 128 * e->blocks);
+			memcpy(&state->hdmi_edid.edid, e->edid,
+			       EDID_BLOCK_SIZE * e->blocks);
 		} else {
 			state->hdmi_edid.present &= ~(0x04 << e->pad);
 			adv7842_s_detect_tx_5v_ctrl(sd);
-- 
2.31.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 09/11] media: ttusb-dec: cleanup an error handling logic
  2021-06-16 12:28 [PATCH 00/11] Address some smatch warnings Mauro Carvalho Chehab
                   ` (7 preceding siblings ...)
  2021-06-16 12:28 ` [PATCH 08/11] media: adv7842: better document EDID block size Mauro Carvalho Chehab
@ 2021-06-16 12:28 ` Mauro Carvalho Chehab
  2021-06-16 12:28 ` [PATCH 10/11] media: dvb-core: frontend: make GET/SET safer Mauro Carvalho Chehab
  2021-06-16 12:28 ` [PATCH 11/11] media: xilinx: simplify get fourcc logic Mauro Carvalho Chehab
  10 siblings, 0 replies; 16+ messages in thread
From: Mauro Carvalho Chehab @ 2021-06-16 12:28 UTC (permalink / raw)
  Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, Allen Pais,
	Hans Verkuil, Mauro Carvalho Chehab, Romain Perier, dingsenjie,
	linux-kernel, linux-media

Simplify the logic at ttusb_dec_send_command().

Besides avoiding some code duplication, as a side effect,
this could remove this false positive return with spatch:

	drivers/media/usb/ttusb-dec/ttusb_dec.c:380 ttusb_dec_send_command() warn: inconsistent returns '&dec->usb_mutex'.
	  Locked on  : 330
	  Unlocked on: 354,365,380

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 drivers/media/usb/ttusb-dec/ttusb_dec.c | 25 +++++++++++--------------
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/drivers/media/usb/ttusb-dec/ttusb_dec.c b/drivers/media/usb/ttusb-dec/ttusb_dec.c
index a852ee5f7ac9..bfda46a36dc5 100644
--- a/drivers/media/usb/ttusb-dec/ttusb_dec.c
+++ b/drivers/media/usb/ttusb-dec/ttusb_dec.c
@@ -324,10 +324,10 @@ static int ttusb_dec_send_command(struct ttusb_dec *dec, const u8 command,
 	if (!b)
 		return -ENOMEM;
 
-	if ((result = mutex_lock_interruptible(&dec->usb_mutex))) {
-		kfree(b);
+	result = mutex_lock_interruptible(&dec->usb_mutex);
+	if (result) {
 		printk("%s: Failed to lock usb mutex.\n", __func__);
-		return result;
+		goto err;
 	}
 
 	b[0] = 0xaa;
@@ -349,9 +349,7 @@ static int ttusb_dec_send_command(struct ttusb_dec *dec, const u8 command,
 	if (result) {
 		printk("%s: command bulk message failed: error %d\n",
 		       __func__, result);
-		mutex_unlock(&dec->usb_mutex);
-		kfree(b);
-		return result;
+		goto err;
 	}
 
 	result = usb_bulk_msg(dec->udev, dec->result_pipe, b,
@@ -360,9 +358,7 @@ static int ttusb_dec_send_command(struct ttusb_dec *dec, const u8 command,
 	if (result) {
 		printk("%s: result bulk message failed: error %d\n",
 		       __func__, result);
-		mutex_unlock(&dec->usb_mutex);
-		kfree(b);
-		return result;
+		goto err;
 	} else {
 		if (debug) {
 			printk(KERN_DEBUG "%s: result: %*ph\n",
@@ -373,12 +369,13 @@ static int ttusb_dec_send_command(struct ttusb_dec *dec, const u8 command,
 			*result_length = b[3];
 		if (cmd_result && b[3] > 0)
 			memcpy(cmd_result, &b[4], b[3]);
-
-		mutex_unlock(&dec->usb_mutex);
-
-		kfree(b);
-		return 0;
 	}
+
+err:
+	mutex_unlock(&dec->usb_mutex);
+
+	kfree(b);
+	return result;
 }
 
 static int ttusb_dec_get_stb_state (struct ttusb_dec *dec, unsigned int *mode,
-- 
2.31.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 10/11] media: dvb-core: frontend: make GET/SET safer
  2021-06-16 12:28 [PATCH 00/11] Address some smatch warnings Mauro Carvalho Chehab
                   ` (8 preceding siblings ...)
  2021-06-16 12:28 ` [PATCH 09/11] media: ttusb-dec: cleanup an error handling logic Mauro Carvalho Chehab
@ 2021-06-16 12:28 ` Mauro Carvalho Chehab
  2021-06-16 12:28 ` [PATCH 11/11] media: xilinx: simplify get fourcc logic Mauro Carvalho Chehab
  10 siblings, 0 replies; 16+ messages in thread
From: Mauro Carvalho Chehab @ 2021-06-16 12:28 UTC (permalink / raw)
  Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab,
	Gustavo A. R. Silva, Hans Verkuil, Mauro Carvalho Chehab,
	linux-kernel, linux-media

The implementation for FE_SET_PROPERTY/FE_GET_PROPERTY has
a debug code that might be explored via spectre.
Improve the logic in order to mitigate such risk.

It should be noticed that, before this patch, the logic
which implements FE_GET_PROPERTY doesn't check the length passed
by the user, which might lead to expose some information. This
is probably not exploitable, though, as the frontend drivers
won't rely on the buffer length value set by userspace, but
it helps to return a valid value back to userspace.

The code was changed to only try to access an array based on
userspace values only when DVB debug is turned on, helping to
reduce the attack surface, as a speculation attack would work
only if DVB dev_dbg() macros are enabled, which is usually
enabled only on test Kernels or by the root user.

As a side effect, a const array size can now be reduced by
~570 bytes, as it now needs to contain just the name of each
DTV command.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 drivers/media/dvb-core/dvb_frontend.c | 213 ++++++++++++++------------
 1 file changed, 113 insertions(+), 100 deletions(-)

diff --git a/drivers/media/dvb-core/dvb_frontend.c b/drivers/media/dvb-core/dvb_frontend.c
index a6915582d1a6..3dccd3af385e 100644
--- a/drivers/media/dvb-core/dvb_frontend.c
+++ b/drivers/media/dvb-core/dvb_frontend.c
@@ -23,6 +23,7 @@
 #include <linux/poll.h>
 #include <linux/semaphore.h>
 #include <linux/module.h>
+#include <linux/nospec.h>
 #include <linux/list.h>
 #include <linux/freezer.h>
 #include <linux/jiffies.h>
@@ -1063,107 +1064,98 @@ static int dvb_frontend_clear_cache(struct dvb_frontend *fe)
 	return 0;
 }
 
-#define _DTV_CMD(n, s, b) \
-[n] = { \
-	.name = #n, \
-	.cmd  = n, \
-	.set  = s,\
-	.buffer = b \
-}
+#define _DTV_CMD(n) \
+	[n] =  #n
 
-struct dtv_cmds_h {
-	char	*name;		/* A display name for debugging purposes */
-
-	__u32	cmd;		/* A unique ID */
-
-	/* Flags */
-	__u32	set:1;		/* Either a set or get property */
-	__u32	buffer:1;	/* Does this property use the buffer? */
-	__u32	reserved:30;	/* Align */
-};
-
-static struct dtv_cmds_h dtv_cmds[DTV_MAX_COMMAND + 1] = {
-	_DTV_CMD(DTV_TUNE, 1, 0),
-	_DTV_CMD(DTV_CLEAR, 1, 0),
+static char *dtv_cmds[DTV_MAX_COMMAND + 1] = {
+	_DTV_CMD(DTV_TUNE),
+	_DTV_CMD(DTV_CLEAR),
 
 	/* Set */
-	_DTV_CMD(DTV_FREQUENCY, 1, 0),
-	_DTV_CMD(DTV_BANDWIDTH_HZ, 1, 0),
-	_DTV_CMD(DTV_MODULATION, 1, 0),
-	_DTV_CMD(DTV_INVERSION, 1, 0),
-	_DTV_CMD(DTV_DISEQC_MASTER, 1, 1),
-	_DTV_CMD(DTV_SYMBOL_RATE, 1, 0),
-	_DTV_CMD(DTV_INNER_FEC, 1, 0),
-	_DTV_CMD(DTV_VOLTAGE, 1, 0),
-	_DTV_CMD(DTV_TONE, 1, 0),
-	_DTV_CMD(DTV_PILOT, 1, 0),
-	_DTV_CMD(DTV_ROLLOFF, 1, 0),
-	_DTV_CMD(DTV_DELIVERY_SYSTEM, 1, 0),
-	_DTV_CMD(DTV_HIERARCHY, 1, 0),
-	_DTV_CMD(DTV_CODE_RATE_HP, 1, 0),
-	_DTV_CMD(DTV_CODE_RATE_LP, 1, 0),
-	_DTV_CMD(DTV_GUARD_INTERVAL, 1, 0),
-	_DTV_CMD(DTV_TRANSMISSION_MODE, 1, 0),
-	_DTV_CMD(DTV_INTERLEAVING, 1, 0),
+	_DTV_CMD(DTV_FREQUENCY),
+	_DTV_CMD(DTV_BANDWIDTH_HZ),
+	_DTV_CMD(DTV_MODULATION),
+	_DTV_CMD(DTV_INVERSION),
+	_DTV_CMD(DTV_DISEQC_MASTER),
+	_DTV_CMD(DTV_SYMBOL_RATE),
+	_DTV_CMD(DTV_INNER_FEC),
+	_DTV_CMD(DTV_VOLTAGE),
+	_DTV_CMD(DTV_TONE),
+	_DTV_CMD(DTV_PILOT),
+	_DTV_CMD(DTV_ROLLOFF),
+	_DTV_CMD(DTV_DELIVERY_SYSTEM),
+	_DTV_CMD(DTV_HIERARCHY),
+	_DTV_CMD(DTV_CODE_RATE_HP),
+	_DTV_CMD(DTV_CODE_RATE_LP),
+	_DTV_CMD(DTV_GUARD_INTERVAL),
+	_DTV_CMD(DTV_TRANSMISSION_MODE),
+	_DTV_CMD(DTV_INTERLEAVING),
 
-	_DTV_CMD(DTV_ISDBT_PARTIAL_RECEPTION, 1, 0),
-	_DTV_CMD(DTV_ISDBT_SOUND_BROADCASTING, 1, 0),
-	_DTV_CMD(DTV_ISDBT_SB_SUBCHANNEL_ID, 1, 0),
-	_DTV_CMD(DTV_ISDBT_SB_SEGMENT_IDX, 1, 0),
-	_DTV_CMD(DTV_ISDBT_SB_SEGMENT_COUNT, 1, 0),
-	_DTV_CMD(DTV_ISDBT_LAYER_ENABLED, 1, 0),
-	_DTV_CMD(DTV_ISDBT_LAYERA_FEC, 1, 0),
-	_DTV_CMD(DTV_ISDBT_LAYERA_MODULATION, 1, 0),
-	_DTV_CMD(DTV_ISDBT_LAYERA_SEGMENT_COUNT, 1, 0),
-	_DTV_CMD(DTV_ISDBT_LAYERA_TIME_INTERLEAVING, 1, 0),
-	_DTV_CMD(DTV_ISDBT_LAYERB_FEC, 1, 0),
-	_DTV_CMD(DTV_ISDBT_LAYERB_MODULATION, 1, 0),
-	_DTV_CMD(DTV_ISDBT_LAYERB_SEGMENT_COUNT, 1, 0),
-	_DTV_CMD(DTV_ISDBT_LAYERB_TIME_INTERLEAVING, 1, 0),
-	_DTV_CMD(DTV_ISDBT_LAYERC_FEC, 1, 0),
-	_DTV_CMD(DTV_ISDBT_LAYERC_MODULATION, 1, 0),
-	_DTV_CMD(DTV_ISDBT_LAYERC_SEGMENT_COUNT, 1, 0),
-	_DTV_CMD(DTV_ISDBT_LAYERC_TIME_INTERLEAVING, 1, 0),
+	_DTV_CMD(DTV_ISDBT_PARTIAL_RECEPTION),
+	_DTV_CMD(DTV_ISDBT_SOUND_BROADCASTING),
+	_DTV_CMD(DTV_ISDBT_SB_SUBCHANNEL_ID),
+	_DTV_CMD(DTV_ISDBT_SB_SEGMENT_IDX),
+	_DTV_CMD(DTV_ISDBT_SB_SEGMENT_COUNT),
+	_DTV_CMD(DTV_ISDBT_LAYER_ENABLED),
+	_DTV_CMD(DTV_ISDBT_LAYERA_FEC),
+	_DTV_CMD(DTV_ISDBT_LAYERA_MODULATION),
+	_DTV_CMD(DTV_ISDBT_LAYERA_SEGMENT_COUNT),
+	_DTV_CMD(DTV_ISDBT_LAYERA_TIME_INTERLEAVING),
+	_DTV_CMD(DTV_ISDBT_LAYERB_FEC),
+	_DTV_CMD(DTV_ISDBT_LAYERB_MODULATION),
+	_DTV_CMD(DTV_ISDBT_LAYERB_SEGMENT_COUNT),
+	_DTV_CMD(DTV_ISDBT_LAYERB_TIME_INTERLEAVING),
+	_DTV_CMD(DTV_ISDBT_LAYERC_FEC),
+	_DTV_CMD(DTV_ISDBT_LAYERC_MODULATION),
+	_DTV_CMD(DTV_ISDBT_LAYERC_SEGMENT_COUNT),
+	_DTV_CMD(DTV_ISDBT_LAYERC_TIME_INTERLEAVING),
 
-	_DTV_CMD(DTV_STREAM_ID, 1, 0),
-	_DTV_CMD(DTV_DVBT2_PLP_ID_LEGACY, 1, 0),
-	_DTV_CMD(DTV_SCRAMBLING_SEQUENCE_INDEX, 1, 0),
-	_DTV_CMD(DTV_LNA, 1, 0),
+	_DTV_CMD(DTV_STREAM_ID),
+	_DTV_CMD(DTV_DVBT2_PLP_ID_LEGACY),
+	_DTV_CMD(DTV_SCRAMBLING_SEQUENCE_INDEX),
+	_DTV_CMD(DTV_LNA),
 
 	/* Get */
-	_DTV_CMD(DTV_DISEQC_SLAVE_REPLY, 0, 1),
-	_DTV_CMD(DTV_API_VERSION, 0, 0),
+	_DTV_CMD(DTV_DISEQC_SLAVE_REPLY),
+	_DTV_CMD(DTV_API_VERSION),
 
-	_DTV_CMD(DTV_ENUM_DELSYS, 0, 0),
+	_DTV_CMD(DTV_ENUM_DELSYS),
 
-	_DTV_CMD(DTV_ATSCMH_PARADE_ID, 1, 0),
-	_DTV_CMD(DTV_ATSCMH_RS_FRAME_ENSEMBLE, 1, 0),
+	_DTV_CMD(DTV_ATSCMH_PARADE_ID),
+	_DTV_CMD(DTV_ATSCMH_RS_FRAME_ENSEMBLE),
 
-	_DTV_CMD(DTV_ATSCMH_FIC_VER, 0, 0),
-	_DTV_CMD(DTV_ATSCMH_NOG, 0, 0),
-	_DTV_CMD(DTV_ATSCMH_TNOG, 0, 0),
-	_DTV_CMD(DTV_ATSCMH_SGN, 0, 0),
-	_DTV_CMD(DTV_ATSCMH_PRC, 0, 0),
-	_DTV_CMD(DTV_ATSCMH_RS_FRAME_MODE, 0, 0),
-	_DTV_CMD(DTV_ATSCMH_RS_CODE_MODE_PRI, 0, 0),
-	_DTV_CMD(DTV_ATSCMH_RS_CODE_MODE_SEC, 0, 0),
-	_DTV_CMD(DTV_ATSCMH_SCCC_BLOCK_MODE, 0, 0),
-	_DTV_CMD(DTV_ATSCMH_SCCC_CODE_MODE_A, 0, 0),
-	_DTV_CMD(DTV_ATSCMH_SCCC_CODE_MODE_B, 0, 0),
-	_DTV_CMD(DTV_ATSCMH_SCCC_CODE_MODE_C, 0, 0),
-	_DTV_CMD(DTV_ATSCMH_SCCC_CODE_MODE_D, 0, 0),
+	_DTV_CMD(DTV_ATSCMH_FIC_VER),
+	_DTV_CMD(DTV_ATSCMH_NOG),
+	_DTV_CMD(DTV_ATSCMH_TNOG),
+	_DTV_CMD(DTV_ATSCMH_SGN),
+	_DTV_CMD(DTV_ATSCMH_PRC),
+	_DTV_CMD(DTV_ATSCMH_RS_FRAME_MODE),
+	_DTV_CMD(DTV_ATSCMH_RS_CODE_MODE_PRI),
+	_DTV_CMD(DTV_ATSCMH_RS_CODE_MODE_SEC),
+	_DTV_CMD(DTV_ATSCMH_SCCC_BLOCK_MODE),
+	_DTV_CMD(DTV_ATSCMH_SCCC_CODE_MODE_A),
+	_DTV_CMD(DTV_ATSCMH_SCCC_CODE_MODE_B),
+	_DTV_CMD(DTV_ATSCMH_SCCC_CODE_MODE_C),
+	_DTV_CMD(DTV_ATSCMH_SCCC_CODE_MODE_D),
 
 	/* Statistics API */
-	_DTV_CMD(DTV_STAT_SIGNAL_STRENGTH, 0, 0),
-	_DTV_CMD(DTV_STAT_CNR, 0, 0),
-	_DTV_CMD(DTV_STAT_PRE_ERROR_BIT_COUNT, 0, 0),
-	_DTV_CMD(DTV_STAT_PRE_TOTAL_BIT_COUNT, 0, 0),
-	_DTV_CMD(DTV_STAT_POST_ERROR_BIT_COUNT, 0, 0),
-	_DTV_CMD(DTV_STAT_POST_TOTAL_BIT_COUNT, 0, 0),
-	_DTV_CMD(DTV_STAT_ERROR_BLOCK_COUNT, 0, 0),
-	_DTV_CMD(DTV_STAT_TOTAL_BLOCK_COUNT, 0, 0),
+	_DTV_CMD(DTV_STAT_SIGNAL_STRENGTH),
+	_DTV_CMD(DTV_STAT_CNR),
+	_DTV_CMD(DTV_STAT_PRE_ERROR_BIT_COUNT),
+	_DTV_CMD(DTV_STAT_PRE_TOTAL_BIT_COUNT),
+	_DTV_CMD(DTV_STAT_POST_ERROR_BIT_COUNT),
+	_DTV_CMD(DTV_STAT_POST_TOTAL_BIT_COUNT),
+	_DTV_CMD(DTV_STAT_ERROR_BLOCK_COUNT),
+	_DTV_CMD(DTV_STAT_TOTAL_BLOCK_COUNT),
 };
 
+static char *dtv_cmd_name(u32 cmd)
+{
+	cmd = array_index_nospec(cmd, DTV_MAX_COMMAND);
+	return dtv_cmds[cmd];
+}
+
+
 /* Synchronise the legacy tuning parameters into the cache, so that demodulator
  * drivers can use a single set_frontend tuning function, regardless of whether
  * it's being used for the legacy or new API, reducing code and complexity.
@@ -1346,6 +1338,7 @@ static int dtv_property_process_get(struct dvb_frontend *fe,
 				    struct file *file)
 {
 	int ncaps;
+	unsigned int len = 1;
 
 	switch (tvp->cmd) {
 	case DTV_ENUM_DELSYS:
@@ -1355,6 +1348,7 @@ static int dtv_property_process_get(struct dvb_frontend *fe,
 			ncaps++;
 		}
 		tvp->u.buffer.len = ncaps;
+		len = ncaps;
 		break;
 	case DTV_FREQUENCY:
 		tvp->u.data = c->frequency;
@@ -1532,27 +1526,51 @@ static int dtv_property_process_get(struct dvb_frontend *fe,
 	/* Fill quality measures */
 	case DTV_STAT_SIGNAL_STRENGTH:
 		tvp->u.st = c->strength;
+		if (tvp->u.buffer.len > MAX_DTV_STATS * sizeof(u32))
+			tvp->u.buffer.len = MAX_DTV_STATS * sizeof(u32);
+		len = tvp->u.buffer.len;
 		break;
 	case DTV_STAT_CNR:
 		tvp->u.st = c->cnr;
+		if (tvp->u.buffer.len > MAX_DTV_STATS * sizeof(u32))
+			tvp->u.buffer.len = MAX_DTV_STATS * sizeof(u32);
+		len = tvp->u.buffer.len;
 		break;
 	case DTV_STAT_PRE_ERROR_BIT_COUNT:
 		tvp->u.st = c->pre_bit_error;
+		if (tvp->u.buffer.len > MAX_DTV_STATS * sizeof(u32))
+			tvp->u.buffer.len = MAX_DTV_STATS * sizeof(u32);
+		len = tvp->u.buffer.len;
 		break;
 	case DTV_STAT_PRE_TOTAL_BIT_COUNT:
 		tvp->u.st = c->pre_bit_count;
+		if (tvp->u.buffer.len > MAX_DTV_STATS * sizeof(u32))
+			tvp->u.buffer.len = MAX_DTV_STATS * sizeof(u32);
+		len = tvp->u.buffer.len;
 		break;
 	case DTV_STAT_POST_ERROR_BIT_COUNT:
 		tvp->u.st = c->post_bit_error;
+		if (tvp->u.buffer.len > MAX_DTV_STATS * sizeof(u32))
+			tvp->u.buffer.len = MAX_DTV_STATS * sizeof(u32);
+		len = tvp->u.buffer.len;
 		break;
 	case DTV_STAT_POST_TOTAL_BIT_COUNT:
 		tvp->u.st = c->post_bit_count;
+		if (tvp->u.buffer.len > MAX_DTV_STATS * sizeof(u32))
+			tvp->u.buffer.len = MAX_DTV_STATS * sizeof(u32);
+		len = tvp->u.buffer.len;
 		break;
 	case DTV_STAT_ERROR_BLOCK_COUNT:
 		tvp->u.st = c->block_error;
+		if (tvp->u.buffer.len > MAX_DTV_STATS * sizeof(u32))
+			tvp->u.buffer.len = MAX_DTV_STATS * sizeof(u32);
+		len = tvp->u.buffer.len;
 		break;
 	case DTV_STAT_TOTAL_BLOCK_COUNT:
 		tvp->u.st = c->block_count;
+		if (tvp->u.buffer.len > MAX_DTV_STATS * sizeof(u32))
+			tvp->u.buffer.len = MAX_DTV_STATS * sizeof(u32);
+		len = tvp->u.buffer.len;
 		break;
 	default:
 		dev_dbg(fe->dvb->device,
@@ -1561,18 +1579,13 @@ static int dtv_property_process_get(struct dvb_frontend *fe,
 		return -EINVAL;
 	}
 
-	if (!dtv_cmds[tvp->cmd].buffer)
-		dev_dbg(fe->dvb->device,
-			"%s: GET cmd 0x%08x (%s) = 0x%08x\n",
-			__func__, tvp->cmd, dtv_cmds[tvp->cmd].name,
-			tvp->u.data);
-	else
-		dev_dbg(fe->dvb->device,
-			"%s: GET cmd 0x%08x (%s) len %d: %*ph\n",
-			__func__,
-			tvp->cmd, dtv_cmds[tvp->cmd].name,
-			tvp->u.buffer.len,
-			tvp->u.buffer.len, tvp->u.buffer.data);
+	if (len < 1)
+		len = 1;
+
+	dev_dbg(fe->dvb->device,
+		"%s: GET cmd 0x%08x (%s) len %d: %*ph\n",
+		__func__, tvp->cmd, dtv_cmd_name(tvp->cmd),
+		tvp->u.buffer.len, tvp->u.buffer.len, tvp->u.buffer.data);
 
 	return 0;
 }
@@ -1870,7 +1883,7 @@ static int dtv_property_process_set(struct dvb_frontend *fe,
 	else
 		dev_dbg(fe->dvb->device,
 			"%s: SET cmd 0x%08x (%s) to 0x%08x\n",
-			__func__, cmd, dtv_cmds[cmd].name, data);
+			__func__, cmd, dtv_cmd_name(cmd), data);
 	switch (cmd) {
 	case DTV_CLEAR:
 		/*
-- 
2.31.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 11/11] media: xilinx: simplify get fourcc logic
  2021-06-16 12:28 [PATCH 00/11] Address some smatch warnings Mauro Carvalho Chehab
                   ` (9 preceding siblings ...)
  2021-06-16 12:28 ` [PATCH 10/11] media: dvb-core: frontend: make GET/SET safer Mauro Carvalho Chehab
@ 2021-06-16 12:28 ` Mauro Carvalho Chehab
  10 siblings, 0 replies; 16+ messages in thread
From: Mauro Carvalho Chehab @ 2021-06-16 12:28 UTC (permalink / raw)
  Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, Hyun Kwon,
	Laurent Pinchart, Mauro Carvalho Chehab, Michal Simek,
	linux-arm-kernel, linux-kernel, linux-media

Right now, there are two calls for xvip_get_format_by_fourcc().
If the first one fails, it is called again in order to pick
the first available format: V4L2_PIX_FMT_YUYV.

This ends by producing a smatch warnings:
	drivers/media/platform/xilinx/xilinx-dma.c:555 __xvip_dma_try_format() error: 'info' dereferencing possible ERR_PTR()
	drivers/media/platform/xilinx/xilinx-dma.c: drivers/media/platform/xilinx/xilinx-dma.c:664 xvip_dma_init() error: 'dma->fmtinfo' dereferencing possible ERR_PTR()

as it is hard for an static analyzer to ensure that calling
xvip_get_format_by_fourcc(XVIP_DMA_DEF_FORMAT) won't return an
error.

So, better to optimize the logic, ensuring that the function
will never return an error.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 drivers/media/platform/xilinx/xilinx-dma.c | 5 +----
 drivers/media/platform/xilinx/xilinx-vip.c | 6 +++---
 2 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/media/platform/xilinx/xilinx-dma.c b/drivers/media/platform/xilinx/xilinx-dma.c
index 2a56201cb853..338c3661d809 100644
--- a/drivers/media/platform/xilinx/xilinx-dma.c
+++ b/drivers/media/platform/xilinx/xilinx-dma.c
@@ -26,7 +26,6 @@
 #include "xilinx-vip.h"
 #include "xilinx-vipp.h"
 
-#define XVIP_DMA_DEF_FORMAT		V4L2_PIX_FMT_YUYV
 #define XVIP_DMA_DEF_WIDTH		1920
 #define XVIP_DMA_DEF_HEIGHT		1080
 
@@ -549,8 +548,6 @@ __xvip_dma_try_format(struct xvip_dma *dma, struct v4l2_pix_format *pix,
 	 * requested format isn't supported.
 	 */
 	info = xvip_get_format_by_fourcc(pix->pixelformat);
-	if (IS_ERR(info))
-		info = xvip_get_format_by_fourcc(XVIP_DMA_DEF_FORMAT);
 
 	pix->pixelformat = info->fourcc;
 	pix->field = V4L2_FIELD_NONE;
@@ -660,7 +657,7 @@ int xvip_dma_init(struct xvip_composite_device *xdev, struct xvip_dma *dma,
 	INIT_LIST_HEAD(&dma->queued_bufs);
 	spin_lock_init(&dma->queued_lock);
 
-	dma->fmtinfo = xvip_get_format_by_fourcc(XVIP_DMA_DEF_FORMAT);
+	dma->fmtinfo = xvip_get_format_by_fourcc(V4L2_PIX_FMT_YUYV);
 	dma->format.pixelformat = dma->fmtinfo->fourcc;
 	dma->format.colorspace = V4L2_COLORSPACE_SRGB;
 	dma->format.field = V4L2_FIELD_NONE;
diff --git a/drivers/media/platform/xilinx/xilinx-vip.c b/drivers/media/platform/xilinx/xilinx-vip.c
index 6ad61b08a31a..a4eb57683411 100644
--- a/drivers/media/platform/xilinx/xilinx-vip.c
+++ b/drivers/media/platform/xilinx/xilinx-vip.c
@@ -70,8 +70,8 @@ EXPORT_SYMBOL_GPL(xvip_get_format_by_code);
  * @fourcc: the format 4CC
  *
  * Return: a pointer to the format information structure corresponding to the
- * given V4L2 format @fourcc, or ERR_PTR if no corresponding format can be
- * found.
+ * given V4L2 format @fourcc. If not found, return a pointer to the first
+ * available format (V4L2_PIX_FMT_YUYV).
  */
 const struct xvip_video_format *xvip_get_format_by_fourcc(u32 fourcc)
 {
@@ -84,7 +84,7 @@ const struct xvip_video_format *xvip_get_format_by_fourcc(u32 fourcc)
 			return format;
 	}
 
-	return ERR_PTR(-EINVAL);
+	return &xvip_video_formats[0];
 }
 EXPORT_SYMBOL_GPL(xvip_get_format_by_fourcc);
 
-- 
2.31.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 08/11] media: adv7842: better document EDID block size
  2021-06-16 12:28 ` [PATCH 08/11] media: adv7842: better document EDID block size Mauro Carvalho Chehab
@ 2021-06-16 12:32   ` Hans Verkuil
  2021-06-17  6:26   ` kernel test robot
  2021-06-17  8:15   ` kernel test robot
  2 siblings, 0 replies; 16+ messages in thread
From: Hans Verkuil @ 2021-06-16 12:32 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, linux-kernel, linux-media

On 16/06/2021 14:28, Mauro Carvalho Chehab wrote:
> While the logic there is correct, it leads to smatch warnings:
> 	/home/hans/work/build/media-git/drivers/media/i2c/adv7842.c:2538 adv7842_set_edid() error: memcpy() '&state->vga_edid.edid' too small (128 vs 512)
> 
> Because the code tricks static analyzers by doing:
> 	memcpy(&state->hdmi_edid.edid, e->edid, 128 * e->blocks);
> 
> for ADV7842_EDID_PORT_VGA, where a logic before that makes
> e->blocks being either 0 or 1.
> 
> Yet, it is ugly to see the "128" magic number all spread about the
> EDID code. So, while here, replace 128 (and 4 x 128) by macros:
> 
> And ensure that the logic which copy into the VGA block
> will use EDID_MAX_VGA_BLOCKS.

Please drop this patch, there is already another patch from me for adv7842 in a pending PR.

> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> ---
>  drivers/media/i2c/adv7842.c | 33 ++++++++++++++++++++++-----------
>  1 file changed, 22 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/media/i2c/adv7842.c b/drivers/media/i2c/adv7842.c
> index 78e61fe6f2f0..30bddab320b9 100644
> --- a/drivers/media/i2c/adv7842.c
> +++ b/drivers/media/i2c/adv7842.c
> @@ -85,6 +85,10 @@ struct adv7842_format_info {
>  	u8 op_format_sel;
>  };
>  
> +#define EDID_BLOCK_SIZE		128
> +#define EDID_MAX_HDMI_BLOCKS	4
> +#define EDID_MAX_VGA_BLOCKS	1
> +
>  struct adv7842_state {
>  	struct adv7842_platform_data pdata;
>  	struct v4l2_subdev sd;
> @@ -98,12 +102,12 @@ struct adv7842_state {
>  
>  	v4l2_std_id norm;
>  	struct {
> -		u8 edid[512];
> +		u8 edid[EDID_BLOCK_SIZE * EDID_MAX_HDMI_BLOCKS];
>  		u32 blocks;
>  		u32 present;
>  	} hdmi_edid;
>  	struct {
> -		u8 edid[128];
> +		u8 edid[EDID_MAX_VGA_BLOCKS * EDID_MAX_VGA_BLOCKS];
>  		u32 blocks;
>  		u32 present;
>  	} vga_edid;
> @@ -732,12 +736,13 @@ static int edid_write_vga_segment(struct v4l2_subdev *sd)
>  	/* edid segment pointer '1' for VGA port */
>  	rep_write_and_or(sd, 0x77, 0xef, 0x10);
>  
> -	for (i = 0; !err && i < blocks * 128; i += I2C_SMBUS_BLOCK_MAX)
> +	for (i = 0; && i < blocks * EDID_BLOCK_SIZE; i += I2C_SMBUS_BLOCK_MAX) {

Besides, this change won't compile, so:

Nacked-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>

Regards,

	Hans

>  		err = i2c_smbus_write_i2c_block_data(state->i2c_edid, i,
>  						     I2C_SMBUS_BLOCK_MAX,
>  						     edid + i);
> -	if (err)
> -		return err;
> +		if (err)
> +			return err;
> +	}
>  
>  	/* Calculates the checksums and enables I2C access
>  	 * to internal EDID ram from VGA DDC port.
> @@ -785,7 +790,7 @@ static int edid_write_hdmi_segment(struct v4l2_subdev *sd, u8 port)
>  		return 0;
>  	}
>  
> -	pa = v4l2_get_edid_phys_addr(edid, blocks * 128, &spa_loc);
> +	pa = v4l2_get_edid_phys_addr(edid, blocks * EDID_BLOCK_SIZE, &spa_loc);
>  	err = v4l2_phys_addr_validate(pa, &parent_pa, NULL);
>  	if (err)
>  		return err;
> @@ -800,7 +805,7 @@ static int edid_write_hdmi_segment(struct v4l2_subdev *sd, u8 port)
>  	}
>  
>  
> -	for (i = 0; !err && i < blocks * 128; i += I2C_SMBUS_BLOCK_MAX) {
> +	for (i = 0; !err && i < blocks * EDID_BLOCK_SIZE; i += I2C_SMBUS_BLOCK_MAX) {
>  		/* set edid segment pointer for HDMI ports */
>  		if (i % 256 == 0)
>  			rep_write_and_or(sd, 0x77, 0xef, i >= 256 ? 0x10 : 0x00);
> @@ -2491,7 +2496,9 @@ static int adv7842_get_edid(struct v4l2_subdev *sd, struct v4l2_edid *edid)
>  	if (edid->start_block + edid->blocks > blocks)
>  		edid->blocks = blocks - edid->start_block;
>  
> -	memcpy(edid->edid, data + edid->start_block * 128, edid->blocks * 128);
> +	memcpy(edid->edid,
> +	       data + edid->start_block * EDID_BLOCK_SIZE,
> +	       edid->blocks * EDID_BLOCK_SIZE);
>  
>  	return 0;
>  }
> @@ -2506,9 +2513,12 @@ static int adv7842_get_edid(struct v4l2_subdev *sd, struct v4l2_edid *edid)
>  static int adv7842_set_edid(struct v4l2_subdev *sd, struct v4l2_edid *e)
>  {
>  	struct adv7842_state *state = to_state(sd);
> -	unsigned int max_blocks = e->pad == ADV7842_EDID_PORT_VGA ? 1 : 4;
> +	unsigned int max_blocks;
>  	int err = 0;
>  
> +	max_blocks = e->pad == ADV7842_EDID_PORT_VGA ?
> +		     EDID_MAX_VGA_BLOCKS  : EDID_MAX_HDMI_BLOCKS;
> +
>  	memset(e->reserved, 0, sizeof(e->reserved));
>  
>  	if (e->pad > ADV7842_EDID_PORT_VGA)
> @@ -2535,7 +2545,7 @@ static int adv7842_set_edid(struct v4l2_subdev *sd, struct v4l2_edid *e)
>  		state->vga_edid.blocks = e->blocks;
>  		state->vga_edid.present = e->blocks ? 0x1 : 0x0;
>  		if (e->blocks)
> -			memcpy(&state->vga_edid.edid, e->edid, 128 * e->blocks);
> +			memcpy(&state->vga_edid.edid, e->edid, EDID_BLOCK_SIZE);
>  		err = edid_write_vga_segment(sd);
>  		break;
>  	case ADV7842_EDID_PORT_A:
> @@ -2544,7 +2554,8 @@ static int adv7842_set_edid(struct v4l2_subdev *sd, struct v4l2_edid *e)
>  		state->hdmi_edid.blocks = e->blocks;
>  		if (e->blocks) {
>  			state->hdmi_edid.present |= 0x04 << e->pad;
> -			memcpy(&state->hdmi_edid.edid, e->edid, 128 * e->blocks);
> +			memcpy(&state->hdmi_edid.edid, e->edid,
> +			       EDID_BLOCK_SIZE * e->blocks);
>  		} else {
>  			state->hdmi_edid.present &= ~(0x04 << e->pad);
>  			adv7842_s_detect_tx_5v_ctrl(sd);
> 


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 04/11] media: sun6i-csi: add a missing return code
  2021-06-16 12:28 ` [PATCH 04/11] media: sun6i-csi: add a missing return code Mauro Carvalho Chehab
@ 2021-06-17  5:37   ` yong
  0 siblings, 0 replies; 16+ messages in thread
From: yong @ 2021-06-17  5:37 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: linuxarm, mauro.chehab, Chen-Yu Tsai, Jernej Skrabec,
	Mauro Carvalho Chehab, Maxime Ripard, linux-arm-kernel,
	linux-kernel, linux-media, linux-sunxi

Hi,

It does not mean there is a error. As the comment, it just check if the
csi is powered.

On Wed, 16 Jun 2021 14:28:30 +0200
Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote:

> As pointed by smatch, there's a missing return code:
> 
> 	drivers/media/platform/sunxi/sun6i-csi/sun6i_video.c:485
> sun6i_video_open() warn: missing error code 'ret'
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> ---
>  drivers/media/platform/sunxi/sun6i-csi/sun6i_video.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_video.c
> b/drivers/media/platform/sunxi/sun6i-csi/sun6i_video.c index
> 3181d0781b61..07b2161392d2 100644 ---
> a/drivers/media/platform/sunxi/sun6i-csi/sun6i_video.c +++
> b/drivers/media/platform/sunxi/sun6i-csi/sun6i_video.c @@ -481,8
> +481,10 @@ static int sun6i_video_open(struct file *file) goto
> fh_release; 
>  	/* check if already powered */
> -	if (!v4l2_fh_is_singular_file(file))
> +	if (!v4l2_fh_is_singular_file(file)) {
> +		ret = -EBUSY;
>  		goto unlock;
> +	}
>  
>  	ret = sun6i_csi_set_power(video->csi, true);
>  	if (ret < 0)

Thanks

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 08/11] media: adv7842: better document EDID block size
  2021-06-16 12:28 ` [PATCH 08/11] media: adv7842: better document EDID block size Mauro Carvalho Chehab
  2021-06-16 12:32   ` Hans Verkuil
@ 2021-06-17  6:26   ` kernel test robot
  2021-06-17  8:15   ` kernel test robot
  2 siblings, 0 replies; 16+ messages in thread
From: kernel test robot @ 2021-06-17  6:26 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: kbuild-all, linux-media, linuxarm, mauro.chehab,
	Mauro Carvalho Chehab, Hans Verkuil, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 3687 bytes --]

Hi Mauro,

I love your patch! Perhaps something to improve:

[auto build test WARNING on linuxtv-media/master]
[also build test WARNING on next-20210616]
[cannot apply to v5.13-rc6]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Mauro-Carvalho-Chehab/Address-some-smatch-warnings/20210617-091510
base:   git://linuxtv.org/media_tree.git master
config: arc-allyesconfig (attached as .config)
compiler: arceb-elf-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/3bdf84a7467fed26b64ffe547f5989d73060a30e
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Mauro-Carvalho-Chehab/Address-some-smatch-warnings/20210617-091510
        git checkout 3bdf84a7467fed26b64ffe547f5989d73060a30e
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arc 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   drivers/media/i2c/adv7842.c: In function 'edid_write_vga_segment':
>> drivers/media/i2c/adv7842.c:739:19: warning: comparison between pointer and integer
     739 |  for (i = 0; && i < blocks * EDID_BLOCK_SIZE; i += I2C_SMBUS_BLOCK_MAX) {
         |                   ^
   drivers/media/i2c/adv7842.c:739:2: error: label 'i' used but not defined
     739 |  for (i = 0; && i < blocks * EDID_BLOCK_SIZE; i += I2C_SMBUS_BLOCK_MAX) {
         |  ^~~


vim +739 drivers/media/i2c/adv7842.c

   715	
   716	static int edid_write_vga_segment(struct v4l2_subdev *sd)
   717	{
   718		struct i2c_client *client = v4l2_get_subdevdata(sd);
   719		struct adv7842_state *state = to_state(sd);
   720		const u8 *edid = state->vga_edid.edid;
   721		u32 blocks = state->vga_edid.blocks;
   722		int err = 0;
   723		int i;
   724	
   725		v4l2_dbg(2, debug, sd, "%s: write EDID on VGA port\n", __func__);
   726	
   727		if (!state->vga_edid.present)
   728			return 0;
   729	
   730		/* HPA disable on port A and B */
   731		io_write_and_or(sd, 0x20, 0xcf, 0x00);
   732	
   733		/* Disable I2C access to internal EDID ram from VGA DDC port */
   734		rep_write_and_or(sd, 0x7f, 0x7f, 0x00);
   735	
   736		/* edid segment pointer '1' for VGA port */
   737		rep_write_and_or(sd, 0x77, 0xef, 0x10);
   738	
 > 739		for (i = 0; && i < blocks * EDID_BLOCK_SIZE; i += I2C_SMBUS_BLOCK_MAX) {
   740			err = i2c_smbus_write_i2c_block_data(state->i2c_edid, i,
   741							     I2C_SMBUS_BLOCK_MAX,
   742							     edid + i);
   743			if (err)
   744				return err;
   745		}
   746	
   747		/* Calculates the checksums and enables I2C access
   748		 * to internal EDID ram from VGA DDC port.
   749		 */
   750		rep_write_and_or(sd, 0x7f, 0x7f, 0x80);
   751	
   752		for (i = 0; i < 1000; i++) {
   753			if (rep_read(sd, 0x79) & 0x20)
   754				break;
   755			mdelay(1);
   756		}
   757		if (i == 1000) {
   758			v4l_err(client, "error enabling edid on VGA port\n");
   759			return -EIO;
   760		}
   761	
   762		/* enable hotplug after 200 ms */
   763		schedule_delayed_work(&state->delayed_work_enable_hotplug, HZ / 5);
   764	
   765		return 0;
   766	}
   767	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 68154 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 08/11] media: adv7842: better document EDID block size
  2021-06-16 12:28 ` [PATCH 08/11] media: adv7842: better document EDID block size Mauro Carvalho Chehab
  2021-06-16 12:32   ` Hans Verkuil
  2021-06-17  6:26   ` kernel test robot
@ 2021-06-17  8:15   ` kernel test robot
  2 siblings, 0 replies; 16+ messages in thread
From: kernel test robot @ 2021-06-17  8:15 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: kbuild-all, linux-media, linuxarm, mauro.chehab,
	Mauro Carvalho Chehab, Hans Verkuil, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 3707 bytes --]

Hi Mauro,

I love your patch! Yet something to improve:

[auto build test ERROR on linuxtv-media/master]
[also build test ERROR on next-20210616]
[cannot apply to v5.13-rc6]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Mauro-Carvalho-Chehab/Address-some-smatch-warnings/20210617-091510
base:   git://linuxtv.org/media_tree.git master
config: alpha-randconfig-r025-20210617 (attached as .config)
compiler: alpha-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/3bdf84a7467fed26b64ffe547f5989d73060a30e
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Mauro-Carvalho-Chehab/Address-some-smatch-warnings/20210617-091510
        git checkout 3bdf84a7467fed26b64ffe547f5989d73060a30e
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=alpha 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All error/warnings (new ones prefixed by >>):

   drivers/media/i2c/adv7842.c: In function 'edid_write_vga_segment':
>> drivers/media/i2c/adv7842.c:739:19: warning: comparison between pointer and integer
     739 |  for (i = 0; && i < blocks * EDID_BLOCK_SIZE; i += I2C_SMBUS_BLOCK_MAX) {
         |                   ^
>> drivers/media/i2c/adv7842.c:739:2: error: label 'i' used but not defined
     739 |  for (i = 0; && i < blocks * EDID_BLOCK_SIZE; i += I2C_SMBUS_BLOCK_MAX) {
         |  ^~~


vim +/i +739 drivers/media/i2c/adv7842.c

   715	
   716	static int edid_write_vga_segment(struct v4l2_subdev *sd)
   717	{
   718		struct i2c_client *client = v4l2_get_subdevdata(sd);
   719		struct adv7842_state *state = to_state(sd);
   720		const u8 *edid = state->vga_edid.edid;
   721		u32 blocks = state->vga_edid.blocks;
   722		int err = 0;
   723		int i;
   724	
   725		v4l2_dbg(2, debug, sd, "%s: write EDID on VGA port\n", __func__);
   726	
   727		if (!state->vga_edid.present)
   728			return 0;
   729	
   730		/* HPA disable on port A and B */
   731		io_write_and_or(sd, 0x20, 0xcf, 0x00);
   732	
   733		/* Disable I2C access to internal EDID ram from VGA DDC port */
   734		rep_write_and_or(sd, 0x7f, 0x7f, 0x00);
   735	
   736		/* edid segment pointer '1' for VGA port */
   737		rep_write_and_or(sd, 0x77, 0xef, 0x10);
   738	
 > 739		for (i = 0; && i < blocks * EDID_BLOCK_SIZE; i += I2C_SMBUS_BLOCK_MAX) {
   740			err = i2c_smbus_write_i2c_block_data(state->i2c_edid, i,
   741							     I2C_SMBUS_BLOCK_MAX,
   742							     edid + i);
   743			if (err)
   744				return err;
   745		}
   746	
   747		/* Calculates the checksums and enables I2C access
   748		 * to internal EDID ram from VGA DDC port.
   749		 */
   750		rep_write_and_or(sd, 0x7f, 0x7f, 0x80);
   751	
   752		for (i = 0; i < 1000; i++) {
   753			if (rep_read(sd, 0x79) & 0x20)
   754				break;
   755			mdelay(1);
   756		}
   757		if (i == 1000) {
   758			v4l_err(client, "error enabling edid on VGA port\n");
   759			return -EIO;
   760		}
   761	
   762		/* enable hotplug after 200 ms */
   763		schedule_delayed_work(&state->delayed_work_enable_hotplug, HZ / 5);
   764	
   765		return 0;
   766	}
   767	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 28567 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2021-06-17  8:17 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-16 12:28 [PATCH 00/11] Address some smatch warnings Mauro Carvalho Chehab
2021-06-16 12:28 ` [PATCH 01/11] media: dvb_ca_en50221: avoid speculation from CA slot Mauro Carvalho Chehab
2021-06-16 12:28 ` [PATCH 02/11] media: dvb_net: avoid speculation from net slot Mauro Carvalho Chehab
2021-06-16 12:28 ` [PATCH 03/11] media: dvbdev: fix error logic at dvb_register_device() Mauro Carvalho Chehab
2021-06-16 12:28 ` [PATCH 04/11] media: sun6i-csi: add a missing return code Mauro Carvalho Chehab
2021-06-17  5:37   ` yong
2021-06-16 12:28 ` [PATCH 05/11] media: saa7134: use more meaninful goto labels Mauro Carvalho Chehab
2021-06-16 12:28 ` [PATCH 06/11] media: saa7134: fix saa7134_initdev error handling logic Mauro Carvalho Chehab
2021-06-16 12:28 ` [PATCH 07/11] media: siano: fix device register error path Mauro Carvalho Chehab
2021-06-16 12:28 ` [PATCH 08/11] media: adv7842: better document EDID block size Mauro Carvalho Chehab
2021-06-16 12:32   ` Hans Verkuil
2021-06-17  6:26   ` kernel test robot
2021-06-17  8:15   ` kernel test robot
2021-06-16 12:28 ` [PATCH 09/11] media: ttusb-dec: cleanup an error handling logic Mauro Carvalho Chehab
2021-06-16 12:28 ` [PATCH 10/11] media: dvb-core: frontend: make GET/SET safer Mauro Carvalho Chehab
2021-06-16 12:28 ` [PATCH 11/11] media: xilinx: simplify get fourcc logic Mauro Carvalho Chehab

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).