Re: [PATCH V9 3/3] audit: add audit by children of executable path

On Fri, Aug 7, 2015 at 12:03 PM, Richard Guy Briggs <rgb(a)redhat.com&gt; wrote: > On 15/08/07, Paul Moore wrote: >> On Friday, August 07, 2015 02:37:15 AM Richard Guy Briggs wrote: >> > On 15/08/06, Paul Moore wrote: >> > >> > > I guess what I'm saying is that I'm not currently convinced that >> > > there is enough value in this to offset the risk I feel the loop >> > > presents. I understand the use cases that you are mentioning, the >> > > are the same as the last time we discussed this, but I'm going to >> > > need something better than that. >> > >> > Can you better describe the loop that concerns you? I don't quite see >> > it. >> >> It would be the only loop in the patch, look at the for loop in >> audit_filter_rules() which iterates up the process' parent chain. > > Sorry, I should reword that... What risk do you see in that loop? It > works up the task ancestry tree until it triggers, or hits init for that > PID namespace that terminates the loop. Do you see a risk in the > numerical pids rolling underneath the loop? I suppose there is some risk of PID overlap, and while that is a concern, it isn't my first. My main concern is that a malicious user could add an extra level of burden to the system by making an absurdly tall process tree and then hammer the system with trivial, short lived syscalls. Granted, there are userspace limits which would bound the impact to some extent, but there is no way to really reduce the risk. You could further put hard limits on the loop, but what good would that do? Malicious users would just know to blow past that limit before they did their Evil Deeds. I'll say it again; I'm not completely opposed to something like this - perhaps in some modified form - but I have yet to see a need for this functionality that is great enough to counter the risk.

> I *do* notice that find_task_by_vpid(pid_t) must be replaced with > find_task_by_pid_ns(pid_t, &init_pid_ns), since task_struct->pid is > always stored in the initial PID namespace. Another thing that needs to be resolved.

paul moore