[haiku-bugs] [Haiku] #6750: chmod is broken for non-superuser
- From: "grahamh" <trac@xxxxxxxxxxxx>
- Date: Mon, 25 Oct 2010 21:14:51 -0000
#6750: chmod is broken for non-superuser
---------------------------------+-----------------------------
Reporter: grahamh | Owner: axeld
Type: bug | Status: new
Priority: normal | Milestone: R1
Component: File Systems/BFS | Version: R1/Development
Keywords: chmod, write_stat | Blocked By:
Has a Patch: 0 | Platform: All
Blocking: |
---------------------------------+-----------------------------
== Description ==
Permissions checking doesn't produce correct POSIX behaviour for chmod
(and probably chown, etc). This won't be noticed from the desktop, as
Haiku's default user is a superuser. However, when you try to run
something like a CGI script from apache, which needs to run as a non-
privileged user, it can lose control of files it creates. For example,
this causes a Foswiki install to break.
== How to reproduce ==
{{{
/data> uname -a
Haiku shredder 1 r39138 Oct 25 2010 07:43:53 BePC Haiku
# make a directory
/data> mkdir j
# and set it to be owned by user "sshd"
/data> chown sshd:users j
# log in as "sshd"
/data> su -l -s bash sshd
Welcome to the Haiku shell.
# go back to the directory, and make a file
/var/empty> cd /data/j
/data/j> touch file
/data/j> ls -l file
-rw-r--r-- 1 sshd users 0 Oct 25 21:38 file
# set it to read-only
/data/j> chmod 444 file
/data/j> ls -l file
-r--r--r-- 1 sshd users 0 Oct 25 21:38 file
# now set it back to writable
/data/j> chmod 644 file
chmod: changing permissions of `file': Operation not allowed
# uh-oh...
}}}
== Analysis ==
The bfs add-on has a single CheckPermissions function (part of the Inode
object), which is supposed to decide whether any given operation that the
VFS layer has passed in is allowed by the inode's read-write flags. It's
called from a number of hooks in kernel_interface.cpp, including the
bfs_write_stat hook which implements chmod (and several other things).
However, chmod() is a special case for access permissions compared to
other hooks such as open(). POSIX states that a file's owner should always
be able to set it back to writable - otherwise it's possible to put a file
in a state where you can no longer use it.
Note: the same issue is present in ext2fs, and possibly others.
== Suggested fix ==
The following patch makes it work OK for me (for BFS), although I doubt it
conforms to any style guides :-)
{{{
Index: Inode.cpp
===================================================================
--- Inode.cpp (revision 38949)
+++ Inode.cpp (working copy)
@@ -487,7 +487,7 @@
status_t
-Inode::CheckPermissions(int accessMode) const
+Inode::CheckPermissions(int accessMode, int write_stat /* optional */)
const
{
uid_t user = geteuid();
gid_t group = getegid();
@@ -505,6 +505,11 @@
}
}
+ // users are always allowed to chmod() files they own
+ if (write_stat && user == (uid_t)fNode.UserID()) {
+ return B_OK;
+ }
+
// shift mode bits, to check directly against accessMode
mode_t mode = Mode();
if (user == (uid_t)fNode.UserID())
Index: kernel_interface.cpp
===================================================================
--- kernel_interface.cpp (revision 38949)
+++ kernel_interface.cpp (working copy)
@@ -777,7 +777,14 @@
// TODO: we should definitely check a bit more if the new stats
are
// valid - or even better, the VFS should check this before
calling us
- status_t status = inode->CheckPermissions(W_OK);
+ status_t status;
+ // chmod, chown need to allow a file's owner to change its
+ // permissions even if it's set to read-only
+ const uint32 ownership = B_STAT_MODE | B_STAT_GID | B_STAT_UID;
+ if ((mask & ownership) && !(mask & ~ownership))
+ status = inode->CheckPermissions(W_OK, true);
+ else
+ status = inode->CheckPermissions(W_OK);
if (status < B_OK)
RETURN_ERROR(status);
Index: Inode.h
===================================================================
--- Inode.h (revision 38949)
+++ Inode.h (working copy)
@@ -99,7 +99,7 @@
Volume* GetVolume() const
{ return fVolume; }
- status_t
CheckPermissions(int accessMode) const;
+ status_t
CheckPermissions(int accessMode, int write_stat = false) const;
// small_data access methods
small_data*
FindSmallData(const bfs_inode* node,
}}}
--
Ticket URL: <http://dev.haiku-os.org/ticket/6750>
Haiku <http://dev.haiku-os.org>
Haiku - the operating system.
Other related posts:
