[apparmor] [patch] fix disabling printk_ratelimit in aa-genprof

[Christian Boltz]

Hello,

Am Montag, 9. Juni 2014 schrieb Seth Arnold:
> On Mon, Jun 09, 2014 at 08:33:28PM +0200, Christian Boltz wrote:
> > aa-genprof failed to set /proc/sys/kernel/printk_ratelimit to 0
> > (unlimited) because the "if not value:" check matches 0.
> > 
> > This patch replaces the check with "... is None".
> > 
> > 
> > === modified file 'utils/aa-genprof'
> > --- utils/aa-genprof    2014-05-21 19:42:43 +0000
> > +++ utils/aa-genprof    2014-06-09 18:31:07 +0000
> > @@ -33,7 +33,7 @@
> > 
> >      return value
> >  
> >  def sysctl_write(path, value):
> > -    if not value:
> > 
> > +    if value is None:
> >          return
> >      
> >      with open(path, 'w') as f_out:
> >          f_out.write(str(value))
> 
> Why do we even have the check? Wouldn't it make more sense to throw an
> error condition if sysctl_write() is being used incorrectly? (either
> when str(value) blows up, in usual-python-style, or with an explicit
> check of expected value ranges?)

Well, there are exactly two calls of that function:

a) to disable the rate limit
   sysctl_write(ratelimit_sysctl, 0)

b) to restore the rate limit
   sysctl_write(ratelimit_sysctl, ratelimit_saved)

If you follow ratelimit_saved downwards, you'll find:

ratelimit_saved = sysctl_read(ratelimit_sysctl)

def sysctl_read(path):                                                                                                                                       
    value = None
    with open(path, 'r') as f_in:
        value = int(f_in.readline())
    return value

So if for some reason sysctl_read fails to read the old limit, it will 
return None instead of the value. And it's probably a bad idea to write 
"None" to /proc/sys/kernel/printk_ratelimit ;-)

That's of course something that shouldn't happen [1], but IMHO it's not 
bad enough to let aa-genprof bail out. 

However, a warning can't hurt. Slightly updated patch:

=== modified file 'utils/aa-genprof'                                                                                                        
--- utils/aa-genprof    2014-05-21 19:42:43 +0000                                                                                           
+++ utils/aa-genprof    2014-06-09 18:48:51 +0000                                                                                           
@@ -33,7 +33,8 @@
     return value
 
 def sysctl_write(path, value):
-    if not value:
+    if value is None:
+        warn('Not writing invalid value "None" to %s'%path)
         return
     with open(path, 'w') as f_out:
         f_out.write(str(value))



Regards,

Christian Boltz

[1] I'd expect that sysctl_read() throws a backtrace if that happens
-- 
Ich glaube aber nicht, dass der DDR Ram hat. Er hat seinen Rechner
doch erst vor einem Jahr gekauft! Die werden Ihm da doch nicht
uralt-Speicherbausteine hereingesteckt haben. Maximal kann er also
"Ex-DDR"-Speicher haben (Sprich Infineon, denn die Produzieren ja
auch in Dresden ...). [Konrad Neitzel in suse-linux]