Bug 3880 - vfs.stp embedded-C functions need more deref() protection
Summary: vfs.stp embedded-C functions need more deref() protection
Status: RESOLVED FIXED
Alias: None
Product: systemtap
Classification: Unclassified
Component: tapsets (show other bugs)
Version: unspecified
: P2 normal
Target Milestone: ---
Assignee: Josh Stone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-01-17 19:11 UTC by Frank Ch. Eigler
Modified: 2007-03-08 16:55 UTC (History)
0 users

See Also:
Host:
Target:
Build:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
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.