Cleanup some deref handling in the task tapset

Some of this is just cosmetic, but there is one big takeaway: there's a
error-goto between kread calls and the CATCH_DEREF_FAULT.  You must not
allow this to bypass any resource management, like unlocking a resource
that you grabbed!

* tapset/task.stp (pid2task): No derefs, so remove the CATCH.
  (task_gid, task_egid, task_uid, task_euid): Move the CATCH within the
  #ifdef branch that actually needs it.
  (task_open_file_handles, task_max_file_handles): Ensure that we always
  call rcu_read_unlock if we locked it!

1 files changed, 20 insertions, 11 deletions

diff --git a/tapset/task.stp b/tapset/task.stp
index 8776a014..3bb65413 100644
--- a/tapset/task.stp
+++ b/tapset/task.stp
@@ -81,7 +81,6 @@ function pid2task:long (pid:long) %{ /* pure */
     rcu_read_unlock();
 #endif /* 2.6.31 */
     THIS->__retvalue = (long)t;
-    CATCH_DEREF_FAULT();
 %}
 
 // Return the name of the given process id
@@ -105,6 +104,7 @@ function task_gid:long (task:long) %{ /* pure */
     struct task_struct *t = (struct task_struct *)(long)THIS->task;
 #ifdef STAPCONF_TASK_UID
     THIS->__retvalue = kread(&(t->gid));
+    CATCH_DEREF_FAULT();
 #else
     /* XXX: We can't easily kread this rcu-protected field. */
     /* XXX: no task_gid() in 2.6.28 */
@@ -114,8 +114,6 @@ function task_gid:long (task:long) %{ /* pure */
     rcu_read_unlock();
     THIS->__retvalue = cred->gid;
 #endif
-
-    CATCH_DEREF_FAULT();
 %}
 
 
@@ -124,6 +122,7 @@ function task_egid:long (task:long) %{ /* pure */
     struct task_struct *t = (struct task_struct *)(long)THIS->task;
 #ifdef STAPCONF_TASK_UID
     THIS->__retvalue = kread(&(t->egid));
+    CATCH_DEREF_FAULT();
 #else
     /* XXX: We can't easily kread this rcu-protected field. */
     /* XXX: no task_egid() in 2.6.28 */
@@ -133,7 +132,6 @@ function task_egid:long (task:long) %{ /* pure */
     rcu_read_unlock();
     THIS->__retvalue = cred->egid;
 #endif
-    CATCH_DEREF_FAULT();
 %}
 
 
@@ -142,12 +140,11 @@ function task_uid:long (task:long) %{ /* pure */
     struct task_struct *t = (struct task_struct *)(long)THIS->task;
 #ifdef STAPCONF_TASK_UID
     THIS->__retvalue = kread(&(t->uid));
+    CATCH_DEREF_FAULT();
 #else
     /* XXX: We can't easily kread this rcu-protected field. */
     THIS->__retvalue = task_uid (t);
 #endif
-
-    CATCH_DEREF_FAULT();
 %}
 
 
@@ -156,11 +153,11 @@ function task_euid:long (task:long) %{ /* pure */
     struct task_struct *t = (struct task_struct *)(long)THIS->task;
 #ifdef STAPCONF_TASK_UID
     THIS->__retvalue = kread(&(t->euid));
+    CATCH_DEREF_FAULT();
 #else
     /* XXX: We can't easily kread this rcu-protected field. */
     THIS->__retvalue = task_euid (t);
 #endif
-    CATCH_DEREF_FAULT();
 %}
 
 
@@ -195,6 +192,7 @@ function task_cpu:long (task:long)
 function task_open_file_handles:long (task:long)
 %( kernel_v >= "2.6.15" %?
 %{ /* pure */
+    int locked = 0;
     unsigned int count=0, fd, max;
     struct task_struct *t;
     struct files_struct *fs;
@@ -203,31 +201,36 @@ function task_open_file_handles:long (task:long)
     fs = kread(&(t->files));
     f = kread(&(fs->fdt));
     rcu_read_lock();
+    locked = 1;
     max = kread(&(f->max_fds));
     for (fd = 0; fd < max; fd++) {
                 if ( kread(&(f->fd[fd])) != NULL)
                         count ++;
         }
-    rcu_read_unlock();
     THIS->__retvalue = count;
     CATCH_DEREF_FAULT();
+    if (locked)
+        rcu_read_unlock();
 %}
 %:
 %{ /* pure */
+    int locked = 0;
     unsigned int count=0, fd, max;
     struct task_struct *t;
     struct files_struct *f;
     t = (struct task_struct *)(long)THIS->task;
     f = kread(&(t->files));
     rcu_read_lock();
+    locked = 1;
     max = kread(&(f->max_fds));
     for (fd = 0; fd < max; fd++) {
                 if ( kread(&(f->fd[fd])) != NULL)
                         count ++;
         }
-    rcu_read_unlock();
     THIS->__retvalue = count;
     CATCH_DEREF_FAULT();
+    if (locked)
+        rcu_read_unlock();
 %}
 %)
 
@@ -236,6 +239,7 @@ function task_open_file_handles:long (task:long)
 function task_max_file_handles:long (task:long)
 %( kernel_v >= "2.6.15" %?
 %{ /* pure */
+    int locked = 0;
     struct task_struct *t;
     struct files_struct *fs;
     struct fdtable *f;
@@ -243,19 +247,24 @@ function task_max_file_handles:long (task:long)
     fs = kread (&(t->files));
     f = kread(&(fs->fdt));
     rcu_read_lock();
+    locked = 1;
     THIS->__retvalue = kread(&(f->max_fds));
-    rcu_read_unlock();
     CATCH_DEREF_FAULT();
+    if (locked)
+        rcu_read_unlock();
 %}
 %:
 %{ /* pure */
+    int locked = 0;
     struct task_struct *t;
     struct files_struct *f;
     t = (struct task_struct *)(long)THIS->task;
     f = kread(&(t->files));
     rcu_read_lock();
+    locked = 1;
     THIS->__retvalue = kread(&(f->max_fds));
-    rcu_read_unlock();
     CATCH_DEREF_FAULT();
+    if (locked)
+        rcu_read_unlock();
 %}
 %)