cron job: media_tree daily build: ERRORS

[Hans Verkuil]

This message is generated daily by a cron job that builds media_tree for
the kernels and architectures in the list below.

Results of the daily build of media_tree:

date:   Mon Jan  8 05:00:15 CET 2018
media-tree git hash:e3ee691dbf24096ea51b3200946b11d68ce75361
media_build git hash:   46c9dc0a08499791cedfc7ee0df387e475f075a2
v4l-utils git hash: 8aa401d119afaeb1b4fe4d2994789cd3e9396554
gcc version:i686-linux-gcc (GCC) 7.1.0
sparse version: v0.5.0-3911-g6f737e1f
smatch version: v0.5.0-3911-g6f737e1f
host hardware:  x86_64
host os:4.13.0-164

linux-git-arm-at91: ERRORS
linux-git-arm-davinci: OK
linux-git-arm-multi: ERRORS
linux-git-arm-pxa: OK
linux-git-arm-stm32: OK
linux-git-blackfin-bf561: ERRORS
linux-git-i686: OK
linux-git-m32r: OK
linux-git-mips: ERRORS
linux-git-powerpc64: OK
linux-git-sh: OK
linux-git-x86_64: OK
linux-2.6.36.4-i686: ERRORS
linux-2.6.37.6-i686: ERRORS
linux-2.6.38.8-i686: ERRORS
linux-2.6.39.4-i686: ERRORS
linux-3.0.60-i686: ERRORS
linux-3.1.10-i686: ERRORS
linux-3.2.37-i686: ERRORS
linux-3.3.8-i686: ERRORS
linux-3.4.27-i686: ERRORS
linux-3.5.7-i686: ERRORS
linux-3.6.11-i686: ERRORS
linux-3.7.4-i686: ERRORS
linux-3.8-i686: ERRORS
linux-3.9.2-i686: WARNINGS
linux-3.10.1-i686: WARNINGS
linux-3.11.1-i686: ERRORS
linux-3.12.67-i686: ERRORS
linux-3.13.11-i686: ERRORS
linux-3.14.9-i686: ERRORS
linux-3.15.2-i686: ERRORS
linux-3.16.7-i686: ERRORS
linux-3.17.8-i686: ERRORS
linux-3.18.7-i686: ERRORS
linux-3.19-i686: ERRORS
linux-4.0.9-i686: ERRORS
linux-4.1.33-i686: ERRORS
linux-4.2.8-i686: ERRORS
linux-4.3.6-i686: ERRORS
linux-4.4.22-i686: ERRORS
linux-4.5.7-i686: ERRORS
linux-4.6.7-i686: ERRORS
linux-4.7.5-i686: ERRORS
linux-4.8-i686: ERRORS
linux-4.9.26-i686: ERRORS
linux-4.10.14-i686: WARNINGS
linux-4.11-i686: WARNINGS
linux-4.12.1-i686: WARNINGS
linux-4.13-i686: WARNINGS
linux-4.14-i686: WARNINGS
linux-2.6.36.4-x86_64: ERRORS
linux-2.6.37.6-x86_64: ERRORS
linux-2.6.38.8-x86_64: ERRORS
linux-2.6.39.4-x86_64: ERRORS
linux-3.0.60-x86_64: ERRORS
linux-3.1.10-x86_64: ERRORS
linux-3.2.37-x86_64: ERRORS
linux-3.3.8-x86_64: ERRORS
linux-3.4.27-x86_64: ERRORS
linux-3.5.7-x86_64: ERRORS
linux-3.6.11-x86_64: ERRORS
linux-3.7.4-x86_64: ERRORS
linux-3.8-x86_64: ERRORS
linux-3.9.2-x86_64: WARNINGS
linux-3.10.1-x86_64: WARNINGS
linux-3.11.1-x86_64: ERRORS
linux-3.12.67-x86_64: ERRORS
linux-3.13.11-x86_64: ERRORS
linux-3.14.9-x86_64: ERRORS
linux-3.15.2-x86_64: ERRORS
linux-3.16.7-x86_64: ERRORS
linux-3.17.8-x86_64: ERRORS
linux-3.18.7-x86_64: ERRORS
linux-3.19-x86_64: ERRORS
linux-4.0.9-x86_64: ERRORS
linux-4.1.33-x86_64: ERRORS
linux-4.2.8-x86_64: ERRORS
linux-4.3.6-x86_64: ERRORS
linux-4.4.22-x86_64: ERRORS
linux-4.5.7-x86_64: ERRORS
linux-4.6.7-x86_64: ERRORS
linux-4.7.5-x86_64: ERRORS
linux-4.8-x86_64: ERRORS
linux-4.9.26-x86_64: ERRORS
linux-4.10.14-x86_64: WARNINGS
linux-4.11-x86_64: WARNINGS
linux-4.12.1-x86_64: WARNINGS
linux-4.13-x86_64: WARNINGS
linux-4.14-x86_64: WARNINGS
apps: OK
spec-git: OK
smatch: OK

Detailed results are available here:

http://www.xs4all.nl/~hverkuil/logs/Monday.log

Full logs are available here:

http://www.xs4all.nl/~hverkuil/logs/Monday.tar.bz2

The Media Infrastructure API from this daily build is here:

http://www.xs4all.nl/~hverkuil/spec/index.html

Re: dvb usb issues since kernel 4.9

[Linus Torvalds]

On Sat, Jan 6, 2018 at 11:54 AM, Mauro Carvalho Chehab
 wrote:
>
> Em Sat, 6 Jan 2018 16:04:16 +0100
> "Josef Griebichler"  escreveu:
>>
>> the causing commit has been identified.
>> After reverting commit 
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=4cd13c21b207e80ddb1144c576500098f2d5f882
>> its working again.
>
> Just replying to me won't magically fix this. The ones that were involved on
> this patch should also be c/c, plus USB people. Just added them.

Actually, you seem to have added an odd subset of the people involved.

For example, Ingo - who actually committed that patch - wasn't on the cc.

I do think we need to simply revert that patch. It's very simple: it
has been reported to lead to actual problems for people, and we don't
fix one problem and then say "well, it fixed something else" when
something breaks.

When something breaks, we either unbreak it, or we revert the change
that caused the breakage.

It's really that simple. That's what "no regressions" means.  We don't
accept changes that cause regressions. This one did.

  Linus

Re: [PATCH 07/18] [media] uvcvideo: prevent bounds-check bypass via speculative execution

[Dan Williams]

On Sun, Jan 7, 2018 at 1:09 AM, Greg KH  wrote:
[..]
> Sorry for the confusion, no, I don't mean the "taint tracking", I mean
> the generic pattern of "speculative out of bounds access" that we are
> fixing here.
>
> Yes, as you mentioned before, there are tons of false-positives in the
> tree, as to find the real problems you have to show that userspace
> controls the access index.  But if we have a generic pattern that can
> rewrite that type of logic into one where it does not matter at all
> (i.e. like the ebpf proposed changes), then it would not be an issue if
> they are false or not, we just rewrite them all to be safe.
>
> We need to find some way not only to fix these issues now (like you are
> doing with this series), but to prevent them from every coming back into
> the codebase again.  It's that second part that we need to keep in the
> back of our minds here, while doing the first portion of this work.

I understand the goal, but I'm not sure any of our current annotation
mechanisms are suitable. We have:

__attribute__((noderef, address_space(x)))

...for the '__user' annotation and other pointers that must not be
de-referenced without a specific accessor. We also have:

__attribute__((bitwise))

...for values that should not be consumed directly without a specific
conversion like endian swapping.

The problem is that we need to see if a value derived from a userspace
controlled input is used to trigger a chain of dependent reads. As far
as I can see the annotation would need to be guided by taint analysis
to be useful, at which point we can just "annotate" the problem spot
with nospec_array_ptr(). Otherwise it seems the scope of a
"__nospec_array_index" annotation would have a low signal to noise
ratio.

Stopping speculation past a uacess_begin() boundary appears to handle
a wide swath of potential problems, and the rest likely needs taint
analysis, at least for now.

All that to say, yes, we need better tooling and infrastructure going forward.

[GIT PULL FOR v4.16] RC fixes

[Sean Young]

Hi Mauro,

I've been testing the lirc changes on various distributions and versions;
there were some regressions.

Thanks,

Sean

The following changes since commit 6f0e5fd39143a59c22d60e7befc4f33f22aeed2f:

  media: vb2: add a new warning about pending buffers (2018-01-03 05:30:35 
-0500)

are available in the Git repository at:

  git://linuxtv.org/syoung/media_tree.git for-v4.16c

for you to fetch changes up to 6dc84d93e557c787ae74a808d93a8493f85a7800:

  media: rc: do not remove first bit if leader pulse is present (2018-01-05 
15:13:15 +)


Colin Ian King (1):
  media: lirc: don't kfree the uninitialized pointer txbuf

Sean Young (5):
  media: lirc: add module alias for lirc_dev
  media: lirc: lirc daemon fails to detect raw IR device
  media: lirc: lirc mode ioctls deal with current mode
  media: rc: clean up leader pulse/space for manchester encoding
  media: rc: do not remove first bit if leader pulse is present

 Documentation/media/uapi/rc/lirc-get-features.rst  | 24 ++---
 Documentation/media/uapi/rc/lirc-get-rec-mode.rst  | 42 --
 Documentation/media/uapi/rc/lirc-get-send-mode.rst | 38 +++-
 drivers/media/rc/ir-mce_kbd-decoder.c  |  6 ++--
 drivers/media/rc/ir-rc5-decoder.c  | 22 ++--
 drivers/media/rc/ir-rc6-decoder.c  | 31 +---
 drivers/media/rc/lirc_dev.c| 11 +++---
 drivers/media/rc/rc-core-priv.h| 10 +++---
 drivers/media/rc/rc-ir-raw.c   | 13 +++
 9 files changed, 112 insertions(+), 85 deletions(-)

Aw: Re: dvb usb issues since kernel 4.9

[Josef Griebichler]

Hi,

here I provide lsusb from my affected hardware (technotrend s2-4600).
http://ix.io/DLY

With this hardware I had errors when recording with tvheadend. Livetv was ok, 
only channel switching made some problems sometimes. Please see attached 
tvheadend service logs.

I also provide dmesg (libreelec on rpi3 with kernel 4.14.10 with revert of the 
mentioned commit).
http://ix.io/DM2


Regards
Josef
 
 

Gesendet: Sonntag, 07. Januar 2018 um 16:41 Uhr
Von: "Alan Stern" 
An: "Mauro Carvalho Chehab" 
Cc: "Josef Griebichler" , "Greg Kroah-Hartman" 
, linux-...@vger.kernel.org, "Eric Dumazet" 
, "Rik van Riel" , "Paolo Abeni" 
, "Hannes Frederic Sowa" , "Jesper 
Dangaard Brouer" , linux-kernel 
, netdev , "Jonathan 
Corbet" , LMML , "Peter Zijlstra" 
, "David Miller" , 
torva...@linux-foundation.org
Betreff: Re: dvb usb issues since kernel 4.9
On Sun, 7 Jan 2018, Mauro Carvalho Chehab wrote: > > > It seems that the 
original patch were designed to solve some IRQ issues > > > with network cards 
with causes data losses on high traffic. However, > > > it is also causing bad 
effects on sustained high bandwidth demands > > > required by DVB cards, at 
least on some USB host drivers. > > > > > > Alan/Greg/Eric/David: > > > > > > 
Any ideas about how to fix it without causing regressions to > > > network? > > 
> > It would be good to know what hardware was involved on the x86 system > > 
and to have some timing data. Can we see the output from lsusb and > > usbmon, 
running on a vanilla kernel that gets plenty of video glitches? > > From 
Josef's report, and from the BZ, the affected hardware seems > to be based on 
Montage Technology M88DS3103/M88TS2022 chipset. What type of USB host 
controller does the x86_64 system use? EHCI or xHCI? > The driver it uses is at 
drivers/media/usb/dvb-usb-v2/dvbsky.c, > with shares a USB implementation that 
is used by a lot more drivers. > The URB handling code is at: > > 
drivers/media/usb/dvb-usb-v2/usb_urb.c > > This particular driver allocates 8 
buffers with 4096 bytes each > for bulk transfers, using transfer_flags = 
URB_NO_TRANSFER_DMA_MAP. > > This become a popular USB hardware nowadays. I 
have one S960c > myself, so I can send you the lsusb from it. You should 
notice, however, > that a DVB-C/DVB-S2 channel can easily provide very high 
sustained bit > rates. Here, on my DVB-S2 provider, a typical transponder 
produces 58 Mpps > of payload after removing URB headers. You mentioned earlier 
that the driver uses bulk transfers. In USB-2.0, the maximum possible payload 
data transfer rate using bulk transfers is 53248 bytes/ms, which is 53.248 MB/s 
(i.e., lower than 58 MB/s). And even this is possible only if almost nothing 
else is using the bus at the same time. > A 10 minutes record with the > entire 
data (with typically contains 5-10 channels) can easily go > above 4 GB, just 
to reproduce 1-2 glitches. So, I'm not sure if > a usbmon dump would be useful. 
It might not be helpful at all. However, I'm not interested in the payload data 
(which would be unintelligible to me anyway) but rather the timing of URB 
submissions and completions. A usbmon trace which didn't keep much of the 
payload data would only require on the order of 50 MB per minute -- and Josef 
said that glitches usually would show up within a minute or so. > I'm enclosing 
the lsusb from a S960C device, with is based on those > Montage chipsets: What 
I wanted to see was the output from "lsusb" on the affected system, not the 
output from "lsusb -v -s B:D" on your system. > > Overall, this may be a very 
difficult problem to solve. The > > 4cd13c21b207 commit was intended to improve 
throughput at the cost of > > increased latency. But then what do you do when 
the latency becomes > > too high for the video subsystem to handle? > > Latency 
can't be too high, otherwise frames will be dropped. Yes, that's the whole 
point. > Even if the Kernel itself doesn't drop, if the delay goes higher > 
than a certain threshold, userspace will need to drop, as it > should be 
presenting audio and video on real time. Yet, typically, > userspace will delay 
it by one or two seconds, with would mean > 1500-3500 buffers, with I suspect 
it is a lot more than the hardware > limits. So I suspect that the hardware 
starves free buffers a way > before userspace, as media hardware don't have 
unlimited buffers > inside them, as they assume that the Kernel/userspace will 
be fast > enough to sustain bit rates up to 66 Mbps of payload. The timing 
information would tell us how large the latency is. In any case, you might be 
able to attack the problem simply by using more than 8 

[PATCH v2 2/2] media: ov9650: add device tree binding

[Akinobu Mita]

Now the ov9650 driver supports device tree probing.  So this adds a
device tree binding documentation.

Cc: Jacopo Mondi 
Cc: H. Nikolaus Schaller 
Cc: Hugues Fruchet 
Cc: Sakari Ailus 
Cc: Mauro Carvalho Chehab 
Cc: Rob Herring 
Signed-off-by: Akinobu Mita 
---
* Changelog v2
- Split binding documentation, suggested by Rob Herring and Jacopo Mondi
- Improve the wording for compatible property in the binding documentation,
  suggested by Jacopo Mondi
- Improve the description for the device node in the binding documentation,
  suggested by Sakari Ailus

 .../devicetree/bindings/media/i2c/ov9650.txt   | 36 ++
 1 file changed, 36 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ov9650.txt

diff --git a/Documentation/devicetree/bindings/media/i2c/ov9650.txt 
b/Documentation/devicetree/bindings/media/i2c/ov9650.txt
new file mode 100644
index 000..506dfc5
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/ov9650.txt
@@ -0,0 +1,36 @@
+* Omnivision OV9650/OV9652 CMOS sensor
+
+Required Properties:
+- compatible: shall be one of
+   "ovti,ov9650"
+   "ovti,ov9652"
+- clocks: reference to the xvclk input clock.
+
+Optional Properties:
+- reset-gpios: reference to the GPIO connected to the resetb pin, if any.
+  Active is high.
+- powerdown-gpios: reference to the GPIO connected to the pwdn pin, if any.
+  Active is high.
+
+The device node shall contain one 'port' child node with one child 'endpoint'
+subnode for its digital output video port, in accordance with the video
+interface bindings defined in Documentation/devicetree/bindings/media/
+video-interfaces.txt.
+
+Example:
+
+ {
+   ov9650: camera@30 {
+   compatible = "ovti,ov9650";
+   reg = <0x30>;
+   reset-gpios = <_gpio_0 0 GPIO_ACTIVE_HIGH>;
+   powerdown-gpios = <_gpio_0 1 GPIO_ACTIVE_HIGH>;
+   clocks = <>;
+
+   port {
+   ov9650_0: endpoint {
+   remote-endpoint = <_in0>;
+   };
+   };
+   };
+};
-- 
2.7.4

[PATCH v2 0/2] media: ov9650: support device tree probing

[Akinobu Mita]

This patchset adds device tree probing for ov9650 driver. This contains
an actual driver change and a newly added binding documentation part.

* Changelog v2
- Split binding documentation, suggested by Rob Herring and Jacopo Mondi
- Improve the wording for compatible property in the binding documentation,
  suggested by Jacopo Mondi
- Improve the description for the device node in the binding documentation,
  suggested by Sakari Ailus
- Remove ov965x_gpio_set() helper and open-code it, suggested by Jacopo Mondi
  and Sakari Ailus
- Call clk_prepare_enable() in s_power callback instead of probe, suggested
  by Sakari Ailus
- Unify clk and gpio configuration in a single if-else block and, also add
  a check either platform data or fwnode is actually specified, suggested
  by Jacopo Mondi
- Add CONFIG_OF guards, suggested by Jacopo Mondi

Akinobu Mita (2):
  media: ov9650: support device tree probing
  media: ov9650: add device tree binding

 .../devicetree/bindings/media/i2c/ov9650.txt   |  36 ++
 drivers/media/i2c/ov9650.c | 130 +++--
 2 files changed, 128 insertions(+), 38 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ov9650.txt

Cc: Jacopo Mondi 
Cc: H. Nikolaus Schaller 
Cc: Hugues Fruchet 
Cc: Sakari Ailus 
Cc: Mauro Carvalho Chehab 
Cc: Rob Herring 
-- 
2.7.4

[PATCH v2 1/2] media: ov9650: support device tree probing

[Akinobu Mita]

The ov9650 driver currently only supports legacy platform data probe.
This change adds device tree probing.

There has been an attempt to add device tree support for ov9650 driver
by Hugues Fruchet as a part of the patchset that adds support of OV9655
camera (http://www.spinics.net/lists/linux-media/msg117903.html), but
it wasn't merged into mainline because creating a separate driver for
OV9655 is preferred.

This is very similar to Hugues's patch, but not supporting new device.

Cc: Jacopo Mondi 
Cc: H. Nikolaus Schaller 
Cc: Hugues Fruchet 
Cc: Sakari Ailus 
Cc: Mauro Carvalho Chehab 
Cc: Rob Herring 
Signed-off-by: Akinobu Mita 
---
* Changelog v2
- Split binding documentation, suggested by Rob Herring and Jacopo Mondi
- Remove ov965x_gpio_set() helper and open-code it, suggested by Jacopo Mondi
  and Sakari Ailus
- Call clk_prepare_enable() in s_power callback instead of probe, suggested
  by Sakari Ailus
- Unify clk and gpio configuration in a single if-else block and, also add
  a check either platform data or fwnode is actually specified, suggested
  by Jacopo Mondi
- Add CONFIG_OF guards, suggested by Jacopo Mondi

 drivers/media/i2c/ov9650.c | 130 -
 1 file changed, 92 insertions(+), 38 deletions(-)

diff --git a/drivers/media/i2c/ov9650.c b/drivers/media/i2c/ov9650.c
index 69433e1..99a3eab 100644
--- a/drivers/media/i2c/ov9650.c
+++ b/drivers/media/i2c/ov9650.c
@@ -11,8 +11,10 @@
  * it under the terms of the GNU General Public License version 2 as
  * published by the Free Software Foundation.
  */
+#include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -249,9 +251,10 @@ struct ov965x {
struct v4l2_subdev sd;
struct media_pad pad;
enum v4l2_mbus_type bus_type;
-   int gpios[NUM_GPIOS];
+   struct gpio_desc *gpios[NUM_GPIOS];
/* External master clock frequency */
unsigned long mclk_frequency;
+   struct clk *clk;
 
/* Protects the struct fields below */
struct mutex lock;
@@ -513,24 +516,27 @@ static int ov965x_set_color_matrix(struct ov965x *ov965x)
return 0;
 }
 
-static void ov965x_gpio_set(int gpio, int val)
-{
-   if (gpio_is_valid(gpio))
-   gpio_set_value(gpio, val);
-}
-
-static void __ov965x_set_power(struct ov965x *ov965x, int on)
+static int __ov965x_set_power(struct ov965x *ov965x, int on)
 {
if (on) {
-   ov965x_gpio_set(ov965x->gpios[GPIO_PWDN], 0);
-   ov965x_gpio_set(ov965x->gpios[GPIO_RST], 0);
+   int ret = clk_prepare_enable(ov965x->clk);
+
+   if (ret)
+   return ret;
+
+   gpiod_set_value_cansleep(ov965x->gpios[GPIO_PWDN], 0);
+   gpiod_set_value_cansleep(ov965x->gpios[GPIO_RST], 0);
msleep(25);
} else {
-   ov965x_gpio_set(ov965x->gpios[GPIO_RST], 1);
-   ov965x_gpio_set(ov965x->gpios[GPIO_PWDN], 1);
+   gpiod_set_value_cansleep(ov965x->gpios[GPIO_RST], 1);
+   gpiod_set_value_cansleep(ov965x->gpios[GPIO_PWDN], 1);
+
+   clk_disable_unprepare(ov965x->clk);
}
 
ov965x->streaming = 0;
+
+   return 0;
 }
 
 static int ov965x_s_power(struct v4l2_subdev *sd, int on)
@@ -543,8 +549,8 @@ static int ov965x_s_power(struct v4l2_subdev *sd, int on)
 
mutex_lock(>lock);
if (ov965x->power == !on) {
-   __ov965x_set_power(ov965x, on);
-   if (on) {
+   ret = __ov965x_set_power(ov965x, on);
+   if (!ret && on) {
ret = ov965x_write_array(client,
 ov965x_init_regs);
ov965x->apply_frame_fmt = 1;
@@ -1408,16 +1414,17 @@ static const struct v4l2_subdev_ops ov965x_subdev_ops = 
{
 /*
  * Reset and power down GPIOs configuration
  */
-static int ov965x_configure_gpios(struct ov965x *ov965x,
- const struct ov9650_platform_data *pdata)
+static int ov965x_configure_gpios_pdata(struct ov965x *ov965x,
+   const struct ov9650_platform_data *pdata)
 {
int ret, i;
+   int gpios[NUM_GPIOS];
 
-   ov965x->gpios[GPIO_PWDN] = pdata->gpio_pwdn;
-   ov965x->gpios[GPIO_RST]  = pdata->gpio_reset;
+   gpios[GPIO_PWDN] = pdata->gpio_pwdn;
+   gpios[GPIO_RST]  = pdata->gpio_reset;
 
for (i = 0; i < ARRAY_SIZE(ov965x->gpios); i++) {
-   int gpio = ov965x->gpios[i];
+   int gpio = gpios[i];
 
if (!gpio_is_valid(gpio))
continue;
@@ -1427,9 +1434,30 @@ static int ov965x_configure_gpios(struct ov965x *ov965x,
return ret;
v4l2_dbg(1, debug, >sd, "set 

Re: dvb usb issues since kernel 4.9

[Alan Stern]

On Sun, 7 Jan 2018, Mauro Carvalho Chehab wrote:

> > > It seems that the original patch were designed to solve some IRQ issues
> > > with network cards with causes data losses on high traffic. However,
> > > it is also causing bad effects on sustained high bandwidth demands
> > > required by DVB cards, at least on some USB host drivers.
> > > 
> > > Alan/Greg/Eric/David:
> > > 
> > > Any ideas about how to fix it without causing regressions to
> > > network?  
> > 
> > It would be good to know what hardware was involved on the x86 system
> > and to have some timing data.  Can we see the output from lsusb and
> > usbmon, running on a vanilla kernel that gets plenty of video glitches?
> 
> From Josef's report, and from the BZ, the affected hardware seems
> to be based on Montage Technology M88DS3103/M88TS2022 chipset.

What type of USB host controller does the x86_64 system use?  EHCI or
xHCI?

> The driver it uses is at drivers/media/usb/dvb-usb-v2/dvbsky.c,
> with shares a USB implementation that is used by a lot more drivers.
> The URB handling code is at:
> 
>   drivers/media/usb/dvb-usb-v2/usb_urb.c
> 
> This particular driver allocates 8 buffers with 4096 bytes each
> for bulk transfers, using transfer_flags = URB_NO_TRANSFER_DMA_MAP.
> 
> This become a popular USB hardware nowadays. I have one S960c
> myself, so I can send you the lsusb from it. You should notice, however,
> that a DVB-C/DVB-S2 channel can easily provide very high sustained bit
> rates. Here, on my DVB-S2 provider, a typical transponder produces 58 Mpps
> of payload after removing URB headers.

You mentioned earlier that the driver uses bulk transfers.  In USB-2.0,
the maximum possible payload data transfer rate using bulk transfers is
53248 bytes/ms, which is 53.248 MB/s (i.e., lower than 58 MB/s).  And
even this is possible only if almost nothing else is using the bus at
the same time.

> A 10 minutes record with the
> entire data (with typically contains 5-10 channels) can easily go
> above 4 GB, just to reproduce 1-2 glitches. So, I'm not sure if
> a usbmon dump would be useful.

It might not be helpful at all.  However, I'm not interested in the 
payload data (which would be unintelligible to me anyway) but rather 
the timing of URB submissions and completions.  A usbmon trace which 
didn't keep much of the payload data would only require on the order of 
50 MB per minute -- and Josef said that glitches usually would show up 
within a minute or so.

> I'm enclosing the lsusb from a S960C device, with is based on those
> Montage chipsets:

What I wanted to see was the output from "lsusb" on the affected
system, not the output from "lsusb -v -s B:D" on your system.

> > Overall, this may be a very difficult problem to solve.  The
> > 4cd13c21b207 commit was intended to improve throughput at the cost of
> > increased latency.  But then what do you do when the latency becomes
> > too high for the video subsystem to handle?
> 
> Latency can't be too high, otherwise frames will be dropped.

Yes, that's the whole point.

> Even if the Kernel itself doesn't drop, if the delay goes higher
> than a certain threshold, userspace will need to drop, as it
> should be presenting audio and video on real time. Yet, typically,
> userspace will delay it by one or two seconds, with would mean
> 1500-3500 buffers, with I suspect it is a lot more than the hardware
> limits. So I suspect that the hardware starves free buffers a way 
> before userspace, as media hardware don't have unlimited buffers
> inside them, as they assume that the Kernel/userspace will be fast
> enough to sustain bit rates up to 66 Mbps of payload.

The timing information would tell us how large the latency is.

In any case, you might be able to attack the problem simply by using
more than 8 buffers.  With just eight 4096-byte buffers, the total
pipeline capacity is only about 0.62 ms (at the maximum possible
transfer rate).  Increasing the number of buffers to 65 would give a
capacity of 5 ms, which is probably a lot better suited for situations
where completions are handled by the ksoftirqd thread.

> Perhaps media drivers could pass some quirk similar to URB_ISO_ASAP,
> in order to revert the kernel logic to prioritize latency instead of
> throughput.

It can't be done without pervasive changes to the USB subsystem, which
I would greatly prefer to avoid.  Besides, this wouldn't really solve
the problem.  Decreasing the latency for one device will cause it to be
increased for others.

Alan Stern

Re: dvb usb issues since kernel 4.9

[Mauro Carvalho Chehab]

Em Sat, 6 Jan 2018 16:44:20 -0500 (EST)
Alan Stern  escreveu:

> On Sat, 6 Jan 2018, Mauro Carvalho Chehab wrote:
> 
> > Hi Josef,
> > 
> > Em Sat, 6 Jan 2018 16:04:16 +0100
> > "Josef Griebichler"  escreveu:
> >   
> > > Hi,
> > > 
> > > the causing commit has been identified.
> > > After reverting commit 
> > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=4cd13c21b207e80ddb1144c576500098f2d5f882
> > > its working again.  
> > 
> > Just replying to me won't magically fix this. The ones that were involved on
> > this patch should also be c/c, plus USB people. Just added them.
> >   
> > > Please have a look into the thread 
> > > https://forum.libreelec.tv/thread/4235-dvb-issue-since-le-switched-to-kernel-4-9-x/?pageNo=13
> > > here are several users aknowledging the revert solves their issues with 
> > > usb dvb cards.  
> > 
> > I read the entire (long) thread there. In order to make easier for the
> > others, from what I understand, the problem happens on both x86 and arm,
> > although almost all comments there are mentioning tests with raspbian
> > Kernel (with uses a different USB host driver than the upstream one).
> > 
> > It happens when watching digital TV DVB-C channels, with usually means
> > a sustained bit rate of 11 MBps to 54 MBps.
> > 
> > The reports mention the dvbsky, with uses USB URB bulk transfers.
> > On every several minutes (5 to 10 mins), the stream suffer "glitches"
> > caused by frame losses.
> > 
> > The part of the thread that contains the bisect is at:
> > 
> > https://forum.libreelec.tv/thread/4235-dvb-issue-since-le-switched-to-kernel-4-9-x/?postID=75965#post75965
> > 
> > It indirectly mentions another comment on the thread with points
> > to:
> > https://github.com/raspberrypi/linux/issues/2134
> > 
> > There, it says that this fix part of the issues:
> > 
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=34f41c0316ed52b0b44542491d89278efdaa70e4
> > 
> > but it affects URB packet losses on a lesser extend.
> > 
> > The main issue is really the logic changes a the core softirq logic.
> > 
> > Using Kernel 4.14.10 on a Raspberry Pi 3 with 4cd13c2 commit reverted
> > fixed the issue. 
> > 
> > Joseph, is the above right? Anything else to mention? Does the
> > same issue affect also on x86 with vanilla Kernel 4.14.10?
> > 
> > -
> > 
> > It seems that the original patch were designed to solve some IRQ issues
> > with network cards with causes data losses on high traffic. However,
> > it is also causing bad effects on sustained high bandwidth demands
> > required by DVB cards, at least on some USB host drivers.
> > 
> > Alan/Greg/Eric/David:
> > 
> > Any ideas about how to fix it without causing regressions to
> > network?  
> 
> It would be good to know what hardware was involved on the x86 system
> and to have some timing data.  Can we see the output from lsusb and
> usbmon, running on a vanilla kernel that gets plenty of video glitches?

>From Josef's report, and from the BZ, the affected hardware seems
to be based on Montage Technology M88DS3103/M88TS2022 chipset.
The driver it uses is at drivers/media/usb/dvb-usb-v2/dvbsky.c,
with shares a USB implementation that is used by a lot more drivers.
The URB handling code is at:

drivers/media/usb/dvb-usb-v2/usb_urb.c

This particular driver allocates 8 buffers with 4096 bytes each
for bulk transfers, using transfer_flags = URB_NO_TRANSFER_DMA_MAP.

This become a popular USB hardware nowadays. I have one S960c
myself, so I can send you the lsusb from it. You should notice, however,
that a DVB-C/DVB-S2 channel can easily provide very high sustained bit
rates. Here, on my DVB-S2 provider, a typical transponder produces 58 Mpps
of payload after removing URB headers. A 10 minutes record with the
entire data (with typically contains 5-10 channels) can easily go
above 4 GB, just to reproduce 1-2 glitches. So, I'm not sure if
a usbmon dump would be useful.

I'm enclosing the lsusb from a S960C device, with is based on those
Montage chipsets:

Bus 002 Device 007: ID 0572:960c Conexant Systems (Rockwell), Inc. DVBSky S960C 
DVB-S2 tuner
Couldn't open device, some information will be missing
Device Descriptor:
  bLength18
  bDescriptorType 1
  bcdUSB   2.00
  bDeviceClass0 (Defined at Interface level)
  bDeviceSubClass 0 
  bDeviceProtocol 0 
  bMaxPacketSize064
  idVendor   0x0572 Conexant Systems (Rockwell), Inc.
  idProduct  0x960c DVBSky S960C DVB-S2 tuner
  bcdDevice0.00
  iManufacturer   1 
  iProduct2 
  iSerial 3 
  bNumConfigurations  1
  Configuration Descriptor:
bLength 9
bDescriptorType 2
wTotalLength  219
bNumInterfaces  1
bConfigurationValue 1
iConfiguration  4 

Re: [PATCH] media: ov9650: support device tree probing

[Sakari Ailus]

Hi Akinobu,

Thanks for the patch. Please see my comments below.

On Sat, Jan 06, 2018 at 02:49:03AM +0900, Akinobu Mita wrote:
> The ov9650 driver currently only supports legacy platform data probe.
> This change adds device tree probing.
> 
> There has been an attempt to add device tree support for ov9650 driver
> by Hugues Fruchet as a part of the patchset that adds support of OV9655
> camera (http://www.spinics.net/lists/linux-media/msg117903.html), but
> it wasn't merged into mainline because creating a separate driver for
> OV9655 is preferred.
> 
> This is very similar to Hugues's patch, but not supporting new device.
> 
> Cc: H. Nikolaus Schaller 
> Cc: Hugues Fruchet 
> Cc: Sakari Ailus 
> Cc: Mauro Carvalho Chehab 
> Cc: Rob Herring 
> Signed-off-by: Akinobu Mita 
> ---
>  .../devicetree/bindings/media/i2c/ov9650.txt   |  35 +++
>  drivers/media/i2c/ov9650.c | 107 
> -
>  2 files changed, 117 insertions(+), 25 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/ov9650.txt
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/ov9650.txt 
> b/Documentation/devicetree/bindings/media/i2c/ov9650.txt
> new file mode 100644
> index 000..aa5024d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/ov9650.txt
> @@ -0,0 +1,35 @@
> +* Omnivision OV9650/OV9652 CMOS sensor
> +
> +Required Properties:
> +- compatible: should be one of
> + "ovti,ov9650"
> + "ovti,ov9652"
> +- clocks: reference to the xvclk input clock.
> +
> +Optional Properties:
> +- reset-gpios: reference to the GPIO connected to the resetb pin, if any.
> +  Active is high.
> +- powerdown-gpios: reference to the GPIO connected to the pwdn pin, if any.
> +  Active is high.
> +
> +The device node must contain one 'port' child node for its digital output

s/must/shall/

Please do say there's one 'endpoint' as well.

> +video port, in accordance with the video interface bindings defined in
> +Documentation/devicetree/bindings/media/video-interfaces.txt.
> +
> +Example:
> +
> + {
> + ov9650: camera@30 {
> + compatible = "ovti,ov9650";
> + reg = <0x30>;
> + reset-gpios = <_gpio_0 0 GPIO_ACTIVE_HIGH>;
> + powerdown-gpios = <_gpio_0 1 GPIO_ACTIVE_HIGH>;
> + clocks = <>;
> +
> + port {
> + ov9650_0: endpoint {
> + remote-endpoint = <_in0>;
> + };
> + };
> + };
> +};
> diff --git a/drivers/media/i2c/ov9650.c b/drivers/media/i2c/ov9650.c
> index 69433e1..1affdc0 100644
> --- a/drivers/media/i2c/ov9650.c
> +++ b/drivers/media/i2c/ov9650.c
> @@ -11,8 +11,10 @@
>   * it under the terms of the GNU General Public License version 2 as
>   * published by the Free Software Foundation.
>   */
> +#include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -249,9 +251,10 @@ struct ov965x {
>   struct v4l2_subdev sd;
>   struct media_pad pad;
>   enum v4l2_mbus_type bus_type;
> - int gpios[NUM_GPIOS];
> + struct gpio_desc *gpios[NUM_GPIOS];
>   /* External master clock frequency */
>   unsigned long mclk_frequency;
> + struct clk *clk;
>  
>   /* Protects the struct fields below */
>   struct mutex lock;
> @@ -513,10 +516,9 @@ static int ov965x_set_color_matrix(struct ov965x *ov965x)
>   return 0;
>  }
>  
> -static void ov965x_gpio_set(int gpio, int val)
> +static void ov965x_gpio_set(struct gpio_desc *gpio, int val)

Could you call gpiod_set_value_cansleep() directly?

>  {
> - if (gpio_is_valid(gpio))
> - gpio_set_value(gpio, val);
> + gpiod_set_value_cansleep(gpio, val);
>  }
>  
>  static void __ov965x_set_power(struct ov965x *ov965x, int on)
> @@ -1408,16 +1410,17 @@ static const struct v4l2_subdev_ops ov965x_subdev_ops 
> = {
>  /*
>   * Reset and power down GPIOs configuration
>   */
> -static int ov965x_configure_gpios(struct ov965x *ov965x,
> -   const struct ov9650_platform_data *pdata)
> +static int ov965x_configure_gpios_pdata(struct ov965x *ov965x,
> + const struct ov9650_platform_data *pdata)
>  {
>   int ret, i;
> + int gpios[NUM_GPIOS];
>  
> - ov965x->gpios[GPIO_PWDN] = pdata->gpio_pwdn;
> - ov965x->gpios[GPIO_RST]  = pdata->gpio_reset;
> + gpios[GPIO_PWDN] = pdata->gpio_pwdn;
> + gpios[GPIO_RST]  = pdata->gpio_reset;
>  
>   for (i = 0; i < ARRAY_SIZE(ov965x->gpios); i++) {
> - int gpio = ov965x->gpios[i];
> + int gpio = gpios[i];
>  
>   if (!gpio_is_valid(gpio))
>   continue;
> @@ -1427,14 +1430,52 @@ static int ov965x_configure_gpios(struct ov965x 
> *ov965x,
>   return ret;
> 

Re: [PATCH 07/18] [media] uvcvideo: prevent bounds-check bypass via speculative execution

[Greg KH]

On Sat, Jan 06, 2018 at 09:41:17AM -0800, Dan Williams wrote:
> On Sat, Jan 6, 2018 at 1:40 AM, Greg KH  wrote:
> > On Sat, Jan 06, 2018 at 10:09:07AM +0100, Greg KH wrote:
> >> On Fri, Jan 05, 2018 at 05:10:32PM -0800, Dan Williams wrote:
> >> > Static analysis reports that 'index' may be a user controlled value that
> >> > is used as a data dependency to read 'pin' from the
> >> > 'selector->baSourceID' array. In order to avoid potential leaks of
> >> > kernel memory values, block speculative execution of the instruction
> >> > stream that could issue reads based on an invalid value of 'pin'.
> >> >
> >> > Based on an original patch by Elena Reshetova.
> >> >
> >> > Cc: Laurent Pinchart 
> >> > Cc: Mauro Carvalho Chehab 
> >> > Cc: linux-media@vger.kernel.org
> >> > Signed-off-by: Elena Reshetova 
> >> > Signed-off-by: Dan Williams 
> >> > ---
> >> >  drivers/media/usb/uvc/uvc_v4l2.c |7 +--
> >> >  1 file changed, 5 insertions(+), 2 deletions(-)
> >> >
> >> > diff --git a/drivers/media/usb/uvc/uvc_v4l2.c 
> >> > b/drivers/media/usb/uvc/uvc_v4l2.c
> >> > index 3e7e283a44a8..7442626dc20e 100644
> >> > --- a/drivers/media/usb/uvc/uvc_v4l2.c
> >> > +++ b/drivers/media/usb/uvc/uvc_v4l2.c
> >> > @@ -22,6 +22,7 @@
> >> >  #include 
> >> >  #include 
> >> >  #include 
> >> > +#include 
> >> >
> >> >  #include 
> >> >  #include 
> >> > @@ -810,6 +811,7 @@ static int uvc_ioctl_enum_input(struct file *file, 
> >> > void *fh,
> >> > struct uvc_entity *iterm = NULL;
> >> > u32 index = input->index;
> >> > int pin = 0;
> >> > +   __u8 *elem;
> >> >
> >> > if (selector == NULL ||
> >> > (chain->dev->quirks & UVC_QUIRK_IGNORE_SELECTOR_UNIT)) {
> >> > @@ -820,8 +822,9 @@ static int uvc_ioctl_enum_input(struct file *file, 
> >> > void *fh,
> >> > break;
> >> > }
> >> > pin = iterm->id;
> >> > -   } else if (index < selector->bNrInPins) {
> >> > -   pin = selector->baSourceID[index];
> >> > +   } else if ((elem = nospec_array_ptr(selector->baSourceID, index,
> >> > +   selector->bNrInPins))) {
> >> > +   pin = *elem;
> >>
> >> I dug through this before, and I couldn't find where index came from
> >> userspace, I think seeing the coverity rule would be nice.
> >
> > Ok, I take it back, this looks correct.  Ugh, the v4l ioctl api is
> > crazy complex (rightfully so), it's amazing that coverity could navigate
> > that whole thing :)
> >
> > While I'm all for fixing this type of thing, I feel like we need to do
> > something "else" for this as playing whack-a-mole for this pattern is
> > going to be a never-ending battle for all drivers for forever.  Either
> > we need some way to mark this data path to make it easy for tools like
> > sparse to flag easily, or we need to catch the issue in the driver
> > subsystems, which unfortunatly, would harm the drivers that don't have
> > this type of issue (like here.)
> >
> > I'm guessing that other operating systems, which don't have the luxury
> > of auditing all of their drivers are going for the "big hammer in the
> > subsystem" type of fix, right?
> >
> > I don't have a good answer for this, but if there was some better way to
> > rewrite these types of patterns to just prevent the need for the
> > nospec_array_ptr() type thing, that might be the best overall for
> > everyone.  Much like ebpf did with their changes.  That way a simple
> > coccinelle rule would be able to catch the pattern and rewrite it.
> >
> > Or am I just dreaming?
> 
> At least on the coccinelle front you're dreaming. Julia already took a
> look and said:
> 
> "I don't think Coccinelle would be good for doing this (ie
> implementing taint analysis) because the dataflow is too complicated."

Sorry for the confusion, no, I don't mean the "taint tracking", I mean
the generic pattern of "speculative out of bounds access" that we are
fixing here.

Yes, as you mentioned before, there are tons of false-positives in the
tree, as to find the real problems you have to show that userspace
controls the access index.  But if we have a generic pattern that can
rewrite that type of logic into one where it does not matter at all
(i.e. like the ebpf proposed changes), then it would not be an issue if
they are false or not, we just rewrite them all to be safe.

We need to find some way not only to fix these issues now (like you are
doing with this series), but to prevent them from every coming back into
the codebase again.  It's that second part that we need to keep in the
back of our minds here, while doing the first portion of this work.

> Perhaps the Coverity instance Dave mentioned at Ksummit 2012 has a
> role to play here?

We have a coverity instance that all kernel developers have access to
(just sign up and we grant it.)  We have at least one person