[PATCH] drm, video: fix use-before-NULL-check

2010-08-27 Thread Kees Cook
Fix potential crashes due to use-before-NULL situations.

Signed-off-by: Kees Cook kees.c...@canonical.com
---
 drivers/gpu/drm/drm_fb_helper.c   |3 ++-
 drivers/media/video/em28xx/em28xx-video.c |3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index de82e20..8dd7e6f 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -94,10 +94,11 @@ static bool 
drm_fb_helper_connector_parse_command_line(struct drm_fb_helper_conn
int i;
enum drm_connector_force force = DRM_FORCE_UNSPECIFIED;
struct drm_fb_helper_cmdline_mode *cmdline_mode;
-   struct drm_connector *connector = fb_helper_conn-connector;
+   struct drm_connector *connector;
 
if (!fb_helper_conn)
return false;
+   connector = fb_helper_conn-connector;
 
cmdline_mode = fb_helper_conn-cmdline_mode;
if (!mode_option)
diff --git a/drivers/media/video/em28xx/em28xx-video.c 
b/drivers/media/video/em28xx/em28xx-video.c
index 7b9ec6e..95a4b60 100644
--- a/drivers/media/video/em28xx/em28xx-video.c
+++ b/drivers/media/video/em28xx/em28xx-video.c
@@ -277,12 +277,13 @@ static void em28xx_copy_vbi(struct em28xx *dev,
 {
void *startwrite, *startread;
int  offset;
-   int bytesperline = dev-vbi_width;
+   int bytesperline;
 
if (dev == NULL) {
em28xx_isocdbg(dev is null\n);
return;
}
+   bytesperline = dev-vbi_width;
 
if (dma_q == NULL) {
em28xx_isocdbg(dma_q is null\n);
-- 
1.7.1


-- 
Kees Cook
Ubuntu Security Team
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] video: fix potential use-before-NULL-check crash

2010-10-06 Thread Kees Cook
Moves use to after NULL-check.

Signed-off-by: Kees Cook kees.c...@canonical.com
---

Sent before as part of https://patchwork.kernel.org/patch/138711/ but it
still hasn't been applied.

---
 drivers/media/video/em28xx/em28xx-video.c |3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/media/video/em28xx/em28xx-video.c 
b/drivers/media/video/em28xx/em28xx-video.c
index 7b9ec6e..95a4b60 100644
--- a/drivers/media/video/em28xx/em28xx-video.c
+++ b/drivers/media/video/em28xx/em28xx-video.c
@@ -277,12 +277,13 @@ static void em28xx_copy_vbi(struct em28xx *dev,
 {
void *startwrite, *startread;
int  offset;
-   int bytesperline = dev-vbi_width;
+   int bytesperline;
 
if (dev == NULL) {
em28xx_isocdbg(dev is null\n);
return;
}
+   bytesperline = dev-vbi_width;
 
if (dma_q == NULL) {
em28xx_isocdbg(dma_q is null\n);
-- 
1.7.1


-- 
Kees Cook
Ubuntu Security Team
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] dvb: fix potential format string leak

2013-09-16 Thread Kees Cook
Make sure that a format string cannot accidentally leak into the printk
buffer.

Signed-off-by: Kees Cook keesc...@chromium.org
---
 drivers/media/dvb-frontends/dib9000.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/dvb-frontends/dib9000.c 
b/drivers/media/dvb-frontends/dib9000.c
index 6201c59..61b2cfe 100644
--- a/drivers/media/dvb-frontends/dib9000.c
+++ b/drivers/media/dvb-frontends/dib9000.c
@@ -649,7 +649,7 @@ static int dib9000_risc_debug_buf(struct dib9000_state 
*state, u16 * data, u8 si
b[2 * (size - 2) - 1] = '\0';   /* Bullet proof the buffer */
if (*b == '~') {
b++;
-   dprintk(b);
+   dprintk(%s, b);
} else
dprintk(RISC%d: %d.%04d %s, state-fe_id, ts / 1, ts % 
1, *b ? b : emtpy);
return 1;
-- 
1.7.9.5


-- 
Kees Cook
Chrome OS Security
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] doc: no singing

2013-12-05 Thread Kees Cook
Stop that, stop that! You're not going to do a song while I'm here.

Signed-off-by: Kees Cook keesc...@chromium.org
---
https://lkml.org/lkml/2013/12/4/786
http://www.youtube.com/watch?v=g3YiPC91QUk#t=62
---
 Documentation/cgroups/resource_counter.txt |2 +-
 Documentation/video4linux/si476x.txt   |2 +-
 arch/score/lib/checksum.S  |2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/cgroups/resource_counter.txt 
b/Documentation/cgroups/resource_counter.txt
index c4d99ed0b418..caa6d662b230 100644
--- a/Documentation/cgroups/resource_counter.txt
+++ b/Documentation/cgroups/resource_counter.txt
@@ -95,7 +95,7 @@ to work with it.
 
  f. u64 res_counter_uncharge_until
(struct res_counter *rc, struct res_counter *top,
-unsinged long val)
+unsigned long val)
 
Almost same as res_cunter_uncharge() but propagation of uncharge
stops when rc == top. This is useful when kill a res_coutner in
diff --git a/Documentation/video4linux/si476x.txt 
b/Documentation/video4linux/si476x.txt
index 2f9b4875ab8a..616607955aaf 100644
--- a/Documentation/video4linux/si476x.txt
+++ b/Documentation/video4linux/si476x.txt
@@ -147,7 +147,7 @@ The drivers exposes following files:
   
   0x12 | readfreq  | Current tuned frequency
   
-  0x14 | freqoff   | Singed frequency offset in units of
+  0x14 | freqoff   | Signed frequency offset in units of
|   | 2ppm
   
   0x15 | rssi  | Signed value of RSSI in dBuV
diff --git a/arch/score/lib/checksum.S b/arch/score/lib/checksum.S
index 706157edc7d5..1141f2b4a501 100644
--- a/arch/score/lib/checksum.S
+++ b/arch/score/lib/checksum.S
@@ -137,7 +137,7 @@ ENTRY(csum_partial)
ldi r25, 0
mv r10, r5
cmpi.c  r5, 0x8
-   blt small_csumcpy   /*  8(singed) bytes to copy */
+   blt small_csumcpy   /*  8(signed) bytes to copy */
cmpi.c  r5, 0x0
beq out
andri.c r25, src, 0x1   /* odd buffer? */
-- 
1.7.9.5


-- 
Kees Cook
Chrome OS Security
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] media: rc-core: use %s in rc_map_get() module load

2014-03-11 Thread Kees Cook
rc_map_get() takes a single string literal for the module to load,
so make sure it cannot be used as a format string in the call to
request_module().

Signed-off-by: Kees Cook keesc...@chromium.org
---
On another security note, this raw request_module() call should have
some kind of prefix associated with it to make sure it can't be used to
load arbitrary modules. For example, request_module(probe-%s, name)
or something, as done for crypto, binfmts, etc.
---
 drivers/media/rc/rc-main.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
index 02e2f38c9c85..dca97aa0a51e 100644
--- a/drivers/media/rc/rc-main.c
+++ b/drivers/media/rc/rc-main.c
@@ -62,7 +62,7 @@ struct rc_map *rc_map_get(const char *name)
map = seek_rc_map(name);
 #ifdef MODULE
if (!map) {
-   int rc = request_module(name);
+   int rc = request_module(%s, name);
if (rc  0) {
printk(KERN_ERR Couldn't load IR keymap %s\n, name);
return NULL;
-- 
1.7.9.5


-- 
Kees Cook
Chrome OS Security
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/1] media: dib9000: avoid out of bound access

2014-06-18 Thread Kees Cook
On Wed, Jun 18, 2014 at 3:02 PM, Heinrich Schuchardt xypron.g...@gmx.de wrote:
 The current test to avoid out of bound access to mb[] is insufficient.
 For len = 19 non-existent mb[10] will be accessed.

 A check in the for loop is insufficient to avoid out of bound access in
 dib9000_mbx_send_attr.

 Signed-off-by: Heinrich Schuchardt xypron.g...@gmx.de
 ---
  drivers/media/dvb-frontends/dib9000.c | 5 -
  1 file changed, 4 insertions(+), 1 deletion(-)

 diff --git a/drivers/media/dvb-frontends/dib9000.c 
 b/drivers/media/dvb-frontends/dib9000.c
 index e540cfb..6a71917 100644
 --- a/drivers/media/dvb-frontends/dib9000.c
 +++ b/drivers/media/dvb-frontends/dib9000.c
 @@ -1040,10 +1040,13 @@ static int dib9000_risc_apb_access_write(struct 
 dib9000_state *state, u32 addres
 if (address = 1024 || !state-platform.risc.fw_is_running)
 return -EINVAL;

 +   if (len  18)
 +   return -EINVAL;
 +
 /* dprintk( APB access thru wr fw %d %x, address, attribute); */

 mb[0] = (unsigned short)address;
 -   for (i = 0; i  len  i  20; i += 2)
 +   for (i = 0; i  len; i += 2)
 mb[1 + (i / 2)] = (b[i]  8 | b[i + 1]);

Good catch on the mb[] access! However, I think there is still more to
fix since b[i + 1] can read past the end of b: Say b is defined as u8
b[3]. Passing len 3 means the second loop, with i==2 will access b[2]
and b[3], the latter is out of range.

-Kees


 dib9000_mbx_send_attr(state, OUT_MSG_BRIDGE_APB_W, mb, 1 + len / 2, 
 attribute);
 --
 2.0.0




-- 
Kees Cook
Chrome OS Security
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/1] media: dib9000: avoid out of bound access

2014-06-18 Thread Kees Cook
On Wed, Jun 18, 2014 at 6:41 PM, Heinrich Schuchardt xypron.g...@gmx.de wrote:
 On 19.06.2014 01:50, Kees Cook wrote:

 On Wed, Jun 18, 2014 at 3:02 PM, Heinrich Schuchardt xypron.g...@gmx.de
 wrote:

 The current test to avoid out of bound access to mb[] is insufficient.
 For len = 19 non-existent mb[10] will be accessed.

 A check in the for loop is insufficient to avoid out of bound access in
 dib9000_mbx_send_attr.

 Signed-off-by: Heinrich Schuchardt xypron.g...@gmx.de
 ---
   drivers/media/dvb-frontends/dib9000.c | 5 -
   1 file changed, 4 insertions(+), 1 deletion(-)

 diff --git a/drivers/media/dvb-frontends/dib9000.c
 b/drivers/media/dvb-frontends/dib9000.c
 index e540cfb..6a71917 100644
 --- a/drivers/media/dvb-frontends/dib9000.c
 +++ b/drivers/media/dvb-frontends/dib9000.c
 @@ -1040,10 +1040,13 @@ static int dib9000_risc_apb_access_write(struct
 dib9000_state *state, u32 addres
  if (address = 1024 || !state-platform.risc.fw_is_running)
  return -EINVAL;

 +   if (len  18)
 +   return -EINVAL;
 +
  /* dprintk( APB access thru wr fw %d %x, address, attribute);
 */

  mb[0] = (unsigned short)address;
 -   for (i = 0; i  len  i  20; i += 2)
 +   for (i = 0; i  len; i += 2)
  mb[1 + (i / 2)] = (b[i]  8 | b[i + 1]);


 Good catch on the mb[] access! However, I think there is still more to
 fix since b[i + 1] can read past the end of b: Say b is defined as u8
 b[3]. Passing len 3 means the second loop, with i==2 will access b[2]
 and b[3], the latter is out of range.


 b[] and len are provided by the caller of dib9000_risc_apb_access_write.
 dib9000_risc_apb_access_write cannot verify if the length of b[] matches len
 at all.

 Currently dib9000_risc_apb_access_write cannot handle odd values of len.
 This holds even true if b[] has been padded with zero to an even length: For
 odd values of len the last byte is not passed to dib9000_mbx_send_attr.

 What is left unclear is how odd values of len should be handled correctly:

 Should the caller provide a b[] padded with 0 to the next even number of
 bytes,
 or should dib9000_risc_apb_access_write take care not to read more then len
 bytes,
 or should odd values of len cause an error EINVAL.

 From what I read in the coding one source of the value of len is
 tuner_attach(), which is called from outside the dib9000 driver.

How about:

for (i = 0; i  len; i += 2) {
u16 val = b[i]  8;
if (i + 1  len)
 val |= b[i + 1];
mb[1 + (i / 2)] = val;

That's defensive, and would have the same effect of callers doing the padding.

-Kees


 Heinrich



 -Kees


  dib9000_mbx_send_attr(state, OUT_MSG_BRIDGE_APB_W, mb, 1 + len /
 2, attribute);
 --
 2.0.0








-- 
Kees Cook
Chrome OS Security
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/1 v2] media: dib9000: avoid out of bound access

2014-06-19 Thread Kees Cook
On Thu, Jun 19, 2014 at 7:49 AM, Heinrich Schuchardt xypron.g...@gmx.de wrote:
 This updated patch also fixes out of bound access to b[].

 In dib9000_risc_apb_access_write() an out of bound access to mb[].

 The current test to avoid out of bound access to mb[] is insufficient.
 For len = 19 non-existent mb[10] will be accessed.

 For odd values of len b[] is accessed out of bounds.

 For large values of len an of bound access to mb[] may occur in
 dib9000_mbx_send_attr.

 Signed-off-by: Heinrich Schuchardt xypron.g...@gmx.de
 ---
  drivers/media/dvb-frontends/dib9000.c | 13 +
  1 file changed, 9 insertions(+), 4 deletions(-)

 diff --git a/drivers/media/dvb-frontends/dib9000.c 
 b/drivers/media/dvb-frontends/dib9000.c
 index e540cfb..f75dec4 100644
 --- a/drivers/media/dvb-frontends/dib9000.c
 +++ b/drivers/media/dvb-frontends/dib9000.c
 @@ -1040,13 +1040,18 @@ static int dib9000_risc_apb_access_write(struct 
 dib9000_state *state, u32 addres
 if (address = 1024 || !state-platform.risc.fw_is_running)
 return -EINVAL;

 +   if (len  18)
 +   return -EINVAL;
 +
 /* dprintk( APB access thru wr fw %d %x, address, attribute); */

 -   mb[0] = (unsigned short)address;
 -   for (i = 0; i  len  i  20; i += 2)
 -   mb[1 + (i / 2)] = (b[i]  8 | b[i + 1]);
 +   mb[0] = (u16)address;
 +   for (i = 0; i + 1  len; i += 2)
 +   mb[1 + i / 2] = b[i]  8 | b[i + 1];
 +   if (len  1)
 +   mb[1 + len / 2] = b[len - 1]  8;

 -   dib9000_mbx_send_attr(state, OUT_MSG_BRIDGE_APB_W, mb, 1 + len / 2, 
 attribute);
 +   dib9000_mbx_send_attr(state, OUT_MSG_BRIDGE_APB_W, mb, (3 + len) / 2, 
 attribute);
 return dib9000_mbx_get_message_attr(state, IN_MSG_END_BRIDGE_APB_RW, 
 mb, s, attribute) == 1 ? 0 : -EINVAL;
  }

 --
 2.0.0


That looks great, thanks!

Reviewed-by: Kees Cook keesc...@chromium.org

-Kees


-- 
Kees Cook
Chrome OS Security
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] [media] af9035: make sure loading modules is const

2014-10-20 Thread Kees Cook
Make sure that loaded modules are const char strings so we don't
load arbitrary modules in the future, nor allow for format string
leaks in the module request call.

Signed-off-by: Kees Cook keesc...@chromium.org
---
 drivers/media/usb/dvb-usb-v2/af9035.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/usb/dvb-usb-v2/af9035.c 
b/drivers/media/usb/dvb-usb-v2/af9035.c
index 00758c83eec7..1896ab218b11 100644
--- a/drivers/media/usb/dvb-usb-v2/af9035.c
+++ b/drivers/media/usb/dvb-usb-v2/af9035.c
@@ -193,8 +193,8 @@ static int af9035_wr_reg_mask(struct dvb_usb_device *d, u32 
reg, u8 val,
return af9035_wr_regs(d, reg, val, 1);
 }
 
-static int af9035_add_i2c_dev(struct dvb_usb_device *d, char *type, u8 addr,
-   void *platform_data, struct i2c_adapter *adapter)
+static int af9035_add_i2c_dev(struct dvb_usb_device *d, const char *type,
+   u8 addr, void *platform_data, struct i2c_adapter *adapter)
 {
int ret, num;
struct state *state = d_to_priv(d);
@@ -221,7 +221,7 @@ static int af9035_add_i2c_dev(struct dvb_usb_device *d, 
char *type, u8 addr,
goto err;
}
 
-   request_module(board_info.type);
+   request_module(%s, board_info.type);
 
/* register I2C device */
client = i2c_new_device(adapter, board_info);
-- 
1.9.1


-- 
Kees Cook
Chrome OS Security
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] [media] anysee: make sure loading modules is const

2014-10-20 Thread Kees Cook
Make sure that loaded modules are const char strings so we don't
load arbitrary modules in the future, nor allow for format string
leaks in the module request call.

Signed-off-by: Kees Cook keesc...@chromium.org
---
 drivers/media/usb/dvb-usb-v2/anysee.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/usb/dvb-usb-v2/anysee.c 
b/drivers/media/usb/dvb-usb-v2/anysee.c
index d3c5f230e97a..ae917c042a52 100644
--- a/drivers/media/usb/dvb-usb-v2/anysee.c
+++ b/drivers/media/usb/dvb-usb-v2/anysee.c
@@ -630,8 +630,8 @@ error:
return ret;
 }
 
-static int anysee_add_i2c_dev(struct dvb_usb_device *d, char *type, u8 addr,
-   void *platform_data)
+static int anysee_add_i2c_dev(struct dvb_usb_device *d, const char *type,
+   u8 addr, void *platform_data)
 {
int ret, num;
struct anysee_state *state = d_to_priv(d);
@@ -659,7 +659,7 @@ static int anysee_add_i2c_dev(struct dvb_usb_device *d, 
char *type, u8 addr,
goto err;
}
 
-   request_module(board_info.type);
+   request_module(%s, board_info.type);
 
/* register I2C device */
client = i2c_new_device(adapter, board_info);
-- 
1.9.1


-- 
Kees Cook
Chrome OS Security
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0435/1285] Replace numeric parameter like 0444 with macro

2016-08-02 Thread Kees Cook
There are so many of these, I wonder if it'd be better to just do one
giant patch, or at least break them up by subsystem instead of by
individual source file...

-Kees

On Tue, Aug 2, 2016 at 4:11 AM, Baole Ni <baolex...@intel.com> wrote:
> I find that the developers often just specified the numeric value
> when calling a macro which is defined with a parameter for access permission.
> As we know, these numeric value for access permission have had the 
> corresponding macro,
> and that using macro can improve the robustness and readability of the code,
> thus, I suggest replacing the numeric parameter with the macro.
>
> Signed-off-by: Chuansheng Liu <chuansheng@intel.com>
> Signed-off-by: Baole Ni <baolex...@intel.com>
> ---
>  drivers/media/i2c/tvp5150.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
> index 0b6d46c..d8ffd88 100644
> --- a/drivers/media/i2c/tvp5150.c
> +++ b/drivers/media/i2c/tvp5150.c
> @@ -33,7 +33,7 @@ MODULE_LICENSE("GPL");
>
>
>  static int debug;
> -module_param(debug, int, 0644);
> +module_param(debug, int, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
>  MODULE_PARM_DESC(debug, "Debug level (0-2)");
>
>  struct tvp5150 {
> --
> 2.9.2
>



-- 
Kees Cook
Chrome OS & Brillo Security
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/1] subsystem:linux-media CVE-2016-5400

2016-07-15 Thread Kees Cook
[fixing Mauro's email...]

On Fri, Jul 15, 2016 at 11:52 AM, Kees Cook <keesc...@google.com> wrote:
> On Fri, Jul 15, 2016 at 8:40 AM, James Patrick-Evans <ja...@jmp-e.com> wrote:
>> This patch addresses CVE-2016-5400, a local DOS vulnerability caused by a
>> memory leak in the airspy usb device driver. The vulnerability is triggered
>> when more than 64 usb devices register with v4l2 of type VFL_TYPE_SDR or
>> VFL_TYPE_SUBDEV.A badusb device can emulate 64 of these devices then through
>> continual emulated connect/disconnect of the 65th device, cause the kernel
>> to run out of RAM and crash the kernel. The vulnerability exists in kernel
>> versions from 3.17 to current 4.7.
>> The memory leak is caused by the probe function of the airspy driver
>> mishandeling errors and not freeing the corresponding control structures
>> when an error occours registering the device to v4l2 core.
>
> Thanks for getting this fixed!
>
>> Signed-off-by: James Patrick-Evans <ja...@jmp-e.com>
>
> Reviewed-by: Kees Cook <keesc...@chromium.org>
>
>> ---
>>  drivers/media/usb/airspy/airspy.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/media/usb/airspy/airspy.c
>> b/drivers/media/usb/airspy/airspy.c
>> index 87c1293..6c3ac8b 100644
>> --- a/drivers/media/usb/airspy/airspy.c
>> +++ b/drivers/media/usb/airspy/airspy.c
>> @@ -1072,7 +1072,7 @@ static int airspy_probe(struct usb_interface *intf,
>> if (ret) {
>> dev_err(s->dev, "Failed to register as video device (%d)\n",
>> ret);
>> -   goto err_unregister_v4l2_dev;
>> +   goto err_free_controls;
>> }
>> dev_info(s->dev, "Registered as %s\n",
>> video_device_node_name(>vdev));
>> --
>> 1.9.1
>>
>
> -Kees
>
> --
> Kees Cook
> Brillo & Chrome OS Security



-- 
Kees Cook
Brillo & Chrome OS Security
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/1] subsystem:linux-media CVE-2016-5400

2016-07-15 Thread Kees Cook
On Fri, Jul 15, 2016 at 8:40 AM, James Patrick-Evans <ja...@jmp-e.com> wrote:
> This patch addresses CVE-2016-5400, a local DOS vulnerability caused by a
> memory leak in the airspy usb device driver. The vulnerability is triggered
> when more than 64 usb devices register with v4l2 of type VFL_TYPE_SDR or
> VFL_TYPE_SUBDEV.A badusb device can emulate 64 of these devices then through
> continual emulated connect/disconnect of the 65th device, cause the kernel
> to run out of RAM and crash the kernel. The vulnerability exists in kernel
> versions from 3.17 to current 4.7.
> The memory leak is caused by the probe function of the airspy driver
> mishandeling errors and not freeing the corresponding control structures
> when an error occours registering the device to v4l2 core.

Thanks for getting this fixed!

> Signed-off-by: James Patrick-Evans <ja...@jmp-e.com>

Reviewed-by: Kees Cook <keesc...@chromium.org>

> ---
>  drivers/media/usb/airspy/airspy.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/usb/airspy/airspy.c
> b/drivers/media/usb/airspy/airspy.c
> index 87c1293..6c3ac8b 100644
> --- a/drivers/media/usb/airspy/airspy.c
> +++ b/drivers/media/usb/airspy/airspy.c
> @@ -1072,7 +1072,7 @@ static int airspy_probe(struct usb_interface *intf,
> if (ret) {
> dev_err(s->dev, "Failed to register as video device (%d)\n",
> ret);
> -   goto err_unregister_v4l2_dev;
> +   goto err_free_controls;
> }
> dev_info(s->dev, "Registered as %s\n",
> video_device_node_name(>vdev));
> --
> 1.9.1
>

-Kees

-- 
Kees Cook
Brillo & Chrome OS Security
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] solo6x10: use designated initializers

2017-01-06 Thread Kees Cook
On Mon, Dec 19, 2016 at 11:56 AM, Andrey Utkin
<andrey.ut...@corp.bluecherry.net> wrote:
> On Fri, Dec 16, 2016 at 05:05:36PM -0800, Kees Cook wrote:
>> Prepare to mark sensitive kernel structures for randomization by making
>> sure they're using designated initializers. These were identified during
>> allyesconfig builds of x86, arm, and arm64, with most initializer fixes
>> extracted from grsecurity.
>
> Ok I've reviewed all the patchset, googled a bit and now I see what's
> going on.
>
>>
>> Signed-off-by: Kees Cook <keesc...@chromium.org>
>> ---
>>  drivers/media/pci/solo6x10/solo6x10-g723.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/media/pci/solo6x10/solo6x10-g723.c 
>> b/drivers/media/pci/solo6x10/solo6x10-g723.c
>> index 6a35107aca25..36e93540bb49 100644
>> --- a/drivers/media/pci/solo6x10/solo6x10-g723.c
>> +++ b/drivers/media/pci/solo6x10/solo6x10-g723.c
>> @@ -350,7 +350,7 @@ static int solo_snd_pcm_init(struct solo_dev *solo_dev)
>>
>>  int solo_g723_init(struct solo_dev *solo_dev)
>>  {
>> - static struct snd_device_ops ops = { NULL };
>> + static struct snd_device_ops ops = { };
>
> I'm not that keen on syntax subtleties, but...
>  * Empty initializer is not quite "designated" as I can judge.
>  * From brief googling I see that empty initializer is not valid in
>some C standards.
>
> Since `ops` is static, what about this?
> For the variant given below, you have my signoff.
>
>> --- a/drivers/media/pci/solo6x10/solo6x10-g723.c
>> +++ b/drivers/media/pci/solo6x10/solo6x10-g723.c
>> @@ -350,7 +350,7 @@ static int solo_snd_pcm_init(struct solo_dev *solo_dev)
>>
>>  int solo_g723_init(struct solo_dev *solo_dev)
>>  {
>> - static struct snd_device_ops ops = { NULL };
>> + static struct snd_device_ops ops;

Ah! Yes, thanks. That works fine too. :) Can this be const as well?

-Kees

-- 
Kees Cook
Nexus Security
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] mtk-vcodec: use designated initializers

2016-12-16 Thread Kees Cook
Prepare to mark sensitive kernel structures for randomization by making
sure they're using designated initializers. These were identified during
allyesconfig builds of x86, arm, and arm64, with most initializer fixes
extracted from grsecurity.

Signed-off-by: Kees Cook <keesc...@chromium.org>
---
 drivers/media/platform/mtk-vcodec/venc/venc_h264_if.c | 8 
 drivers/media/platform/mtk-vcodec/venc/venc_vp8_if.c  | 8 
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/media/platform/mtk-vcodec/venc/venc_h264_if.c 
b/drivers/media/platform/mtk-vcodec/venc/venc_h264_if.c
index b76c80bdf30b..4eb3be37ba14 100644
--- a/drivers/media/platform/mtk-vcodec/venc/venc_h264_if.c
+++ b/drivers/media/platform/mtk-vcodec/venc/venc_h264_if.c
@@ -665,10 +665,10 @@ static int h264_enc_deinit(unsigned long handle)
 }
 
 static const struct venc_common_if venc_h264_if = {
-   h264_enc_init,
-   h264_enc_encode,
-   h264_enc_set_param,
-   h264_enc_deinit,
+   .init = h264_enc_init,
+   .encode = h264_enc_encode,
+   .set_param = h264_enc_set_param,
+   .deinit = h264_enc_deinit,
 };
 
 const struct venc_common_if *get_h264_enc_comm_if(void);
diff --git a/drivers/media/platform/mtk-vcodec/venc/venc_vp8_if.c 
b/drivers/media/platform/mtk-vcodec/venc/venc_vp8_if.c
index 544f57186243..a6fa145f2c54 100644
--- a/drivers/media/platform/mtk-vcodec/venc/venc_vp8_if.c
+++ b/drivers/media/platform/mtk-vcodec/venc/venc_vp8_if.c
@@ -470,10 +470,10 @@ static int vp8_enc_deinit(unsigned long handle)
 }
 
 static const struct venc_common_if venc_vp8_if = {
-   vp8_enc_init,
-   vp8_enc_encode,
-   vp8_enc_set_param,
-   vp8_enc_deinit,
+   .init = vp8_enc_init,
+   .encode = vp8_enc_encode,
+   .set_param = vp8_enc_set_param,
+   .deinit = vp8_enc_deinit,
 };
 
 const struct venc_common_if *get_vp8_enc_comm_if(void);
-- 
2.7.4


-- 
Kees Cook
Nexus Security
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] solo6x10: use designated initializers

2016-12-16 Thread Kees Cook
Prepare to mark sensitive kernel structures for randomization by making
sure they're using designated initializers. These were identified during
allyesconfig builds of x86, arm, and arm64, with most initializer fixes
extracted from grsecurity.

Signed-off-by: Kees Cook <keesc...@chromium.org>
---
 drivers/media/pci/solo6x10/solo6x10-g723.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/pci/solo6x10/solo6x10-g723.c 
b/drivers/media/pci/solo6x10/solo6x10-g723.c
index 6a35107aca25..36e93540bb49 100644
--- a/drivers/media/pci/solo6x10/solo6x10-g723.c
+++ b/drivers/media/pci/solo6x10/solo6x10-g723.c
@@ -350,7 +350,7 @@ static int solo_snd_pcm_init(struct solo_dev *solo_dev)
 
 int solo_g723_init(struct solo_dev *solo_dev)
 {
-   static struct snd_device_ops ops = { NULL };
+   static struct snd_device_ops ops = { };
struct snd_card *card;
struct snd_kcontrol_new kctl;
char name[32];
-- 
2.7.4


-- 
Kees Cook
Nexus Security
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 17/31] media/i2c/tc358743: Initialize timer

2017-09-20 Thread Kees Cook
This converts to use setup_timer() to set callback and data, though it
doesn't look like this would have worked with timer checking enabled
since no init_timer() was ever called before.

Cc: Mats Randgaard <matra...@cisco.com>
Cc: Mauro Carvalho Chehab <mche...@kernel.org>
Cc: linux-media@vger.kernel.org
Signed-off-by: Kees Cook <keesc...@chromium.org>
---
 drivers/media/i2c/tc358743.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c
index e6f5c363ccab..07ad6a3ff1ec 100644
--- a/drivers/media/i2c/tc358743.c
+++ b/drivers/media/i2c/tc358743.c
@@ -1964,8 +1964,8 @@ static int tc358743_probe(struct i2c_client *client,
} else {
INIT_WORK(>work_i2c_poll,
  tc358743_work_i2c_poll);
-   state->timer.data = (unsigned long)state;
-   state->timer.function = tc358743_irq_poll_timer;
+   setup_timer(>timer, tc358743_irq_poll_timer,
+   (unsigned long)state);
state->timer.expires = jiffies +
   msecs_to_jiffies(POLL_INTERVAL_MS);
add_timer(>timer);
-- 
2.7.4



[PATCH] media: saa7134: Convert timers to use timer_setup()

2017-10-04 Thread Kees Cook
In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly.

Cc: Mauro Carvalho Chehab <mche...@kernel.org>
Cc: Sakari Ailus <sakari.ai...@linux.intel.com>
Cc: Sean Young <s...@mess.org>
Cc: Geliang Tang <geliangt...@gmail.com>
Cc: Hans Verkuil <hans.verk...@cisco.com>
Cc: linux-media@vger.kernel.org
Cc: Thomas Gleixner <t...@linutronix.de>
Signed-off-by: Kees Cook <keesc...@chromium.org>
---
This requires commit 686fef928bba ("timer: Prepare to change timer
callback argument type") in v4.14-rc3, but should be otherwise
stand-alone.
---
 drivers/media/pci/saa7134/saa7134-core.c  | 6 +++---
 drivers/media/pci/saa7134/saa7134-input.c | 9 -
 drivers/media/pci/saa7134/saa7134-ts.c| 3 +--
 drivers/media/pci/saa7134/saa7134-vbi.c   | 3 +--
 drivers/media/pci/saa7134/saa7134-video.c | 3 +--
 drivers/media/pci/saa7134/saa7134.h   | 2 +-
 6 files changed, 11 insertions(+), 15 deletions(-)

diff --git a/drivers/media/pci/saa7134/saa7134-core.c 
b/drivers/media/pci/saa7134/saa7134-core.c
index 7976c5a12ca8..9e76de2411ae 100644
--- a/drivers/media/pci/saa7134/saa7134-core.c
+++ b/drivers/media/pci/saa7134/saa7134-core.c
@@ -338,9 +338,9 @@ void saa7134_buffer_next(struct saa7134_dev *dev,
}
 }
 
-void saa7134_buffer_timeout(unsigned long data)
+void saa7134_buffer_timeout(struct timer_list *t)
 {
-   struct saa7134_dmaqueue *q = (struct saa7134_dmaqueue *)data;
+   struct saa7134_dmaqueue *q = from_timer(q, t, timeout);
struct saa7134_dev *dev = q->dev;
unsigned long flags;
 
@@ -378,7 +378,7 @@ void saa7134_stop_streaming(struct saa7134_dev *dev, struct 
saa7134_dmaqueue *q)
}
}
spin_unlock_irqrestore(>slock, flags);
-   saa7134_buffer_timeout((unsigned long)q); /* also calls 
del_timer(>timeout) */
+   saa7134_buffer_timeout(>timeout); /* also calls 
del_timer(>timeout) */
 }
 EXPORT_SYMBOL_GPL(saa7134_stop_streaming);
 
diff --git a/drivers/media/pci/saa7134/saa7134-input.c 
b/drivers/media/pci/saa7134/saa7134-input.c
index 9337e4615519..2d5abeddc079 100644
--- a/drivers/media/pci/saa7134/saa7134-input.c
+++ b/drivers/media/pci/saa7134/saa7134-input.c
@@ -447,10 +447,10 @@ void saa7134_input_irq(struct saa7134_dev *dev)
}
 }
 
-static void saa7134_input_timer(unsigned long data)
+static void saa7134_input_timer(struct timer_list *t)
 {
-   struct saa7134_dev *dev = (struct saa7134_dev *)data;
-   struct saa7134_card_ir *ir = dev->remote;
+   struct saa7134_card_ir *ir = from_timer(ir, t, timer);
+   struct saa7134_dev *dev = ir->dev->priv;
 
build_key(dev);
mod_timer(>timer, jiffies + msecs_to_jiffies(ir->polling));
@@ -507,8 +507,7 @@ static int __saa7134_ir_start(void *priv)
ir->running = true;
 
if (ir->polling) {
-   setup_timer(>timer, saa7134_input_timer,
-   (unsigned long)dev);
+   timer_setup(>timer, saa7134_input_timer, 0);
ir->timer.expires = jiffies + HZ;
add_timer(>timer);
}
diff --git a/drivers/media/pci/saa7134/saa7134-ts.c 
b/drivers/media/pci/saa7134/saa7134-ts.c
index 7414878af9e0..2be703617e29 100644
--- a/drivers/media/pci/saa7134/saa7134-ts.c
+++ b/drivers/media/pci/saa7134/saa7134-ts.c
@@ -223,8 +223,7 @@ int saa7134_ts_init1(struct saa7134_dev *dev)
dev->ts.nr_packets = ts_nr_packets;
 
INIT_LIST_HEAD(>ts_q.queue);
-   setup_timer(>ts_q.timeout, saa7134_buffer_timeout,
-   (unsigned long)(>ts_q));
+   timer_setup(>ts_q.timeout, saa7134_buffer_timeout, 0);
dev->ts_q.dev  = dev;
dev->ts_q.need_two = 1;
dev->ts_started= 0;
diff --git a/drivers/media/pci/saa7134/saa7134-vbi.c 
b/drivers/media/pci/saa7134/saa7134-vbi.c
index bcad9b2d9bb3..af494c6147f1 100644
--- a/drivers/media/pci/saa7134/saa7134-vbi.c
+++ b/drivers/media/pci/saa7134/saa7134-vbi.c
@@ -181,8 +181,7 @@ struct vb2_ops saa7134_vbi_qops = {
 int saa7134_vbi_init1(struct saa7134_dev *dev)
 {
INIT_LIST_HEAD(>vbi_q.queue);
-   setup_timer(>vbi_q.timeout, saa7134_buffer_timeout,
-   (unsigned long)(>vbi_q));
+   timer_setup(>vbi_q.timeout, saa7134_buffer_timeout, 0);
dev->vbi_q.dev  = dev;
 
if (vbibufs < 2)
diff --git a/drivers/media/pci/saa7134/saa7134-video.c 
b/drivers/media/pci/saa7134/saa7134-video.c
index 51d42bbf969e..82d2a24644e4 100644
--- a/drivers/media/pci/saa7134/saa7134-video.c
+++ b/drivers/media/pci/saa7134/saa7134-video.c
@@ -2145,8 +2145,7 @@ int saa7134_video_init1(struct saa7134_dev *dev)
dev->automute   = 0;
 
INIT_LIST_HEAD(>video_q.queue)

[PATCH] media/saa7146: Convert timers to use timer_setup()

2017-10-04 Thread Kees Cook
In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly. This requires adding a pointer to
hold the timer's target file, as there won't be a way to pass this in the
future.

Cc: Hans Verkuil <hverk...@xs4all.nl>
Cc: Mauro Carvalho Chehab <mche...@kernel.org>
Cc: linux-media@vger.kernel.org
Cc: Thomas Gleixner <t...@linutronix.de>
Signed-off-by: Kees Cook <keesc...@chromium.org>
---
This requires commit 686fef928bba ("timer: Prepare to change timer
callback argument type") in v4.14-rc3, but should be otherwise
stand-alone.
---
 drivers/media/common/saa7146/saa7146_fops.c | 2 +-
 drivers/media/common/saa7146/saa7146_vbi.c  | 9 +
 include/media/drv-intf/saa7146_vv.h | 1 +
 3 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/media/common/saa7146/saa7146_fops.c 
b/drivers/media/common/saa7146/saa7146_fops.c
index 930d2c94d5d3..c4664f0da874 100644
--- a/drivers/media/common/saa7146/saa7146_fops.c
+++ b/drivers/media/common/saa7146/saa7146_fops.c
@@ -559,7 +559,7 @@ int saa7146_vv_init(struct saa7146_dev* dev, struct 
saa7146_ext_vv *ext_vv)
vbi->start[1] = 312;
vbi->count[1] = 16;
 
-   init_timer(>vbi_read_timeout);
+   timer_setup(>vbi_read_timeout, NULL, 0);
 
vv->ov_fb.capability = V4L2_FBUF_CAP_LIST_CLIPPING;
vv->ov_fb.flags = V4L2_FBUF_FLAG_PRIMARY;
diff --git a/drivers/media/common/saa7146/saa7146_vbi.c 
b/drivers/media/common/saa7146/saa7146_vbi.c
index d79e4d7ecd9f..f65c0b934c22 100644
--- a/drivers/media/common/saa7146/saa7146_vbi.c
+++ b/drivers/media/common/saa7146/saa7146_vbi.c
@@ -348,9 +348,10 @@ static void vbi_stop(struct saa7146_fh *fh, struct file 
*file)
spin_unlock_irqrestore(>slock, flags);
 }
 
-static void vbi_read_timeout(unsigned long data)
+static void vbi_read_timeout(struct timer_list *t)
 {
-   struct file *file = (struct file*)data;
+   struct saa7146_vv *vv = from_timer(vv, t, vbi_read_timeout);
+   struct file *file = vv->vbi_read_timeout_file;
struct saa7146_fh *fh = file->private_data;
struct saa7146_dev *dev = fh->dev;
 
@@ -401,8 +402,8 @@ static int vbi_open(struct saa7146_dev *dev, struct file 
*file)
sizeof(struct saa7146_buf),
file, >v4l2_lock);
 
-   vv->vbi_read_timeout.function = vbi_read_timeout;
-   vv->vbi_read_timeout.data = (unsigned long)file;
+   vv->vbi_read_timeout.function = (TIMER_FUNC_TYPE)vbi_read_timeout;
+   vv->vbi_read_timeout_file = file;
 
/* initialize the brs */
if ( 0 != (SAA7146_USE_PORT_B_FOR_VBI & dev->ext_vv_data->flags)) {
diff --git a/include/media/drv-intf/saa7146_vv.h 
b/include/media/drv-intf/saa7146_vv.h
index 0da6ccc0615b..fbe36d05fb79 100644
--- a/include/media/drv-intf/saa7146_vv.h
+++ b/include/media/drv-intf/saa7146_vv.h
@@ -107,6 +107,7 @@ struct saa7146_vv
struct saa7146_dmaqueue vbi_dmaq;
struct v4l2_vbi_format  vbi_fmt;
struct timer_list   vbi_read_timeout;
+   struct file *vbi_read_timeout_file;
/* vbi workaround interrupt queue */
wait_queue_head_t   vbi_wq;
int vbi_fieldcount;
-- 
2.7.4


-- 
Kees Cook
Pixel Security


[PATCH] media: saa7146: Convert timers to use timer_setup()

2017-10-04 Thread Kees Cook
In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly.

Cc: Hans Verkuil <hverk...@xs4all.nl>
Cc: Mauro Carvalho Chehab <mche...@kernel.org>
Cc: linux-media@vger.kernel.org
Cc: Thomas Gleixner <t...@linutronix.de>
Signed-off-by: Kees Cook <keesc...@chromium.org>
---
This requires commit 686fef928bba ("timer: Prepare to change timer
callback argument type") in v4.14-rc3, but should be otherwise
stand-alone.
---
 drivers/media/common/saa7146/saa7146_fops.c  | 4 ++--
 drivers/media/common/saa7146/saa7146_vbi.c   | 3 +--
 drivers/media/common/saa7146/saa7146_video.c | 3 +--
 include/media/drv-intf/saa7146_vv.h  | 2 +-
 4 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/media/common/saa7146/saa7146_fops.c 
b/drivers/media/common/saa7146/saa7146_fops.c
index c4664f0da874..8c87d6837c49 100644
--- a/drivers/media/common/saa7146/saa7146_fops.c
+++ b/drivers/media/common/saa7146/saa7146_fops.c
@@ -163,9 +163,9 @@ void saa7146_buffer_next(struct saa7146_dev *dev,
}
 }
 
-void saa7146_buffer_timeout(unsigned long data)
+void saa7146_buffer_timeout(struct timer_list *t)
 {
-   struct saa7146_dmaqueue *q = (struct saa7146_dmaqueue*)data;
+   struct saa7146_dmaqueue *q = from_timer(q, t, timeout);
struct saa7146_dev *dev = q->dev;
unsigned long flags;
 
diff --git a/drivers/media/common/saa7146/saa7146_vbi.c 
b/drivers/media/common/saa7146/saa7146_vbi.c
index f65c0b934c22..0f7599ad4958 100644
--- a/drivers/media/common/saa7146/saa7146_vbi.c
+++ b/drivers/media/common/saa7146/saa7146_vbi.c
@@ -366,8 +366,7 @@ static void vbi_init(struct saa7146_dev *dev, struct 
saa7146_vv *vv)
 
INIT_LIST_HEAD(>vbi_dmaq.queue);
 
-   setup_timer(>vbi_dmaq.timeout, saa7146_buffer_timeout,
-   (unsigned long)(>vbi_dmaq));
+   timer_setup(>vbi_dmaq.timeout, saa7146_buffer_timeout, 0);
vv->vbi_dmaq.dev  = dev;
 
init_waitqueue_head(>vbi_wq);
diff --git a/drivers/media/common/saa7146/saa7146_video.c 
b/drivers/media/common/saa7146/saa7146_video.c
index 37b4654dc21c..ed9b5300dcce 100644
--- a/drivers/media/common/saa7146/saa7146_video.c
+++ b/drivers/media/common/saa7146/saa7146_video.c
@@ -1201,8 +1201,7 @@ static void video_init(struct saa7146_dev *dev, struct 
saa7146_vv *vv)
 {
INIT_LIST_HEAD(>video_dmaq.queue);
 
-   setup_timer(>video_dmaq.timeout, saa7146_buffer_timeout,
-   (unsigned long)(>video_dmaq));
+   timer_setup(>video_dmaq.timeout, saa7146_buffer_timeout, 0);
vv->video_dmaq.dev  = dev;
 
/* set some default values */
diff --git a/include/media/drv-intf/saa7146_vv.h 
b/include/media/drv-intf/saa7146_vv.h
index fbe36d05fb79..b1f25240968f 100644
--- a/include/media/drv-intf/saa7146_vv.h
+++ b/include/media/drv-intf/saa7146_vv.h
@@ -184,7 +184,7 @@ int saa7146_unregister_device(struct video_device *vid, 
struct saa7146_dev *dev)
 void saa7146_buffer_finish(struct saa7146_dev *dev, struct saa7146_dmaqueue 
*q, int state);
 void saa7146_buffer_next(struct saa7146_dev *dev, struct saa7146_dmaqueue 
*q,int vbi);
 int saa7146_buffer_queue(struct saa7146_dev *dev, struct saa7146_dmaqueue *q, 
struct saa7146_buf *buf);
-void saa7146_buffer_timeout(unsigned long data);
+void saa7146_buffer_timeout(struct timer_list *t);
 void saa7146_dma_free(struct saa7146_dev* dev,struct videobuf_queue *q,
    struct saa7146_buf *buf);
 
-- 
2.7.4


-- 
Kees Cook
Pixel Security


[PATCH] media: serial_ir: Convert timers to use timer_setup()

2017-10-04 Thread Kees Cook
In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly.

Cc: Sean Young <s...@mess.org>
Cc: Mauro Carvalho Chehab <mche...@kernel.org>
Cc: linux-media@vger.kernel.org
Cc: Thomas Gleixner <t...@linutronix.de>
Signed-off-by: Kees Cook <keesc...@chromium.org>
---
This requires commit 686fef928bba ("timer: Prepare to change timer
callback argument type") in v4.14-rc3, but should be otherwise
stand-alone.
---
 drivers/media/rc/serial_ir.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/media/rc/serial_ir.c b/drivers/media/rc/serial_ir.c
index 8b66926bc16a..8bf5637b3a69 100644
--- a/drivers/media/rc/serial_ir.c
+++ b/drivers/media/rc/serial_ir.c
@@ -470,7 +470,7 @@ static int hardware_init_port(void)
return 0;
 }
 
-static void serial_ir_timeout(unsigned long arg)
+static void serial_ir_timeout(struct timer_list *unused)
 {
DEFINE_IR_RAW_EVENT(ev);
 
@@ -540,8 +540,7 @@ static int serial_ir_probe(struct platform_device *dev)
 
serial_ir.rcdev = rcdev;
 
-   setup_timer(_ir.timeout_timer, serial_ir_timeout,
-   (unsigned long)_ir);
+   timer_setup(_ir.timeout_timer, serial_ir_timeout, 0);
 
result = devm_request_irq(>dev, irq, serial_ir_irq_handler,
  share_irq ? IRQF_SHARED : 0,
-- 
2.7.4


-- 
Kees Cook
Pixel Security


[PATCH] input: Convert timers to use timer_setup()

2017-10-04 Thread Kees Cook
In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly.

One input_dev user hijacks the input_dev software autorepeat timer to
perform its own repeat management. However, there is not path back
to the existing status variable, so add a generic one to the input
structure and use that instead.

Cc: Dmitry Torokhov <dmitry.torok...@gmail.com>
Cc: Mauro Carvalho Chehab <mche...@kernel.org>
Cc: Hans Verkuil <hans.verk...@cisco.com>
Cc: Sakari Ailus <sakari.ai...@linux.intel.com>
Cc: Geliang Tang <geliangt...@gmail.com>
Cc: linux-in...@vger.kernel.org
Cc: linux-media@vger.kernel.org
Cc: Thomas Gleixner <t...@linutronix.de>
Signed-off-by: Kees Cook <keesc...@chromium.org>
---
This requires commit 686fef928bba ("timer: Prepare to change timer
callback argument type") in v4.14-rc3, but should be otherwise
stand-alone.
---
 drivers/input/input.c   | 12 ++--
 drivers/media/pci/ttpci/av7110.h|  1 -
 drivers/media/pci/ttpci/av7110_ir.c | 16 
 include/linux/input.h   |  2 ++
 4 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/drivers/input/input.c b/drivers/input/input.c
index d268fdc23c64..497ad2dcb699 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -76,7 +76,7 @@ static void input_start_autorepeat(struct input_dev *dev, int 
code)
 {
if (test_bit(EV_REP, dev->evbit) &&
dev->rep[REP_PERIOD] && dev->rep[REP_DELAY] &&
-   dev->timer.data) {
+   dev->timer.function) {
dev->repeat_key = code;
mod_timer(>timer,
  jiffies + msecs_to_jiffies(dev->rep[REP_DELAY]));
@@ -179,9 +179,9 @@ static void input_pass_event(struct input_dev *dev,
  * dev->event_lock here to avoid racing with input_event
  * which may cause keys get "stuck".
  */
-static void input_repeat_key(unsigned long data)
+static void input_repeat_key(struct timer_list *t)
 {
-   struct input_dev *dev = (void *) data;
+   struct input_dev *dev = from_timer(dev, t, timer);
unsigned long flags;
 
spin_lock_irqsave(>event_lock, flags);
@@ -1790,7 +1790,7 @@ struct input_dev *input_allocate_device(void)
device_initialize(>dev);
mutex_init(>mutex);
spin_lock_init(>event_lock);
-   init_timer(>timer);
+   timer_setup(>timer, NULL, 0);
INIT_LIST_HEAD(>h_list);
INIT_LIST_HEAD(>node);
 
@@ -2053,8 +2053,8 @@ static void devm_input_device_unregister(struct device 
*dev, void *res)
  */
 void input_enable_softrepeat(struct input_dev *dev, int delay, int period)
 {
-   dev->timer.data = (unsigned long) dev;
-   dev->timer.function = input_repeat_key;
+   dev->timer.function = (TIMER_FUNC_TYPE)input_repeat_key;
+   dev->timer_data = 0;
dev->rep[REP_DELAY] = delay;
dev->rep[REP_PERIOD] = period;
 }
diff --git a/drivers/media/pci/ttpci/av7110.h b/drivers/media/pci/ttpci/av7110.h
index 347827925c14..b98a4f3006df 100644
--- a/drivers/media/pci/ttpci/av7110.h
+++ b/drivers/media/pci/ttpci/av7110.h
@@ -93,7 +93,6 @@ struct infrared {
u8  inversion;
u16 last_key;
u16 last_toggle;
-   u8  delay_timer_finished;
 };
 
 
diff --git a/drivers/media/pci/ttpci/av7110_ir.c 
b/drivers/media/pci/ttpci/av7110_ir.c
index ca05198de2c2..a883caa6488c 100644
--- a/drivers/media/pci/ttpci/av7110_ir.c
+++ b/drivers/media/pci/ttpci/av7110_ir.c
@@ -155,16 +155,16 @@ static void av7110_emit_key(unsigned long parm)
if (timer_pending(>keyup_timer)) {
del_timer(>keyup_timer);
if (ir->last_key != keycode || toggle != ir->last_toggle) {
-   ir->delay_timer_finished = 0;
+   ir->input_dev->timer_data = 0;
input_event(ir->input_dev, EV_KEY, ir->last_key, 0);
input_event(ir->input_dev, EV_KEY, keycode, 1);
input_sync(ir->input_dev);
-   } else if (ir->delay_timer_finished) {
+   } else if (ir->input_dev->timer_data) {
input_event(ir->input_dev, EV_KEY, keycode, 2);
input_sync(ir->input_dev);
}
} else {
-   ir->delay_timer_finished = 0;
+   ir->input_dev->timer_data = 0;
input_event(ir->input_dev, EV_KEY, keycode, 1);
input_sync(ir->input_dev);
}
@@ -206,11 +206,12 @@ static void input_register_keys(struct infrared *ir)
 
 

[PATCH] staging/atomisp: Convert timers to use timer_setup()

2017-10-04 Thread Kees Cook
In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly.

Cc: Mauro Carvalho Chehab <mche...@kernel.org>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
Cc: Alan Cox <a...@linux.intel.com>
Cc: Daeseok Youn <daeseok.y...@gmail.com>
Cc: Arnd Bergmann <a...@arndb.de>
Cc: linux-media@vger.kernel.org
Cc: de...@driverdev.osuosl.org
Cc: Thomas Gleixner <t...@linutronix.de>
Signed-off-by: Kees Cook <keesc...@chromium.org>
---
This requires commit 686fef928bba ("timer: Prepare to change timer
callback argument type") in v4.14-rc3, but should be otherwise
stand-alone.
---
 drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c  | 13 -
 drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.h  |  6 +-
 .../media/atomisp/pci/atomisp2/atomisp_compat_css20.c |  2 +-
 drivers/staging/media/atomisp/pci/atomisp2/atomisp_v4l2.c | 15 +--
 4 files changed, 11 insertions(+), 25 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c 
b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
index f48bf451c1f5..6a38cd64b2a1 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
@@ -1660,20 +1660,15 @@ void atomisp_css_flush(struct atomisp_device *isp)
dev_dbg(isp->dev, "atomisp css flush done\n");
 }
 
-#ifndef ISP2401
-void atomisp_wdt(unsigned long isp_addr)
-#else
-void atomisp_wdt(unsigned long pipe_addr)
-#endif
+void atomisp_wdt(struct timer_list *t)
 {
 #ifndef ISP2401
-   struct atomisp_device *isp = (struct atomisp_device *)isp_addr;
+   struct atomisp_sub_device *asd = from_timer(asd, t, wdt);
 #else
-   struct atomisp_video_pipe *pipe =
-   (struct atomisp_video_pipe *)pipe_addr;
+   struct atomisp_video_pipe *pipe = from_timer(pipe, t, wdt);
struct atomisp_sub_device *asd = pipe->asd;
-   struct atomisp_device *isp = asd->isp;
 #endif
+   struct atomisp_device *isp = asd->isp;
 
 #ifdef ISP2401
atomic_inc(>wdt_count);
diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.h 
b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.h
index 31ba4e613d13..4bb83864da2e 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.h
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.h
@@ -85,11 +85,7 @@ static inline void __iomem 
*atomisp_get_io_virt_addr(unsigned int address)
 void atomisp_msi_irq_init(struct atomisp_device *isp, struct pci_dev *dev);
 void atomisp_msi_irq_uninit(struct atomisp_device *isp, struct pci_dev *dev);
 void atomisp_wdt_work(struct work_struct *work);
-#ifndef ISP2401
-void atomisp_wdt(unsigned long isp_addr);
-#else
-void atomisp_wdt(unsigned long pipe_addr);
-#endif
+void atomisp_wdt(struct timer_list *t);
 void atomisp_setup_flash(struct atomisp_sub_device *asd);
 irqreturn_t atomisp_isr(int irq, void *dev);
 irqreturn_t atomisp_isr_thread(int irq, void *isp_ptr);
diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_css20.c 
b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_css20.c
index 05897b747349..0b907474e024 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_css20.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_css20.c
@@ -4515,7 +4515,7 @@ int atomisp_css_isr_thread(struct atomisp_device *isp,
for (i = 0; i < isp->num_of_streams; i++)
atomisp_wdt_stop(>asd[i], 0);
 #ifndef ISP2401
-   atomisp_wdt((unsigned long)isp);
+   atomisp_wdt(>asd[0].wdt);
 #else
queue_work(isp->wdt_work_queue, >wdt_work);
 #endif
diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_v4l2.c 
b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_v4l2.c
index 663aa916e3ca..76dc779f32c3 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_v4l2.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_v4l2.c
@@ -1155,17 +1155,12 @@ static int init_atomisp_wdts(struct atomisp_device *isp)
struct atomisp_sub_device *asd = >asd[i];
asd = >asd[i];
 #ifndef ISP2401
-   setup_timer(>wdt, atomisp_wdt, (unsigned long)isp);
+   timer_setup(>wdt, atomisp_wdt, 0);
 #else
-   setup_timer(>video_out_capture.wdt,
-   atomisp_wdt, (unsigned long)>video_out_capture);
-   setup_timer(>video_out_preview.wdt,
-   atomisp_wdt, (unsigned long)>video_out_preview);
-   setup_timer(>video_out_vf.wdt,
-   atomisp_wdt, (unsigned long)>video_out_vf);
-   setup_timer(>v

Re: [PATCH] staging/atomisp: Convert timers to use timer_setup()

2017-10-17 Thread Kees Cook
On Tue, Oct 17, 2017 at 1:23 AM, Sakari Ailus <sakari.ai...@iki.fi> wrote:
> On Mon, Oct 16, 2017 at 04:24:56PM -0700, Kees Cook wrote:
>> In preparation for unconditionally passing the struct timer_list pointer to
>> all timer callbacks, switch to using the new timer_setup() and from_timer()
>> to pass the timer pointer explicitly.
>>
>> Cc: Mauro Carvalho Chehab <mche...@kernel.org>
>> Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
>> Cc: Alan Cox <a...@linux.intel.com>
>> Cc: Daeseok Youn <daeseok.y...@gmail.com>
>> Cc: Arnd Bergmann <a...@arndb.de>
>> Cc: linux-media@vger.kernel.org
>> Cc: de...@driverdev.osuosl.org
>> Signed-off-by: Kees Cook <keesc...@chromium.org>
>
> This appears to be the same as the patch I've applied previously.

Okay, sorry for the noise. I didn't see it in -next when I did my
rebase this week.

-Kees

-- 
Kees Cook
Pixel Security


[PATCH] media/saa7146: Convert timers to use timer_setup()

2017-10-16 Thread Kees Cook
In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly. This requires adding a pointer to
hold the timer's target file, as there won't be a way to pass this in the
future.

Cc: Hans Verkuil <hverk...@xs4all.nl>
Cc: Mauro Carvalho Chehab <mche...@kernel.org>
Cc: linux-media@vger.kernel.org
Signed-off-by: Kees Cook <keesc...@chromium.org>
---
 drivers/media/common/saa7146/saa7146_fops.c | 2 +-
 drivers/media/common/saa7146/saa7146_vbi.c  | 9 +
 include/media/drv-intf/saa7146_vv.h | 1 +
 3 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/media/common/saa7146/saa7146_fops.c 
b/drivers/media/common/saa7146/saa7146_fops.c
index 930d2c94d5d3..c4664f0da874 100644
--- a/drivers/media/common/saa7146/saa7146_fops.c
+++ b/drivers/media/common/saa7146/saa7146_fops.c
@@ -559,7 +559,7 @@ int saa7146_vv_init(struct saa7146_dev* dev, struct 
saa7146_ext_vv *ext_vv)
vbi->start[1] = 312;
vbi->count[1] = 16;
 
-   init_timer(>vbi_read_timeout);
+   timer_setup(>vbi_read_timeout, NULL, 0);
 
vv->ov_fb.capability = V4L2_FBUF_CAP_LIST_CLIPPING;
vv->ov_fb.flags = V4L2_FBUF_FLAG_PRIMARY;
diff --git a/drivers/media/common/saa7146/saa7146_vbi.c 
b/drivers/media/common/saa7146/saa7146_vbi.c
index 69525ca4f52c..ae66c2325228 100644
--- a/drivers/media/common/saa7146/saa7146_vbi.c
+++ b/drivers/media/common/saa7146/saa7146_vbi.c
@@ -348,9 +348,10 @@ static void vbi_stop(struct saa7146_fh *fh, struct file 
*file)
spin_unlock_irqrestore(>slock, flags);
 }
 
-static void vbi_read_timeout(unsigned long data)
+static void vbi_read_timeout(struct timer_list *t)
 {
-   struct file *file = (struct file*)data;
+   struct saa7146_vv *vv = from_timer(vv, t, vbi_read_timeout);
+   struct file *file = vv->vbi_read_timeout_file;
struct saa7146_fh *fh = file->private_data;
struct saa7146_dev *dev = fh->dev;
 
@@ -401,8 +402,8 @@ static int vbi_open(struct saa7146_dev *dev, struct file 
*file)
sizeof(struct saa7146_buf),
file, >v4l2_lock);
 
-   vv->vbi_read_timeout.function = vbi_read_timeout;
-   vv->vbi_read_timeout.data = (unsigned long)file;
+   vv->vbi_read_timeout.function = (TIMER_FUNC_TYPE)vbi_read_timeout;
+   vv->vbi_read_timeout_file = file;
 
/* initialize the brs */
if ( 0 != (SAA7146_USE_PORT_B_FOR_VBI & dev->ext_vv_data->flags)) {
diff --git a/include/media/drv-intf/saa7146_vv.h 
b/include/media/drv-intf/saa7146_vv.h
index 736f4f2d8290..926c5b145279 100644
--- a/include/media/drv-intf/saa7146_vv.h
+++ b/include/media/drv-intf/saa7146_vv.h
@@ -107,6 +107,7 @@ struct saa7146_vv
struct saa7146_dmaqueue vbi_dmaq;
struct v4l2_vbi_format  vbi_fmt;
struct timer_list   vbi_read_timeout;
+   struct file *vbi_read_timeout_file;
/* vbi workaround interrupt queue */
wait_queue_head_t   vbi_wq;
    int vbi_fieldcount;
-- 
2.7.4


-- 
Kees Cook
Pixel Security


[PATCH] media: tvaudio: Convert timers to use timer_setup()

2017-10-16 Thread Kees Cook
In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly.

Cc: Mauro Carvalho Chehab <mche...@kernel.org>
Cc: linux-media@vger.kernel.org
Signed-off-by: Kees Cook <keesc...@chromium.org>
---
 drivers/media/i2c/tvaudio.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/media/i2c/tvaudio.c b/drivers/media/i2c/tvaudio.c
index ce86534450ac..16a1e08ce06c 100644
--- a/drivers/media/i2c/tvaudio.c
+++ b/drivers/media/i2c/tvaudio.c
@@ -300,9 +300,9 @@ static int chip_cmd(struct CHIPSTATE *chip, char *name, 
audiocmd *cmd)
  *   if available, ...
  */
 
-static void chip_thread_wake(unsigned long data)
+static void chip_thread_wake(struct timer_list *t)
 {
-   struct CHIPSTATE *chip = (struct CHIPSTATE*)data;
+   struct CHIPSTATE *chip = from_timer(chip, t, wt);
wake_up_process(chip->thread);
 }
 
@@ -1995,7 +1995,7 @@ static int tvaudio_probe(struct i2c_client *client, const 
struct i2c_device_id *
v4l2_ctrl_handler_setup(>hdl);
 
chip->thread = NULL;
-   init_timer(>wt);
+   timer_setup(>wt, chip_thread_wake, 0);
if (desc->flags & CHIP_NEED_CHECKMODE) {
if (!desc->getrxsubchans || !desc->setaudmode) {
/* This shouldn't be happen. Warn user, but keep working
@@ -2005,8 +2005,6 @@ static int tvaudio_probe(struct i2c_client *client, const 
struct i2c_device_id *
return 0;
}
/* start async thread */
-   chip->wt.function = chip_thread_wake;
-   chip->wt.data = (unsigned long)chip;
chip->thread = kthread_run(chip_thread, chip, "%s",
   client->name);
    if (IS_ERR(chip->thread)) {
-- 
2.7.4


-- 
Kees Cook
Pixel Security


[PATCH] media: input: Convert timers to use timer_setup()

2017-10-16 Thread Kees Cook
In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly.

One input_dev user hijacks the input_dev software autorepeat timer to
perform its own repeat management. However, there is no path back to the
existing status variable, so add a generic one to the input structure and
use that instead.

Cc: Dmitry Torokhov <dmitry.torok...@gmail.com>
Cc: Mauro Carvalho Chehab <mche...@kernel.org>
Cc: Hans Verkuil <hans.verk...@cisco.com>
Cc: Sakari Ailus <sakari.ai...@linux.intel.com>
Cc: Geliang Tang <geliangt...@gmail.com>
Cc: linux-in...@vger.kernel.org
Cc: linux-media@vger.kernel.org
Signed-off-by: Kees Cook <keesc...@chromium.org>
Acked-by: Pali Rohár <pali.ro...@gmail.com>
---
 drivers/input/input.c   | 12 ++--
 drivers/media/pci/ttpci/av7110.h|  1 -
 drivers/media/pci/ttpci/av7110_ir.c | 16 
 include/linux/input.h   |  2 ++
 4 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/drivers/input/input.c b/drivers/input/input.c
index d268fdc23c64..497ad2dcb699 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -76,7 +76,7 @@ static void input_start_autorepeat(struct input_dev *dev, int 
code)
 {
if (test_bit(EV_REP, dev->evbit) &&
dev->rep[REP_PERIOD] && dev->rep[REP_DELAY] &&
-   dev->timer.data) {
+   dev->timer.function) {
dev->repeat_key = code;
mod_timer(>timer,
  jiffies + msecs_to_jiffies(dev->rep[REP_DELAY]));
@@ -179,9 +179,9 @@ static void input_pass_event(struct input_dev *dev,
  * dev->event_lock here to avoid racing with input_event
  * which may cause keys get "stuck".
  */
-static void input_repeat_key(unsigned long data)
+static void input_repeat_key(struct timer_list *t)
 {
-   struct input_dev *dev = (void *) data;
+   struct input_dev *dev = from_timer(dev, t, timer);
unsigned long flags;
 
spin_lock_irqsave(>event_lock, flags);
@@ -1790,7 +1790,7 @@ struct input_dev *input_allocate_device(void)
device_initialize(>dev);
mutex_init(>mutex);
spin_lock_init(>event_lock);
-   init_timer(>timer);
+   timer_setup(>timer, NULL, 0);
INIT_LIST_HEAD(>h_list);
INIT_LIST_HEAD(>node);
 
@@ -2053,8 +2053,8 @@ static void devm_input_device_unregister(struct device 
*dev, void *res)
  */
 void input_enable_softrepeat(struct input_dev *dev, int delay, int period)
 {
-   dev->timer.data = (unsigned long) dev;
-   dev->timer.function = input_repeat_key;
+   dev->timer.function = (TIMER_FUNC_TYPE)input_repeat_key;
+   dev->timer_data = 0;
dev->rep[REP_DELAY] = delay;
dev->rep[REP_PERIOD] = period;
 }
diff --git a/drivers/media/pci/ttpci/av7110.h b/drivers/media/pci/ttpci/av7110.h
index 347827925c14..b98a4f3006df 100644
--- a/drivers/media/pci/ttpci/av7110.h
+++ b/drivers/media/pci/ttpci/av7110.h
@@ -93,7 +93,6 @@ struct infrared {
u8  inversion;
u16 last_key;
u16 last_toggle;
-   u8  delay_timer_finished;
 };
 
 
diff --git a/drivers/media/pci/ttpci/av7110_ir.c 
b/drivers/media/pci/ttpci/av7110_ir.c
index ca05198de2c2..a883caa6488c 100644
--- a/drivers/media/pci/ttpci/av7110_ir.c
+++ b/drivers/media/pci/ttpci/av7110_ir.c
@@ -155,16 +155,16 @@ static void av7110_emit_key(unsigned long parm)
if (timer_pending(>keyup_timer)) {
del_timer(>keyup_timer);
if (ir->last_key != keycode || toggle != ir->last_toggle) {
-   ir->delay_timer_finished = 0;
+   ir->input_dev->timer_data = 0;
input_event(ir->input_dev, EV_KEY, ir->last_key, 0);
input_event(ir->input_dev, EV_KEY, keycode, 1);
input_sync(ir->input_dev);
-   } else if (ir->delay_timer_finished) {
+   } else if (ir->input_dev->timer_data) {
input_event(ir->input_dev, EV_KEY, keycode, 2);
input_sync(ir->input_dev);
}
} else {
-   ir->delay_timer_finished = 0;
+   ir->input_dev->timer_data = 0;
input_event(ir->input_dev, EV_KEY, keycode, 1);
input_sync(ir->input_dev);
}
@@ -206,11 +206,12 @@ static void input_register_keys(struct infrared *ir)
 
 
 /* called by the input driver after rep[REP_DELAY] ms */
-static void input_repeat_key(unsigned long parm)
+static void input_repeat_key(struct timer_lis

[PATCH] media: dvb-core: Convert timers to use timer_setup()

2017-10-16 Thread Kees Cook
In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly.

Cc: Mauro Carvalho Chehab <mche...@kernel.org>
Cc: devendra sharma <devendra.sharma9...@gmail.com>
Cc: linux-media@vger.kernel.org
Signed-off-by: Kees Cook <keesc...@chromium.org>
---
 drivers/media/dvb-core/dmxdev.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/media/dvb-core/dmxdev.c b/drivers/media/dvb-core/dmxdev.c
index 18e4230865be..3ddd44e1ee77 100644
--- a/drivers/media/dvb-core/dmxdev.c
+++ b/drivers/media/dvb-core/dmxdev.c
@@ -329,9 +329,9 @@ static int dvb_dmxdev_set_buffer_size(struct dmxdev_filter 
*dmxdevfilter,
return 0;
 }
 
-static void dvb_dmxdev_filter_timeout(unsigned long data)
+static void dvb_dmxdev_filter_timeout(struct timer_list *t)
 {
-   struct dmxdev_filter *dmxdevfilter = (struct dmxdev_filter *)data;
+   struct dmxdev_filter *dmxdevfilter = from_timer(dmxdevfilter, t, timer);
 
dmxdevfilter->buffer.error = -ETIMEDOUT;
spin_lock_irq(>dev->lock);
@@ -346,8 +346,6 @@ static void dvb_dmxdev_filter_timer(struct dmxdev_filter 
*dmxdevfilter)
 
del_timer(>timer);
if (para->timeout) {
-   dmxdevfilter->timer.function = dvb_dmxdev_filter_timeout;
-   dmxdevfilter->timer.data = (unsigned long)dmxdevfilter;
dmxdevfilter->timer.expires =
jiffies + 1 + (HZ / 2 + HZ * para->timeout) / 1000;
add_timer(>timer);
@@ -754,7 +752,7 @@ static int dvb_demux_open(struct inode *inode, struct file 
*file)
dvb_ringbuffer_init(>buffer, NULL, 8192);
dmxdevfilter->type = DMXDEV_TYPE_NONE;
dvb_dmxdev_filter_state_set(dmxdevfilter, DMXDEV_STATE_ALLOCATED);
-   init_timer(>timer);
+   timer_setup(>timer, dvb_dmxdev_filter_timeout, 0);
 
dvbdev->users++;
 
-- 
2.7.4


-- 
Kees Cook
Pixel Security


[PATCH] media: saa7134: Convert timers to use timer_setup()

2017-10-16 Thread Kees Cook
In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly.

Cc: Mauro Carvalho Chehab <mche...@kernel.org>
Cc: Sakari Ailus <sakari.ai...@linux.intel.com>
Cc: Sean Young <s...@mess.org>
Cc: Geliang Tang <geliangt...@gmail.com>
Cc: Hans Verkuil <hans.verk...@cisco.com>
Cc: linux-media@vger.kernel.org
Signed-off-by: Kees Cook <keesc...@chromium.org>
---
 drivers/media/pci/saa7134/saa7134-core.c  | 6 +++---
 drivers/media/pci/saa7134/saa7134-input.c | 9 -
 drivers/media/pci/saa7134/saa7134-ts.c| 3 +--
 drivers/media/pci/saa7134/saa7134-vbi.c   | 3 +--
 drivers/media/pci/saa7134/saa7134-video.c | 3 +--
 drivers/media/pci/saa7134/saa7134.h   | 2 +-
 6 files changed, 11 insertions(+), 15 deletions(-)

diff --git a/drivers/media/pci/saa7134/saa7134-core.c 
b/drivers/media/pci/saa7134/saa7134-core.c
index 7976c5a12ca8..9e76de2411ae 100644
--- a/drivers/media/pci/saa7134/saa7134-core.c
+++ b/drivers/media/pci/saa7134/saa7134-core.c
@@ -338,9 +338,9 @@ void saa7134_buffer_next(struct saa7134_dev *dev,
}
 }
 
-void saa7134_buffer_timeout(unsigned long data)
+void saa7134_buffer_timeout(struct timer_list *t)
 {
-   struct saa7134_dmaqueue *q = (struct saa7134_dmaqueue *)data;
+   struct saa7134_dmaqueue *q = from_timer(q, t, timeout);
struct saa7134_dev *dev = q->dev;
unsigned long flags;
 
@@ -378,7 +378,7 @@ void saa7134_stop_streaming(struct saa7134_dev *dev, struct 
saa7134_dmaqueue *q)
}
}
spin_unlock_irqrestore(>slock, flags);
-   saa7134_buffer_timeout((unsigned long)q); /* also calls 
del_timer(>timeout) */
+   saa7134_buffer_timeout(>timeout); /* also calls 
del_timer(>timeout) */
 }
 EXPORT_SYMBOL_GPL(saa7134_stop_streaming);
 
diff --git a/drivers/media/pci/saa7134/saa7134-input.c 
b/drivers/media/pci/saa7134/saa7134-input.c
index 9337e4615519..2d5abeddc079 100644
--- a/drivers/media/pci/saa7134/saa7134-input.c
+++ b/drivers/media/pci/saa7134/saa7134-input.c
@@ -447,10 +447,10 @@ void saa7134_input_irq(struct saa7134_dev *dev)
}
 }
 
-static void saa7134_input_timer(unsigned long data)
+static void saa7134_input_timer(struct timer_list *t)
 {
-   struct saa7134_dev *dev = (struct saa7134_dev *)data;
-   struct saa7134_card_ir *ir = dev->remote;
+   struct saa7134_card_ir *ir = from_timer(ir, t, timer);
+   struct saa7134_dev *dev = ir->dev->priv;
 
build_key(dev);
mod_timer(>timer, jiffies + msecs_to_jiffies(ir->polling));
@@ -507,8 +507,7 @@ static int __saa7134_ir_start(void *priv)
ir->running = true;
 
if (ir->polling) {
-   setup_timer(>timer, saa7134_input_timer,
-   (unsigned long)dev);
+   timer_setup(>timer, saa7134_input_timer, 0);
ir->timer.expires = jiffies + HZ;
add_timer(>timer);
}
diff --git a/drivers/media/pci/saa7134/saa7134-ts.c 
b/drivers/media/pci/saa7134/saa7134-ts.c
index 7414878af9e0..2be703617e29 100644
--- a/drivers/media/pci/saa7134/saa7134-ts.c
+++ b/drivers/media/pci/saa7134/saa7134-ts.c
@@ -223,8 +223,7 @@ int saa7134_ts_init1(struct saa7134_dev *dev)
dev->ts.nr_packets = ts_nr_packets;
 
INIT_LIST_HEAD(>ts_q.queue);
-   setup_timer(>ts_q.timeout, saa7134_buffer_timeout,
-   (unsigned long)(>ts_q));
+   timer_setup(>ts_q.timeout, saa7134_buffer_timeout, 0);
dev->ts_q.dev  = dev;
dev->ts_q.need_two = 1;
dev->ts_started= 0;
diff --git a/drivers/media/pci/saa7134/saa7134-vbi.c 
b/drivers/media/pci/saa7134/saa7134-vbi.c
index bcad9b2d9bb3..af494c6147f1 100644
--- a/drivers/media/pci/saa7134/saa7134-vbi.c
+++ b/drivers/media/pci/saa7134/saa7134-vbi.c
@@ -181,8 +181,7 @@ struct vb2_ops saa7134_vbi_qops = {
 int saa7134_vbi_init1(struct saa7134_dev *dev)
 {
INIT_LIST_HEAD(>vbi_q.queue);
-   setup_timer(>vbi_q.timeout, saa7134_buffer_timeout,
-   (unsigned long)(>vbi_q));
+   timer_setup(>vbi_q.timeout, saa7134_buffer_timeout, 0);
dev->vbi_q.dev  = dev;
 
if (vbibufs < 2)
diff --git a/drivers/media/pci/saa7134/saa7134-video.c 
b/drivers/media/pci/saa7134/saa7134-video.c
index 51d42bbf969e..82d2a24644e4 100644
--- a/drivers/media/pci/saa7134/saa7134-video.c
+++ b/drivers/media/pci/saa7134/saa7134-video.c
@@ -2145,8 +2145,7 @@ int saa7134_video_init1(struct saa7134_dev *dev)
dev->automute   = 0;
 
INIT_LIST_HEAD(>video_q.queue);
-   setup_timer(>video_q.timeout, saa7134_buffer_timeout,
-   (unsigned long)(>video_q));
+   timer_setup(>video_q.timeout, saa7134_buffer_timeout, 0);
dev->vide

[PATCH] media: saa7146: Convert timers to use timer_setup()

2017-10-16 Thread Kees Cook
In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly.

Cc: Hans Verkuil <hverk...@xs4all.nl>
Cc: Mauro Carvalho Chehab <mche...@kernel.org>
Cc: linux-media@vger.kernel.org
Signed-off-by: Kees Cook <keesc...@chromium.org>
---
 drivers/media/common/saa7146/saa7146_fops.c  | 4 ++--
 drivers/media/common/saa7146/saa7146_vbi.c   | 3 +--
 drivers/media/common/saa7146/saa7146_video.c | 3 +--
 include/media/drv-intf/saa7146_vv.h  | 2 +-
 4 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/media/common/saa7146/saa7146_fops.c 
b/drivers/media/common/saa7146/saa7146_fops.c
index c4664f0da874..8c87d6837c49 100644
--- a/drivers/media/common/saa7146/saa7146_fops.c
+++ b/drivers/media/common/saa7146/saa7146_fops.c
@@ -163,9 +163,9 @@ void saa7146_buffer_next(struct saa7146_dev *dev,
}
 }
 
-void saa7146_buffer_timeout(unsigned long data)
+void saa7146_buffer_timeout(struct timer_list *t)
 {
-   struct saa7146_dmaqueue *q = (struct saa7146_dmaqueue*)data;
+   struct saa7146_dmaqueue *q = from_timer(q, t, timeout);
struct saa7146_dev *dev = q->dev;
unsigned long flags;
 
diff --git a/drivers/media/common/saa7146/saa7146_vbi.c 
b/drivers/media/common/saa7146/saa7146_vbi.c
index ae66c2325228..7fa3147c2d7e 100644
--- a/drivers/media/common/saa7146/saa7146_vbi.c
+++ b/drivers/media/common/saa7146/saa7146_vbi.c
@@ -366,8 +366,7 @@ static void vbi_init(struct saa7146_dev *dev, struct 
saa7146_vv *vv)
 
INIT_LIST_HEAD(>vbi_dmaq.queue);
 
-   setup_timer(>vbi_dmaq.timeout, saa7146_buffer_timeout,
-   (unsigned long)(>vbi_dmaq));
+   timer_setup(>vbi_dmaq.timeout, saa7146_buffer_timeout, 0);
vv->vbi_dmaq.dev  = dev;
 
init_waitqueue_head(>vbi_wq);
diff --git a/drivers/media/common/saa7146/saa7146_video.c 
b/drivers/media/common/saa7146/saa7146_video.c
index 51eeed830de4..2b631eaa65b3 100644
--- a/drivers/media/common/saa7146/saa7146_video.c
+++ b/drivers/media/common/saa7146/saa7146_video.c
@@ -1201,8 +1201,7 @@ static void video_init(struct saa7146_dev *dev, struct 
saa7146_vv *vv)
 {
INIT_LIST_HEAD(>video_dmaq.queue);
 
-   setup_timer(>video_dmaq.timeout, saa7146_buffer_timeout,
-   (unsigned long)(>video_dmaq));
+   timer_setup(>video_dmaq.timeout, saa7146_buffer_timeout, 0);
vv->video_dmaq.dev  = dev;
 
/* set some default values */
diff --git a/include/media/drv-intf/saa7146_vv.h 
b/include/media/drv-intf/saa7146_vv.h
index 926c5b145279..17bbe3851d75 100644
--- a/include/media/drv-intf/saa7146_vv.h
+++ b/include/media/drv-intf/saa7146_vv.h
@@ -184,7 +184,7 @@ int saa7146_unregister_device(struct video_device *vid, 
struct saa7146_dev *dev)
 void saa7146_buffer_finish(struct saa7146_dev *dev, struct saa7146_dmaqueue 
*q, int state);
 void saa7146_buffer_next(struct saa7146_dev *dev, struct saa7146_dmaqueue 
*q,int vbi);
 int saa7146_buffer_queue(struct saa7146_dev *dev, struct saa7146_dmaqueue *q, 
struct saa7146_buf *buf);
-void saa7146_buffer_timeout(unsigned long data);
+void saa7146_buffer_timeout(struct timer_list *t);
 void saa7146_dma_free(struct saa7146_dev* dev,struct videobuf_queue *q,
    struct saa7146_buf *buf);
 
-- 
2.7.4


-- 
Kees Cook
Pixel Security


[PATCH] media: tc358743: Convert timers to use timer_setup()

2017-10-16 Thread Kees Cook
In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly.

Cc: Mats Randgaard <matra...@cisco.com>
Cc: Mauro Carvalho Chehab <mche...@kernel.org>
Cc: linux-media@vger.kernel.org
Signed-off-by: Kees Cook <keesc...@chromium.org>
---
 drivers/media/i2c/tc358743.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c
index a9355032076f..359f63d7dfca 100644
--- a/drivers/media/i2c/tc358743.c
+++ b/drivers/media/i2c/tc358743.c
@@ -1481,9 +1481,9 @@ static irqreturn_t tc358743_irq_handler(int irq, void 
*dev_id)
return handled ? IRQ_HANDLED : IRQ_NONE;
 }
 
-static void tc358743_irq_poll_timer(unsigned long arg)
+static void tc358743_irq_poll_timer(struct timer_list *t)
 {
-   struct tc358743_state *state = (struct tc358743_state *)arg;
+   struct tc358743_state *state = from_timer(state, t, timer);
unsigned int msecs;
 
schedule_work(>work_i2c_poll);
@@ -2147,8 +2147,7 @@ static int tc358743_probe(struct i2c_client *client,
} else {
INIT_WORK(>work_i2c_poll,
  tc358743_work_i2c_poll);
-   setup_timer(>timer, tc358743_irq_poll_timer,
-   (unsigned long)state);
+   timer_setup(>timer, tc358743_irq_poll_timer, 0);
state->timer.expires = jiffies +
   msecs_to_jiffies(POLL_INTERVAL_MS);
add_timer(>timer);
-- 
2.7.4


-- 
Kees Cook
Pixel Security


[PATCH] media: serial_ir: Convert timers to use timer_setup()

2017-10-16 Thread Kees Cook
In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly.

Cc: Sean Young <s...@mess.org>
Cc: Mauro Carvalho Chehab <mche...@kernel.org>
Cc: linux-media@vger.kernel.org
Signed-off-by: Kees Cook <keesc...@chromium.org>
---
 drivers/media/rc/serial_ir.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/media/rc/serial_ir.c b/drivers/media/rc/serial_ir.c
index 8b66926bc16a..8bf5637b3a69 100644
--- a/drivers/media/rc/serial_ir.c
+++ b/drivers/media/rc/serial_ir.c
@@ -470,7 +470,7 @@ static int hardware_init_port(void)
return 0;
 }
 
-static void serial_ir_timeout(unsigned long arg)
+static void serial_ir_timeout(struct timer_list *unused)
 {
DEFINE_IR_RAW_EVENT(ev);
 
@@ -540,8 +540,7 @@ static int serial_ir_probe(struct platform_device *dev)
 
serial_ir.rcdev = rcdev;
 
-   setup_timer(_ir.timeout_timer, serial_ir_timeout,
-   (unsigned long)_ir);
+   timer_setup(_ir.timeout_timer, serial_ir_timeout, 0);
 
result = devm_request_irq(>dev, irq, serial_ir_irq_handler,
  share_irq ? IRQF_SHARED : 0,
-- 
2.7.4


-- 
Kees Cook
Pixel Security


[PATCH] staging/atomisp: Convert timers to use timer_setup()

2017-10-16 Thread Kees Cook
In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly.

Cc: Mauro Carvalho Chehab <mche...@kernel.org>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
Cc: Alan Cox <a...@linux.intel.com>
Cc: Daeseok Youn <daeseok.y...@gmail.com>
Cc: Arnd Bergmann <a...@arndb.de>
Cc: linux-media@vger.kernel.org
Cc: de...@driverdev.osuosl.org
Signed-off-by: Kees Cook <keesc...@chromium.org>
---
 drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c  | 13 -
 drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.h  |  6 +-
 .../media/atomisp/pci/atomisp2/atomisp_compat_css20.c |  2 +-
 drivers/staging/media/atomisp/pci/atomisp2/atomisp_v4l2.c | 15 +--
 4 files changed, 11 insertions(+), 25 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c 
b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
index b0c647f4d250..f3cf4ecba630 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
@@ -1661,20 +1661,15 @@ void atomisp_css_flush(struct atomisp_device *isp)
dev_dbg(isp->dev, "atomisp css flush done\n");
 }
 
-#ifndef ISP2401
-void atomisp_wdt(unsigned long isp_addr)
-#else
-void atomisp_wdt(unsigned long pipe_addr)
-#endif
+void atomisp_wdt(struct timer_list *t)
 {
 #ifndef ISP2401
-   struct atomisp_device *isp = (struct atomisp_device *)isp_addr;
+   struct atomisp_sub_device *asd = from_timer(asd, t, wdt);
 #else
-   struct atomisp_video_pipe *pipe =
-   (struct atomisp_video_pipe *)pipe_addr;
+   struct atomisp_video_pipe *pipe = from_timer(pipe, t, wdt);
struct atomisp_sub_device *asd = pipe->asd;
-   struct atomisp_device *isp = asd->isp;
 #endif
+   struct atomisp_device *isp = asd->isp;
 
 #ifdef ISP2401
atomic_inc(>wdt_count);
diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.h 
b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.h
index 31ba4e613d13..4bb83864da2e 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.h
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.h
@@ -85,11 +85,7 @@ static inline void __iomem 
*atomisp_get_io_virt_addr(unsigned int address)
 void atomisp_msi_irq_init(struct atomisp_device *isp, struct pci_dev *dev);
 void atomisp_msi_irq_uninit(struct atomisp_device *isp, struct pci_dev *dev);
 void atomisp_wdt_work(struct work_struct *work);
-#ifndef ISP2401
-void atomisp_wdt(unsigned long isp_addr);
-#else
-void atomisp_wdt(unsigned long pipe_addr);
-#endif
+void atomisp_wdt(struct timer_list *t);
 void atomisp_setup_flash(struct atomisp_sub_device *asd);
 irqreturn_t atomisp_isr(int irq, void *dev);
 irqreturn_t atomisp_isr_thread(int irq, void *isp_ptr);
diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_css20.c 
b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_css20.c
index 5027fd20d966..b190f9958066 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_css20.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_css20.c
@@ -4526,7 +4526,7 @@ int atomisp_css_isr_thread(struct atomisp_device *isp,
for (i = 0; i < isp->num_of_streams; i++)
atomisp_wdt_stop(>asd[i], 0);
 #ifndef ISP2401
-   atomisp_wdt((unsigned long)isp);
+   atomisp_wdt(>asd[0].wdt);
 #else
queue_work(isp->wdt_work_queue, >wdt_work);
 #endif
diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_v4l2.c 
b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_v4l2.c
index e85b3819bffa..20aff7faf44a 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_v4l2.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_v4l2.c
@@ -1150,17 +1150,12 @@ static int init_atomisp_wdts(struct atomisp_device *isp)
struct atomisp_sub_device *asd = >asd[i];
asd = >asd[i];
 #ifndef ISP2401
-   setup_timer(>wdt, atomisp_wdt, (unsigned long)isp);
+   timer_setup(>wdt, atomisp_wdt, 0);
 #else
-   setup_timer(>video_out_capture.wdt,
-   atomisp_wdt, (unsigned long)>video_out_capture);
-   setup_timer(>video_out_preview.wdt,
-   atomisp_wdt, (unsigned long)>video_out_preview);
-   setup_timer(>video_out_vf.wdt,
-   atomisp_wdt, (unsigned long)>video_out_vf);
-   setup_timer(>video_out_video_capture.wdt,
-   atomisp_wdt,
-   (unsigned long)>video_out_video_capture);
+   timer_setup(>video_out_capture.wdt, atomisp_wdt, 0);
+   

[PATCH 17/31] media/i2c/tc358743: Initialize timer

2017-08-31 Thread Kees Cook
This converts to use setup_timer() to set callback and data, though it
doesn't look like this would have worked with timer checking enabled
since no init_timer() was ever called before.

Cc: Mats Randgaard <matra...@cisco.com>
Cc: Mauro Carvalho Chehab <mche...@kernel.org>
Cc: linux-media@vger.kernel.org
Signed-off-by: Kees Cook <keesc...@chromium.org>
---
 drivers/media/i2c/tc358743.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c
index 5788af238b86..94e722e0f4e0 100644
--- a/drivers/media/i2c/tc358743.c
+++ b/drivers/media/i2c/tc358743.c
@@ -1964,8 +1964,8 @@ static int tc358743_probe(struct i2c_client *client,
} else {
INIT_WORK(>work_i2c_poll,
  tc358743_work_i2c_poll);
-   state->timer.data = (unsigned long)state;
-   state->timer.function = tc358743_irq_poll_timer;
+   setup_timer(>timer, tc358743_irq_poll_timer,
+   (unsigned long)state);
state->timer.expires = jiffies +
   msecs_to_jiffies(POLL_INTERVAL_MS);
add_timer(>timer);
-- 
2.7.4



[PATCH] staging: atomisp: i2c: Convert timers to use timer_setup()

2017-10-24 Thread Kees Cook
In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly.

Signed-off-by: Kees Cook <keesc...@chromium.org>
---
 drivers/staging/media/atomisp/i2c/lm3554.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/media/atomisp/i2c/lm3554.c 
b/drivers/staging/media/atomisp/i2c/lm3554.c
index 679176f7c542..a815f208409f 100644
--- a/drivers/staging/media/atomisp/i2c/lm3554.c
+++ b/drivers/staging/media/atomisp/i2c/lm3554.c
@@ -171,10 +171,9 @@ static int lm3554_set_config1(struct lm3554 *flash)
 /* 
-
  * Hardware trigger
  */
-static void lm3554_flash_off_delay(long unsigned int arg)
+static void lm3554_flash_off_delay(struct timer_list *t)
 {
-   struct v4l2_subdev *sd = i2c_get_clientdata((struct i2c_client *)arg);
-   struct lm3554 *flash = to_lm3554(sd);
+   struct lm3554 *flash = from_timer(flash, t, flash_off_delay);
struct lm3554_platform_data *pdata = flash->pdata;
 
gpio_set_value(pdata->gpio_strobe, 0);
@@ -915,8 +914,7 @@ static int lm3554_probe(struct i2c_client *client,
 
mutex_init(>power_lock);
 
-   setup_timer(>flash_off_delay, lm3554_flash_off_delay,
-   (unsigned long)client);
+   timer_setup(>flash_off_delay, lm3554_flash_off_delay, 0);
 
err = lm3554_gpio_init(client);
if (err) {
-- 
2.7.4


-- 
Kees Cook
Pixel Security


Re: [PATCH v2] media: ttpci: remove autorepeat handling and use timer_setup

2017-11-02 Thread Kees Cook
On Tue, Oct 31, 2017 at 1:11 PM, Sean Young <s...@mess.org> wrote:
> Leave the autorepeat handling up to the input layer, and move
> to the new timer API.
>
> Compile tested only.
>
> Signed-off-by: Sean Young <s...@mess.org>

Hi! Just checking up on this... the input timer conversion is blocked
by getting this sorted out, so I'd love to have something either
media, input, or timer tree can carry. :)

Thanks!

-Kees

> ---
> v2:
>  - fixes and improvements from Dmitry Torokhov
>
>  drivers/media/pci/ttpci/av7110.h|  2 +-
>  drivers/media/pci/ttpci/av7110_ir.c | 56 
> +
>  2 files changed, 21 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/media/pci/ttpci/av7110.h 
> b/drivers/media/pci/ttpci/av7110.h
> index 347827925c14..bcb72ecbedc0 100644
> --- a/drivers/media/pci/ttpci/av7110.h
> +++ b/drivers/media/pci/ttpci/av7110.h
> @@ -93,7 +93,7 @@ struct infrared {
> u8  inversion;
> u16 last_key;
> u16 last_toggle;
> -   u8  delay_timer_finished;
> +   boolkeypressed;
>  };
>
>
> diff --git a/drivers/media/pci/ttpci/av7110_ir.c 
> b/drivers/media/pci/ttpci/av7110_ir.c
> index ca05198de2c2..ee414803e6b5 100644
> --- a/drivers/media/pci/ttpci/av7110_ir.c
> +++ b/drivers/media/pci/ttpci/av7110_ir.c
> @@ -84,15 +84,16 @@ static u16 default_key_map [256] = {
>
>
>  /* key-up timer */
> -static void av7110_emit_keyup(unsigned long parm)
> +static void av7110_emit_keyup(struct timer_list *t)
>  {
> -   struct infrared *ir = (struct infrared *) parm;
> +   struct infrared *ir = from_timer(ir, t, keyup_timer);
>
> -   if (!ir || !test_bit(ir->last_key, ir->input_dev->key))
> +   if (!ir || !ir->keypressed)
> return;
>
> input_report_key(ir->input_dev, ir->last_key, 0);
> input_sync(ir->input_dev);
> +   ir->keypressed = false;
>  }
>
>
> @@ -152,29 +153,18 @@ static void av7110_emit_key(unsigned long parm)
> return;
> }
>
> -   if (timer_pending(>keyup_timer)) {
> -   del_timer(>keyup_timer);
> -   if (ir->last_key != keycode || toggle != ir->last_toggle) {
> -   ir->delay_timer_finished = 0;
> -   input_event(ir->input_dev, EV_KEY, ir->last_key, 0);
> -   input_event(ir->input_dev, EV_KEY, keycode, 1);
> -   input_sync(ir->input_dev);
> -   } else if (ir->delay_timer_finished) {
> -   input_event(ir->input_dev, EV_KEY, keycode, 2);
> -   input_sync(ir->input_dev);
> -   }
> -   } else {
> -   ir->delay_timer_finished = 0;
> -   input_event(ir->input_dev, EV_KEY, keycode, 1);
> -   input_sync(ir->input_dev);
> -   }
> +   if (ir->keypressed &&
> +   (ir->last_key != keycode || toggle != ir->last_toggle))
> +   input_event(ir->input_dev, EV_KEY, ir->last_key, 0);
>
> +   input_event(ir->input_dev, EV_KEY, keycode, 1);
> +   input_sync(ir->input_dev);
> +
> +   ir->keypressed = true;
> ir->last_key = keycode;
> ir->last_toggle = toggle;
>
> -   ir->keyup_timer.expires = jiffies + UP_TIMEOUT;
> -   add_timer(>keyup_timer);
> -
> +   mod_timer(>keyup_timer, jiffies + UP_TIMEOUT);
>  }
>
>
> @@ -204,16 +194,6 @@ static void input_register_keys(struct infrared *ir)
> ir->input_dev->keycodemax = ARRAY_SIZE(ir->key_map);
>  }
>
> -
> -/* called by the input driver after rep[REP_DELAY] ms */
> -static void input_repeat_key(unsigned long parm)
> -{
> -   struct infrared *ir = (struct infrared *) parm;
> -
> -   ir->delay_timer_finished = 1;
> -}
> -
> -
>  /* check for configuration changes */
>  int av7110_check_ir_config(struct av7110 *av7110, int force)
>  {
> @@ -333,8 +313,7 @@ int av7110_ir_init(struct av7110 *av7110)
> av_list[av_cnt++] = av7110;
> av7110_check_ir_config(av7110, true);
>
> -   setup_timer(>ir.keyup_timer, av7110_emit_keyup,
> -   (unsigned long)>ir);
> +   timer_setup(>ir.keyup_timer, av7110_emit_keyup, 0);
>
> input_dev = input_allocate_device();
> if (!input_dev)
> @@ -365,8 +344,13 @@ int av7110_ir_init(struct av7110 *av7110)
> input_free_device(input_dev);
> return

Re: [PATCH v2] media: ttpci: remove autorepeat handling and use timer_setup

2017-11-03 Thread Kees Cook
On Fri, Nov 3, 2017 at 3:17 PM, Dmitry Torokhov
<dmitry.torok...@gmail.com> wrote:
> On Thu, Nov 02, 2017 at 10:16:58PM -0200, Mauro Carvalho Chehab wrote:
>> Em Thu, 2 Nov 2017 16:50:37 -0700
>> Dmitry Torokhov <dmitry.torok...@gmail.com> escreveu:
>>
>> > On Thu, Nov 02, 2017 at 04:24:27PM -0700, Kees Cook wrote:
>> > > On Tue, Oct 31, 2017 at 1:11 PM, Sean Young <s...@mess.org> wrote:
>> > > > Leave the autorepeat handling up to the input layer, and move
>> > > > to the new timer API.
>> > > >
>> > > > Compile tested only.
>> > > >
>> > > > Signed-off-by: Sean Young <s...@mess.org>
>> > >
>> > > Hi! Just checking up on this... the input timer conversion is blocked
>> > > by getting this sorted out, so I'd love to have something either
>> > > media, input, or timer tree can carry. :)
>> >
>> > Acked-by: Dmitry Torokhov <dmitry.torok...@gmail.com>
>> >
>> > From my POV the patch is good. Mauro, do you want me to take it through
>> > my tree, or maybe you could create an immutable branch off 4.14-rc5 (or
>> > 6) with this commit and I will pull it in and then can apply Kees input
>> > core conversion patch?
>>
>> Feel free to apply it into your tree with my ack:
>>
>> Acked-by: Mauro Carvalho Chehab <mche...@s-opensource.com>
>
> Applied and pulled Kees' patch to the input core (dropping the timer_data
> business) on top.

Awesome, thanks! :)

-Kees

-- 
Kees Cook
Pixel Security


Re: [PATCH] media: pvrusb2: Convert timers to use timer_setup()

2017-10-25 Thread Kees Cook
Eek, sorry, this uses timer_setup_on_stack() which is only in -next.
If you can Ack this, I can carry it in the timer tree.

Thanks!

-Kees

On Tue, Oct 24, 2017 at 5:22 PM, Kees Cook <keesc...@chromium.org> wrote:
> In preparation for unconditionally passing the struct timer_list pointer to
> all timer callbacks, switch to using the new timer_setup() and from_timer()
> to pass the timer pointer explicitly.
>
> Cc: Mike Isely <is...@pobox.com>
> Cc: Mauro Carvalho Chehab <mche...@kernel.org>
> Cc: linux-media@vger.kernel.org
> Signed-off-by: Kees Cook <keesc...@chromium.org>
> ---
>  drivers/media/usb/pvrusb2/pvrusb2-hdw.c | 64 
> ++---
>  1 file changed, 36 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c 
> b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c
> index ad5b25b89699..8289ee482f49 100644
> --- a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c
> +++ b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c
> @@ -330,10 +330,10 @@ static void pvr2_hdw_state_log_state(struct pvr2_hdw *);
>  static int pvr2_hdw_cmd_usbstream(struct pvr2_hdw *hdw,int runFl);
>  static int pvr2_hdw_commit_setup(struct pvr2_hdw *hdw);
>  static int pvr2_hdw_get_eeprom_addr(struct pvr2_hdw *hdw);
> -static void pvr2_hdw_quiescent_timeout(unsigned long);
> -static void pvr2_hdw_decoder_stabilization_timeout(unsigned long);
> -static void pvr2_hdw_encoder_wait_timeout(unsigned long);
> -static void pvr2_hdw_encoder_run_timeout(unsigned long);
> +static void pvr2_hdw_quiescent_timeout(struct timer_list *);
> +static void pvr2_hdw_decoder_stabilization_timeout(struct timer_list *);
> +static void pvr2_hdw_encoder_wait_timeout(struct timer_list *);
> +static void pvr2_hdw_encoder_run_timeout(struct timer_list *);
>  static int pvr2_issue_simple_cmd(struct pvr2_hdw *,u32);
>  static int pvr2_send_request_ex(struct pvr2_hdw *hdw,
> unsigned int timeout,int probe_fl,
> @@ -2373,18 +2373,15 @@ struct pvr2_hdw *pvr2_hdw_create(struct usb_interface 
> *intf,
> }
> if (!hdw) goto fail;
>
> -   setup_timer(>quiescent_timer, pvr2_hdw_quiescent_timeout,
> -   (unsigned long)hdw);
> +   timer_setup(>quiescent_timer, pvr2_hdw_quiescent_timeout, 0);
>
> -   setup_timer(>decoder_stabilization_timer,
> -   pvr2_hdw_decoder_stabilization_timeout,
> -   (unsigned long)hdw);
> +   timer_setup(>decoder_stabilization_timer,
> +   pvr2_hdw_decoder_stabilization_timeout, 0);
>
> -   setup_timer(>encoder_wait_timer, pvr2_hdw_encoder_wait_timeout,
> -   (unsigned long)hdw);
> +   timer_setup(>encoder_wait_timer, pvr2_hdw_encoder_wait_timeout,
> +   0);
>
> -   setup_timer(>encoder_run_timer, pvr2_hdw_encoder_run_timeout,
> -   (unsigned long)hdw);
> +   timer_setup(>encoder_run_timer, pvr2_hdw_encoder_run_timeout, 0);
>
> hdw->master_state = PVR2_STATE_DEAD;
>
> @@ -3539,10 +3536,16 @@ static void pvr2_ctl_read_complete(struct urb *urb)
> complete(>ctl_done);
>  }
>
> +struct hdw_timer {
> +   struct timer_list timer;
> +   struct pvr2_hdw *hdw;
> +};
>
> -static void pvr2_ctl_timeout(unsigned long data)
> +static void pvr2_ctl_timeout(struct timer_list *t)
>  {
> -   struct pvr2_hdw *hdw = (struct pvr2_hdw *)data;
> +   struct hdw_timer *timer = from_timer(timer, t, timer);
> +   struct pvr2_hdw *hdw = timer->hdw;
> +
> if (hdw->ctl_write_pend_flag || hdw->ctl_read_pend_flag) {
> hdw->ctl_timeout_flag = !0;
> if (hdw->ctl_write_pend_flag)
> @@ -3564,7 +3567,10 @@ static int pvr2_send_request_ex(struct pvr2_hdw *hdw,
>  {
> unsigned int idx;
> int status = 0;
> -   struct timer_list timer;
> +   struct hdw_timer timer = {
> +   .hdw = hdw,
> +   };
> +
> if (!hdw->ctl_lock_held) {
> pvr2_trace(PVR2_TRACE_ERROR_LEGS,
>"Attempted to execute control transfer without 
> lock!!");
> @@ -3621,8 +3627,8 @@ static int pvr2_send_request_ex(struct pvr2_hdw *hdw,
> hdw->ctl_timeout_flag = 0;
> hdw->ctl_write_pend_flag = 0;
> hdw->ctl_read_pend_flag = 0;
> -   setup_timer(, pvr2_ctl_timeout, (unsigned long)hdw);
> -   timer.expires = jiffies + timeout;
> +   timer_setup_on_stack(, pvr2_ctl_timeout, 0);
> +   timer.timer.expires = jiffies + timeout;
>
> if (write_len && write_data) {
> hdw->cmd_debug_s

[PATCH] media: pci: Convert timers to use timer_setup()

2017-10-24 Thread Kees Cook
In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly.

Cc: Mauro Carvalho Chehab <mche...@kernel.org>
Cc: Andy Walls <awa...@md.metrocast.net>
Cc: Sergey Kozlov <se...@netup.ru>
Cc: Abylay Ospan <aos...@netup.ru>
Cc: Ezequiel Garcia <ezequ...@vanguardiasur.com.ar>
Cc: Hans Verkuil <hansv...@cisco.com>
Cc: Arvind Yadav <arvind.yadav...@gmail.com>
Cc: Geliang Tang <geliangt...@gmail.com>
Cc: Sean Young <s...@mess.org>
Cc: Sakari Ailus <sakari.ai...@linux.intel.com>
Cc: "Pali Rohár" <pali.ro...@gmail.com>
Cc: linux-media@vger.kernel.org
Signed-off-by: Kees Cook <keesc...@chromium.org>
---
 drivers/media/pci/bt8xx/bttv-driver.c  |  6 +++---
 drivers/media/pci/bt8xx/bttv-input.c   | 19 ++-
 drivers/media/pci/bt8xx/bttvp.h|  1 +
 drivers/media/pci/cx18/cx18-fileops.c  |  4 ++--
 drivers/media/pci/cx18/cx18-fileops.h  |  2 +-
 drivers/media/pci/cx18/cx18-streams.c  |  2 +-
 drivers/media/pci/ivtv/ivtv-driver.c   |  3 +--
 drivers/media/pci/ivtv/ivtv-irq.c  |  4 ++--
 drivers/media/pci/ivtv/ivtv-irq.h  |  2 +-
 drivers/media/pci/netup_unidvb/netup_unidvb_core.c |  7 +++
 drivers/media/pci/ttpci/av7110_ir.c|  7 +++
 drivers/media/pci/tw686x/tw686x-core.c |  7 +++
 12 files changed, 31 insertions(+), 33 deletions(-)

diff --git a/drivers/media/pci/bt8xx/bttv-driver.c 
b/drivers/media/pci/bt8xx/bttv-driver.c
index 227086a2e99c..b366a7e1d976 100644
--- a/drivers/media/pci/bt8xx/bttv-driver.c
+++ b/drivers/media/pci/bt8xx/bttv-driver.c
@@ -3652,9 +3652,9 @@ bttv_irq_wakeup_vbi(struct bttv *btv, struct bttv_buffer 
*wakeup,
wake_up(>vb.done);
 }
 
-static void bttv_irq_timeout(unsigned long data)
+static void bttv_irq_timeout(struct timer_list *t)
 {
-   struct bttv *btv = (struct bttv *)data;
+   struct bttv *btv = from_timer(btv, t, timeout);
struct bttv_buffer_set old,new;
struct bttv_buffer *ovbi;
struct bttv_buffer *item;
@@ -4043,7 +4043,7 @@ static int bttv_probe(struct pci_dev *dev, const struct 
pci_device_id *pci_id)
INIT_LIST_HEAD(>capture);
INIT_LIST_HEAD(>vcapture);
 
-   setup_timer(>timeout, bttv_irq_timeout, (unsigned long)btv);
+   timer_setup(>timeout, bttv_irq_timeout, 0);
 
btv->i2c_rc = -1;
btv->tuner_type  = UNSET;
diff --git a/drivers/media/pci/bt8xx/bttv-input.c 
b/drivers/media/pci/bt8xx/bttv-input.c
index 73d655d073d6..ac7674700685 100644
--- a/drivers/media/pci/bt8xx/bttv-input.c
+++ b/drivers/media/pci/bt8xx/bttv-input.c
@@ -133,10 +133,10 @@ void bttv_input_irq(struct bttv *btv)
ir_handle_key(btv);
 }
 
-static void bttv_input_timer(unsigned long data)
+static void bttv_input_timer(struct timer_list *t)
 {
-   struct bttv *btv = (struct bttv*)data;
-   struct bttv_ir *ir = btv->remote;
+   struct bttv_ir *ir = from_timer(ir, t, timer);
+   struct bttv *btv = ir->btv;
 
if (btv->c.type == BTTV_BOARD_ENLTV_FM_2)
ir_enltv_handle_key(btv);
@@ -189,9 +189,9 @@ static u32 bttv_rc5_decode(unsigned int code)
return rc5;
 }
 
-static void bttv_rc5_timer_end(unsigned long data)
+static void bttv_rc5_timer_end(struct timer_list *t)
 {
-   struct bttv_ir *ir = (struct bttv_ir *)data;
+   struct bttv_ir *ir = from_timer(ir, t, timer);
ktime_t tv;
u32 gap, rc5, scancode;
u8 toggle, command, system;
@@ -296,15 +296,15 @@ static int bttv_rc5_irq(struct bttv *btv)
 
 /* -- */
 
-static void bttv_ir_start(struct bttv *btv, struct bttv_ir *ir)
+static void bttv_ir_start(struct bttv_ir *ir)
 {
if (ir->polling) {
-   setup_timer(>timer, bttv_input_timer, (unsigned long)btv);
+   timer_setup(>timer, bttv_input_timer, 0);
ir->timer.expires  = jiffies + msecs_to_jiffies(1000);
add_timer(>timer);
} else if (ir->rc5_gpio) {
/* set timer_end for code completion */
-   setup_timer(>timer, bttv_rc5_timer_end, (unsigned long)ir);
+   timer_setup(>timer, bttv_rc5_timer_end, 0);
ir->shift_by = 1;
ir->rc5_remote_gap = ir_rc5_remote_gap;
}
@@ -531,6 +531,7 @@ int bttv_input_init(struct bttv *btv)
 
/* init input device */
ir->dev = rc;
+   ir->btv = btv;
 
snprintf(ir->name, sizeof(ir->name), "bttv IR (card=%d)",
 btv->c.type);
@@ -553,7 +554,7 @@ int bttv_input_init(struct bttv *btv)
rc->driver_name = MOD

Re: [PATCH] media: input: Convert timers to use timer_setup()

2017-10-24 Thread Kees Cook
On Thu, Oct 19, 2017 at 3:48 PM, Dmitry Torokhov
<dmitry.torok...@gmail.com> wrote:
> On Thu, Oct 19, 2017 at 03:45:38PM -0700, Kees Cook wrote:
>> On Thu, Oct 19, 2017 at 3:32 PM, Dmitry Torokhov
>> <dmitry.torok...@gmail.com> wrote:
>> > On Mon, Oct 16, 2017 at 04:14:43PM -0700, Kees Cook wrote:
>> >> In preparation for unconditionally passing the struct timer_list pointer 
>> >> to
>> >> all timer callbacks, switch to using the new timer_setup() and 
>> >> from_timer()
>> >> to pass the timer pointer explicitly.
>> >>
>> >> One input_dev user hijacks the input_dev software autorepeat timer to
>> >> perform its own repeat management. However, there is no path back to the
>> >> existing status variable, so add a generic one to the input structure and
>> >> use that instead.
>> >
>> > That is too bad and it should not be doing this. I'd rather av7110 used
>> > its own private timer for that.
>>
>> Yeah, that was a pretty weird case. I couldn't see how to avoid it,
>> though. I didn't see a way to hook the autorepeat, but I'm not too
>> familiar with the code here.
>
> You just need to manage the private timer in the driver and not mess up
> with the input core if input core's autorepeat does not provide the
> desired behavior...

I don't know how to fix this, but I still need to do this refactoring.
What's the correct step forward here? Should I temporarily disable the
timer in av7110?

Seems like the hijacking was introduced in ee820a648fb3 ("V4L/DVB
(5334): Dvb-ttpci: Infrared remote control refactoring").

Thanks!

-Kees

-- 
Kees Cook
Pixel Security


[PATCH] media: s2255: Convert timers to use timer_setup()

2017-10-24 Thread Kees Cook
In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly.

Cc: Mauro Carvalho Chehab <mche...@kernel.org>
Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
Cc: Sakari Ailus <sakari.ai...@linux.intel.com>
Cc: Bhumika Goyal <bhumi...@gmail.com>
Cc: Mike Isely <is...@pobox.com>
Cc: Arvind Yadav <arvind.yadav...@gmail.com>
Cc: linux-media@vger.kernel.org
Signed-off-by: Kees Cook <keesc...@chromium.org>
---
 drivers/media/usb/s2255/s2255drv.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/media/usb/s2255/s2255drv.c 
b/drivers/media/usb/s2255/s2255drv.c
index b2f239c4ba42..7fee5766587a 100644
--- a/drivers/media/usb/s2255/s2255drv.c
+++ b/drivers/media/usb/s2255/s2255drv.c
@@ -485,9 +485,10 @@ static void s2255_reset_dsppower(struct s2255_dev *dev)
 
 /* kickstarts the firmware loading. from probe
  */
-static void s2255_timer(unsigned long user_data)
+static void s2255_timer(struct timer_list *t)
 {
-   struct s2255_fw *data = (struct s2255_fw *)user_data;
+   struct s2255_dev *dev = from_timer(dev, t, timer);
+   struct s2255_fw *data = dev->fw_data;
if (usb_submit_urb(data->fw_urb, GFP_ATOMIC) < 0) {
pr_err("s2255: can't submit urb\n");
atomic_set(>fw_state, S2255_FW_FAILED);
@@ -2283,7 +2284,7 @@ static int s2255_probe(struct usb_interface *interface,
dev_err(>dev, "Could not find bulk-in endpoint\n");
goto errorEP;
}
-   setup_timer(>timer, s2255_timer, (unsigned long)dev->fw_data);
+   timer_setup(>timer, s2255_timer, 0);
init_waitqueue_head(>fw_data->wait_fw);
for (i = 0; i < MAX_CHANNELS; i++) {
struct s2255_vc *vc = >vc[i];
-- 
2.7.4


-- 
Kees Cook
Pixel Security


[PATCH] media: radio: Convert timers to use timer_setup()

2017-10-24 Thread Kees Cook
In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly.

Cc: Hans Verkuil <hverk...@xs4all.nl>
Cc: Mauro Carvalho Chehab <mche...@kernel.org>
Cc: "David S. Miller" <da...@davemloft.net>
Cc: Johannes Berg <johannes.b...@intel.com>
Cc: linux-media@vger.kernel.org
Signed-off-by: Kees Cook <keesc...@chromium.org>
---
 drivers/media/radio/radio-cadet.c | 7 +++
 drivers/media/radio/wl128x/fmdrv_common.c | 7 +++
 2 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/media/radio/radio-cadet.c 
b/drivers/media/radio/radio-cadet.c
index 6888b7db449d..7575e5370a49 100644
--- a/drivers/media/radio/radio-cadet.c
+++ b/drivers/media/radio/radio-cadet.c
@@ -281,9 +281,9 @@ static bool cadet_has_rds_data(struct cadet *dev)
 }
 
 
-static void cadet_handler(unsigned long data)
+static void cadet_handler(struct timer_list *t)
 {
-   struct cadet *dev = (void *)data;
+   struct cadet *dev = from_timer(dev, t, readtimer);
 
/* Service the RDS fifo */
if (mutex_trylock(>lock)) {
@@ -309,7 +309,6 @@ static void cadet_handler(unsigned long data)
/*
 * Clean up and exit
 */
-   setup_timer(>readtimer, cadet_handler, data);
dev->readtimer.expires = jiffies + msecs_to_jiffies(50);
add_timer(>readtimer);
 }
@@ -318,7 +317,7 @@ static void cadet_start_rds(struct cadet *dev)
 {
dev->rdsstat = 1;
outb(0x80, dev->io);/* Select RDS fifo */
-   setup_timer(>readtimer, cadet_handler, (unsigned long)dev);
+   timer_setup(>readtimer, cadet_handler, 0);
dev->readtimer.expires = jiffies + msecs_to_jiffies(50);
add_timer(>readtimer);
 }
diff --git a/drivers/media/radio/wl128x/fmdrv_common.c 
b/drivers/media/radio/wl128x/fmdrv_common.c
index ab3428bf63fe..800d69c3f80b 100644
--- a/drivers/media/radio/wl128x/fmdrv_common.c
+++ b/drivers/media/radio/wl128x/fmdrv_common.c
@@ -543,13 +543,13 @@ static inline void fm_irq_common_cmd_resp_helper(struct 
fmdev *fmdev, u8 stage)
  * interrupt process. Therefore reset stage index to re-enable default
  * interrupts. So that next interrupt will be processed as usual.
  */
-static void int_timeout_handler(unsigned long data)
+static void int_timeout_handler(struct timer_list *t)
 {
struct fmdev *fmdev;
struct fm_irq *fmirq;
 
fmdbg("irq: timeout,trying to re-enable fm interrupts\n");
-   fmdev = (struct fmdev *)data;
+   fmdev = from_timer(fmdev, t, irq_info.timer);
fmirq = >irq_info;
fmirq->retry++;
 
@@ -1550,8 +1550,7 @@ int fmc_prepare(struct fmdev *fmdev)
atomic_set(>tx_cnt, 1);
fmdev->resp_comp = NULL;
 
-   setup_timer(>irq_info.timer, _timeout_handler,
-   (unsigned long)fmdev);
+   timer_setup(>irq_info.timer, int_timeout_handler, 0);
/*TODO: add FM_STIC_EVENT later */
fmdev->irq_info.mask = FM_MAL_EVENT;
 
-- 
2.7.4


-- 
Kees Cook
Pixel Security


[PATCH] media: pvrusb2: Convert timers to use timer_setup()

2017-10-24 Thread Kees Cook
In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly.

Cc: Mike Isely <is...@pobox.com>
Cc: Mauro Carvalho Chehab <mche...@kernel.org>
Cc: linux-media@vger.kernel.org
Signed-off-by: Kees Cook <keesc...@chromium.org>
---
 drivers/media/usb/pvrusb2/pvrusb2-hdw.c | 64 ++---
 1 file changed, 36 insertions(+), 28 deletions(-)

diff --git a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c 
b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c
index ad5b25b89699..8289ee482f49 100644
--- a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c
+++ b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c
@@ -330,10 +330,10 @@ static void pvr2_hdw_state_log_state(struct pvr2_hdw *);
 static int pvr2_hdw_cmd_usbstream(struct pvr2_hdw *hdw,int runFl);
 static int pvr2_hdw_commit_setup(struct pvr2_hdw *hdw);
 static int pvr2_hdw_get_eeprom_addr(struct pvr2_hdw *hdw);
-static void pvr2_hdw_quiescent_timeout(unsigned long);
-static void pvr2_hdw_decoder_stabilization_timeout(unsigned long);
-static void pvr2_hdw_encoder_wait_timeout(unsigned long);
-static void pvr2_hdw_encoder_run_timeout(unsigned long);
+static void pvr2_hdw_quiescent_timeout(struct timer_list *);
+static void pvr2_hdw_decoder_stabilization_timeout(struct timer_list *);
+static void pvr2_hdw_encoder_wait_timeout(struct timer_list *);
+static void pvr2_hdw_encoder_run_timeout(struct timer_list *);
 static int pvr2_issue_simple_cmd(struct pvr2_hdw *,u32);
 static int pvr2_send_request_ex(struct pvr2_hdw *hdw,
unsigned int timeout,int probe_fl,
@@ -2373,18 +2373,15 @@ struct pvr2_hdw *pvr2_hdw_create(struct usb_interface 
*intf,
}
if (!hdw) goto fail;
 
-   setup_timer(>quiescent_timer, pvr2_hdw_quiescent_timeout,
-   (unsigned long)hdw);
+   timer_setup(>quiescent_timer, pvr2_hdw_quiescent_timeout, 0);
 
-   setup_timer(>decoder_stabilization_timer,
-   pvr2_hdw_decoder_stabilization_timeout,
-   (unsigned long)hdw);
+   timer_setup(>decoder_stabilization_timer,
+   pvr2_hdw_decoder_stabilization_timeout, 0);
 
-   setup_timer(>encoder_wait_timer, pvr2_hdw_encoder_wait_timeout,
-   (unsigned long)hdw);
+   timer_setup(>encoder_wait_timer, pvr2_hdw_encoder_wait_timeout,
+   0);
 
-   setup_timer(>encoder_run_timer, pvr2_hdw_encoder_run_timeout,
-   (unsigned long)hdw);
+   timer_setup(>encoder_run_timer, pvr2_hdw_encoder_run_timeout, 0);
 
hdw->master_state = PVR2_STATE_DEAD;
 
@@ -3539,10 +3536,16 @@ static void pvr2_ctl_read_complete(struct urb *urb)
complete(>ctl_done);
 }
 
+struct hdw_timer {
+   struct timer_list timer;
+   struct pvr2_hdw *hdw;
+};
 
-static void pvr2_ctl_timeout(unsigned long data)
+static void pvr2_ctl_timeout(struct timer_list *t)
 {
-   struct pvr2_hdw *hdw = (struct pvr2_hdw *)data;
+   struct hdw_timer *timer = from_timer(timer, t, timer);
+   struct pvr2_hdw *hdw = timer->hdw;
+
if (hdw->ctl_write_pend_flag || hdw->ctl_read_pend_flag) {
hdw->ctl_timeout_flag = !0;
if (hdw->ctl_write_pend_flag)
@@ -3564,7 +3567,10 @@ static int pvr2_send_request_ex(struct pvr2_hdw *hdw,
 {
unsigned int idx;
int status = 0;
-   struct timer_list timer;
+   struct hdw_timer timer = {
+   .hdw = hdw,
+   };
+
if (!hdw->ctl_lock_held) {
pvr2_trace(PVR2_TRACE_ERROR_LEGS,
   "Attempted to execute control transfer without 
lock!!");
@@ -3621,8 +3627,8 @@ static int pvr2_send_request_ex(struct pvr2_hdw *hdw,
hdw->ctl_timeout_flag = 0;
hdw->ctl_write_pend_flag = 0;
hdw->ctl_read_pend_flag = 0;
-   setup_timer(, pvr2_ctl_timeout, (unsigned long)hdw);
-   timer.expires = jiffies + timeout;
+   timer_setup_on_stack(, pvr2_ctl_timeout, 0);
+   timer.timer.expires = jiffies + timeout;
 
if (write_len && write_data) {
hdw->cmd_debug_state = 2;
@@ -3677,7 +3683,7 @@ status);
}
 
/* Start timer */
-   add_timer();
+   add_timer();
 
/* Now wait for all I/O to complete */
hdw->cmd_debug_state = 4;
@@ -3687,7 +3693,7 @@ status);
hdw->cmd_debug_state = 5;
 
/* Stop timer */
-   del_timer_sync();
+   del_timer_sync();
 
hdw->cmd_debug_state = 6;
status = 0;
@@ -3769,6 +3775,8 @@ status);
if ((status < 0) && (!probe_fl)) {
pvr2_hdw_render_useless(hdw);
}
+   destroy_timer_on_stack();
+
return status;
 }
 
@@ -4366,9 +4374,9 @@ static int state_eval_encoder_run(struct pvr2_hdw *hdw)
 
 
 

[PATCH] media: rc: Convert timers to use timer_setup()

2017-10-24 Thread Kees Cook
In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly.

Cc: Maxim Levitsky <maximlevit...@gmail.com>
Cc: Mauro Carvalho Chehab <mche...@kernel.org>
Cc: Sean Young <s...@mess.org>
Cc: James Hogan <jho...@kernel.org>
Cc: Hans Verkuil <hans.verk...@cisco.com>
Cc: "Antti Seppälä" <a.sepp...@gmail.com>
Cc: Heiner Kallweit <hkallwe...@gmail.com>
Cc: "David Härdeman" <da...@hardeman.nu>
Cc: Andi Shyti <andi.sh...@samsung.com>
Cc: linux-media@vger.kernel.org
Signed-off-by: Kees Cook <keesc...@chromium.org>
---
 drivers/media/rc/ene_ir.c |  7 +++
 drivers/media/rc/igorplugusb.c|  6 +++---
 drivers/media/rc/img-ir/img-ir-hw.c   | 13 ++---
 drivers/media/rc/img-ir/img-ir-raw.c  |  6 +++---
 drivers/media/rc/imon.c   |  7 +++
 drivers/media/rc/ir-mce_kbd-decoder.c |  7 +++
 drivers/media/rc/rc-ir-raw.c  |  8 
 drivers/media/rc/rc-main.c|  7 +++
 drivers/media/rc/sir_ir.c |  4 ++--
 9 files changed, 30 insertions(+), 35 deletions(-)

diff --git a/drivers/media/rc/ene_ir.c b/drivers/media/rc/ene_ir.c
index af7ba23e16e1..71b8c9bbf6c4 100644
--- a/drivers/media/rc/ene_ir.c
+++ b/drivers/media/rc/ene_ir.c
@@ -670,9 +670,9 @@ static void ene_tx_sample(struct ene_device *dev)
 }
 
 /* timer to simulate tx done interrupt */
-static void ene_tx_irqsim(unsigned long data)
+static void ene_tx_irqsim(struct timer_list *t)
 {
-   struct ene_device *dev = (struct ene_device *)data;
+   struct ene_device *dev = from_timer(dev, t, tx_sim_timer);
unsigned long flags;
 
spin_lock_irqsave(>hw_lock, flags);
@@ -1045,8 +1045,7 @@ static int ene_probe(struct pnp_dev *pnp_dev, const 
struct pnp_device_id *id)
 
if (!dev->hw_learning_and_tx_capable && txsim) {
dev->hw_learning_and_tx_capable = true;
-   setup_timer(>tx_sim_timer, ene_tx_irqsim,
-   (long unsigned int)dev);
+   timer_setup(>tx_sim_timer, ene_tx_irqsim, 0);
pr_warn("Simulation of TX activated\n");
}
 
diff --git a/drivers/media/rc/igorplugusb.c b/drivers/media/rc/igorplugusb.c
index 4b715eb995f8..f563ddd7f739 100644
--- a/drivers/media/rc/igorplugusb.c
+++ b/drivers/media/rc/igorplugusb.c
@@ -137,9 +137,9 @@ static void igorplugusb_cmd(struct igorplugusb *ir, int cmd)
dev_err(ir->dev, "submit urb failed: %d", ret);
 }
 
-static void igorplugusb_timer(unsigned long data)
+static void igorplugusb_timer(struct timer_list *t)
 {
-   struct igorplugusb *ir = (struct igorplugusb *)data;
+   struct igorplugusb *ir = from_timer(ir, t, timer);
 
igorplugusb_cmd(ir, GET_INFRACODE);
 }
@@ -174,7 +174,7 @@ static int igorplugusb_probe(struct usb_interface *intf,
 
ir->dev = >dev;
 
-   setup_timer(>timer, igorplugusb_timer, (unsigned long)ir);
+   timer_setup(>timer, igorplugusb_timer, 0);
 
ir->request.bRequest = GET_INFRACODE;
ir->request.bRequestType = USB_TYPE_VENDOR | USB_DIR_IN;
diff --git a/drivers/media/rc/img-ir/img-ir-hw.c 
b/drivers/media/rc/img-ir/img-ir-hw.c
index 82fdf4cc0824..f54bc5d23893 100644
--- a/drivers/media/rc/img-ir/img-ir-hw.c
+++ b/drivers/media/rc/img-ir/img-ir-hw.c
@@ -867,9 +867,9 @@ static void img_ir_handle_data(struct img_ir_priv *priv, 
u32 len, u64 raw)
 }
 
 /* timer function to end waiting for repeat. */
-static void img_ir_end_timer(unsigned long arg)
+static void img_ir_end_timer(struct timer_list *t)
 {
-   struct img_ir_priv *priv = (struct img_ir_priv *)arg;
+   struct img_ir_priv *priv = from_timer(priv, t, hw.end_timer);
 
spin_lock_irq(>lock);
img_ir_end_repeat(priv);
@@ -881,9 +881,9 @@ static void img_ir_end_timer(unsigned long arg)
  * cleared when invalid interrupts were generated due to a quirk in the
  * img-ir decoder.
  */
-static void img_ir_suspend_timer(unsigned long arg)
+static void img_ir_suspend_timer(struct timer_list *t)
 {
-   struct img_ir_priv *priv = (struct img_ir_priv *)arg;
+   struct img_ir_priv *priv = from_timer(priv, t, hw.suspend_timer);
 
spin_lock_irq(>lock);
/*
@@ -1055,9 +1055,8 @@ int img_ir_probe_hw(struct img_ir_priv *priv)
img_ir_probe_hw_caps(priv);
 
/* Set up the end timer */
-   setup_timer(>end_timer, img_ir_end_timer, (unsigned long)priv);
-   setup_timer(>suspend_timer, img_ir_suspend_timer,
-   (unsigned long)priv);
+   timer_setup(>end_timer, img_ir_end_timer, 0);
+   timer_setup(>suspend_timer, img_ir_suspend_timer, 0);
 
/* Register a clock notifier */
if (!IS_ERR(priv->clk)) {
diff --

Re: [PATCH] media: av7110: switch to useing timer_setup()

2017-10-30 Thread Kees Cook
On Tue, Oct 24, 2017 at 5:40 PM, Dmitry Torokhov
<dmitry.torok...@gmail.com> wrote:
> In preparation for unconditionally passing the struct timer_list pointer to
> all timer callbacks, switch to using the new timer_setup() and from_timer()
> to pass the timer pointer explicitly.
>
> Also stop poking into input core internals and override its autorepeat
> timer function. I am not sure why we have such convoluted autorepeat
> handling in this driver instead of letting input core handle autorepeat,
> but this preserves current behavior of allowing controlling autorepeat
> delay and forcing autorepeat period to be whatever the hardware has.
>
> Signed-off-by: Dmitry Torokhov <dmitry.torok...@gmail.com>

Reviewed-by: Kees Cook <keesc...@chromium.org>
(with the Subject typo fixed)

Hans, since this depends on the input side not changing first, I think
it makes sense for Dmitry to carry this in the Input tree before the
Input timer updates. Would that be okay?

Thanks!

-Kees

> ---
>
> Note that this has not been tested on the hardware. But it should
> compile, so I have that going for me.
>
>  drivers/media/pci/ttpci/av7110.h|  4 ++--
>  drivers/media/pci/ttpci/av7110_ir.c | 40 
> +
>  2 files changed, 20 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/media/pci/ttpci/av7110.h 
> b/drivers/media/pci/ttpci/av7110.h
> index 347827925c14..0aa3c6f01853 100644
> --- a/drivers/media/pci/ttpci/av7110.h
> +++ b/drivers/media/pci/ttpci/av7110.h
> @@ -80,10 +80,11 @@ struct av7110;
>
>  /* infrared remote control */
>  struct infrared {
> -   u16 key_map[256];
> +   u16 key_map[256];
> struct input_dev*input_dev;
> charinput_phys[32];
> struct timer_list   keyup_timer;
> +   unsigned long   keydown_time;
> struct tasklet_struct   ir_tasklet;
> void(*ir_handler)(struct av7110 *av7110, u32 
> ircom);
> u32 ir_command;
> @@ -93,7 +94,6 @@ struct infrared {
> u8  inversion;
> u16 last_key;
> u16 last_toggle;
> -   u8  delay_timer_finished;
>  };
>
>
> diff --git a/drivers/media/pci/ttpci/av7110_ir.c 
> b/drivers/media/pci/ttpci/av7110_ir.c
> index ca05198de2c2..b602e64b3412 100644
> --- a/drivers/media/pci/ttpci/av7110_ir.c
> +++ b/drivers/media/pci/ttpci/av7110_ir.c
> @@ -84,9 +84,9 @@ static u16 default_key_map [256] = {
>
>
>  /* key-up timer */
> -static void av7110_emit_keyup(unsigned long parm)
> +static void av7110_emit_keyup(struct timer_list *t)
>  {
> -   struct infrared *ir = (struct infrared *) parm;
> +   struct infrared *ir = from_timer(ir, keyup_timer, t);
>
> if (!ir || !test_bit(ir->last_key, ir->input_dev->key))
> return;
> @@ -152,19 +152,20 @@ static void av7110_emit_key(unsigned long parm)
> return;
> }
>
> -   if (timer_pending(>keyup_timer)) {
> -   del_timer(>keyup_timer);
> +   if (del_timer(>keyup_timer)) {
> if (ir->last_key != keycode || toggle != ir->last_toggle) {
> -   ir->delay_timer_finished = 0;
> +   ir->keydown_time = jiffies;
> input_event(ir->input_dev, EV_KEY, ir->last_key, 0);
> input_event(ir->input_dev, EV_KEY, keycode, 1);
> input_sync(ir->input_dev);
> -   } else if (ir->delay_timer_finished) {
> +   } else if (time_after(jiffies, ir->keydown_time +
> +   msecs_to_jiffies(
> +   ir->input_dev->rep[REP_PERIOD]))) {
> input_event(ir->input_dev, EV_KEY, keycode, 2);
> input_sync(ir->input_dev);
> }
> } else {
> -   ir->delay_timer_finished = 0;
> +   ir->keydown_time = jiffies;
> input_event(ir->input_dev, EV_KEY, keycode, 1);
> input_sync(ir->input_dev);
> }
> @@ -172,9 +173,7 @@ static void av7110_emit_key(unsigned long parm)
> ir->last_key = keycode;
> ir->last_toggle = toggle;
>
> -   ir->keyup_timer.expires = jiffies + UP_TIMEOUT;
> -   add_timer(>keyup_timer);
> -
> +   mod_timer(>keyup_timer, jiffies + UP_TIMEOUT);
>  }
>
>
> @@ -184,12 +183,19 @@ static void input_register_keys(struct inf

Re: [PATCH] media: media-device: fix ioctl function types

2018-04-30 Thread Kees Cook
On Fri, Apr 27, 2018 at 12:54 PM, Sami Tolvanen <samitolva...@google.com> wrote:
> This change fixes function types for media device ioctls to avoid
> indirect call mismatches with Control-Flow Integrity checking.
>
> Signed-off-by: Sami Tolvanen <samitolva...@google.com>

Thanks for sending these!

Reviewed-by: Kees Cook <keesc...@chromium.org>

-Kees

> ---
>  drivers/media/media-device.c | 21 +++--
>  1 file changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
> index 35e81f7c0d2f..bc5c024906e6 100644
> --- a/drivers/media/media-device.c
> +++ b/drivers/media/media-device.c
> @@ -54,9 +54,10 @@ static int media_device_close(struct file *filp)
> return 0;
>  }
>
> -static int media_device_get_info(struct media_device *dev,
> -struct media_device_info *info)
> +static long media_device_get_info(struct media_device *dev, void *arg)
>  {
> +   struct media_device_info *info = (struct media_device_info *)arg;
> +
> memset(info, 0, sizeof(*info));
>
> if (dev->driver_name[0])
> @@ -93,9 +94,9 @@ static struct media_entity *find_entity(struct media_device 
> *mdev, u32 id)
> return NULL;
>  }
>
> -static long media_device_enum_entities(struct media_device *mdev,
> -  struct media_entity_desc *entd)
> +static long media_device_enum_entities(struct media_device *mdev, void *arg)
>  {
> +   struct media_entity_desc *entd = (struct media_entity_desc *)arg;
> struct media_entity *ent;
>
> ent = find_entity(mdev, entd->id);
> @@ -146,9 +147,9 @@ static void media_device_kpad_to_upad(const struct 
> media_pad *kpad,
> upad->flags = kpad->flags;
>  }
>
> -static long media_device_enum_links(struct media_device *mdev,
> -   struct media_links_enum *links)
> +static long media_device_enum_links(struct media_device *mdev, void *arg)
>  {
> +   struct media_links_enum *links = (struct media_links_enum *)arg;
> struct media_entity *entity;
>
> entity = find_entity(mdev, links->entity);
> @@ -195,9 +196,9 @@ static long media_device_enum_links(struct media_device 
> *mdev,
> return 0;
>  }
>
> -static long media_device_setup_link(struct media_device *mdev,
> -   struct media_link_desc *linkd)
> +static long media_device_setup_link(struct media_device *mdev, void *arg)
>  {
> +   struct media_link_desc *linkd = (struct media_link_desc *)arg;
> struct media_link *link = NULL;
> struct media_entity *source;
> struct media_entity *sink;
> @@ -225,9 +226,9 @@ static long media_device_setup_link(struct media_device 
> *mdev,
> return __media_entity_setup_link(link, linkd->flags);
>  }
>
> -static long media_device_get_topology(struct media_device *mdev,
> - struct media_v2_topology *topo)
> +static long media_device_get_topology(struct media_device *mdev, void *arg)
>  {
> +   struct media_v2_topology *topo = (struct media_v2_topology *)arg;
> struct media_entity *entity;
> struct media_interface *intf;
> struct media_pad *pad;
> --
> 2.17.0.441.gb46fe60e1d-goog
>



-- 
Kees Cook
Pixel Security


Re: [PATCH] media: v4l2-ioctl: fix function types for IOCTL_INFO_STD

2018-04-30 Thread Kees Cook
On Fri, Apr 27, 2018 at 11:59 AM, Sami Tolvanen <samitolva...@google.com> wrote:
> This change fixes indirect call mismatches with Control-Flow Integrity
> checking, which are caused by calling standard ioctls using a function
> pointer that doesn't match the type of the actual function.
>
> Signed-off-by: Sami Tolvanen <samitolva...@google.com>

I think this actually makes things much more readable in the end. Thanks!

Reviewed-by: Kees Cook <keesc...@chromium.org>

-Kees


> ---
>  drivers/media/v4l2-core/v4l2-ioctl.c | 72 ++--
>  1 file changed, 46 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c 
> b/drivers/media/v4l2-core/v4l2-ioctl.c
> index f48c505550e0..d50a06ab3509 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -2489,11 +2489,8 @@ struct v4l2_ioctl_info {
> unsigned int ioctl;
> u32 flags;
> const char * const name;
> -   union {
> -   u32 offset;
> -   int (*func)(const struct v4l2_ioctl_ops *ops,
> -   struct file *file, void *fh, void *p);
> -   } u;
> +   int (*func)(const struct v4l2_ioctl_ops *ops, struct file *file,
> +   void *fh, void *p);
> void (*debug)(const void *arg, bool write_only);
>  };
>
> @@ -2501,27 +2498,24 @@ struct v4l2_ioctl_info {
>  #define INFO_FL_PRIO   (1 << 0)
>  /* This control can be valid if the filehandle passes a control handler. */
>  #define INFO_FL_CTRL   (1 << 1)
> -/* This is a standard ioctl, no need for special code */
> -#define INFO_FL_STD(1 << 2)
>  /* This is ioctl has its own function */
> -#define INFO_FL_FUNC   (1 << 3)
> +#define INFO_FL_FUNC   (1 << 2)
>  /* Queuing ioctl */
> -#define INFO_FL_QUEUE  (1 << 4)
> +#define INFO_FL_QUEUE  (1 << 3)
>  /* Always copy back result, even on error */
> -#define INFO_FL_ALWAYS_COPY(1 << 5)
> +#define INFO_FL_ALWAYS_COPY(1 << 4)
>  /* Zero struct from after the field to the end */
>  #define INFO_FL_CLEAR(v4l2_struct, field)  \
> ((offsetof(struct v4l2_struct, field) + \
>   sizeof(((struct v4l2_struct *)0)->field)) << 16)
>  #define INFO_FL_CLEAR_MASK (_IOC_SIZEMASK << 16)
>
> -#define IOCTL_INFO_STD(_ioctl, _vidioc, _debug, _flags)  
>   \
> -   [_IOC_NR(_ioctl)] = {   \
> -   .ioctl = _ioctl,\
> -   .flags = _flags | INFO_FL_STD,  \
> -   .name = #_ioctl,\
> -   .u.offset = offsetof(struct v4l2_ioctl_ops, _vidioc),   \
> -   .debug = _debug,\
> +#define DEFINE_IOCTL_STD_FNC(_vidioc)  \
> +   static int __v4l_ ## _vidioc ## _fnc(   \
> +   const struct v4l2_ioctl_ops *ops,   \
> +   struct file *file, void *fh, void *p)   \
> +   {   \
> +   return ops->_vidioc(file, fh, p);   \
> }
>
>  #define IOCTL_INFO_FNC(_ioctl, _func, _debug, _flags)  \
> @@ -2529,10 +2523,42 @@ struct v4l2_ioctl_info {
> .ioctl = _ioctl,\
> .flags = _flags | INFO_FL_FUNC, \
> .name = #_ioctl,\
> -   .u.func = _func,\
> +   .func = _func,  \
> .debug = _debug,\
> }
>
> +#define IOCTL_INFO_STD(_ioctl, _vidioc, _debug, _flags)\
> +   IOCTL_INFO_FNC(_ioctl, __v4l_ ## _vidioc ## _fnc, _debug, _flags)
> +
> +DEFINE_IOCTL_STD_FNC(vidioc_g_fbuf)
> +DEFINE_IOCTL_STD_FNC(vidioc_s_fbuf)
> +DEFINE_IOCTL_STD_FNC(vidioc_expbuf)
> +DEFINE_IOCTL_STD_FNC(vidioc_g_std)
> +DEFINE_IOCTL_STD_FNC(vidioc_g_audio)
> +DEFINE_IOCTL_STD_FNC(vidioc_s_audio)
> +DEFINE_IOCTL_STD_FNC(vidioc_g_input)
> +DEFINE_IOCTL_STD_FNC(vidioc_g_edid)
> +DEFINE_IOCTL_STD_FNC(vidioc_s_edid)
> +DEFINE_IOCTL_STD_FNC(vidioc_g_output)
> +DEFINE_IOCTL_STD_FNC(vidioc_g_audout)
> +DEFINE_IOCTL_STD_FNC(vidioc_s_audout)
> +DEFINE_IOCTL_STD_FNC(vidioc_g_jpegcomp)
> +DEFINE_IOCTL_STD_FNC(vidioc_s_jpegcom

Re: [PATCH] media: input: Convert timers to use timer_setup()

2017-10-19 Thread Kees Cook
On Thu, Oct 19, 2017 at 3:32 PM, Dmitry Torokhov
<dmitry.torok...@gmail.com> wrote:
> On Mon, Oct 16, 2017 at 04:14:43PM -0700, Kees Cook wrote:
>> In preparation for unconditionally passing the struct timer_list pointer to
>> all timer callbacks, switch to using the new timer_setup() and from_timer()
>> to pass the timer pointer explicitly.
>>
>> One input_dev user hijacks the input_dev software autorepeat timer to
>> perform its own repeat management. However, there is no path back to the
>> existing status variable, so add a generic one to the input structure and
>> use that instead.
>
> That is too bad and it should not be doing this. I'd rather av7110 used
> its own private timer for that.

Yeah, that was a pretty weird case. I couldn't see how to avoid it,
though. I didn't see a way to hook the autorepeat, but I'm not too
familiar with the code here.

(I am finding such bizarre stuff with this refactoring...)

-Kees

-- 
Kees Cook
Pixel Security


Re: [PATCH 3/3] media: replace strncpy() by strscpy()

2018-09-10 Thread Kees Cook
On Mon, Sep 10, 2018 at 5:19 AM, Mauro Carvalho Chehab
 wrote:
> The strncpy() function is being deprecated upstream. Replace
> it by the safer strscpy().

This one I'm quite concerned about. This could lead to kernel memory
exposures if any of the callers depend on strncpy()'s trailing
NUL-padding to clear a buffer of prior contents.

How did you validate that for these changes?

-Kees

-- 
Kees Cook
Pixel Security


Re: [PATCH 1/3] media: use strscpy() instead of strlcpy()

2018-09-10 Thread Kees Cook
On Mon, Sep 10, 2018 at 5:19 AM, Mauro Carvalho Chehab
 wrote:
> The implementation of strscpy() is more robust and safer.
>
> That's now the recommended way to copy NUL terminated strings.

This looks fine since I don't see anything using the strlcpy() return
value (the return value meaning between strlcpy() and strscpy()
differs).

Reviewed-by: Kees Cook 

-Kees

-- 
Kees Cook
Pixel Security


Re: [PATCH 2/3] media: replace strcpy() by strscpy()

2018-09-10 Thread Kees Cook
On Mon, Sep 10, 2018 at 5:19 AM, Mauro Carvalho Chehab
 wrote:
> The strcpy() function is being deprecated upstream. Replace
> it by the safer strscpy().

Did you verify that all the destination buffers here are arrays and
not pointers? For example:

struct thing {
  char buffer[64];
  char *ptr;
}

strscpy(instance->buffer, source, sizeof(instance->buffer));

is correct.

But:

strscpy(instance->ptr, source, sizeof(instance->ptr));

will not be and will truncate strings to sizeof(char *).

If you _did_ verify this, I'd love to know more about your tooling. :)

-Kees

-- 
Kees Cook
Pixel Security


Re: [PATCH 3/3] media: replace strncpy() by strscpy()

2018-09-10 Thread Kees Cook
On Mon, Sep 10, 2018 at 11:34 AM, Mauro Carvalho Chehab
 wrote:
> Em Mon, 10 Sep 2018 09:18:05 -0700
> Kees Cook  escreveu:
>
>> On Mon, Sep 10, 2018 at 5:19 AM, Mauro Carvalho Chehab
>>  wrote:
>> > The strncpy() function is being deprecated upstream. Replace
>> > it by the safer strscpy().
>>
>> This one I'm quite concerned about. This could lead to kernel memory
>> exposures if any of the callers depend on strncpy()'s trailing
>> NUL-padding to clear a buffer of prior contents.
>>
>> How did you validate that for these changes?
>
> That's actually easy for those familiar with the V4L2 API. There are
> several fields at either uAPI or kAPI (or both) that have strings.
>
> For example, a video input has a name.
>
> So, for one familiar with the V4L2 API, it is clear that something
> like:
>
> +   strscpy(inp->name, zr->card.input[inp->index].name,
> +   sizeof(inp->name));
>
> Is just filling the uAPI with the name of Input, with is, typically,
> something like:
> S-Video
> Television
> Radio
> Composite
>
> A visual inspection of the patch shows that, on almost all cases, it is
> either filling a device driver's name (used mainly for debug routines),
> a video Input, a format description string, or the video caps fields
> name and driver.

It looks like the ioctl path also pre-clears the output buffer before
handing it over to the per-driver routines, so I think this looks
okay. It's a large patch, but if you're comfortable with it, go for
it. :)

-Kees

-- 
Kees Cook
Pixel Security


Re: [PATCH][RFC] kernel.h: provide array iterator

2018-03-15 Thread Kees Cook
On Thu, Mar 15, 2018 at 3:00 AM, Kieran Bingham
<kieran.bing...@ideasonboard.com> wrote:
> Simplify array iteration with a helper to iterate each entry in an array.
> Utilise the existing ARRAY_SIZE macro to identify the length of the array
> and pointer arithmetic to process each item as a for loop.
>
> Signed-off-by: Kieran Bingham <kieran.bing...@ideasonboard.com>
> ---
>  include/linux/kernel.h | 10 ++
>  1 file changed, 10 insertions(+)
>
> The use of static arrays to store data is a common use case throughout the
> kernel. Along with that is the obvious need to iterate that data.
>
> In fact there are just shy of 5000 instances of iterating a static array:
> git grep "for .*ARRAY_SIZE" | wc -l
> 4943

I suspect the main question is "Does this macro make the code easier to read?"

I think it does, and we have other kinds of iterators like this in the
kernel already. Would it be worth building a Coccinelle script to do
the 5000 replacements?

-Kees

>
> When working on the UVC driver - I found that I needed to split one such
> iteration into two parts, and at the same time felt that this could be
> refactored to be cleaner / easier to read.
>
> I do however worry that this simple short patch might not be desired or could
> also be heavily bikeshedded due to it's potential wide spread use (though
> perhaps that would be a good thing to have more users) ...  but here it is,
> along with an example usage below which is part of a separate series.
>
> The aim is to simplify iteration on static arrays, in the same way that we 
> have
> iterators for lists. The use of the ARRAY_SIZE macro, provides all the
> protections given by "__must_be_array(arr)" to this macro too.
>
> Regards
>
> Kieran
>
> =
> Example Usage from a pending UVC development:
>
> +#define for_each_uvc_urb(uvc_urb, uvc_streaming) \
> +   for_each_array_element(uvc_urb, uvc_streaming->uvc_urb)
>
>  /*
>   * Uninitialize isochronous/bulk URBs and free transfer buffers.
>   */
>  static void uvc_uninit_video(struct uvc_streaming *stream, int free_buffers)
>  {
> -   struct urb *urb;
> -   unsigned int i;
> +   struct uvc_urb *uvc_urb;
>
> uvc_video_stats_stop(stream);
>
> -   for (i = 0; i < UVC_URBS; ++i) {
> -   struct uvc_urb *uvc_urb = >uvc_urb[i];
> +   for_each_uvc_urb(uvc_urb, stream)
> +   usb_kill_urb(uvc_urb->urb);
>
> -   urb = uvc_urb->urb;
> -   if (urb == NULL)
> -   continue;
> +   flush_workqueue(stream->async_wq);
>
> -   usb_kill_urb(urb);
> -   usb_free_urb(urb);
> +   for_each_uvc_urb(uvc_urb, stream) {
> +   usb_free_urb(uvc_urb->urb);
> uvc_urb->urb = NULL;
> }
>
> if (free_buffers)
> uvc_free_urb_buffers(stream);
>  }
> =
>
>
>
>
> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> index ce51455e2adf..95d7dae248b7 100644
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -70,6 +70,16 @@
>   */
>  #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + 
> __must_be_array(arr))
>
> +/**
> + * for_each_array_element - Iterate all items in an array
> + * @elem: pointer of array type for iteration cursor
> + * @array: array to be iterated
> + */
> +#define for_each_array_element(elem, array) \
> +   for (elem = &(array)[0]; \
> +elem < &(array)[ARRAY_SIZE(array)]; \
> +++elem)
> +
>  #define u64_to_user_ptr(x) (   \
>  {  \
> typecheck(u64, x);  \
> --
> 2.7.4
>



-- 
Kees Cook
Pixel Security