re: drm/nvd0/disp: initial crtc object implementation

re: drm/nvd0/disp: initial crtc object implementation

[Dan Carpenter]

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

Re: drm/nvd0/disp: initial crtc object implementation

[Ben Skeggs]

----- 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
> 
> 

[patch] drm/nv50/disp: min/max are reversed in nv50_crtc_gamma_set()

[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++) {

[patch] drm/nv50/disp: min/max are reversed in nv50_crtc_gamma_set()

[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++) {