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
----- 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
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++) {
dri-devel@lists.freedesktop.org
-
Ben Skeggs -
Dan Carpenter