* re: drm/nvd0/disp: initial crtc object implementation
@ 2013-11-26 21:30 Dan Carpenter
2013-11-26 23:22 ` Ben Skeggs
0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2013-11-26 21:30 UTC (permalink / raw)
To: bskeggs; +Cc: dri-devel
Hello Ben Skeggs,
The patch 438d99e3b175: "drm/nvd0/disp: initial crtc object
implementation" from Jul 5, 2011, leads to the following
static checker warning: "drivers/gpu/drm/nouveau/nv50_display.c:1272
nv50_crtc_gamma_set()
error: buffer overflow 'nv_crtc->lut.r' 256 <= 256"
drivers/gpu/drm/nouveau/nv50_display.c
1263 static void
1264 nv50_crtc_gamma_set(struct drm_crtc *crtc, u16 *r, u16 *g, u16 *b,
1265 uint32_t start, uint32_t size)
1266 {
1267 struct nouveau_crtc *nv_crtc = nouveau_crtc(crtc);
1268 u32 end = max(start + size, (u32)256);
1269 u32 i;
1270
1271 for (i = start; i < end; i++) {
1272 nv_crtc->lut.r[i] = r[i];
^^^^^^^^
These arrays have 256 elements so going beyond seems like a bug. Should
the end = max() be a min() or something?
1273 nv_crtc->lut.g[i] = g[i];
1274 nv_crtc->lut.b[i] = b[i];
1275 }
1276
1277 nv50_crtc_lut_load(crtc);
1278 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: drm/nvd0/disp: initial crtc object implementation
2013-11-26 21:30 drm/nvd0/disp: initial crtc object implementation Dan Carpenter
@ 2013-11-26 23:22 ` Ben Skeggs
2013-11-27 22:18 ` Dan Carpenter
0 siblings, 1 reply; 4+ messages in thread
From: Ben Skeggs @ 2013-11-26 23:22 UTC (permalink / raw)
To: Dan Carpenter; +Cc: dri-devel
----- Original Message -----
> From: "Dan Carpenter" <dan.carpenter@oracle.com>
> To: bskeggs@redhat.com
> Cc: dri-devel@lists.freedesktop.org
> Sent: Wednesday, 27 November, 2013 7:30:27 AM
> Subject: re: drm/nvd0/disp: initial crtc object implementation
>
> Hello Ben Skeggs,
>
> The patch 438d99e3b175: "drm/nvd0/disp: initial crtc object
> implementation" from Jul 5, 2011, leads to the following
> static checker warning: "drivers/gpu/drm/nouveau/nv50_display.c:1272
> nv50_crtc_gamma_set()
> error: buffer overflow 'nv_crtc->lut.r' 256 <= 256"
>
> drivers/gpu/drm/nouveau/nv50_display.c
> 1263 static void
> 1264 nv50_crtc_gamma_set(struct drm_crtc *crtc, u16 *r, u16 *g, u16 *b,
> 1265 uint32_t start, uint32_t size)
> 1266 {
> 1267 struct nouveau_crtc *nv_crtc = nouveau_crtc(crtc);
> 1268 u32 end = max(start + size, (u32)256);
> 1269 u32 i;
> 1270
> 1271 for (i = start; i < end; i++) {
> 1272 nv_crtc->lut.r[i] = r[i];
> ^^^^^^^^
> These arrays have 256 elements so going beyond seems like a bug. Should
> the end = max() be a min() or something?
Yes, should definitely be a min. Did you want to cook the patch or shall I?
Thanks,
Ben.
>
> 1273 nv_crtc->lut.g[i] = g[i];
> 1274 nv_crtc->lut.b[i] = b[i];
> 1275 }
> 1276
> 1277 nv50_crtc_lut_load(crtc);
> 1278 }
>
> regards,
> dan carpenter
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread* [patch] drm/nv50/disp: min/max are reversed in nv50_crtc_gamma_set()
2013-11-26 23:22 ` Ben Skeggs
@ 2013-11-27 22:18 ` Dan Carpenter
0 siblings, 0 replies; 4+ messages in thread
From: Dan Carpenter @ 2013-11-27 22:18 UTC (permalink / raw)
To: David Airlie; +Cc: kernel-janitors, Ben Skeggs, dri-devel
We should be taking the minimum here instead of the max. It could lead
to a buffer overflow.
Fixes: 438d99e3b175 ('drm/nvd0/disp: initial crtc object implementation')
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/drivers/gpu/drm/nouveau/nv50_display.c b/drivers/gpu/drm/nouveau/nv50_display.c
index f8e66c08b11a..4e384a2f99c3 100644
--- a/drivers/gpu/drm/nouveau/nv50_display.c
+++ b/drivers/gpu/drm/nouveau/nv50_display.c
@@ -1265,7 +1265,7 @@ nv50_crtc_gamma_set(struct drm_crtc *crtc, u16 *r, u16 *g, u16 *b,
uint32_t start, uint32_t size)
{
struct nouveau_crtc *nv_crtc = nouveau_crtc(crtc);
- u32 end = max(start + size, (u32)256);
+ u32 end = min_t(u32, start + size, 256);
u32 i;
for (i = start; i < end; i++) {
^ permalink raw reply related [flat|nested] 4+ messages in thread* [patch] drm/nv50/disp: min/max are reversed in nv50_crtc_gamma_set()
@ 2013-11-27 22:18 ` Dan Carpenter
0 siblings, 0 replies; 4+ messages in thread
From: Dan Carpenter @ 2013-11-27 22:18 UTC (permalink / raw)
To: David Airlie; +Cc: kernel-janitors, Ben Skeggs, dri-devel
We should be taking the minimum here instead of the max. It could lead
to a buffer overflow.
Fixes: 438d99e3b175 ('drm/nvd0/disp: initial crtc object implementation')
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/drivers/gpu/drm/nouveau/nv50_display.c b/drivers/gpu/drm/nouveau/nv50_display.c
index f8e66c08b11a..4e384a2f99c3 100644
--- a/drivers/gpu/drm/nouveau/nv50_display.c
+++ b/drivers/gpu/drm/nouveau/nv50_display.c
@@ -1265,7 +1265,7 @@ nv50_crtc_gamma_set(struct drm_crtc *crtc, u16 *r, u16 *g, u16 *b,
uint32_t start, uint32_t size)
{
struct nouveau_crtc *nv_crtc = nouveau_crtc(crtc);
- u32 end = max(start + size, (u32)256);
+ u32 end = min_t(u32, start + size, 256);
u32 i;
for (i = start; i < end; i++) {
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-11-27 22:18 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-11-26 21:30 drm/nvd0/disp: initial crtc object implementation Dan Carpenter
2013-11-26 23:22 ` Ben Skeggs
2013-11-27 22:18 ` [patch] drm/nv50/disp: min/max are reversed in nv50_crtc_gamma_set() Dan Carpenter
2013-11-27 22:18 ` Dan Carpenter
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.