[PATCH] drm/dumb-buffers: Integer overflow in drm_mode_create_ioctl()

[PATCH] drm/dumb-buffers: Integer overflow in drm_mode_create_ioctl()

[Dan Carpenter]

There is a comment here which says that DIV_ROUND_UP() and that's where
the problem comes from.  Say you pick:

	args->bpp = UINT_MAX - 7;
	args->width = 4;
	args->height = 1;

The integer overflow in DIV_ROUND_UP() means "cpp" is UINT_MAX / 8 and
because of how we picked args->width that means cpp < UINT_MAX / 4.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
Btw, DIV_ROUND_UP() integer overflows have been a recurring source of
bugs so I have an unreleased static checker warning specific for that.
This line triggers three warnings for me on my unreleased code:

drivers/gpu/drm/drm_dumb_buffers.c:69 drm_mode_create_dumb_ioctl() warn: negative user subtract: 0-u32max - 1
drivers/gpu/drm/drm_dumb_buffers.c:69 drm_mode_create_dumb_ioctl() warn: potential integer overflow from user '(args->bpp) + (8)'
drivers/gpu/drm/drm_dumb_buffers.c:69 drm_mode_create_dumb_ioctl() warn: potential integer overflow in 'DIV_ROUND_UP'

It's a pretty common idiom in the kernel to overflow and then test for
it later so I'm not able to release this code because of the number of
false positives that this idiom causes...

diff --git a/drivers/gpu/drm/drm_dumb_buffers.c b/drivers/gpu/drm/drm_dumb_buffers.c
index 39ac15ce4702..45b0b5bbb5f8 100644
--- a/drivers/gpu/drm/drm_dumb_buffers.c
+++ b/drivers/gpu/drm/drm_dumb_buffers.c
@@ -65,7 +65,8 @@ int drm_mode_create_dumb_ioctl(struct drm_device *dev,
 		return -EINVAL;
 
 	/* overflow checks for 32bit size calculations */
-	/* NOTE: DIV_ROUND_UP() can overflow */
+	if (args->bpp > UINT_MAX - 8)
+		return -EINVAL;
 	cpp = DIV_ROUND_UP(args->bpp, 8);
 	if (!cpp || cpp > 0xffffffffU / args->width)
 		return -EINVAL;

[PATCH] drm/dumb-buffers: Integer overflow in drm_mode_create_ioctl()

[Dan Carpenter]

There is a comment here which says that DIV_ROUND_UP() and that's where
the problem comes from.  Say you pick:

	args->bpp = UINT_MAX - 7;
	args->width = 4;
	args->height = 1;

The integer overflow in DIV_ROUND_UP() means "cpp" is UINT_MAX / 8 and
because of how we picked args->width that means cpp < UINT_MAX / 4.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
Btw, DIV_ROUND_UP() integer overflows have been a recurring source of
bugs so I have an unreleased static checker warning specific for that.
This line triggers three warnings for me on my unreleased code:

drivers/gpu/drm/drm_dumb_buffers.c:69 drm_mode_create_dumb_ioctl() warn: negative user subtract: 0-u32max - 1
drivers/gpu/drm/drm_dumb_buffers.c:69 drm_mode_create_dumb_ioctl() warn: potential integer overflow from user '(args->bpp) + (8)'
drivers/gpu/drm/drm_dumb_buffers.c:69 drm_mode_create_dumb_ioctl() warn: potential integer overflow in 'DIV_ROUND_UP'

It's a pretty common idiom in the kernel to overflow and then test for
it later so I'm not able to release this code because of the number of
false positives that this idiom causes...

diff --git a/drivers/gpu/drm/drm_dumb_buffers.c b/drivers/gpu/drm/drm_dumb_buffers.c
index 39ac15ce4702..45b0b5bbb5f8 100644
--- a/drivers/gpu/drm/drm_dumb_buffers.c
+++ b/drivers/gpu/drm/drm_dumb_buffers.c
@@ -65,7 +65,8 @@ int drm_mode_create_dumb_ioctl(struct drm_device *dev,
 		return -EINVAL;
 
 	/* overflow checks for 32bit size calculations */
-	/* NOTE: DIV_ROUND_UP() can overflow */
+	if (args->bpp > UINT_MAX - 8)
+		return -EINVAL;
 	cpp = DIV_ROUND_UP(args->bpp, 8);
 	if (!cpp || cpp > 0xffffffffU / args->width)
 		return -EINVAL;
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2] drm/dumb-buffers: Integer overflow in drm_mode_create_ioctl()

[Dan Carpenter]

There is a comment here which says that DIV_ROUND_UP() can overflow and
that's where the problem comes from.  Say you pick:

	args->bpp = UINT_MAX - 7;
	args->width = 4;
	args->height = 1;

The integer overflow in DIV_ROUND_UP() means "cpp" is UINT_MAX / 8 and
because of how we picked args->width that means cpp < UINT_MAX / 4.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
v2: correct a typo in the commit message

diff --git a/drivers/gpu/drm/drm_dumb_buffers.c b/drivers/gpu/drm/drm_dumb_buffers.c
index 39ac15ce4702..45b0b5bbb5f8 100644
--- a/drivers/gpu/drm/drm_dumb_buffers.c
+++ b/drivers/gpu/drm/drm_dumb_buffers.c
@@ -65,7 +65,8 @@ int drm_mode_create_dumb_ioctl(struct drm_device *dev,
 		return -EINVAL;
 
 	/* overflow checks for 32bit size calculations */
-	/* NOTE: DIV_ROUND_UP() can overflow */
+	if (args->bpp > UINT_MAX - 8)
+		return -EINVAL;
 	cpp = DIV_ROUND_UP(args->bpp, 8);
 	if (!cpp || cpp > 0xffffffffU / args->width)
 		return -EINVAL;

[PATCH v2] drm/dumb-buffers: Integer overflow in drm_mode_create_ioctl()

[Dan Carpenter]

There is a comment here which says that DIV_ROUND_UP() can overflow and
that's where the problem comes from.  Say you pick:

	args->bpp = UINT_MAX - 7;
	args->width = 4;
	args->height = 1;

The integer overflow in DIV_ROUND_UP() means "cpp" is UINT_MAX / 8 and
because of how we picked args->width that means cpp < UINT_MAX / 4.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
v2: correct a typo in the commit message

diff --git a/drivers/gpu/drm/drm_dumb_buffers.c b/drivers/gpu/drm/drm_dumb_buffers.c
index 39ac15ce4702..45b0b5bbb5f8 100644
--- a/drivers/gpu/drm/drm_dumb_buffers.c
+++ b/drivers/gpu/drm/drm_dumb_buffers.c
@@ -65,7 +65,8 @@ int drm_mode_create_dumb_ioctl(struct drm_device *dev,
 		return -EINVAL;
 
 	/* overflow checks for 32bit size calculations */
-	/* NOTE: DIV_ROUND_UP() can overflow */
+	if (args->bpp > UINT_MAX - 8)
+		return -EINVAL;
 	cpp = DIV_ROUND_UP(args->bpp, 8);
 	if (!cpp || cpp > 0xffffffffU / args->width)
 		return -EINVAL;
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2] drm/dumb-buffers: Integer overflow in drm_mode_create_ioctl()

[Chris Wilson]

Quoting Dan Carpenter (2018-05-09 09:12:54)
> There is a comment here which says that DIV_ROUND_UP() can overflow and
> that's where the problem comes from.  Say you pick:
> 
>         args->bpp = UINT_MAX - 7;
>         args->width = 4;
>         args->height = 1;
> 
> The integer overflow in DIV_ROUND_UP() means "cpp" is UINT_MAX / 8 and
> because of how we picked args->width that means cpp < UINT_MAX / 4.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> v2: correct a typo in the commit message
> 
> diff --git a/drivers/gpu/drm/drm_dumb_buffers.c b/drivers/gpu/drm/drm_dumb_buffers.c
> index 39ac15ce4702..45b0b5bbb5f8 100644
> --- a/drivers/gpu/drm/drm_dumb_buffers.c
> +++ b/drivers/gpu/drm/drm_dumb_buffers.c
> @@ -65,7 +65,8 @@ int drm_mode_create_dumb_ioctl(struct drm_device *dev,
>                 return -EINVAL;
>  
>         /* overflow checks for 32bit size calculations */
> -       /* NOTE: DIV_ROUND_UP() can overflow */
> +       if (args->bpp > UINT_MAX - 8)
> +               return -EINVAL;

#define __KERNEL_DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d)

both args->bpp and cpp are u32, so should we use U32_MAX to be typesafe?

>         cpp = DIV_ROUND_UP(args->bpp, 8);
>         if (!cpp || cpp > 0xffffffffU / args->width)

And these constants should also be U32_MAX?
-Chris

Re: [PATCH v2] drm/dumb-buffers: Integer overflow in drm_mode_create_ioctl()

[Chris Wilson]

Quoting Dan Carpenter (2018-05-09 09:12:54)
> There is a comment here which says that DIV_ROUND_UP() can overflow and
> that's where the problem comes from.  Say you pick:
> 
>         args->bpp = UINT_MAX - 7;
>         args->width = 4;
>         args->height = 1;
> 
> The integer overflow in DIV_ROUND_UP() means "cpp" is UINT_MAX / 8 and
> because of how we picked args->width that means cpp < UINT_MAX / 4.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> v2: correct a typo in the commit message
> 
> diff --git a/drivers/gpu/drm/drm_dumb_buffers.c b/drivers/gpu/drm/drm_dumb_buffers.c
> index 39ac15ce4702..45b0b5bbb5f8 100644
> --- a/drivers/gpu/drm/drm_dumb_buffers.c
> +++ b/drivers/gpu/drm/drm_dumb_buffers.c
> @@ -65,7 +65,8 @@ int drm_mode_create_dumb_ioctl(struct drm_device *dev,
>                 return -EINVAL;
>  
>         /* overflow checks for 32bit size calculations */
> -       /* NOTE: DIV_ROUND_UP() can overflow */
> +       if (args->bpp > UINT_MAX - 8)
> +               return -EINVAL;

#define __KERNEL_DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d)

both args->bpp and cpp are u32, so should we use U32_MAX to be typesafe?

>         cpp = DIV_ROUND_UP(args->bpp, 8);
>         if (!cpp || cpp > 0xffffffffU / args->width)

And these constants should also be U32_MAX?
-Chris
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2] drm/dumb-buffers: Integer overflow in drm_mode_create_ioctl()

[Dan Carpenter]

On Wed, May 09, 2018 at 09:18:57AM +0100, Chris Wilson wrote:
> Quoting Dan Carpenter (2018-05-09 09:12:54)
> > There is a comment here which says that DIV_ROUND_UP() can overflow and
> > that's where the problem comes from.  Say you pick:
> > 
> >         args->bpp = UINT_MAX - 7;
> >         args->width = 4;
> >         args->height = 1;
> > 
> > The integer overflow in DIV_ROUND_UP() means "cpp" is UINT_MAX / 8 and
> > because of how we picked args->width that means cpp < UINT_MAX / 4.
> > 
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > ---
> > v2: correct a typo in the commit message
> > 
> > diff --git a/drivers/gpu/drm/drm_dumb_buffers.c b/drivers/gpu/drm/drm_dumb_buffers.c
> > index 39ac15ce4702..45b0b5bbb5f8 100644
> > --- a/drivers/gpu/drm/drm_dumb_buffers.c
> > +++ b/drivers/gpu/drm/drm_dumb_buffers.c
> > @@ -65,7 +65,8 @@ int drm_mode_create_dumb_ioctl(struct drm_device *dev,
> >                 return -EINVAL;
> >  
> >         /* overflow checks for 32bit size calculations */
> > -       /* NOTE: DIV_ROUND_UP() can overflow */
> > +       if (args->bpp > UINT_MAX - 8)
> > +               return -EINVAL;
> 
> #define __KERNEL_DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d)
> 
> both args->bpp and cpp are u32, so should we use U32_MAX to be typesafe?
> 

They will always be equivalent so it's just a style issue...  I don't
have a strong preference so I can use U32_MAX if that's prefered.

> >         cpp = DIV_ROUND_UP(args->bpp, 8);
> >         if (!cpp || cpp > 0xffffffffU / args->width)
> 
> And these constants should also be U32_MAX?

Yeah.  I considered changing those but one never knows how much style to
clean up...  I'll redo in the respin.

regards,
dan carpenter

Re: [PATCH v2] drm/dumb-buffers: Integer overflow in drm_mode_create_ioctl()

[Dan Carpenter]

On Wed, May 09, 2018 at 09:18:57AM +0100, Chris Wilson wrote:
> Quoting Dan Carpenter (2018-05-09 09:12:54)
> > There is a comment here which says that DIV_ROUND_UP() can overflow and
> > that's where the problem comes from.  Say you pick:
> > 
> >         args->bpp = UINT_MAX - 7;
> >         args->width = 4;
> >         args->height = 1;
> > 
> > The integer overflow in DIV_ROUND_UP() means "cpp" is UINT_MAX / 8 and
> > because of how we picked args->width that means cpp < UINT_MAX / 4.
> > 
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > ---
> > v2: correct a typo in the commit message
> > 
> > diff --git a/drivers/gpu/drm/drm_dumb_buffers.c b/drivers/gpu/drm/drm_dumb_buffers.c
> > index 39ac15ce4702..45b0b5bbb5f8 100644
> > --- a/drivers/gpu/drm/drm_dumb_buffers.c
> > +++ b/drivers/gpu/drm/drm_dumb_buffers.c
> > @@ -65,7 +65,8 @@ int drm_mode_create_dumb_ioctl(struct drm_device *dev,
> >                 return -EINVAL;
> >  
> >         /* overflow checks for 32bit size calculations */
> > -       /* NOTE: DIV_ROUND_UP() can overflow */
> > +       if (args->bpp > UINT_MAX - 8)
> > +               return -EINVAL;
> 
> #define __KERNEL_DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d)
> 
> both args->bpp and cpp are u32, so should we use U32_MAX to be typesafe?
> 

They will always be equivalent so it's just a style issue...  I don't
have a strong preference so I can use U32_MAX if that's prefered.

> >         cpp = DIV_ROUND_UP(args->bpp, 8);
> >         if (!cpp || cpp > 0xffffffffU / args->width)
> 
> And these constants should also be U32_MAX?

Yeah.  I considered changing those but one never knows how much style to
clean up...  I'll redo in the respin.

regards,
dan carpenter

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2] drm/dumb-buffers: Integer overflow in drm_mode_create_ioctl()

[Dan Carpenter]

There is a comment here which says that DIV_ROUND_UP() and that's where
the problem comes from.  Say you pick:

	args->bpp = UINT_MAX - 7;
	args->width = 4;
	args->height = 1;

The integer overflow in DIV_ROUND_UP() means "cpp" is UINT_MAX / 8 and
because of how we picked args->width that means cpp < UINT_MAX / 4.

I've fixed it by preventing the integer overflow in DIV_ROUND_UP().  I
removed the check for !cpp because it's not possible after this change.
I also changed all the 0xffffffffU references to U32_MAX.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
v2:  additional cleanups

diff --git a/drivers/gpu/drm/drm_dumb_buffers.c b/drivers/gpu/drm/drm_dumb_buffers.c
index 39ac15ce4702..9e2ae02f31e0 100644
--- a/drivers/gpu/drm/drm_dumb_buffers.c
+++ b/drivers/gpu/drm/drm_dumb_buffers.c
@@ -65,12 +65,13 @@ int drm_mode_create_dumb_ioctl(struct drm_device *dev,
 		return -EINVAL;
 
 	/* overflow checks for 32bit size calculations */
-	/* NOTE: DIV_ROUND_UP() can overflow */
+	if (args->bpp > U32_MAX - 8)
+		return -EINVAL;
 	cpp = DIV_ROUND_UP(args->bpp, 8);
-	if (!cpp || cpp > 0xffffffffU / args->width)
+	if (cpp > U32_MAX / args->width)
 		return -EINVAL;
 	stride = cpp * args->width;
-	if (args->height > 0xffffffffU / stride)
+	if (args->height > U32_MAX / stride)
 		return -EINVAL;
 
 	/* test for wrap-around */

[PATCH v2] drm/dumb-buffers: Integer overflow in drm_mode_create_ioctl()

[Dan Carpenter]

There is a comment here which says that DIV_ROUND_UP() and that's where
the problem comes from.  Say you pick:

	args->bpp = UINT_MAX - 7;
	args->width = 4;
	args->height = 1;

The integer overflow in DIV_ROUND_UP() means "cpp" is UINT_MAX / 8 and
because of how we picked args->width that means cpp < UINT_MAX / 4.

I've fixed it by preventing the integer overflow in DIV_ROUND_UP().  I
removed the check for !cpp because it's not possible after this change.
I also changed all the 0xffffffffU references to U32_MAX.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
v2:  additional cleanups

diff --git a/drivers/gpu/drm/drm_dumb_buffers.c b/drivers/gpu/drm/drm_dumb_buffers.c
index 39ac15ce4702..9e2ae02f31e0 100644
--- a/drivers/gpu/drm/drm_dumb_buffers.c
+++ b/drivers/gpu/drm/drm_dumb_buffers.c
@@ -65,12 +65,13 @@ int drm_mode_create_dumb_ioctl(struct drm_device *dev,
 		return -EINVAL;
 
 	/* overflow checks for 32bit size calculations */
-	/* NOTE: DIV_ROUND_UP() can overflow */
+	if (args->bpp > U32_MAX - 8)
+		return -EINVAL;
 	cpp = DIV_ROUND_UP(args->bpp, 8);
-	if (!cpp || cpp > 0xffffffffU / args->width)
+	if (cpp > U32_MAX / args->width)
 		return -EINVAL;
 	stride = cpp * args->width;
-	if (args->height > 0xffffffffU / stride)
+	if (args->height > U32_MAX / stride)
 		return -EINVAL;
 
 	/* test for wrap-around */
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2] drm/dumb-buffers: Integer overflow in drm_mode_create_ioctl()

[Chris Wilson]

Quoting Dan Carpenter (2018-05-16 15:00:26)
> There is a comment here which says that DIV_ROUND_UP() and that's where
> the problem comes from.  Say you pick:
> 
>         args->bpp = UINT_MAX - 7;
>         args->width = 4;
>         args->height = 1;
> 
> The integer overflow in DIV_ROUND_UP() means "cpp" is UINT_MAX / 8 and
> because of how we picked args->width that means cpp < UINT_MAX / 4.
> 
> I've fixed it by preventing the integer overflow in DIV_ROUND_UP().  I
> removed the check for !cpp because it's not possible after this change.
> I also changed all the 0xffffffffU references to U32_MAX.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

I agree with Daniel that the !cpp check after DIV_ROUND_UP was
sufficient to guard the current code, but switching to a more idiomatic
style of overflow checking has its benefits too. Plus I like having
U32_MAX to compare the type ranges against.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

Re: [PATCH v2] drm/dumb-buffers: Integer overflow in drm_mode_create_ioctl()

[Chris Wilson]

Quoting Dan Carpenter (2018-05-16 15:00:26)
> There is a comment here which says that DIV_ROUND_UP() and that's where
> the problem comes from.  Say you pick:
> 
>         args->bpp = UINT_MAX - 7;
>         args->width = 4;
>         args->height = 1;
> 
> The integer overflow in DIV_ROUND_UP() means "cpp" is UINT_MAX / 8 and
> because of how we picked args->width that means cpp < UINT_MAX / 4.
> 
> I've fixed it by preventing the integer overflow in DIV_ROUND_UP().  I
> removed the check for !cpp because it's not possible after this change.
> I also changed all the 0xffffffffU references to U32_MAX.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

I agree with Daniel that the !cpp check after DIV_ROUND_UP was
sufficient to guard the current code, but switching to a more idiomatic
style of overflow checking has its benefits too. Plus I like having
U32_MAX to compare the type ranges against.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2] drm/dumb-buffers: Integer overflow in drm_mode_create_ioctl()

[Dan Carpenter]

On Wed, May 16, 2018 at 03:26:07PM +0100, Chris Wilson wrote:
> Quoting Dan Carpenter (2018-05-16 15:00:26)
> > There is a comment here which says that DIV_ROUND_UP() and that's where
> > the problem comes from.  Say you pick:
> > 
> >         args->bpp = UINT_MAX - 7;
> >         args->width = 4;
> >         args->height = 1;
> > 
> > The integer overflow in DIV_ROUND_UP() means "cpp" is UINT_MAX / 8 and
> > because of how we picked args->width that means cpp < UINT_MAX / 4.
> > 
> > I've fixed it by preventing the integer overflow in DIV_ROUND_UP().  I
> > removed the check for !cpp because it's not possible after this change.
> > I also changed all the 0xffffffffU references to U32_MAX.
> > 
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> I agree with Daniel that the !cpp check after DIV_ROUND_UP was
> sufficient to guard the current code, but switching to a more idiomatic
> style of overflow checking has its benefits too. Plus I like having
> U32_MAX to compare the type ranges against.
> 

I'm not totally sure what it means to say that the integer overflow is
sufficient.  The overflow check is definitely buggy but if you mean that
it isn't exploitable then you're probably right.  Anyway, let's say you
use use the values I provided in my changelog.  Then I believe we can
reach vgem_gem_dumb_create():

static int vgem_gem_dumb_create(struct drm_file *file, struct drm_device *dev,
   205                                  struct drm_mode_create_dumb *args)
   206  {
   207          struct drm_gem_object *gem_object;
   208          u64 pitch, size;
   209  
   210          pitch = args->width * DIV_ROUND_UP(args->bpp, 8);
                                      ^^^^^^^^^^^^^^^^^^^^^^^^^^
This is now:	pitch = 4           * (U32_MAX / 8);

I would get a static checker warning here because of the integer
overflow in DIV_ROUND_UP().  I'll push that check soon.

   211          size = args->height * pitch;

This is now:	size = 1 * U32_MAX / 2;

   212          if (size = 0)
   213                  return -EINVAL;
   214  
   215          gem_object = vgem_gem_create(dev, file, &args->handle, size);

Then this fails because we can't allocate U32_MAX / 2 bytes.  There
are some other potential numbers we could try instead of the ones I
gave.  On 32bit arches  __vgem_gem_create() has integer overflows if
size is over U32_MAX - PAGE_SIZE so it gets a bit complicated...

   216          if (IS_ERR(gem_object))
   217                  return PTR_ERR(gem_object);
   218  
   219          args->size = gem_object->size;
   220          args->pitch = pitch;
   221  
   222          DRM_DEBUG_DRIVER("Created object of size %lld\n", size);
   223  
   224          return 0;
   225  }

regards,
dan carpenter

Re: [PATCH v2] drm/dumb-buffers: Integer overflow in drm_mode_create_ioctl()

[Dan Carpenter]

On Wed, May 16, 2018 at 03:26:07PM +0100, Chris Wilson wrote:
> Quoting Dan Carpenter (2018-05-16 15:00:26)
> > There is a comment here which says that DIV_ROUND_UP() and that's where
> > the problem comes from.  Say you pick:
> > 
> >         args->bpp = UINT_MAX - 7;
> >         args->width = 4;
> >         args->height = 1;
> > 
> > The integer overflow in DIV_ROUND_UP() means "cpp" is UINT_MAX / 8 and
> > because of how we picked args->width that means cpp < UINT_MAX / 4.
> > 
> > I've fixed it by preventing the integer overflow in DIV_ROUND_UP().  I
> > removed the check for !cpp because it's not possible after this change.
> > I also changed all the 0xffffffffU references to U32_MAX.
> > 
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> I agree with Daniel that the !cpp check after DIV_ROUND_UP was
> sufficient to guard the current code, but switching to a more idiomatic
> style of overflow checking has its benefits too. Plus I like having
> U32_MAX to compare the type ranges against.
> 

I'm not totally sure what it means to say that the integer overflow is
sufficient.  The overflow check is definitely buggy but if you mean that
it isn't exploitable then you're probably right.  Anyway, let's say you
use use the values I provided in my changelog.  Then I believe we can
reach vgem_gem_dumb_create():

static int vgem_gem_dumb_create(struct drm_file *file, struct drm_device *dev,
   205                                  struct drm_mode_create_dumb *args)
   206  {
   207          struct drm_gem_object *gem_object;
   208          u64 pitch, size;
   209  
   210          pitch = args->width * DIV_ROUND_UP(args->bpp, 8);
                                      ^^^^^^^^^^^^^^^^^^^^^^^^^^
This is now:	pitch = 4           * (U32_MAX / 8);

I would get a static checker warning here because of the integer
overflow in DIV_ROUND_UP().  I'll push that check soon.

   211          size = args->height * pitch;

This is now:	size = 1 * U32_MAX / 2;

   212          if (size == 0)
   213                  return -EINVAL;
   214  
   215          gem_object = vgem_gem_create(dev, file, &args->handle, size);

Then this fails because we can't allocate U32_MAX / 2 bytes.  There
are some other potential numbers we could try instead of the ones I
gave.  On 32bit arches  __vgem_gem_create() has integer overflows if
size is over U32_MAX - PAGE_SIZE so it gets a bit complicated...

   216          if (IS_ERR(gem_object))
   217                  return PTR_ERR(gem_object);
   218  
   219          args->size = gem_object->size;
   220          args->pitch = pitch;
   221  
   222          DRM_DEBUG_DRIVER("Created object of size %lld\n", size);
   223  
   224          return 0;
   225  }

regards,
dan carpenter
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2] drm/dumb-buffers: Integer overflow in drm_mode_create_ioctl()

[Chris Wilson]

Quoting Dan Carpenter (2018-05-16 15:52:57)
> On Wed, May 16, 2018 at 03:26:07PM +0100, Chris Wilson wrote:
> > Quoting Dan Carpenter (2018-05-16 15:00:26)
> > > There is a comment here which says that DIV_ROUND_UP() and that's where
> > > the problem comes from.  Say you pick:
> > > 
> > >         args->bpp = UINT_MAX - 7;
> > >         args->width = 4;
> > >         args->height = 1;
> > > 
> > > The integer overflow in DIV_ROUND_UP() means "cpp" is UINT_MAX / 8 and
> > > because of how we picked args->width that means cpp < UINT_MAX / 4.
> > > 
> > > I've fixed it by preventing the integer overflow in DIV_ROUND_UP().  I
> > > removed the check for !cpp because it's not possible after this change.
> > > I also changed all the 0xffffffffU references to U32_MAX.
> > > 
> > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > 
> > I agree with Daniel that the !cpp check after DIV_ROUND_UP was
> > sufficient to guard the current code, but switching to a more idiomatic
> > style of overflow checking has its benefits too. Plus I like having
> > U32_MAX to compare the type ranges against.
> > 
> 
> I'm not totally sure what it means to say that the integer overflow is
> sufficient.  The overflow check is definitely buggy but if you mean that
> it isn't exploitable then you're probably right.  Anyway, let's say you
> use use the values I provided in my changelog.  Then I believe we can
> reach vgem_gem_dumb_create():

But we are talking about

	cpp = (U32_MAX + 8) / 8

which is 0. So the !cpp does catch the overflow.

Or am I completely off in unsigned wraparound?
-Chris
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2] drm/dumb-buffers: Integer overflow in drm_mode_create_ioctl()

[Dan Carpenter]

On Wed, May 16, 2018 at 03:56:55PM +0100, Chris Wilson wrote:
> Quoting Dan Carpenter (2018-05-16 15:52:57)
> > On Wed, May 16, 2018 at 03:26:07PM +0100, Chris Wilson wrote:
> > > Quoting Dan Carpenter (2018-05-16 15:00:26)
> > > > There is a comment here which says that DIV_ROUND_UP() and that's where
> > > > the problem comes from.  Say you pick:
> > > > 
> > > >         args->bpp = UINT_MAX - 7;
> > > >         args->width = 4;
> > > >         args->height = 1;
> > > > 
> > > > The integer overflow in DIV_ROUND_UP() means "cpp" is UINT_MAX / 8 and
> > > > because of how we picked args->width that means cpp < UINT_MAX / 4.
> > > > 
> > > > I've fixed it by preventing the integer overflow in DIV_ROUND_UP().  I
> > > > removed the check for !cpp because it's not possible after this change.
> > > > I also changed all the 0xffffffffU references to U32_MAX.
> > > > 
> > > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > > 
> > > I agree with Daniel that the !cpp check after DIV_ROUND_UP was
> > > sufficient to guard the current code, but switching to a more idiomatic
> > > style of overflow checking has its benefits too. Plus I like having
> > > U32_MAX to compare the type ranges against.
> > > 
> > 
> > I'm not totally sure what it means to say that the integer overflow is
> > sufficient.  The overflow check is definitely buggy but if you mean that
> > it isn't exploitable then you're probably right.  Anyway, let's say you
> > use use the values I provided in my changelog.  Then I believe we can
> > reach vgem_gem_dumb_create():
> 
> But we are talking about
> 
> 	cpp = (U32_MAX + 8) / 8
> 

No, no.  That's not the problem.  The problem is the - 1 subtraction.  I
explained poorly.  The issue is when "bpp" is U32_MAX - 7.  The define
looks like:

#define __KERNEL_DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d))

So "n" is (U32_MAX - 7) and "d" is 8.  "(n) + (d)" wraps to zero.  We do
the "- 1" subtraction so that gives us U32_MAX.  And finally we divide
by "d" to get "U32_MAX / 8".

regards,
dan carpenter

Re: [PATCH v2] drm/dumb-buffers: Integer overflow in drm_mode_create_ioctl()

[Dan Carpenter]

On Wed, May 16, 2018 at 03:56:55PM +0100, Chris Wilson wrote:
> Quoting Dan Carpenter (2018-05-16 15:52:57)
> > On Wed, May 16, 2018 at 03:26:07PM +0100, Chris Wilson wrote:
> > > Quoting Dan Carpenter (2018-05-16 15:00:26)
> > > > There is a comment here which says that DIV_ROUND_UP() and that's where
> > > > the problem comes from.  Say you pick:
> > > > 
> > > >         args->bpp = UINT_MAX - 7;
> > > >         args->width = 4;
> > > >         args->height = 1;
> > > > 
> > > > The integer overflow in DIV_ROUND_UP() means "cpp" is UINT_MAX / 8 and
> > > > because of how we picked args->width that means cpp < UINT_MAX / 4.
> > > > 
> > > > I've fixed it by preventing the integer overflow in DIV_ROUND_UP().  I
> > > > removed the check for !cpp because it's not possible after this change.
> > > > I also changed all the 0xffffffffU references to U32_MAX.
> > > > 
> > > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > > 
> > > I agree with Daniel that the !cpp check after DIV_ROUND_UP was
> > > sufficient to guard the current code, but switching to a more idiomatic
> > > style of overflow checking has its benefits too. Plus I like having
> > > U32_MAX to compare the type ranges against.
> > > 
> > 
> > I'm not totally sure what it means to say that the integer overflow is
> > sufficient.  The overflow check is definitely buggy but if you mean that
> > it isn't exploitable then you're probably right.  Anyway, let's say you
> > use use the values I provided in my changelog.  Then I believe we can
> > reach vgem_gem_dumb_create():
> 
> But we are talking about
> 
> 	cpp = (U32_MAX + 8) / 8
> 

No, no.  That's not the problem.  The problem is the - 1 subtraction.  I
explained poorly.  The issue is when "bpp" is U32_MAX - 7.  The define
looks like:

#define __KERNEL_DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d))

So "n" is (U32_MAX - 7) and "d" is 8.  "(n) + (d)" wraps to zero.  We do
the "- 1" subtraction so that gives us U32_MAX.  And finally we divide
by "d" to get "U32_MAX / 8".

regards,
dan carpenter
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2] drm/dumb-buffers: Integer overflow in drm_mode_create_ioctl()

[Chris Wilson]

Quoting Dan Carpenter (2018-05-16 16:15:54)
> On Wed, May 16, 2018 at 03:56:55PM +0100, Chris Wilson wrote:
> > Quoting Dan Carpenter (2018-05-16 15:52:57)
> > > On Wed, May 16, 2018 at 03:26:07PM +0100, Chris Wilson wrote:
> > > > Quoting Dan Carpenter (2018-05-16 15:00:26)
> > > > > There is a comment here which says that DIV_ROUND_UP() and that's where
> > > > > the problem comes from.  Say you pick:
> > > > > 
> > > > >         args->bpp = UINT_MAX - 7;
> > > > >         args->width = 4;
> > > > >         args->height = 1;
> > > > > 
> > > > > The integer overflow in DIV_ROUND_UP() means "cpp" is UINT_MAX / 8 and
> > > > > because of how we picked args->width that means cpp < UINT_MAX / 4.
> > > > > 
> > > > > I've fixed it by preventing the integer overflow in DIV_ROUND_UP().  I
> > > > > removed the check for !cpp because it's not possible after this change.
> > > > > I also changed all the 0xffffffffU references to U32_MAX.
> > > > > 
> > > > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > > > 
> > > > I agree with Daniel that the !cpp check after DIV_ROUND_UP was
> > > > sufficient to guard the current code, but switching to a more idiomatic
> > > > style of overflow checking has its benefits too. Plus I like having
> > > > U32_MAX to compare the type ranges against.
> > > > 
> > > 
> > > I'm not totally sure what it means to say that the integer overflow is
> > > sufficient.  The overflow check is definitely buggy but if you mean that
> > > it isn't exploitable then you're probably right.  Anyway, let's say you
> > > use use the values I provided in my changelog.  Then I believe we can
> > > reach vgem_gem_dumb_create():
> > 
> > But we are talking about
> > 
> >       cpp = (U32_MAX + 8) / 8
> > 
> 
> No, no.  That's not the problem.  The problem is the - 1 subtraction.  I
> explained poorly.  The issue is when "bpp" is U32_MAX - 7.  The define
> looks like:
> 
> #define __KERNEL_DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d))
> 
> So "n" is (U32_MAX - 7) and "d" is 8.  "(n) + (d)" wraps to zero.  We do
> the "- 1" subtraction so that gives us U32_MAX.  And finally we divide
> by "d" to get "U32_MAX / 8".

D'oh. Thanks for spelling it out.
-Chris
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2] drm/dumb-buffers: Integer overflow in drm_mode_create_ioctl()

[Dan Carpenter]

Btw, I've looked at this some more and I'm 99% sure there is no way to
exploit it.  The "if (PAGE_ALIGN(size) = 0)" prevents the integer
overflow in __vgem_gem_create() that I was worried about.

regards,
dan carpenter

Re: [PATCH v2] drm/dumb-buffers: Integer overflow in drm_mode_create_ioctl()

[Dan Carpenter]

Btw, I've looked at this some more and I'm 99% sure there is no way to
exploit it.  The "if (PAGE_ALIGN(size) == 0)" prevents the integer
overflow in __vgem_gem_create() that I was worried about.

regards,
dan carpenter


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2] drm/dumb-buffers: Integer overflow in drm_mode_create_ioctl()

[Daniel Vetter]

On Wed, May 16, 2018 at 05:00:26PM +0300, Dan Carpenter wrote:
> There is a comment here which says that DIV_ROUND_UP() and that's where
> the problem comes from.  Say you pick:
> 
> 	args->bpp = UINT_MAX - 7;
> 	args->width = 4;
> 	args->height = 1;
> 
> The integer overflow in DIV_ROUND_UP() means "cpp" is UINT_MAX / 8 and
> because of how we picked args->width that means cpp < UINT_MAX / 4.
> 
> I've fixed it by preventing the integer overflow in DIV_ROUND_UP().  I
> removed the check for !cpp because it's not possible after this change.
> I also changed all the 0xffffffffU references to U32_MAX.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> v2:  additional cleanups

Thanks a lot for respinning, applied to drm-misc-fixes.
-Daniel

> 
> diff --git a/drivers/gpu/drm/drm_dumb_buffers.c b/drivers/gpu/drm/drm_dumb_buffers.c
> index 39ac15ce4702..9e2ae02f31e0 100644
> --- a/drivers/gpu/drm/drm_dumb_buffers.c
> +++ b/drivers/gpu/drm/drm_dumb_buffers.c
> @@ -65,12 +65,13 @@ int drm_mode_create_dumb_ioctl(struct drm_device *dev,
>  		return -EINVAL;
>  
>  	/* overflow checks for 32bit size calculations */
> -	/* NOTE: DIV_ROUND_UP() can overflow */
> +	if (args->bpp > U32_MAX - 8)
> +		return -EINVAL;
>  	cpp = DIV_ROUND_UP(args->bpp, 8);
> -	if (!cpp || cpp > 0xffffffffU / args->width)
> +	if (cpp > U32_MAX / args->width)
>  		return -EINVAL;
>  	stride = cpp * args->width;
> -	if (args->height > 0xffffffffU / stride)
> +	if (args->height > U32_MAX / stride)
>  		return -EINVAL;
>  
>  	/* test for wrap-around */
> _______________________________________________
> 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

Re: [PATCH v2] drm/dumb-buffers: Integer overflow in drm_mode_create_ioctl()

[Daniel Vetter]

On Wed, May 16, 2018 at 05:00:26PM +0300, Dan Carpenter wrote:
> There is a comment here which says that DIV_ROUND_UP() and that's where
> the problem comes from.  Say you pick:
> 
> 	args->bpp = UINT_MAX - 7;
> 	args->width = 4;
> 	args->height = 1;
> 
> The integer overflow in DIV_ROUND_UP() means "cpp" is UINT_MAX / 8 and
> because of how we picked args->width that means cpp < UINT_MAX / 4.
> 
> I've fixed it by preventing the integer overflow in DIV_ROUND_UP().  I
> removed the check for !cpp because it's not possible after this change.
> I also changed all the 0xffffffffU references to U32_MAX.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> v2:  additional cleanups

Thanks a lot for respinning, applied to drm-misc-fixes.
-Daniel

> 
> diff --git a/drivers/gpu/drm/drm_dumb_buffers.c b/drivers/gpu/drm/drm_dumb_buffers.c
> index 39ac15ce4702..9e2ae02f31e0 100644
> --- a/drivers/gpu/drm/drm_dumb_buffers.c
> +++ b/drivers/gpu/drm/drm_dumb_buffers.c
> @@ -65,12 +65,13 @@ int drm_mode_create_dumb_ioctl(struct drm_device *dev,
>  		return -EINVAL;
>  
>  	/* overflow checks for 32bit size calculations */
> -	/* NOTE: DIV_ROUND_UP() can overflow */
> +	if (args->bpp > U32_MAX - 8)
> +		return -EINVAL;
>  	cpp = DIV_ROUND_UP(args->bpp, 8);
> -	if (!cpp || cpp > 0xffffffffU / args->width)
> +	if (cpp > U32_MAX / args->width)
>  		return -EINVAL;
>  	stride = cpp * args->width;
> -	if (args->height > 0xffffffffU / stride)
> +	if (args->height > U32_MAX / stride)
>  		return -EINVAL;
>  
>  	/* test for wrap-around */
> _______________________________________________
> 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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/dumb-buffers: Integer overflow in drm_mode_create_ioctl()

[Daniel Vetter]

On Wed, May 09, 2018 at 10:22:49AM +0300, Dan Carpenter wrote:
> There is a comment here which says that DIV_ROUND_UP() and that's where
> the problem comes from.  Say you pick:
> 
> 	args->bpp = UINT_MAX - 7;
> 	args->width = 4;
> 	args->height = 1;
> 
> The integer overflow in DIV_ROUND_UP() means "cpp" is UINT_MAX / 8 and
> because of how we picked args->width that means cpp < UINT_MAX / 4.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> Btw, DIV_ROUND_UP() integer overflows have been a recurring source of
> bugs so I have an unreleased static checker warning specific for that.
> This line triggers three warnings for me on my unreleased code:
> 
> drivers/gpu/drm/drm_dumb_buffers.c:69 drm_mode_create_dumb_ioctl() warn: negative user subtract: 0-u32max - 1
> drivers/gpu/drm/drm_dumb_buffers.c:69 drm_mode_create_dumb_ioctl() warn: potential integer overflow from user '(args->bpp) + (8)'
> drivers/gpu/drm/drm_dumb_buffers.c:69 drm_mode_create_dumb_ioctl() warn: potential integer overflow in 'DIV_ROUND_UP'
> 
> It's a pretty common idiom in the kernel to overflow and then test for
> it later so I'm not able to release this code because of the number of
> false positives that this idiom causes...
> 
> diff --git a/drivers/gpu/drm/drm_dumb_buffers.c b/drivers/gpu/drm/drm_dumb_buffers.c
> index 39ac15ce4702..45b0b5bbb5f8 100644
> --- a/drivers/gpu/drm/drm_dumb_buffers.c
> +++ b/drivers/gpu/drm/drm_dumb_buffers.c
> @@ -65,7 +65,8 @@ int drm_mode_create_dumb_ioctl(struct drm_device *dev,
>  		return -EINVAL;
>  
>  	/* overflow checks for 32bit size calculations */
> -	/* NOTE: DIV_ROUND_UP() can overflow */
> +	if (args->bpp > UINT_MAX - 8)
> +		return -EINVAL;
>  	cpp = DIV_ROUND_UP(args->bpp, 8);
>  	if (!cpp || cpp > 0xffffffffU / args->width)

The !cpp check is now redundant, this was our minimal overflow check. Note
that we only really care for cpp != 0 and that the size calculation doesn't
overflow. Userspace specifying a completely bogus bpp value is ok
otherwise (reasonable values only go up to about 128). So I think there's
no security issue here.

Anyway, can you pls respin with the !cpp check removed? See also

commit 6a77e80e55cacace60ff03aa717a6d364a401d2b (HEAD -> stuff)
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Mon Apr 30 17:04:10 2018 +0200

    backlight: remove obsolete comment for ->state

for context.

Thanks, Daniel

>  		return -EINVAL;
> _______________________________________________
> 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

Re: [PATCH] drm/dumb-buffers: Integer overflow in drm_mode_create_ioctl()

[Daniel Vetter]

On Wed, May 09, 2018 at 10:22:49AM +0300, Dan Carpenter wrote:
> There is a comment here which says that DIV_ROUND_UP() and that's where
> the problem comes from.  Say you pick:
> 
> 	args->bpp = UINT_MAX - 7;
> 	args->width = 4;
> 	args->height = 1;
> 
> The integer overflow in DIV_ROUND_UP() means "cpp" is UINT_MAX / 8 and
> because of how we picked args->width that means cpp < UINT_MAX / 4.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> Btw, DIV_ROUND_UP() integer overflows have been a recurring source of
> bugs so I have an unreleased static checker warning specific for that.
> This line triggers three warnings for me on my unreleased code:
> 
> drivers/gpu/drm/drm_dumb_buffers.c:69 drm_mode_create_dumb_ioctl() warn: negative user subtract: 0-u32max - 1
> drivers/gpu/drm/drm_dumb_buffers.c:69 drm_mode_create_dumb_ioctl() warn: potential integer overflow from user '(args->bpp) + (8)'
> drivers/gpu/drm/drm_dumb_buffers.c:69 drm_mode_create_dumb_ioctl() warn: potential integer overflow in 'DIV_ROUND_UP'
> 
> It's a pretty common idiom in the kernel to overflow and then test for
> it later so I'm not able to release this code because of the number of
> false positives that this idiom causes...
> 
> diff --git a/drivers/gpu/drm/drm_dumb_buffers.c b/drivers/gpu/drm/drm_dumb_buffers.c
> index 39ac15ce4702..45b0b5bbb5f8 100644
> --- a/drivers/gpu/drm/drm_dumb_buffers.c
> +++ b/drivers/gpu/drm/drm_dumb_buffers.c
> @@ -65,7 +65,8 @@ int drm_mode_create_dumb_ioctl(struct drm_device *dev,
>  		return -EINVAL;
>  
>  	/* overflow checks for 32bit size calculations */
> -	/* NOTE: DIV_ROUND_UP() can overflow */
> +	if (args->bpp > UINT_MAX - 8)
> +		return -EINVAL;
>  	cpp = DIV_ROUND_UP(args->bpp, 8);
>  	if (!cpp || cpp > 0xffffffffU / args->width)

The !cpp check is now redundant, this was our minimal overflow check. Note
that we only really care for cpp != 0 and that the size calculation doesn't
overflow. Userspace specifying a completely bogus bpp value is ok
otherwise (reasonable values only go up to about 128). So I think there's
no security issue here.

Anyway, can you pls respin with the !cpp check removed? See also

commit 6a77e80e55cacace60ff03aa717a6d364a401d2b (HEAD -> stuff)
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Mon Apr 30 17:04:10 2018 +0200

    backlight: remove obsolete comment for ->state

for context.

Thanks, Daniel

>  		return -EINVAL;
> _______________________________________________
> 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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel