Bug 3880

Summary: vfs.stp embedded-C functions need more deref() protection
Product: systemtap Reporter: Frank Ch. Eigler <fche>
Component: tapsetsAssignee: Josh Stone <jistone>
Status: RESOLVED FIXED    
Severity: normal    
Priority: P2    
Version: unspecified   
Target Milestone: ---   
Host: Target:
Build: Last reconfirmed:

Description Frank Ch. Eigler 2007-01-17 19:11:42 UTC
Functions like __file_maxbytes and __file_filename make too many
assumptions about their arguments.  For example, the former only
uses deref() for one link in the pointer chain, whereas it really
needs it for all steps (in case the intermediate pointers are corrupted).
The latter needs to use a protected string-copy, not just a nullness test.

This whole tapset should be reviewed for similar optimism.
Comment 1 Frank Ch. Eigler 2007-01-17 19:35:22 UTC
BTW, adding this to src/HACKING:

   Embedded-C code should avoid making references to the runtime or
-  other code possibly generated by the translator.
+  other code possibly generated by the translator.  Embedded-C code that
+  dereferences pointers should use deref() type functions to check each
+  individual operation if there exists a possibility that the function may
+  be called with invalid pointers or pointer chains.
Comment 2 Josh Stone 2007-02-01 17:30:57 UTC
With the resolution of bug #3079, I'm migrating the tapsets to use the
kread/kwrite macros.  I'll take this bug and audit the tapsets to make sure
we're protecting all unknown-pointer dereferences.
Comment 3 Josh Stone 2007-03-08 16:55:06 UTC
The kread migration is finished.  There's still an issue with the way bdevname
is called (marked with FIXME), but the rest of the code is protected.

I left the explicit NULL checks in, because I didn't want to change the
functionality of the tapset.  Thus it still needs an audit to determine whether
those NULL checks are appropriate.  In cases where NULL is expected and
permitted, leave it as-is, but if NULL should be an error just let the kread
catch it.