All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: prevent division by zero when asking for chipset power
@ 2011-11-10 12:51 Eugeni Dodonov
  2011-11-10 13:21 ` Chris Wilson
  2011-11-11 15:36 ` Jesse Barnes
  0 siblings, 2 replies; 4+ messages in thread
From: Eugeni Dodonov @ 2011-11-10 12:51 UTC (permalink / raw)
  To: intel-gfx; +Cc: stable, Eugeni Dodonov

This prevents an in-kernel division by zero which happens when we are
asking for i915_chipset_val too quickly, or within a race condition
between the power monitoring thread and userspace accesses via debugfs.

The issue can be reproduced easily via the following command:
while ``; do cat /sys/kernel/debug/dri/0/i915_emon_status; done

This is particularly dangerous because it can be triggered by
a non-privileged user by just reading the debugfs entry.

This issue was also found independently by Konstantin Belousov
<kostikbel@gmail.com>, who proposed a similar patch.

Reported-by: Konstantin Belousov <kostikbel@gmail.com>
Acked-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Acked-by: Keith Packard <keithp@keithp.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c |   10 ++++++++++
 drivers/gpu/drm/i915/i915_drv.h |    1 +
 2 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 8a3942c..de199e7 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1453,6 +1453,14 @@ unsigned long i915_chipset_val(struct drm_i915_private *dev_priv)
 
 	diff1 = now - dev_priv->last_time1;
 
+	/* Prevent division-by-zero if we are asking too fast.
+	 * Also, we don't get interesting results in we are polling
+	 * faster than once in 10ms, so just return the saved value
+	 * in such cases.
+	 */
+	if (diff1 <= 10)
+		return dev_priv->chipset_power;
+
 	count1 = I915_READ(DMIEC);
 	count2 = I915_READ(DDREC);
 	count3 = I915_READ(CSIEC);
@@ -1483,6 +1491,8 @@ unsigned long i915_chipset_val(struct drm_i915_private *dev_priv)
 	dev_priv->last_count1 = total_count;
 	dev_priv->last_time1 = now;
 
+	dev_priv->chipset_power = ret;
+
 	return ret;
 }
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 7916bd9..1a2a2d1 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -707,6 +707,7 @@ typedef struct drm_i915_private {
 
 	u64 last_count1;
 	unsigned long last_time1;
+	unsigned long chipset_power;
 	u64 last_count2;
 	struct timespec last_time2;
 	unsigned long gfx_power;
-- 
1.7.7.1

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

* Re: [PATCH] drm/i915: prevent division by zero when asking for chipset power
  2011-11-10 12:51 [PATCH] drm/i915: prevent division by zero when asking for chipset power Eugeni Dodonov
@ 2011-11-10 13:21 ` Chris Wilson
  2011-11-10 15:55   ` Eugeni Dodonov
  2011-11-11 15:36 ` Jesse Barnes
  1 sibling, 1 reply; 4+ messages in thread
From: Chris Wilson @ 2011-11-10 13:21 UTC (permalink / raw)
  To: intel-gfx; +Cc: stable, Eugeni Dodonov

On Thu, 10 Nov 2011 10:51:26 -0200, Eugeni Dodonov <eugeni.dodonov@intel.com> wrote:
> This prevents an in-kernel division by zero which happens when we are
> asking for i915_chipset_val too quickly, or within a race condition
> between the power monitoring thread and userspace accesses via debugfs.
> 
> The issue can be reproduced easily via the following command:
> while ``; do cat /sys/kernel/debug/dri/0/i915_emon_status; done
> 
> This is particularly dangerous because it can be triggered by
> a non-privileged user by just reading the debugfs entry.
> 
> This issue was also found independently by Konstantin Belousov
> <kostikbel@gmail.com>, who proposed a similar patch.
> 
> Reported-by: Konstantin Belousov <kostikbel@gmail.com>
> Acked-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> Acked-by: Keith Packard <keithp@keithp.com>
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Just spotted a typo...
> ---
>  drivers/gpu/drm/i915/i915_dma.c |   10 ++++++++++
>  drivers/gpu/drm/i915/i915_drv.h |    1 +
>  2 files changed, 11 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 8a3942c..de199e7 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1453,6 +1453,14 @@ unsigned long i915_chipset_val(struct drm_i915_private *dev_priv)
>  
>  	diff1 = now - dev_priv->last_time1;
>  
> +	/* Prevent division-by-zero if we are asking too fast.
> +	 * Also, we don't get interesting results in we are polling
s/in/if/
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

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

* [PATCH] drm/i915: prevent division by zero when asking for chipset power
  2011-11-10 13:21 ` Chris Wilson
@ 2011-11-10 15:55   ` Eugeni Dodonov
  0 siblings, 0 replies; 4+ messages in thread
From: Eugeni Dodonov @ 2011-11-10 15:55 UTC (permalink / raw)
  To: intel-gfx; +Cc: stable, Eugeni Dodonov

This prevents an in-kernel division by zero which happens when we are
asking for i915_chipset_val too quickly, or within a race condition
between the power monitoring thread and userspace accesses via debugfs.

The issue can be reproduced easily via the following command:
while ``; do cat /sys/kernel/debug/dri/0/i915_emon_status; done

This is particularly dangerous because it can be triggered by
a non-privileged user by just reading the debugfs entry.

This issue was also found independently by Konstantin Belousov
<kostikbel@gmail.com>, who proposed a similar patch.

Reported-by: Konstantin Belousov <kostikbel@gmail.com>
Acked-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Acked-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: <stable@vger.kernel.org>
Signed-off-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c |   10 ++++++++++
 drivers/gpu/drm/i915/i915_drv.h |    1 +
 2 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 8a3942c..c72b590 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1453,6 +1453,14 @@ unsigned long i915_chipset_val(struct drm_i915_private *dev_priv)
 
 	diff1 = now - dev_priv->last_time1;
 
+	/* Prevent division-by-zero if we are asking too fast.
+	 * Also, we don't get interesting results if we are polling
+	 * faster than once in 10ms, so just return the saved value
+	 * in such cases.
+	 */
+	if (diff1 <= 10)
+		return dev_priv->chipset_power;
+
 	count1 = I915_READ(DMIEC);
 	count2 = I915_READ(DDREC);
 	count3 = I915_READ(CSIEC);
@@ -1483,6 +1491,8 @@ unsigned long i915_chipset_val(struct drm_i915_private *dev_priv)
 	dev_priv->last_count1 = total_count;
 	dev_priv->last_time1 = now;
 
+	dev_priv->chipset_power = ret;
+
 	return ret;
 }
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 7916bd9..1a2a2d1 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -707,6 +707,7 @@ typedef struct drm_i915_private {
 
 	u64 last_count1;
 	unsigned long last_time1;
+	unsigned long chipset_power;
 	u64 last_count2;
 	struct timespec last_time2;
 	unsigned long gfx_power;
-- 
1.7.7.1

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

* Re: [PATCH] drm/i915: prevent division by zero when asking for chipset power
  2011-11-10 12:51 [PATCH] drm/i915: prevent division by zero when asking for chipset power Eugeni Dodonov
  2011-11-10 13:21 ` Chris Wilson
@ 2011-11-11 15:36 ` Jesse Barnes
  1 sibling, 0 replies; 4+ messages in thread
From: Jesse Barnes @ 2011-11-11 15:36 UTC (permalink / raw)
  To: Eugeni Dodonov; +Cc: intel-gfx, stable


[-- Attachment #1.1: Type: text/plain, Size: 1069 bytes --]

On Thu, 10 Nov 2011 10:51:26 -0200
Eugeni Dodonov <eugeni.dodonov@intel.com> wrote:

> This prevents an in-kernel division by zero which happens when we are
> asking for i915_chipset_val too quickly, or within a race condition
> between the power monitoring thread and userspace accesses via debugfs.
> 
> The issue can be reproduced easily via the following command:
> while ``; do cat /sys/kernel/debug/dri/0/i915_emon_status; done
> 
> This is particularly dangerous because it can be triggered by
> a non-privileged user by just reading the debugfs entry.
> 
> This issue was also found independently by Konstantin Belousov
> <kostikbel@gmail.com>, who proposed a similar patch.
> 
> Reported-by: Konstantin Belousov <kostikbel@gmail.com>
> Acked-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> Acked-by: Keith Packard <keithp@keithp.com>
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
> ---

Yep, looks fine, thanks for pushing this Eugeni.

-- 
Jesse Barnes, Intel Open Source Technology Center

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

end of thread, other threads:[~2011-11-11 15:36 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-11-10 12:51 [PATCH] drm/i915: prevent division by zero when asking for chipset power Eugeni Dodonov
2011-11-10 13:21 ` Chris Wilson
2011-11-10 15:55   ` Eugeni Dodonov
2011-11-11 15:36 ` Jesse Barnes

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.