[patch] SELinux: trivial, unify iterator variable naming, part 3

[patch] SELinux: trivial, unify iterator variable naming, part 3

[Vesa-Matti J Kari]

Hello,

Once again, renamed iterators "idx" to "i", to be in harmony
with the old unwritten C convention, and with the rest of the code.

Signed-off-by: Vesa-Matti Kari <vmkari@cc.helsinki.fi>

---

 security/selinux/netif.c |   16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)



--- security-testing-2.6/security/selinux/netif.c	2008-07-20 18:29:22.000000000 +0300
+++ security-testing-2.6-vmk2/security/selinux/netif.c	2008-07-21 02:17:42.000000000 +0300
@@ -67,10 +67,10 @@ static inline u32 sel_netif_hashfn(int i
  */
 static inline struct sel_netif *sel_netif_find(int ifindex)
 {
-	int idx = sel_netif_hashfn(ifindex);
+	int i = sel_netif_hashfn(ifindex);
 	struct sel_netif *netif;

-	list_for_each_entry_rcu(netif, &sel_netif_hash[idx], list)
+	list_for_each_entry_rcu(netif, &sel_netif_hash[i], list)
 		/* all of the devices should normally fit in the hash, so we
 		 * optimize for that case */
 		if (likely(netif->nsec.ifindex == ifindex))
@@ -90,13 +90,13 @@ static inline struct sel_netif *sel_neti
  */
 static int sel_netif_insert(struct sel_netif *netif)
 {
-	int idx;
+	int i;

 	if (sel_netif_total >= SEL_NETIF_HASH_MAX)
 		return -ENOSPC;

-	idx = sel_netif_hashfn(netif->nsec.ifindex);
-	list_add_rcu(&netif->list, &sel_netif_hash[idx]);
+	i = sel_netif_hashfn(netif->nsec.ifindex);
+	list_add_rcu(&netif->list, &sel_netif_hash[i]);
 	sel_netif_total++;

 	return 0;
@@ -257,12 +257,12 @@ static void sel_netif_kill(int ifindex)
  */
 static void sel_netif_flush(void)
 {
-	int idx;
+	int i;
 	struct sel_netif *netif;

 	spin_lock_bh(&sel_netif_lock);
-	for (idx = 0; idx < SEL_NETIF_HASH_SIZE; idx++)
-		list_for_each_entry(netif, &sel_netif_hash[idx], list)
+	for (i = 0; i < SEL_NETIF_HASH_SIZE; i++)
+		list_for_each_entry(netif, &sel_netif_hash[i], list)
 			sel_netif_destroy(netif);
 	spin_unlock_bh(&sel_netif_lock);
 }

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [patch] SELinux: trivial, unify iterator variable naming, part 3

[James Morris]

On Mon, 21 Jul 2008, Vesa-Matti J Kari wrote:

> Hello,
> 
> Once again, renamed iterators "idx" to "i", to be in harmony
> with the old unwritten C convention, and with the rest of the code.
> 
> Signed-off-by: Vesa-Matti Kari <vmkari@cc.helsinki.fi>

I'm not applying these variable renaming patches, as they peturb the code 
for no established benefit.

I suggest reading the following carefully on how to submit patches:
http://www.zipworld.com.au/~akpm/linux/patches/stuff/tpp.txt

It's also generally best to simply use git to create patches (e.g. via 
git-format-patch).


Something that would be particularly useful at this stage would be to see 
if you can reproduce a long delay in booting the current git tree possibly 
related to SELinux policy loading:

[    6.904650] EXT3-fs: mounted filesystem with ordered data mode.
[    7.076411] type=1404 audit(1216904882.076:2): enforcing=1 old_enforcing=0 auid=4294967295 ses=4294967295
[   59.445985] SELinux: 8192 avtab hash slots, 1815416 rules.
[   60.812559] SELinux: 8192 avtab hash slots, 1979772 rules.

And if so, track down what's causing it.


- James
-- 
James Morris
<jmorris@namei.org>

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [patch] SELinux: trivial, unify iterator variable naming, part 3

[Vesa-Matti J Kari]

Hello,

On Thu, 24 Jul 2008, James Morris wrote:

> I'm not applying these variable renaming patches, as they peturb the code
> for no established benefit.

That's all right.

> I suggest reading the following carefully on how to submit patches:
> http://www.zipworld.com.au/~akpm/linux/patches/stuff/tpp.txt

Thanks a lot. I try to be more careful. I suppose I have to resend one
patch, as I forgot to CC the kernel list, and the patch was not really
conforming to the rules specified above, either.

> It's also generally best to simply use git to create patches (e.g. via
> git-format-patch).

OK, I have to learn how to use git.

> Something that would be particularly useful at this stage would be to see
> if you can reproduce a long delay in booting the current git tree possibly
> related to SELinux policy loading:
>
> [    6.904650] EXT3-fs: mounted filesystem with ordered data mode.
> [    7.076411] type=1404 audit(1216904882.076:2): enforcing=1 old_enforcing=0 auid=4294967295 ses=4294967295
> [   59.445985] SELinux: 8192 avtab hash slots, 1815416 rules.
> [   60.812559] SELinux: 8192 avtab hash slots, 1979772 rules.

It boots quite quickly on my Fedora 9, but the boot log shows
significantly less rules than yours. If you really have that much rules, I
guess your hash chains will have to be much longer too...

The 171021 vs 1979772 is strange, ratio being approx. 1:11.

Here is what I have:

EXT3-fs: mounted filesystem with ordered data mode.
type=1404 audit(1216911601.748:2): enforcing=1 old_enforcing=0 auid=4294967295 ses=4294967295
SELinux: 8192 avtab hash slots, 171021 rules.
SELinux: 8192 avtab hash slots, 171021 rules.


RPM on my F9:

Name        : selinux-policy               Relocations: (not relocatable)
Version     : 3.3.1                             Vendor: Fedora Project
Release     : 78.fc9                        Build Date: Mon Jul 14 23:44:19 2008
Install Date: Mon Jul 21 19:19:09 2008         Build Host: xenbuilder4.fedora.phx.redhat.com
Group       : System Environment/Base       Source RPM: selinux-policy-3.3.1-78.fc9.src.rpm
Size        : 8515318                          License: GPLv2+
Signature   : DSA/SHA1, Thu Jul 17 23:47:20 2008, Key ID b44269d04f2a6fd2
Packager    : Fedora Project
URL         : http://serefpolicy.sourceforge.net
Summary     : SELinux policy configuration
Description :
SELinux Reference Policy - modular.
Based off of reference policy: Checked out revision 2624.

> And if so, track down what's causing it.

After the long boot delay, do the SELinux checks work as expected? Any
other weirdness in the boot or audit logs?

Best regards,
vmk
-- 
************************************************************************
               Tietotekniikkaosasto / Helsingin yliopisto
                 IT Department / University of Helsinki
************************************************************************

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [patch] SELinux: trivial, unify iterator variable naming, part 3

[Stephen Smalley]

On Thu, 2008-07-24 at 19:00 +0300, Vesa-Matti J Kari wrote:
> Hello,
> 
> On Thu, 24 Jul 2008, James Morris wrote:
> 
> > I'm not applying these variable renaming patches, as they peturb the code
> > for no established benefit.
> 
> That's all right.
> 
> > I suggest reading the following carefully on how to submit patches:
> > http://www.zipworld.com.au/~akpm/linux/patches/stuff/tpp.txt
> 
> Thanks a lot. I try to be more careful. I suppose I have to resend one
> patch, as I forgot to CC the kernel list, and the patch was not really
> conforming to the rules specified above, either.
> 
> > It's also generally best to simply use git to create patches (e.g. via
> > git-format-patch).
> 
> OK, I have to learn how to use git.
> 
> > Something that would be particularly useful at this stage would be to see
> > if you can reproduce a long delay in booting the current git tree possibly
> > related to SELinux policy loading:
> >
> > [    6.904650] EXT3-fs: mounted filesystem with ordered data mode.
> > [    7.076411] type=1404 audit(1216904882.076:2): enforcing=1 old_enforcing=0 auid=4294967295 ses=4294967295
> > [   59.445985] SELinux: 8192 avtab hash slots, 1815416 rules.
> > [   60.812559] SELinux: 8192 avtab hash slots, 1979772 rules.
> 
> It boots quite quickly on my Fedora 9, but the boot log shows
> significantly less rules than yours. If you really have that much rules, I
> guess your hash chains will have to be much longer too...
> 
> The 171021 vs 1979772 is strange, ratio being approx. 1:11.
> 
> Here is what I have:
> 
> EXT3-fs: mounted filesystem with ordered data mode.
> type=1404 audit(1216911601.748:2): enforcing=1 old_enforcing=0 auid=4294967295 ses=4294967295
> SELinux: 8192 avtab hash slots, 171021 rules.
> SELinux: 8192 avtab hash slots, 171021 rules.

Yes, that looks more reasonable than James' output.
James - semodule -l output?  rpm -V selinux-policy-targeted output?
If you move aside the policy.N file and run semodule -B, do you end up
with the same size policy or one that is more reasonable?

Also, libsepol version is of interest here.



--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [patch] SELinux: trivial, unify iterator variable naming, part 3

[James Morris]

Turns out it was caused by 
CONFIG_SECURITY_SELINUX_POLICYDB_VERSION_MAX_VALUE being set to the 
default of 19.

After setting it to 22 (same as the Fedora kernel), the problem went away.  


- James
-- 
James Morris
<jmorris@namei.org>

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [patch] SELinux: trivial, unify iterator variable naming, part 3

[Stephen Smalley]

On Fri, 2008-07-25 at 23:03 +1000, James Morris wrote:
> Turns out it was caused by 
> CONFIG_SECURITY_SELINUX_POLICYDB_VERSION_MAX_VALUE being set to the 
> default of 19.
> 
> After setting it to 22 (same as the Fedora kernel), the problem went away.  

Makes sense - policy.19 predates the avtab memory optimization work I
did, and requires the policy toolchain to fully expand all
attribute-based rules into individual type pairs.  So that shows how
much memory we are saving from that particular optimization today.



--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [patch] SELinux: trivial, unify iterator variable naming, part 3

[James Morris]

On Fri, 25 Jul 2008, Stephen Smalley wrote:

> On Fri, 2008-07-25 at 23:03 +1000, James Morris wrote:
> > Turns out it was caused by 
> > CONFIG_SECURITY_SELINUX_POLICYDB_VERSION_MAX_VALUE being set to the 
> > default of 19.
> > 
> > After setting it to 22 (same as the Fedora kernel), the problem went away.  
> 
> Makes sense - policy.19 predates the avtab memory optimization work I
> did, and requires the policy toolchain to fully expand all
> attribute-based rules into individual type pairs.  So that shows how
> much memory we are saving from that particular optimization today.

Should we bump that value so that kernel developers don't hit the same 
problem if they have SELinux enabled?  (Many would assume the boot hung).


-- 
James Morris
<jmorris@namei.org>

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [patch] SELinux: trivial, unify iterator variable naming, part 3

[Stephen Smalley]

On Sat, 2008-07-26 at 00:47 +1000, James Morris wrote:
> On Fri, 25 Jul 2008, Stephen Smalley wrote:
> 
> > On Fri, 2008-07-25 at 23:03 +1000, James Morris wrote:
> > > Turns out it was caused by 
> > > CONFIG_SECURITY_SELINUX_POLICYDB_VERSION_MAX_VALUE being set to the 
> > > default of 19.
> > > 
> > > After setting it to 22 (same as the Fedora kernel), the problem went away.  
> > 
> > Makes sense - policy.19 predates the avtab memory optimization work I
> > did, and requires the policy toolchain to fully expand all
> > attribute-based rules into individual type pairs.  So that shows how
> > much memory we are saving from that particular optimization today.
> 
> Should we bump that value so that kernel developers don't hit the same 
> problem if they have SELinux enabled?  (Many would assume the boot hung).

No - the whole point of that config option is to avoid breakage on
Fedora 3 and 4, as noted in the help text.  And the option on which it
depends defaults to n and thus shouldn't be enabled for anyone by
default.

As to whether or not we need to care about Fedora 3 and 4 anymore is
perhaps a reasonable question; if not, then the entire option could go
away.



--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [patch] SELinux: trivial, unify iterator variable naming, part 3

[Paul Moore]

On Friday 25 July 2008 8:45:26 pm Stephen Smalley wrote:
> On Sat, 2008-07-26 at 00:47 +1000, James Morris wrote:
> > On Fri, 25 Jul 2008, Stephen Smalley wrote:
> > > On Fri, 2008-07-25 at 23:03 +1000, James Morris wrote:
> > > > Turns out it was caused by
> > > > CONFIG_SECURITY_SELINUX_POLICYDB_VERSION_MAX_VALUE being set to
> > > > the default of 19.
> > > >
> > > > After setting it to 22 (same as the Fedora kernel), the problem
> > > > went away.
> > >
> > > Makes sense - policy.19 predates the avtab memory optimization
> > > work I did, and requires the policy toolchain to fully expand all
> > > attribute-based rules into individual type pairs.  So that shows
> > > how much memory we are saving from that particular optimization
> > > today.
> >
> > Should we bump that value so that kernel developers don't hit the
> > same problem if they have SELinux enabled?  (Many would assume the
> > boot hung).
>
> No - the whole point of that config option is to avoid breakage on
> Fedora 3 and 4, as noted in the help text.  And the option on which
> it depends defaults to n and thus shouldn't be enabled for anyone by
> default.
>
> As to whether or not we need to care about Fedora 3 and 4 anymore is
> perhaps a reasonable question; if not, then the entire option could
> go away.

I'm thinking of Andrew Morton's crufty old Fedora Core 2 laptop right 
now ...

-- 
paul moore
linux @ hp

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [patch] SELinux: trivial, unify iterator variable naming, part 3

[Stephen Smalley]

On Sat, 2008-07-26 at 10:23 -0400, Paul Moore wrote:
> On Friday 25 July 2008 8:45:26 pm Stephen Smalley wrote:
> > On Sat, 2008-07-26 at 00:47 +1000, James Morris wrote:
> > > On Fri, 25 Jul 2008, Stephen Smalley wrote:
> > > > On Fri, 2008-07-25 at 23:03 +1000, James Morris wrote:
> > > > > Turns out it was caused by
> > > > > CONFIG_SECURITY_SELINUX_POLICYDB_VERSION_MAX_VALUE being set to
> > > > > the default of 19.
> > > > >
> > > > > After setting it to 22 (same as the Fedora kernel), the problem
> > > > > went away.
> > > >
> > > > Makes sense - policy.19 predates the avtab memory optimization
> > > > work I did, and requires the policy toolchain to fully expand all
> > > > attribute-based rules into individual type pairs.  So that shows
> > > > how much memory we are saving from that particular optimization
> > > > today.
> > >
> > > Should we bump that value so that kernel developers don't hit the
> > > same problem if they have SELinux enabled?  (Many would assume the
> > > boot hung).
> >
> > No - the whole point of that config option is to avoid breakage on
> > Fedora 3 and 4, as noted in the help text.  And the option on which
> > it depends defaults to n and thus shouldn't be enabled for anyone by
> > default.
> >
> > As to whether or not we need to care about Fedora 3 and 4 anymore is
> > perhaps a reasonable question; if not, then the entire option could
> > go away.
> 
> I'm thinking of Andrew Morton's crufty old Fedora Core 2 laptop right 
> now ...

The last selinux bug reports I got from Andrew were only for Fedora 5,
so I'm hoping he has retired anything before F5.

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.