| Subject | Re: [AppArmor 39/41] AppArmor: Profile loading and manipulation, pathname matching | From | Andi Kleen <> | Date | 12 Apr 2007 15:46:38 +0200 |
| |
jjohansen@suse.de writes:
[didn't review code fully, just some stuff I noticed]
> + > +struct aa_dfa { > + struct table_header *tables[YYTD_ID_NXT]; > +};
If that is passed in from user space you would need special compat code for 64bit kernels who support 32bit userland. Better to avoid pointers.
> + > + /* get optional subprofiles */ > + if (aa_is_nameX(e, AA_LIST, "hats")) { > + while (!aa_is_nameX(e, AA_LISTEND, NULL)) { > + struct aa_profile *subprofile; > + subprofile = aa_unpack_profile(e);
Is there any check that would guard the recursion from stack overflow on malicious input?
> + /* > + * Replacement needs to allocate a new aa_task_context for each > + * task confined by old_profile. To do this the profile locks > + * are only held when the actual switch is done per task. While > + * looping to allocate a new aa_task_context the old_task list > + * may get shorter if tasks exit/change their profile but will > + * not get longer as new task will not use old_profile detecting > + * that is stale. > + */ > + do { > + new_cxt = aa_alloc_task_context(GFP_KERNEL | __GFP_NOFAIL);
NOFAIL is usually a bad sign. It should be only used if there is no alternative.
-Andi
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
|