Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1467201 > unrolled thread

[PATCH] drm/gma500: dont expose bytes from kernel stack

Started byHeinrich Schuchardt <xypron.glpk@gmx.de>
First post2016-08-21 20:50 +0200
Last post2016-08-22 10:40 +0200
Articles 7 — 5 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH] drm/gma500: dont expose bytes from kernel stack Heinrich Schuchardt <xypron.glpk@gmx.de> - 2016-08-21 20:50 +0200
    Re: [PATCH] drm/gma500: dont expose bytes from kernel stack Joe Perches <joe@perches.com> - 2016-08-21 20:50 +0200
      Re: [PATCH] drm/gma500: dont expose bytes from kernel stack Heinrich Schuchardt <xypron.glpk@gmx.de> - 2016-08-21 21:40 +0200
        Re: [PATCH] drm/gma500: dont expose bytes from kernel stack Joe Perches <joe@perches.com> - 2016-08-21 21:40 +0200
    Re: [PATCH] drm/gma500: dont expose bytes from kernel stack Daniel Vetter <daniel@ffwll.ch> - 2016-08-22 09:40 +0200
      Re: [PATCH] drm/gma500: dont expose bytes from kernel stack One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk> - 2016-08-22 11:20 +0200
    Re: [PATCH] drm/gma500: dont expose bytes from kernel stack Jani Nikula <jani.nikula@linux.intel.com> - 2016-08-22 10:40 +0200

#1467201 — [PATCH] drm/gma500: dont expose bytes from kernel stack

FromHeinrich Schuchardt <xypron.glpk@gmx.de>
Date2016-08-21 20:50 +0200
Subject[PATCH] drm/gma500: dont expose bytes from kernel stack
Message-ID<s8FYJ-3WX-5@gated-at.bofh.it>
Components m1, m2, p2, dot, vco of variable clock should be
initialized to avoid bytes from the kernel stack to be
exposed.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 drivers/gpu/drm/gma500/oaktrail_crtc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/gma500/oaktrail_crtc.c b/drivers/gpu/drm/gma500/oaktrail_crtc.c
index da9fd34..28bd8f3 100644
--- a/drivers/gpu/drm/gma500/oaktrail_crtc.c
+++ b/drivers/gpu/drm/gma500/oaktrail_crtc.c
@@ -138,6 +138,7 @@ static bool mrst_sdvo_find_best_pll(const struct gma_limit_t *limit,
 	u32 target_vco, actual_freq;
 	s32 freq_error, min_error = 100000;
 
+	memset(clock, 0, sizeof(struct gma_clock_t));
 	memset(best_clock, 0, sizeof(*best_clock));
 
 	for (clock.m = limit->m.min; clock.m <= limit->m.max; clock.m++) {
-- 
2.1.4

[toc] | [next] | [standalone]

#1467202

FromJoe Perches <joe@perches.com>
Date2016-08-21 20:50 +0200
Message-ID<s8FYJ-3WX-19@gated-at.bofh.it>
In reply to#1467201
On Sun, 2016-08-21 at 20:39 +0200, Heinrich Schuchardt wrote:
> Components m1, m2, p2, dot, vco of variable clock should be
> initialized to avoid bytes from the kernel stack to be
> exposed.

How was this found? visual code inspection?

And isn't this true for mrst_lvds_find_best_pll as well?

> a@@ -138,6 +138,7 @@ static bool mrst_sdvo_find_best_pll(const struct gma_limit_t *limit,
>  	u32 target_vco, actual_freq;
>  	s32 freq_error, min_error = 100000;
>  
> +	memset(clock, 0, sizeof(struct gma_clock_t));
>  	memset(best_clock, 0, sizeof(*best_clock));
>  
>  	for (clock.m = limit->m.min; clock.m <= limit->m.max; clock.m++) {

[toc] | [prev] | [next] | [standalone]

#1467216

FromHeinrich Schuchardt <xypron.glpk@gmx.de>
Date2016-08-21 21:40 +0200
Message-ID<s8GL8-4yb-3@gated-at.bofh.it>
In reply to#1467202
On 08/21/2016 08:46 PM, Joe Perches wrote:
> On Sun, 2016-08-21 at 20:39 +0200, Heinrich Schuchardt wrote:
>> Components m1, m2, p2, dot, vco of variable clock should be
>> initialized to avoid bytes from the kernel stack to be
>> exposed.
> 
> How was this found? visual code inspection?

cppcheck (http://cppcheck.sourceforge.net/)

[toc] | [prev] | [next] | [standalone]

#1467217

FromJoe Perches <joe@perches.com>
Date2016-08-21 21:40 +0200
Message-ID<s8GL8-4yb-29@gated-at.bofh.it>
In reply to#1467216
On Sun, 2016-08-21 at 21:35 +0200, Heinrich Schuchardt wrote:
> On 08/21/2016 08:46 PM, Joe Perches wrote:
> > On Sun, 2016-08-21 at 20:39 +0200, Heinrich Schuchardt wrote:
> > > Components m1, m2, p2, dot, vco of variable clock should be
> > > initialized to avoid bytes from the kernel stack to be
> > > exposed.
> > How was this found? visual code inspection?
> cppcheck (http://cppcheck.sourceforge.net/)

This should probably be mentioned in the changelog.

What about mrst_lvds_find_best_pll ?

[toc] | [prev] | [next] | [standalone]

#1467425

FromDaniel Vetter <daniel@ffwll.ch>
Date2016-08-22 09:40 +0200
Message-ID<s8RZU-3kt-23@gated-at.bofh.it>
In reply to#1467201
On Sun, Aug 21, 2016 at 08:39:38PM +0200, Heinrich Schuchardt wrote:
> Components m1, m2, p2, dot, vco of variable clock should be
> initialized to avoid bytes from the kernel stack to be
> exposed.
> 
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

Might be a silly question, but where exactly would we expose these bytes?
This isn't directly called by an ioctl, I have no idea how those bytes
might get to userspace ...
-Daniel
> ---
>  drivers/gpu/drm/gma500/oaktrail_crtc.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/gma500/oaktrail_crtc.c b/drivers/gpu/drm/gma500/oaktrail_crtc.c
> index da9fd34..28bd8f3 100644
> --- a/drivers/gpu/drm/gma500/oaktrail_crtc.c
> +++ b/drivers/gpu/drm/gma500/oaktrail_crtc.c
> @@ -138,6 +138,7 @@ static bool mrst_sdvo_find_best_pll(const struct gma_limit_t *limit,
>  	u32 target_vco, actual_freq;
>  	s32 freq_error, min_error = 100000;
>  
> +	memset(clock, 0, sizeof(struct gma_clock_t));
>  	memset(best_clock, 0, sizeof(*best_clock));
>  
>  	for (clock.m = limit->m.min; clock.m <= limit->m.max; clock.m++) {
> -- 
> 2.1.4
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[toc] | [prev] | [next] | [standalone]

#1467492

FromOne Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk>
Date2016-08-22 11:20 +0200
Message-ID<s8TyF-4oi-7@gated-at.bofh.it>
In reply to#1467425
On Mon, 22 Aug 2016 09:29:17 +0200
Daniel Vetter <daniel@ffwll.ch> wrote:

> On Sun, Aug 21, 2016 at 08:39:38PM +0200, Heinrich Schuchardt wrote:
> > Components m1, m2, p2, dot, vco of variable clock should be
> > initialized to avoid bytes from the kernel stack to be
> > exposed.
> > 
> > Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>  
> 
> Might be a silly question, but where exactly would we expose these bytes?
> This isn't directly called by an ioctl, I have no idea how those bytes
> might get to userspace ...

mrst_print_pll displays clock.p2 - which is indeed never cleared 8)

Alan

[toc] | [prev] | [next] | [standalone]

#1467464

FromJani Nikula <jani.nikula@linux.intel.com>
Date2016-08-22 10:40 +0200
Message-ID<s8SVY-3V6-29@gated-at.bofh.it>
In reply to#1467201
On Sun, 21 Aug 2016, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> Components m1, m2, p2, dot, vco of variable clock should be
> initialized to avoid bytes from the kernel stack to be
> exposed.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  drivers/gpu/drm/gma500/oaktrail_crtc.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/gma500/oaktrail_crtc.c b/drivers/gpu/drm/gma500/oaktrail_crtc.c
> index da9fd34..28bd8f3 100644
> --- a/drivers/gpu/drm/gma500/oaktrail_crtc.c
> +++ b/drivers/gpu/drm/gma500/oaktrail_crtc.c
> @@ -138,6 +138,7 @@ static bool mrst_sdvo_find_best_pll(const struct gma_limit_t *limit,
>  	u32 target_vco, actual_freq;
>  	s32 freq_error, min_error = 100000;
>  
> +	memset(clock, 0, sizeof(struct gma_clock_t));

Did you build this? Did you run this?

BR,
Jani.


>  	memset(best_clock, 0, sizeof(*best_clock));
>  
>  	for (clock.m = limit->m.min; clock.m <= limit->m.max; clock.m++) {

-- 
Jani Nikula, Intel Open Source Technology Center

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web