Re: svn commit: r302577 - head/sys/dev/drm2

[Ngie Cooper (yaneurabeya)]

> On Jul 13, 2016, at 16:56, Ngie Cooper  wrote:
> 
> On Wed, Jul 13, 2016 at 4:54 AM, Robert Watson  wrote:
>> On Mon, 11 Jul 2016, Garrett Cooper wrote:
>> 
>>> Add missing default case to capable(..) function definition
>>> 
>>> By definition (enum __drm_capabilities), cases other than CAP_SYS_ADMIN
>>> aren't possible. Add in a KASSERT safety belt and return false in
>>> !INVARIANTS case if an invalid value is passed in, as it would be a
>>> programmer error.
>>> 
>>> This fixes a -Wreturn-type error with gcc 5.3.0.
>>> 
>>> Differential Revision: https://reviews.freebsd.org/D7188
>>> MFC after: 1 week
>>> Reported by:   devel/amd64-gcc (5.3.0)
>>> Reviewed by:   dumbbell
>>> Sponsored by:  EMC / Isilon Storage Division
>> 
>> Per my comment in the review, I think a panic() here would be preferable to
>> a KASSERT(), as it would come without perceptible runtime cost, and failstop
>> the system if we were violating a design-time security invariant.
> 
>Good point. I'll commit the change tonight.

Fixed in r302841. Thanks!



signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: svn commit: r302577 - head/sys/dev/drm2

[Ngie Cooper]

On Wed, Jul 13, 2016 at 4:54 AM, Robert Watson  wrote:
> On Mon, 11 Jul 2016, Garrett Cooper wrote:
>
>>  Add missing default case to capable(..) function definition
>>
>>  By definition (enum __drm_capabilities), cases other than CAP_SYS_ADMIN
>>  aren't possible. Add in a KASSERT safety belt and return false in
>>  !INVARIANTS case if an invalid value is passed in, as it would be a
>>  programmer error.
>>
>>  This fixes a -Wreturn-type error with gcc 5.3.0.
>>
>>  Differential Revision: https://reviews.freebsd.org/D7188
>>  MFC after: 1 week
>>  Reported by:   devel/amd64-gcc (5.3.0)
>>  Reviewed by:   dumbbell
>>  Sponsored by:  EMC / Isilon Storage Division
>
> Per my comment in the review, I think a panic() here would be preferable to
> a KASSERT(), as it would come without perceptible runtime cost, and failstop
> the system if we were violating a design-time security invariant.

Good point. I'll commit the change tonight.
Thanks!
-Ngie
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r302577 - head/sys/dev/drm2

[Robert Watson]

On Mon, 11 Jul 2016, Garrett Cooper wrote:


 Add missing default case to capable(..) function definition

 By definition (enum __drm_capabilities), cases other than CAP_SYS_ADMIN
 aren't possible. Add in a KASSERT safety belt and return false in
 !INVARIANTS case if an invalid value is passed in, as it would be a
 programmer error.

 This fixes a -Wreturn-type error with gcc 5.3.0.

 Differential Revision: https://reviews.freebsd.org/D7188
 MFC after: 1 week
 Reported by:   devel/amd64-gcc (5.3.0)
 Reviewed by:   dumbbell
 Sponsored by:  EMC / Isilon Storage Division


Per my comment in the review, I think a panic() here would be preferable to a 
KASSERT(), as it would come without perceptible runtime cost, and failstop the 
system if we were violating a design-time security invariant.


Robert
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

svn commit: r302577 - head/sys/dev/drm2

[Garrett Cooper]

Author: ngie
Date: Mon Jul 11 17:01:07 2016
New Revision: 302577
URL: https://svnweb.freebsd.org/changeset/base/302577

Log:
  Add missing default case to capable(..) function definition
  
  By definition (enum __drm_capabilities), cases other than CAP_SYS_ADMIN
  aren't possible. Add in a KASSERT safety belt and return false in
  !INVARIANTS case if an invalid value is passed in, as it would be a
  programmer error.
  
  This fixes a -Wreturn-type error with gcc 5.3.0.
  
  Differential Revision:https://reviews.freebsd.org/D7188
  MFC after:1 week
  Reported by:  devel/amd64-gcc (5.3.0)
  Reviewed by:  dumbbell
  Sponsored by: EMC / Isilon Storage Division

Modified:
  head/sys/dev/drm2/drm_os_freebsd.h

Modified: head/sys/dev/drm2/drm_os_freebsd.h
==
--- head/sys/dev/drm2/drm_os_freebsd.h  Mon Jul 11 16:56:51 2016
(r302576)
+++ head/sys/dev/drm2/drm_os_freebsd.h  Mon Jul 11 17:01:07 2016
(r302577)
@@ -438,6 +438,10 @@ capable(enum __drm_capabilities cap)
switch (cap) {
case CAP_SYS_ADMIN:
return DRM_SUSER(curthread);
+   default:
+   KASSERT(false,
+   ("%s: unhandled capability: %0x", __func__, cap));
+   return (false);
}
 }
 
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"