Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751872AbaDYV4g (ORCPT ); Fri, 25 Apr 2014 17:56:36 -0400 Received: from mail-wi0-f173.google.com ([209.85.212.173]:38747 "EHLO mail-wi0-f173.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751283AbaDYV4e (ORCPT ); Fri, 25 Apr 2014 17:56:34 -0400 MIME-Version: 1.0 In-Reply-To: References: <535A5C78.1070901@samsung.com> <535A75C1.3050901@samsung.com> <20140425182310.GA9128@redhat.com> <87sip15iy5.fsf@x220.int.ebiederm.org> <20140425192543.GA11908@redhat.com> <878uqt42q7.fsf@x220.int.ebiederm.org> <20140425200156.GA13727@redhat.com> <874n1h16le.fsf@x220.int.ebiederm.org> <87iopxxfpp.fsf@x220.int.ebiederm.org> Date: Sat, 26 Apr 2014 00:56:32 +0300 Message-ID: Subject: Re: Kernel panic at Ubuntu: IMA + Apparmor From: Dmitry Kasatkin To: "Eric W. Biederman" Cc: Oleg Nesterov , Dmitry Kasatkin , linux-security-module , John Johansen , Mimi Zohar , James Morris , viro@zeniv.linux.org.uk, Linux Kernel Mailing List , kernel-team Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 26 April 2014 00:46, Dmitry Kasatkin wrote: > On 26 April 2014 00:27, Eric W. Biederman wrote: >> Dmitry Kasatkin writes: >> >>> On 25 April 2014 23:45, Eric W. Biederman wrote: >>>> Dmitry Kasatkin writes: >>>> >>>>> On 25 April 2014 23:01, Oleg Nesterov wrote: >>>>>> On 04/25, Eric W. Biederman wrote: >>>>>>> >>>>>>> Oleg Nesterov 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 > > Is it really a show stopper to switch order of 2 functions as quick fix? It was like that before 3.10 and seemed ok... > > -- > Thanks, > Dmitry -- Thanks, Dmitry -- 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/