procfs permissions on 2.6.x

procfs permissions on 2.6.x

[Herbert Poetzl]

Hi Andrew!

stumbled over the following detail ...

usually when somebody tries to modify an inode,
notify_change() calls inode_change_ok() to verify
the user's permissions ... now it seems that
somewhere around 2.5.41, a patch similar to this
one was included into the mainline, and remained
almost unmodified ...

http://www.uwsg.iu.edu/hypermail/linux/kernel/0210.1/1002.html

this probably unintentionally circumvents the 
inode_change_ok() check, so that now any user
can modify inodes of the procfs. 

example:

  $ chmod a-rwx /proc/cmdline

the following patch hopefully fixes this, so
please consider for inclusion ...

TIA,
Herbert


diff -NurpP --minimal linux-2.6.7/fs/proc/generic.c linux-2.6.7-fix/fs/proc/generic.c
--- linux-2.6.7/fs/proc/generic.c	2004-06-16 07:20:26.000000000 +0200
+++ linux-2.6.7-fix/fs/proc/generic.c	2004-07-03 21:50:30.000000000 +0200
@@ -241,8 +241,20 @@ static int proc_notify_change(struct den
 	return error;
 }
 
+static int proc_setattr(struct dentry *dentry, struct iattr *iattr)
+{
+        struct inode *inode = dentry->d_inode;
+        int error;
+
+        error = inode_change_ok(inode, iattr);
+        if (error)
+                return error;
+	error = proc_notify_change(dentry, iattr);
+	return error;
+}
+
 static struct inode_operations proc_file_inode_operations = {
-	.setattr	= proc_notify_change,
+	.setattr	= proc_setattr,
 };
 
 /*
@@ -472,7 +484,7 @@ static struct file_operations proc_dir_o
  */
 static struct inode_operations proc_dir_inode_operations = {
 	.lookup		= proc_lookup,
-	.setattr	= proc_notify_change,
+	.setattr	= proc_setattr,
 };
 
 static int proc_register(struct proc_dir_entry * dir, struct proc_dir_entry * dp)

Re: procfs permissions on 2.6.x

[Christoph Hellwig]

On Sat, Jul 03, 2004 at 10:22:42PM +0200, Herbert Poetzl wrote:
> 
> Hi Andrew!
> 
> stumbled over the following detail ...
> 
> usually when somebody tries to modify an inode,
> notify_change() calls inode_change_ok() to verify
> the user's permissions ... now it seems that
> somewhere around 2.5.41, a patch similar to this
> one was included into the mainline, and remained
> almost unmodified ...
> 
> http://www.uwsg.iu.edu/hypermail/linux/kernel/0210.1/1002.html
> 
> this probably unintentionally circumvents the 
> inode_change_ok() check, so that now any user
> can modify inodes of the procfs. 
> 
> example:
> 
>   $ chmod a-rwx /proc/cmdline
> 
> the following patch hopefully fixes this, so
> please consider for inclusion ...

Actually the patch you reference above looks extremly bogus and should just
be reverted instead.

Re: procfs permissions on 2.6.x

[Andrew Morton]

Christoph Hellwig <hch@infradead.org> wrote:
>
> On Sat, Jul 03, 2004 at 10:22:42PM +0200, Herbert Poetzl wrote:
> > 
> > Hi Andrew!
> > 
> > stumbled over the following detail ...
> > 
> > usually when somebody tries to modify an inode,
> > notify_change() calls inode_change_ok() to verify
> > the user's permissions ... now it seems that
> > somewhere around 2.5.41, a patch similar to this
> > one was included into the mainline, and remained
> > almost unmodified ...
> > 
> > http://www.uwsg.iu.edu/hypermail/linux/kernel/0210.1/1002.html
> > 
> > this probably unintentionally circumvents the 
> > inode_change_ok() check, so that now any user
> > can modify inodes of the procfs. 
> > 
> > example:
> > 
> >   $ chmod a-rwx /proc/cmdline
> > 
> > the following patch hopefully fixes this, so
> > please consider for inclusion ...
> 
> Actually the patch you reference above looks extremly bogus and should just
> be reverted instead.

Why is it "extremely bogus"?  I assume Olaf had a reason for wanting chmod
on procfs files?

Re: procfs permissions on 2.6.x

[Christoph Hellwig]

> > Actually the patch you reference above looks extremly bogus and should just
> > be reverted instead.
> 
> Why is it "extremely bogus"?  I assume Olaf had a reason for wanting chmod
> on procfs files?

Because it turns the question what permissions a procfs file has into
a lottery game.  He only changes the incore inode owner and as soon as
the inode is reclaimed the old ones return.

Re: procfs permissions on 2.6.x

[Andrew Morton]

Christoph Hellwig <hch@infradead.org> wrote:
>
> > > Actually the patch you reference above looks extremly bogus and should just
> > > be reverted instead.
> > 
> > Why is it "extremely bogus"?  I assume Olaf had a reason for wanting chmod
> > on procfs files?
> 
> Because it turns the question what permissions a procfs file has into
> a lottery game.  He only changes the incore inode owner and as soon as
> the inode is reclaimed the old ones return.

procfs inodes aren't reclaimable.

chrisw fixed this bug a couple of days ago.

Re: procfs permissions on 2.6.x

[bert hubert]

On Sat, Jul 03, 2004 at 01:35:56PM -0700, Andrew Morton wrote:

> Why is it "extremely bogus"?  I assume Olaf had a reason for wanting chmod
> on procfs files?

On a related note:
ahu@d800:/sys/devices/system/cpu/cpu0/cpufreq$ ls -l
total 0
-r--------    1 ahu      ahu          4096 2004-07-04 03:18 cpuinfo_cur_freq
-r--r--r--    1 ahu      ahu          4096 2004-07-04 03:18 cpuinfo_max_freq

..

I'm not entirely sure why the current CPU frequency has suddenly become a
state secret, nor why the ownership of these files appears to be uid 1000.

-r--r--r--    1 ahu      ahu          4096 2004-07-04 03:18 cpuinfo_min_freq
-r--r--r--    1 ahu      ahu          4096 2004-07-04 03:18 scaling_available_frequencies
-r--r--r--    1 ahu      ahu          4096 2004-07-04 03:18 scaling_available_governors
-r--r--r--    1 ahu      ahu          4096 2004-07-04 03:18 scaling_cur_freq
-r--r--r--    1 ahu      ahu          4096 2004-07-04 03:18 scaling_driver
-rw-r--r--    1 ahu      ahu          4096 2004-07-04 03:18 scaling_governor
-rw-r--r--    1 ahu      ahu          4096 2004-07-04 03:18 scaling_max_freq
-rw-r--r--    1 ahu      ahu          4096 2004-07-04 03:18 scaling_min_freq

This appears to be even more bogus - uid 1000 can write to the scaling
governor. It looks like the uid that mounted /sys owns these files. Not
entirely sure what happens.

Fixes appear trivial but I wonder about the reasons.

Regards,

bert

-- 
http://www.PowerDNS.com      Open source, database driven DNS Software 
http://lartc.org           Linux Advanced Routing & Traffic Control HOWTO

Re: procfs permissions on 2.6.x

[viro]

On Sat, Jul 03, 2004 at 02:35:58PM -0700, Andrew Morton wrote:
> > Because it turns the question what permissions a procfs file has into
> > a lottery game.  He only changes the incore inode owner and as soon as
> > the inode is reclaimed the old ones return.
> 
> procfs inodes aren't reclaimable.

WTF do you mean "not reclaimable"?  They do get freed under memory pressure
and recreated on later lookups.

Re: procfs permissions on 2.6.x

[Andrew Morton]

viro@parcelfarce.linux.theplanet.co.uk wrote:
>
> On Sat, Jul 03, 2004 at 02:35:58PM -0700, Andrew Morton wrote:
> > > Because it turns the question what permissions a procfs file has into
> > > a lottery game.  He only changes the incore inode owner and as soon as
> > > the inode is reclaimed the old ones return.
> > 
> > procfs inodes aren't reclaimable.
> 
> WTF do you mean "not reclaimable"?

Got confused.

>  They do get freed under memory pressure
> and recreated on later lookups.

Some do.  On my test box 1000-odd /proc inodes get allocated and fully
freed on each `ls -R /proc'.  65 /proc inodes are freed during `ls -lR
/proc/net'.  So maybe it isn't working completely.

But proc_notify_change() copies the inode's uid, gid and mode into the
proc_dir_entry, so they get correctly initialised when the inode is
reinstantiated, so afaict we have no bug here.

Re: procfs permissions on 2.6.x

[viro]

On Sun, Jul 04, 2004 at 02:55:42PM -0700, Andrew Morton wrote:
> 
> Some do.  On my test box 1000-odd /proc inodes get allocated and fully
> freed on each `ls -R /proc'.  65 /proc inodes are freed during `ls -lR
> /proc/net'.  So maybe it isn't working completely.
> 
> But proc_notify_change() copies the inode's uid, gid and mode into the
> proc_dir_entry, so they get correctly initialised when the inode is
> reinstantiated, so afaict we have no bug here.

Why on the earth do we ever want to allow chown/chmod on procfs in the first
place?

Al, who'd missed that stuff back in 2.5.42, but would love to hear explanation
anyway.

Re: procfs permissions on 2.6.x

[Andi Kleen]

viro@parcelfarce.linux.theplanet.co.uk writes:
>
> Why on the earth do we ever want to allow chown/chmod on procfs in the first
> place?

It's useful to stop other people from reading your environment
or command line.

-Andi

Re: procfs permissions on 2.6.x

[FabF]

On Mon, 2004-07-05 at 00:25, Andi Kleen wrote:
> viro@parcelfarce.linux.theplanet.co.uk writes:
> >
> > Why on the earth do we ever want to allow chown/chmod on procfs in the first
> > place?
> 
> It's useful to stop other people from reading your environment
> or command line.
We wouldn't need that feature if mods are accurately defined from the
beginning (ie at creation) at that's already the case AFAICS.

FabF

Re: procfs permissions on 2.6.x

[Andrew Morton]

viro@parcelfarce.linux.theplanet.co.uk wrote:
>
> On Sun, Jul 04, 2004 at 02:55:42PM -0700, Andrew Morton wrote:
> > 
> > Some do.  On my test box 1000-odd /proc inodes get allocated and fully
> > freed on each `ls -R /proc'.  65 /proc inodes are freed during `ls -lR
> > /proc/net'.  So maybe it isn't working completely.
> > 
> > But proc_notify_change() copies the inode's uid, gid and mode into the
> > proc_dir_entry, so they get correctly initialised when the inode is
> > reinstantiated, so afaict we have no bug here.
> 
> Why on the earth do we ever want to allow chown/chmod on procfs in the first
> place?

We always have done, even current 2.4 permits it.  But the changes go away
when the /proc file is closed.

> Al, who'd missed that stuff back in 2.5.42, but would love to hear explanation
> anyway.

That change made the chown/chgrp/chmod changes persist after the file is
closed, which seems sensible.

The alternative would be to disallow these changes altogether.  That might
break something, but it seems doubtful.

As for why anyone would _want_ to change these things: no idea.

Re: procfs permissions on 2.6.x

[Paul Jackson]

> if mods are accurately defined from the beginning

Is there a single "accurate" definition (of /proc file permissions)
on which we agree?  I suspect not.

-- 
                          I won't rest till it's the best ...
                          Programmer, Linux Scalability
                          Paul Jackson <pj@sgi.com> 1.650.933.1373

Re: procfs permissions on 2.6.x

[Clemens Schwaighofer]

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

viro@parcelfarce.linux.theplanet.co.uk wrote:
| On Sun, Jul 04, 2004 at 02:55:42PM -0700, Andrew Morton wrote:
|
|>Some do.  On my test box 1000-odd /proc inodes get allocated and fully
|>freed on each `ls -R /proc'.  65 /proc inodes are freed during `ls -lR
|>/proc/net'.  So maybe it isn't working completely.
|>
|>But proc_notify_change() copies the inode's uid, gid and mode into the
|>proc_dir_entry, so they get correctly initialised when the inode is
|>reinstantiated, so afaict we have no bug here.
|
|
| Why on the earth do we ever want to allow chown/chmod on procfs in the
first
| place?

Well perhaps I am on the wrong track but eg /proc/bus/usb/002/005 is my
digital camera and unless its either world rw or owned by me (user) I
can't get any pictures unless I make myself root.

So yes, I would want to have chown/chmod in procfs ...

- --
Clemens Schwaighofer - IT Engineer & System Administration
==========================================================
TEQUILA\Japan, 6-17-2 Ginza Chuo-ku, Tokyo 104-8167, JAPAN
Tel: +81-(0)3-3545-7703            Fax: +81-(0)3-3545-7343
http://www.tequila.co.jp
==========================================================
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.4 (GNU/Linux)

iD8DBQFA6LPajBz/yQjBxz8RApbcAKCfmBwm92UmiAqOZvEtZq6M215XKACg4Tbl
oF+mx5LOEd9QMrrVomg+lOY=
=qvWj
-----END PGP SIGNATURE-----

Re: procfs permissions on 2.6.x

[viro]

On Mon, Jul 05, 2004 at 10:50:19AM +0900, Clemens Schwaighofer wrote:
> Well perhaps I am on the wrong track but eg /proc/bus/usb/002/005 is my
> digital camera and unless its either world rw or owned by me (user) I
> can't get any pictures unless I make myself root.
> 
> So yes, I would want to have chown/chmod in procfs ...

... except that /proc/bus/usb/* is not in procfs.  BTW, it makes more sense
to change GID of the file in question and make it group-writable, adding the
users who are allowed to mess with it into the group.

Re: procfs permissions on 2.6.x

[Duncan Sands]

> Well perhaps I am on the wrong track but eg /proc/bus/usb/002/005 is my
> digital camera and unless its either world rw or owned by me (user) I
> can't get any pictures unless I make myself root.
> 
> So yes, I would want to have chown/chmod in procfs ...

Hi Clemens, that isn't procfs, it's usbfs.  It's been plonked (*) on top of a procfs
directory, but that doesn't matter.

Ciao,

Duncan.

(*)  I believe the technical term is "mounted".

Re: procfs permissions on 2.6.x

[Clemens Schwaighofer]

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

Duncan Sands wrote:
|>Well perhaps I am on the wrong track but eg /proc/bus/usb/002/005 is my
|>digital camera and unless its either world rw or owned by me (user) I
|>can't get any pictures unless I make myself root.
|>
|>So yes, I would want to have chown/chmod in procfs ...
|
|
| Hi Clemens, that isn't procfs, it's usbfs.  It's been plonked (*) on
top of a procfs
| directory, but that doesn't matter.

total forgot about that :) Because if you don't compile the kernel with
usbfs you don't see it there ...

- --
Clemens Schwaighofer - IT Engineer & System Administration
==========================================================
TEQUILA\Japan, 6-17-2 Ginza Chuo-ku, Tokyo 104-8167, JAPAN
Tel: +81-(0)3-3545-7703            Fax: +81-(0)3-3545-7343
http://www.tequila.co.jp
==========================================================
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.4 (GNU/Linux)

iD8DBQFA6Q3KjBz/yQjBxz8RArACAKDNr+SXs20yVvWdWBYGsbpCAkMqwACgqO2S
srwaWJ2q6Wd0j5AOB4Y5UZM=
=kcN9
-----END PGP SIGNATURE-----

Re: procfs permissions on 2.6.x

[Andy Lutomirski]

Andrew Morton wrote:
>>Why on the earth do we ever want to allow chown/chmod on procfs in the first
>>place?

[...]

> 
> As for why anyone would _want_ to change these things: no idea.

Because I don't want the whole world reading my conntrack state.  And 
I'm sure other people have other opinions there and on other things, so 
that's why we should be able to change it.

--Andy