/proc parent &proc_root == NULL?

/proc parent &proc_root == NULL?

[John Richard Moser]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

proc_misc_init() has both these lines in it:

entry = create_proc_entry("kmsg", S_IRUSR, &proc_root);
proc_root_kcore = create_proc_entry("kcore", S_IRUSR, NULL);

Both entries show up in /proc, as /proc/kmsg and /proc/kcore.  So I ask,
as I can't see after several minutes of examination, what's the
difference?  Why is NULL used for some and &proc_root used for others?

I'm looking at 2.6.10

- --
All content of all messages exchanged herein are left in the
Public Domain, unless otherwise explicitly stated.

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.0 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFB+CIYhDd4aOud5P8RAsVPAKCIzjicPM4e9FrAOFUgf3JJIV8GgACfWh4a
iX1Z8mKOX7RRzHWVnKhx1mQ=
=+MqB
-----END PGP SIGNATURE-----

Re: /proc parent &proc_root == NULL?

[Randy.Dunlap]

John Richard Moser wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> proc_misc_init() has both these lines in it:
> 
> entry = create_proc_entry("kmsg", S_IRUSR, &proc_root);
> proc_root_kcore = create_proc_entry("kcore", S_IRUSR, NULL);
> 
> Both entries show up in /proc, as /proc/kmsg and /proc/kcore.  So I ask,
> as I can't see after several minutes of examination, what's the
> difference?  Why is NULL used for some and &proc_root used for others?
> 
> I'm looking at 2.6.10

create_proc_entry() passes &parent to proc_create().
See proc_create():
...
This is an error path:
	if (!(*parent) && xlate_proc_name(name, parent, &fn) != 0)
		goto out;
but xlate_proc_name() searches for a /proc/.... and returns the 
all-but-final-part-of-name *parent (hope that makes some sense,
see the comments above the function), so it returns &proc_root.

HTH.  If not, fire back.
-- 
~Randy

Re: /proc parent &proc_root == NULL?

[John Richard Moser]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1



Randy.Dunlap wrote:
> John Richard Moser wrote:
> 
>> -----BEGIN PGP SIGNED MESSAGE-----
>> Hash: SHA1
>>
>> proc_misc_init() has both these lines in it:
>>
>> entry = create_proc_entry("kmsg", S_IRUSR, &proc_root);
>> proc_root_kcore = create_proc_entry("kcore", S_IRUSR, NULL);
>>
>> Both entries show up in /proc, as /proc/kmsg and /proc/kcore.  So I ask,
>> as I can't see after several minutes of examination, what's the
>> difference?  Why is NULL used for some and &proc_root used for others?
>>
>> I'm looking at 2.6.10
> 
> 
> create_proc_entry() passes &parent to proc_create().
> See proc_create():
> ...
> This is an error path:
>     if (!(*parent) && xlate_proc_name(name, parent, &fn) != 0)
>         goto out;
> but xlate_proc_name() searches for a /proc/.... and returns the
> all-but-final-part-of-name *parent (hope that makes some sense,
> see the comments above the function), so it returns &proc_root.
> 
> HTH.  If not, fire back.

create_proc_entry("kmsg", S_IRUSR, &proc_root);

So this is asking for proc_root to be filled?

create_proc_entry("kcore", S_IRUSR, NULL);

And this is just saying to shove it in proc's root?


I'm trying to locate a specific proc entry, using this lovely piece of
code I ripped off:

/*
 * Find a proc entry
 * Duplicated from remove_proc_entry()
 */
struct proc_dir_entry **get_proc_entry(const char *name, struct
proc_dir_entry *parent) {
        struct proc_dir_entry **p;
        const char *fn = name;
        int len;
        if (!parent && xlate_proc_name(name, &parent, &fn) != 0)
                goto out;
        len = strlen(fn);
        for (p = &parent->subdir; *p; p=&(*p)->next ) {
                if (!proc_match(len, fn, *p))
                        continue;
                return p;
        }
out:
        return NULL;
}


And I'm trying to figure out if, say, /proc/devices would be found by...

get_proc_entry("devices",NULL);
- -OR-
get_proc_entry("devices",&proc_root);

Oh well.  I'll figure it out eventually.  :)  I've already caused my
kernel to not boot :) figured it out too, it was that very function
above; I replaced a chunk of remove_proc_entry() with a modified version
of that and I'd busted it horribly so it didn't work.  Just more things
to remind me that I know not what it is I do.

- --
All content of all messages exchanged herein are left in the
Public Domain, unless otherwise explicitly stated.

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.0 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFB+FMLhDd4aOud5P8RAp1XAJ9j+ezlZgYuXpTmeaNSlQcC3xkb+ACaAjA8
D3NEZH4Drey2nuMCXZwK6sE=
=o5P7
-----END PGP SIGNATURE-----

Re: /proc parent &proc_root == NULL?

[Al Viro]

On Wed, Jan 26, 2005 at 09:33:48PM -0500, John Richard Moser wrote:
> create_proc_entry("kmsg", S_IRUSR, &proc_root);
> 
> So this is asking for proc_root to be filled?
> 
> create_proc_entry("kcore", S_IRUSR, NULL);
> 
> And this is just saying to shove it in proc's root?

NULL is equivalent to &proc_root in that context; moreover, it's better
style - drivers really shouldn't be refering to what is procfs-private
object.

> I'm trying to locate a specific proc entry, using this lovely piece of
> code I ripped off:

That's not something allowed outside of procfs code - lifetime rules
alone make that a Very Bad Idea(tm).  If that's just debugging - OK,
but if your code really uses that stuff, I want details on the intended
use.  In that case your design is almost certainly asking for trouble.

Re: /proc parent &proc_root == NULL?

[John Richard Moser]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1



Al Viro wrote:
> On Wed, Jan 26, 2005 at 09:33:48PM -0500, John Richard Moser wrote:
> 
>>create_proc_entry("kmsg", S_IRUSR, &proc_root);
>>
>>So this is asking for proc_root to be filled?
>>
>>create_proc_entry("kcore", S_IRUSR, NULL);
>>
>>And this is just saying to shove it in proc's root?
> 
> 
> NULL is equivalent to &proc_root in that context; moreover, it's better
> style - drivers really shouldn't be refering to what is procfs-private
> object.
> 
> 
>>I'm trying to locate a specific proc entry, using this lovely piece of
>>code I ripped off:
> 
> 
> That's not something allowed outside of procfs code - lifetime rules
> alone make that a Very Bad Idea(tm).  If that's just debugging - OK,
> but if your code really uses that stuff, I want details on the intended
> use.  In that case your design is almost certainly asking for trouble.
> 

I'm trying to implement parts of grsecurity on top of a framework
similar to LSM (i.e. ripoff) that I wrote as an academic endeavor to
find out how stuff works and get some real-life kernel coding experience.

This particular problem pertains to proc_misc.c and trying to create a
hook for some grsecurity protections that alter the modes on certain
/proc entries.  The chunk of the patch I'm trying to immitate is:


diff -urNp linux-2.6.10/fs/proc/proc_misc.c linux-2.6.10/fs/proc/proc_misc.c
- --- linux-2.6.10/fs/proc/proc_misc.c    2004-12-24 16:34:00 -0500
+++ linux-2.6.10/fs/proc/proc_misc.c    2005-01-08 15:53:52 -0500
@@ -582,6 +582,8 @@ static void create_seq_entry(char *name,
 void __init proc_misc_init(void)
 {
        struct proc_dir_entry *entry;
+       int gr_mode = 0;
+
        static struct {
                char *name;
                int (*read_proc)(char*,char**,off_t,int,int*,void*);
@@ -596,9 +598,13 @@ void __init proc_misc_init(void)
 #ifdef CONFIG_STRAM_PROC
                {"stram",       stram_read_proc},
 #endif
+#ifndef CONFIG_GRKERNSEC_PROC_ADD
                {"devices",     devices_read_proc},
+#endif
                {"filesystems", filesystems_read_proc},
+#ifndef CONFIG_GRKERNSEC_PROC_ADD
                {"cmdline",     cmdline_read_proc},
+#endif
                {"locks",       locks_read_proc},
                {"execdomains", execdomains_read_proc},
                {NULL,}
@@ -606,27 +612,45 @@ void __init proc_misc_init(void)
        for (p = simple_ones; p->name; p++)
                create_proc_read_entry(p->name, 0, NULL, p->read_proc,
NULL);

+#ifdef CONFIG_GRKERNSEC_PROC_USER
+       gr_mode = S_IRUSR;
+#elif CONFIG_GRKERNSEC_PROC_USERGROUP
+       gr_mode = S_IRUSR | S_IRGRP;
+#endif
+#ifdef CONFIG_GRKERNSEC_PROC_ADD
+       create_proc_read_entry("devices", gr_mode, NULL,
&devices_read_proc, NULL);
+       create_proc_read_entry("cmdline", gr_mode, NULL,
&cmdline_read_proc, NULL);
+#endif
+
        proc_symlink("mounts", NULL, "self/mounts");

        /* And now for trickier ones */
        entry = create_proc_entry("kmsg", S_IRUSR, &proc_root);
        if (entry)
                entry->proc_fops = &proc_kmsg_operations;
+#ifdef CONFIG_GRKERNSEC_PROC_ADD
+       create_seq_entry("cpuinfo", gr_mode, &proc_cpuinfo_operations);
+#else
        create_seq_entry("cpuinfo", 0, &proc_cpuinfo_operations);
+#endif
        create_seq_entry("partitions", 0, &proc_partitions_operations);
        create_seq_entry("stat", 0, &proc_stat_operations);
        create_seq_entry("interrupts", 0, &proc_interrupts_operations);
+#ifdef CONFIG_GRKERNSEC_PROC_ADD
+
create_seq_entry("slabinfo",S_IWUSR|gr_mode,&proc_slabinfo_operations);
+#else

create_seq_entry("slabinfo",S_IWUSR|S_IRUGO,&proc_slabinfo_operations);
+#endif
        create_seq_entry("buddyinfo",S_IRUGO,
&fragmentation_file_operations);
        create_seq_entry("vmstat",S_IRUGO, &proc_vmstat_file_operations);
        create_seq_entry("diskstats", 0, &proc_diskstats_operations);
 #ifdef CONFIG_MODULES
- -       create_seq_entry("modules", 0, &proc_modules_operations);
+       create_seq_entry("modules", gr_mode, &proc_modules_operations);
 #endif
 #ifdef CONFIG_SCHEDSTATS
        create_seq_entry("schedstat", 0, &proc_schedstat_operations);
 #endif
- -#ifdef CONFIG_PROC_KCORE
+#if defined(CONFIG_PROC_KCORE) && !defined(CONFIG_GRKERNSEC_PROC_ADD)
        proc_root_kcore = create_proc_entry("kcore", S_IRUSR, NULL);
        if (proc_root_kcore) {
                proc_root_kcore->proc_fops = &proc_kcore_operations;


The above I've been trying to create a security hook for to test out.
I've learned much already, like that breaking proc breaks your system
and causes panics.  :)

- --
All content of all messages exchanged herein are left in the
Public Domain, unless otherwise explicitly stated.

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.0 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFB+GF1hDd4aOud5P8RAvwKAJ9nPyuifIDloGyNGwNMuCfGvXMKswCgkIHE
kCr8U80DJJWfRSVJTZbXaMs=
=MDiz
-----END PGP SIGNATURE-----

Re: /proc parent &proc_root == NULL?

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 1156 bytes --]

On Wed, 26 Jan 2005 22:35:18 EST, John Richard Moser said:

> This particular problem pertains to proc_misc.c and trying to create a
> hook for some grsecurity protections that alter the modes on certain
> /proc entries.  The chunk of the patch I'm trying to immitate is:

> +#ifdef CONFIG_GRKERNSEC_PROC_ADD
> +       create_seq_entry("cpuinfo", gr_mode, &proc_cpuinfo_operations);
> +#else
>         create_seq_entry("cpuinfo", 0, &proc_cpuinfo_operations);
> +#endif

An alternate way to approach this - leave the permissions alone here.

And then use the security_ops->inode_permission() hook to do something like:

	if ((inode == cpuinfo) && (current->fsuid))
		 return -EPERM;

Writing the proper tests for whether it's the inode you want and whether to
give the request the kiss-of-death are left as an excersize for the programmer.. ;)

You may want to use a properly timed initcall() to create a callback that
happens after proc_misc_init() happens, but before userspace gets going, and
walk through the /proc tree at that time and cache info on the files you care
about, so you don't have to re-walk /proc every time permission() gets called....

[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

Re: /proc parent &proc_root == NULL?

[John Richard Moser]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1



Valdis.Kletnieks@vt.edu wrote:
> On Wed, 26 Jan 2005 22:35:18 EST, John Richard Moser said:
> 
> 
>>This particular problem pertains to proc_misc.c and trying to create a
>>hook for some grsecurity protections that alter the modes on certain
>>/proc entries.  The chunk of the patch I'm trying to immitate is:
> 
> 
>>+#ifdef CONFIG_GRKERNSEC_PROC_ADD
>>+       create_seq_entry("cpuinfo", gr_mode, &proc_cpuinfo_operations);
>>+#else
>>        create_seq_entry("cpuinfo", 0, &proc_cpuinfo_operations);
>>+#endif
> 
> 
> An alternate way to approach this - leave the permissions alone here.
> 
> And then use the security_ops->inode_permission() hook to do something like:
> 
> 	if ((inode == cpuinfo) && (current->fsuid))
> 		 return -EPERM;
> 
> Writing the proper tests for whether it's the inode you want and whether to
> give the request the kiss-of-death are left as an excersize for the programmer.. ;)
> 
> You may want to use a properly timed initcall() to create a callback that
> happens after proc_misc_init() happens, but before userspace gets going, and
> walk through the /proc tree at that time and cache info on the files you care
> about, so you don't have to re-walk /proc every time permission() gets called....

mmm.  I'd thought about that actually-- for modules to get a whack at
this they'd have to be compiled in.  Loaded as modules would break the
security.

Perhaps both.  I could give modules a "Hook" that gave them a crack at
/proc on load, as well as put a hook in *read**read**read**read*
proc_permission()?  (I wrote one there already!  :)

Also, before it expires

http://rafb.net/paste/results/tZ5Jp878.html

Nice for a simple learning excercise huh?  Modules aren't aware of
stacking, and there's no mandatory dummy code (a la security/dummy.c);
but each hook calls a function that does a loop (based on a C99 variadic
macro) through things, so the lack of a dummy module is kind of offset.

- --
All content of all messages exchanged herein are left in the
Public Domain, unless otherwise explicitly stated.

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.0 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFB+I9ZhDd4aOud5P8RAvDyAJ9m7KLA9+KzLg2colO3uhRXaxzOXACfQekQ
eHDZYuZ33Qtbz9q0fgaUhmw=
=k7kW
-----END PGP SIGNATURE-----

Re: /proc parent &proc_root == NULL?

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 295 bytes --]

On Thu, 27 Jan 2005 01:51:05 EST, John Richard Moser said:

> mmm.  I'd thought about that actually-- for modules to get a whack at
> this they'd have to be compiled in.  Loaded as modules would break the
> security.

And that, my friends, is *exactly* why SELinux can't be built as a module ;)

[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

Re: /proc parent &proc_root == NULL?

[John Richard Moser]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1



Valdis.Kletnieks@vt.edu wrote:
> On Thu, 27 Jan 2005 01:51:05 EST, John Richard Moser said:
> 
> 
>>mmm.  I'd thought about that actually-- for modules to get a whack at
>>this they'd have to be compiled in.  Loaded as modules would break the
>>security.
> 
> 
> And that, my friends, is *exactly* why SELinux can't be built as a module ;)

:) So far my little grkernsec module hasn't hit any bumps like that;
though so far I haven't copied much of spender's code.  I'm sure the
chroot() restrictions will easily be make for a loadable module.

At this point, I should be making more important design decisions.  For
example, why am I still doing this?  Isn't there something better for me
to do than clone LSM and GrSecurity, attempt (*cough*) to improve on the
original designs, and then harass kernel devs about problems I'm having
with things that are just meant to be toys for me anyway?


- --
All content of all messages exchanged herein are left in the
Public Domain, unless otherwise explicitly stated.

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.0 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFB+JuehDd4aOud5P8RAg+WAJ451ls4FIMG0wm/r3pa/dPpcasRugCeP5j9
be2STVV+vC2B1ScYYQNmMY0=
=IjCv
-----END PGP SIGNATURE-----

Re: /proc parent &proc_root == NULL?

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 743 bytes --]

On Thu, 27 Jan 2005 01:40:12 EST, Valdis.Kletnieks@vt.edu said:

> You may want to use a properly timed initcall() to create a callback that
> happens after proc_misc_init() happens, but before userspace gets going, and
> walk through the /proc tree at that time and cache info on the files you care
> about, so you don't have to re-walk /proc every time permission() gets called

Blarg.

The proper time to walk the /proc filesystem and cache such info, is, of
course, in the ->sp_post_addmount() LSM hook.  Check @mnt and @mountpoint_nd to
see if it's /proc, and if so, go snarf all the info you need to make your tests
fast..

Not sure if you'd need to redo your cache in sp_post_remount(), for the odd case
where /proc gets remounted....


[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]