Messages in this thread | | | Date | Sat, 26 Apr 2014 00:46:25 +0300 | Subject | Re: Kernel panic at Ubuntu: IMA + Apparmor | From | Dmitry Kasatkin <> |
| |
On 26 April 2014 00:27, Eric W. Biederman <ebiederm@xmission.com> wrote: > Dmitry Kasatkin <dmitry.kasatkin@gmail.com> writes: > >> On 25 April 2014 23:45, Eric W. Biederman <ebiederm@xmission.com> wrote: >>> Dmitry Kasatkin <dmitry.kasatkin@gmail.com> writes: >>> >>>> On 25 April 2014 23:01, Oleg Nesterov <oleg@redhat.com> wrote: >>>>> On 04/25, Eric W. Biederman wrote: >>>>>> >>>>>> Oleg Nesterov <oleg@redhat.com> writes: >>>>>> >>>>>> > Well. I _think_ that __fput() and ima_file_free() in particular should not >>>>>> > depend on current and/or current->nsproxy. If nothing else, fput() can be >>>>>> > called by the unrelated task which looks into /proc/pid/. >>>>>> > >>>>>> > But again, task_work_add() has more and more users, and it seems that even >>>>>> > __fput() paths can do "everything", so perhaps it would be safer to allow >>>>>> > to use ->nsproxy in task_work_run. >>>>>> >>>>>> Like I said, give me a clear motivating case. >>>>> >>>>> I agree, we need a reason. Currently I do not see one. >>>>> >>>>>> Right now not allowing >>>>>> nsproxy is turning up bugs in __fput. Which seems like a good thing. >>>>> >>>>> This is what I certainly agree with ;) >>>>> >>>> >>>> Hi, >>>> >>>> IMA uses kernel_read API which does not know anything about caller. >>>> And of course security frameworks are at guard as usual. >>>> >>>> Exactly after reading first Eric's respons, I thought why to scratch >>>> the head when task work queues are indeed designed for tasks... >>> >>> __fput has no guarantee of running in the task that close the file >>> descriptor. If your code depends on that your code is broken. >>> >>>> And if you to dig for the history, IMA-appraisal was stuck due to >>>> lockdep reporting even though it was on non-everlaping cases. >>>> IIRC files vs. directories... >>>> >>>> After that IIRC Al Viro discussed about delayed fput and IIRC Oleg >>>> (sorry if I am wrong) introduced task work queues. >>>> >>>> So IMA-appraisal was able to be upstreamed... That was ~3.4 time frame, IIRC >>>> >>>> Name space also dated around ~3.4?? >>>> Apparmor namespace change was also around that time. >>>> >>>> 3.10 introduces this name space order change and broke IMA-appraisal. >>> >>> IMA-appraisal is fundamentally broken because I can take a mandatory >>> file lock and prevent IMA-apprasial. >>> >> >> What file lock are you talking about? >> IMA-appraisal does not depends on file locks... > > It honors them. Look at rw_verify_area, in vfs_read, in kernel_read. > > It sure looks like locks_mandatory_area can cause your kernel_read to > fail. > >>> Using kernel_read is what allows this. >>> >>>> Isn't it a clear motivating case??? >>> >>> kernel_read is not appropriate for IMA use. The rest of this is just >>> the messenger. >>> >>> IMA needs to use a cousin of kernel_read that operates at a lower level >>> than vfs_read. A function that all of the permission checks and the >>> fsnotify work. >>> >>> I am sorry to be the bearer of bad news. But kernel_read is totally >>> inappropriate for IMA. >>> >> >> So you break IMA-appraisal and declare that it cannot be used now? > > I didn't break it. I read the code, and I read the back trace to see > where the bug was. > > I see IMA-appraisal trying to read file data as if it were a user space > application in such a way that it can get permission denied for a whole > host of reasons. > > My understanding of IMA-appraisal is that using a code path that can > give use permission denined when performing appraisal is a way for > clever people to attack and avoid IMA-appraisal without violating any > security policy. >
Interesting thing is that file was already open before and LSM gave OK for this. Then re-reading the file on close in fact does not need any LSM permission checks. But as kernel_read API is still the same, it goes via the same checks...
But on close with delayed fput nsproxy is missing ....
> Am I wrong. Is it ok for IMA-appraisal to get permission denied when it > wants to appraise a file? >
IMA is called after may_open...
> Eric
-- Thanks, Dmitry
| |