Re: [PATCH V3 3/5 selinux-next] selinux: sidtab_clone switch to use rwlock.

2018-05-30 Thread J Freyensee



  
+int sidtab_clone(struct sidtab *s, struct sidtab *d)

+{
+   int i, rc = 0;
If s or d are NULL (see if() below), why would we want rc, the return 
value, to be 0?  How about defaulting rc to an error value (-EINVAL)?

+   struct sidtab_node *cur;
+
+   if (!s || !d)
+   goto errout;
+
+   read_lock(>lock);
+   for (i = 0; i < SIDTAB_SIZE; i++) {
+   cur = s->htable[i];
+   while (cur) {
+   if (cur->sid > SECINITSID_NUM)
+   rc =  sidtab_insert(d, cur->sid, >context);
+   if (rc)
+   goto out;
+   cur = cur->next;
+   }
+   }
+out:
+   read_unlock(>lock);
+errout:
+   return rc;
+}


Thanks,
Jay


Re: [PATCH V3 3/5 selinux-next] selinux: sidtab_clone switch to use rwlock.

2018-05-30 Thread J Freyensee



  
+int sidtab_clone(struct sidtab *s, struct sidtab *d)

+{
+   int i, rc = 0;
If s or d are NULL (see if() below), why would we want rc, the return 
value, to be 0?  How about defaulting rc to an error value (-EINVAL)?

+   struct sidtab_node *cur;
+
+   if (!s || !d)
+   goto errout;
+
+   read_lock(>lock);
+   for (i = 0; i < SIDTAB_SIZE; i++) {
+   cur = s->htable[i];
+   while (cur) {
+   if (cur->sid > SECINITSID_NUM)
+   rc =  sidtab_insert(d, cur->sid, >context);
+   if (rc)
+   goto out;
+   cur = cur->next;
+   }
+   }
+out:
+   read_unlock(>lock);
+errout:
+   return rc;
+}


Thanks,
Jay