* Serge E. Hallyn <[email protected]> [2009-01-28 11:13:05]:

> Quoting CAI Qian ([email protected]):
> > Hi,
> > 
> > 
> > --- On Wed, 1/28/09, Serge E. Hallyn <[email protected]> wrote:
> > 
> > > From: Serge E. Hallyn <[email protected]>
> > > Subject: Re: [LTP] [PATCH] proc01: SELinux with attr/* Interface - 
> > > version 2
> > > To: "CAI Qian" <[email protected]>
> > > Cc: "Kamalesh Babulal" <[email protected]>, 
> > > [email protected], [email protected], [email protected], 
> > > [email protected]
> > > Date: Wednesday, January 28, 2009, 11:50 PM
> > > Quoting CAI Qian ([email protected]):
> > > > Hi,
> > > > 
> > > > 
> > > > --- On Wed, 1/28/09, Serge E. Hallyn
> > > <[email protected]> wrote:
> > > > 
> > > > > From: Serge E. Hallyn <[email protected]>
> > > > > Subject: Re: [LTP] [PATCH] proc01: SELinux with
> > > attr/* Interface - version 2
> > > > > To: "CAI Qian" <[email protected]>
> > > > > Cc: "Kamalesh Babulal"
> > > <[email protected]>, [email protected],
> > > [email protected], [email protected],
> > > [email protected]
> > > > > Date: Wednesday, January 28, 2009, 10:57 PM
> > > > > Quoting CAI Qian ([email protected]):
> > > > > > Kamalesh Babulal, well, my approach is that
> > > anyone who
> > > > > cares about 
> > > > > > AppArmor can add a list of files should work
> > > to the
> > > > > code. it is fair that if different LSMs behave
> > > differently,
> > > > > we'll need different lists
> > > > > > (selinux_should_work and
> > > apparmor_should_work) to deal
> > > > > with them.
> > > > > > 
> > > > > > > To make it
> > > > > > > generic can we 
> > > > > > > just skip reading the list of files, if
> > > they
> > > > > return EINVAL
> > > > > > > or else we 
> > > > > > > have to support checking of different
> > > LSM's
> > > > > and add
> > > > > > > support for each of 
> > > > > > > them individually.
> > > > > > > 
> > > > > > 
> > > > > > Yes, but then you will still need to treat
> > > different
> > > > > LSMs differently.
> > > > > > 
> > > > > > >   Agree that the coverage of the
> > > testcase is going
> > > > > to be
> > > > > > > reduced. It will be 
> > > > > > > reduced more because the list which we
> > > are taking
> > > > > care is
> > > > > > > incomplete, 
> > > > > > 
> > > > > > Which ones are missing -- should return
> > > EINVAL with
> > > > > SELinux
> > > > > > disabled? 
> > > > > > 
> > > > > > > we could need to add other files to the
> > > list like
> > > > > nfs to be
> > > > > > > skipped. 
> > > > > > > Sending another patch which will ignore
> > > the file
> > > > > if it
> > > > > > > returns EINVAL or else 
> > > > > > > throw warning.
> > > > > > 
> > > > > > This patch won't able to catch attr/*

Sorry send the wrong patch

> > > entries
> > > > > return
> > > > > > EINVAL while SELinux is enabled. It does not
> > > look like
> > > > > a good
> > > > > > approach to me, because it is a test
> > > coverage
> > > > > regression.
> > > > > > 
> > > > > > CAI Qian
> > > > > 
> > > > > So, just to try and think through this...  If no
> > > LSM is
> > > > > enabled,
> > > > > the files should return -EINVAL.  If they
> > > don't return
> > > > > -EINVAL,
> > > > > is that a situation we care about?  What would it
> > > mean?
> > > > > 
> > > > 
> > > > Yes, Stephen Smalley from National Security Agency of
> > > U.S. told it
> > > > means "security modules (e.g. capability)
> > > don't support any of those 
> > > > interfaces", so if another errno is returned, it
> > > should be brought up
> > > > to attention.
> > > 
> > > Obviously the correct behavior depends upon the security
> > > subsystem,
> > > but you're finagling the checks into fs-specific
> > > checks.
> > > 
> > > It seems to me that if you're going to check for
> > > correct return
> > > values from these functions, you should do so under
> > > testcases/kernel/security.
> > 
> > Well, it is fine to put some of those checks under another separate 
> > test, but it looks to me that it is probably easier and simpler here 
> > to consider those two tightly coupled component all together in this
> > case. Because the test should not simple blindly skip those entries
> > due to the reasons I have stated before, and people may run this test 
> > with SELinux on/off or AppArmor on/off (not consider other Linux 
> > security modules) as those are all valid test environment setup, we'll 
> > then have 3 different unique conditions:
> > 
> > 1. SELinux on (attr/* read successfuly)
> > 2. AppArmor on (???)
> > 3. SELinux off and AppArmor off (attr/* read with -EINVAL)
> 4. TOMOYO on
> 5. Smack on
> ...
> 
> > As the result, the above checking code will need to be present in both
> > proc01 and a new test.
> 
> Then please stick to the simple suggestion from Stephen, keeping
> any selinux- (or any other lsm-)specific code out of proc01.c.
> 
> Which may be what you're suggesting  :)
> 
> -serge

We can just add the files related to LSM, to known failure list. We already 
check
for their return value, if not EINVAL report test failure or else skip.
Added the nfsd files to the list. 

---
 testcases/kernel/fs/proc/proc01.c |    7 +++++++
 1 file changed, 7 insertions(+)

Index: b/testcases/kernel/fs/proc/proc01.c
===================================================================
--- a/testcases/kernel/fs/proc/proc01.c
+++ b/testcases/kernel/fs/proc/proc01.c
@@ -88,6 +88,13 @@ const Mapping known_issues[] =
     {"read", "/proc/xen/privcmd", EINVAL},
     {"read", "/proc/self/mem", EIO},
     {"read", "/proc/self/task/[0-9]*/mem", EIO},
+       {"read", "/proc/self/attr/*", EINVAL},
+       {"read", "/proc/self/task/[0-9]*/attr/*", EINVAL},
+       {"read", "/proc/fs/nfsd/unlock_filesystem", EINVAL},
+       {"read", "/proc/fs/nfsd/unlock_ip", EINVAL},
+       {"read", "/proc/fs/nfsd/filehandle", EINVAL},
+       {"read", "/proc/fs/nfsd/.getfs", EINVAL},
+       {"read", "/proc/fs/nfsd/.getfd", EINVAL},
     {"", "", 0}
   };
 
-- 
Thanks & Regards,
Kamalesh Babulal,
Linux Technology Center,
IBM, ISTL.

------------------------------------------------------------------------------
This SF.net email is sponsored by:
SourcForge Community
SourceForge wants to tell your story.
http://p.sf.net/sfu/sf-spreadtheword
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list

Reply via email to