[apparmor] [patch] Fix rttime default unit in RlimitRule

[John Johansen]

On 06/26/2015 03:15 PM, Christian Boltz wrote:
> Hello,
> 
> RlimitRule accidently used 'ms' (milliseconds) as default unit for
> rttime rules, but rttime without unit means 'us' (microseconds). This
> patch fixes this.
> 
> Also add some tests with 'us' as unit, and two more to cover terribly
> invalid corner cases (and to improve test coverage by 2 lines ;-)
> 
> 
looks right

Acked-by: John Johansen <john.johansen at canonical.com>

> 
> [ 58-RlimitRule-fix-rttime-default-unit.diff ]
> 
> === modified file utils/apparmor/rule/rlimit.py
> --- utils/apparmor/rule/rlimit.py       2015-06-06 14:53:16.868029000 +0200
> +++ utils/apparmor/rule/rlimit.py       2015-06-26 22:50:44.728769405 +0200
> @@ -26,7 +26,7 @@
>  
>  rlimit_size     = ['fsize', 'data', 'stack', 'core', 'rss', 'as', 'memlock', 'msgqueue']  # NUMBER ( 'K' | 'M' | 'G' )
>  rlimit_number   = ['ofile', 'nofile', 'locks', 'sigpending', 'nproc', 'rtprio']
> -rlimit_time     = ['cpu', 'rttime']  # number + time unit (cpu in seconds+, rttime in ms+)
> +rlimit_time     = ['cpu', 'rttime']  # number + time unit (cpu in seconds+, rttime in us+)
>  rlimit_nice     = ['nice']  # a number between -20 and 19.
>  
>  rlimit_all      = rlimit_size + rlimit_number + rlimit_time + rlimit_nice
> @@ -92,11 +92,11 @@
>                  if unit == 'm' and rlimit == 'rttime':
>                      raise AppArmorException('Ambiguous value %s in rlimit %s rule - use "ms" or "minutes"' % (value, rlimit))
>                  if unit != '' and not ('seconds'.startswith(unit) or 'minutes'.startswith(unit) or 'hours'.startswith(unit) or
> -                        (unit == 'ms' and rlimit == 'rttime') ):
> +                        (rlimit == 'rttime' and unit in ['ms', 'us']) ):
>                      raise AppArmorException('Invalid unit in rlimit %s %s rule' % (rlimit, value))
>  
>                  if rlimit == 'rttime':
> -                    self.value_as_int = self.time_to_int(value, 'ms')
> +                    self.value_as_int = self.time_to_int(value, 'us')
>                  else:
>                      self.value_as_int = self.time_to_int(value, 'seconds')
>  
> @@ -181,7 +181,9 @@
>          if unit == '':
>              unit = default_unit
>  
> -        if unit == 'ms':
> +        if unit == 'us':
> +            number = number / 1000000.0
> +        elif unit == 'ms':
>              number = number / 1000.0
>          elif 'seconds'.startswith(unit):
>              pass
> === modified file utils/test/test-rlimit.py
> --- utils/test/test-rlimit.py   2015-06-06 14:53:16.868029000 +0200
> +++ utils/test/test-rlimit.py   2015-06-26 23:01:08.322573480 +0200
> @@ -74,8 +74,11 @@
>          ('set rlimit,'                      , AppArmorException), # missing parts
>          ('set rlimit <= 5,'                 , AppArmorException),
>          ('set rlimit cpu <= ,'              , AppArmorException),
> +        ('set rlimit cpu <= "",'            , AppArmorBug),       # evil quoting trick
>          ('set rlimit foo <= 5,'             , AppArmorException), # unknown rlimit
>          ('set rlimit rttime <= 60m,'        , AppArmorException), # 'm' could mean 'ms' or 'minutes'
> +        ('set rlimit cpu <= 20ms,'          , AppArmorException), # cpu doesn't support 'ms'...
> +        ('set rlimit cpu <= 20us,'          , AppArmorException), # ... or 'us'
>          ('set rlimit nice <= 20MB,'         , AppArmorException), # invalid unit for this rlimit type
>          ('set rlimit cpu <= 20MB,'          , AppArmorException),
>          ('set rlimit data <= 20seconds,'    , AppArmorException),
> @@ -136,6 +139,7 @@
>          ([None  , '1024'           ]    , AppArmorBug), # wrong type for rlimit
>          (['as'  , dict()           ]    , AppArmorBug), # wrong type for value
>          (['as'  , None             ]    , AppArmorBug), # wrong type for value
> +        (['cpu' , '100xy2'         ]    , AppArmorException), # invalid unit
>      ]
>  
>      def _run_test(self, params, expected):
> @@ -447,6 +451,7 @@
>          self.obj = RlimitRule('cpu', '1')
>  
>      tests = [
> +        ('30us'     ,       0.00003),
>          ('40ms'     ,       0.04),
>          ('40'       ,      40),
>          ('40seconds',      40),
> @@ -460,6 +465,9 @@
>      def test_with_ms_as_default(self):
>          self.assertEqual(self.obj.time_to_int('40', 'ms'), 0.04)
>  
> +    def test_with_us_as_default(self):
> +        self.assertEqual(self.obj.time_to_int('30', 'us'), 0.00003)
> +
>      def test_invalid_time_to_int(self):
>          with self.assertRaises(AppArmorException):
>              self.obj.time_to_int('20mice', 'seconds')
> 
> 
> Regards,
> 
> Christian Boltz
>