2005-01-27 00:23:04

by John Richard Moser

[permalink] [raw]
Subject: /proc parent &proc_root == NULL?

-----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-----


2005-01-27 01:44:28

by Randy.Dunlap

[permalink] [raw]
Subject: Re: /proc parent &proc_root == NULL?

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

2005-01-27 02:41:22

by John Richard Moser

[permalink] [raw]
Subject: Re: /proc parent &proc_root == NULL?

-----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-----

2005-01-27 03:17:13

by Al Viro

[permalink] [raw]
Subject: Re: /proc parent &proc_root == NULL?

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.

2005-01-27 03:35:28

by John Richard Moser

[permalink] [raw]
Subject: Re: /proc parent &proc_root == NULL?

-----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-----

2005-01-27 06:41:00

by Valdis Klētnieks

[permalink] [raw]
Subject: Re: /proc parent &proc_root == NULL?

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....


Attachments:
(No filename) (226.00 B)

2005-01-27 06:50:58

by John Richard Moser

[permalink] [raw]
Subject: Re: /proc parent &proc_root == NULL?

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



[email protected] 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-----

2005-01-27 06:54:36

by Valdis Klētnieks

[permalink] [raw]
Subject: Re: /proc parent &proc_root == NULL?

On Thu, 27 Jan 2005 01:40:12 EST, [email protected] 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....


Attachments:
(No filename) (226.00 B)

2005-01-27 07:11:14

by Valdis Klētnieks

[permalink] [raw]
Subject: Re: /proc parent &proc_root == NULL?

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 ;)


Attachments:
(No filename) (226.00 B)

2005-01-27 07:43:12

by John Richard Moser

[permalink] [raw]
Subject: Re: /proc parent &proc_root == NULL?

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



[email protected] 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-----