hal: Branch 'hal-0_5_9-branch'

David Zeuthen david at kemper.freedesktop.org
Thu May 24 11:35:04 PDT 2007


 tools/hal-storage-cleanup-all-mountpoints.c |    7 ++-----
 tools/hal-storage-shared.c                  |    3 +--
 2 files changed, 3 insertions(+), 7 deletions(-)

New commits:
diff-tree e24c420820c9d33aa3c5d28a23f501f938fc1c77 (from 84ccd7f20b88397f174dae44fe97ecec26c1fc26)
Author: David Zeuthen <davidz at redhat.com>
Date:   Thu May 24 14:34:42 2007 -0400

    create lock file with predictable permissions
    
    This was initially reported as a RHEL5 bug #241032, but after analysis
    it was determine there is no security issue
    
    Steve Grubb <sgrubb at redhat.com>:
    > I was working on an IDS script and found this file:
    >
    > ls -l  /media/.hal-mtab-lock
    > --ws--Sr-x 1 root root 0 Nov 15  2006 /media/.hal-mtab-lock
    >
    > Looking at open 3p man page, if you pass the O_CREAT flag to open, it
    > will look for arg 3 to get the mode. In this case it turns out to be
    > the stack contents. I wonder how dangerous this could be if by random
    > chance its setuid root and world writable?
    
    David Zeuthen <davidz at redhat.com>:
    > Steve, thanks for catching that. Using mode 0600 is the right
    > approach; the lock file is private to HAL components all running as
    > uid 0. Also, on HAL startup we need to delete the lock file otherwise
    > it won't get recreated with the appropriate mode.
    
    David Zeuthen <davidz at redhat.com>:
    > Hmm, playing around with passing various modes in; when I pass 04777 I
    > get this
    >
    > $ ls -l /media/.hal-mtab-lock
    > -rwsr-xr-x 1 root root 0 2007-05-23 17:31 /media/.hal-mtab-lock
    >
    > which makes sense as hald calls umask(022) when it daemonizes and this is
    > inherited by the process that creates the lock file. Hence, we will never ever
    > write files in this situation that are world writable. Since the the lock file
    > is always zero bytes we're good. There is no risk of attack here.

diff --git a/tools/hal-storage-cleanup-all-mountpoints.c b/tools/hal-storage-cleanup-all-mountpoints.c
index aa8d657..a997190 100644
--- a/tools/hal-storage-cleanup-all-mountpoints.c
+++ b/tools/hal-storage-cleanup-all-mountpoints.c
@@ -162,9 +162,8 @@ do_cleanup (void)
 int
 main (int argc, char *argv[])
 {
-	if (!lock_hal_mtab ()) {
-		unknown_error ("Cannot obtain lock on /media/.hal-mtab");
-	}
+
+        unlink ("/media/.hal-mtab-lock");
 
 	if (getenv ("HAL_PROP_INFO_UDI") == NULL)
 		usage ();
@@ -174,7 +173,5 @@ main (int argc, char *argv[])
 #endif
 	do_cleanup ();
 
-
-	unlock_hal_mtab ();
 	return 0;
 }
diff --git a/tools/hal-storage-shared.c b/tools/hal-storage-shared.c
index 26e7ccd..7f2ae40 100644
--- a/tools/hal-storage-shared.c
+++ b/tools/hal-storage-shared.c
@@ -637,8 +637,7 @@ lock_hal_mtab (void)
 
 	printf ("%d: XYA attempting to get lock on /media/.hal-mtab-lock\n", getpid ());
 
-	lock_mtab_fd = open ("/media/.hal-mtab-lock", O_CREAT | O_RDWR);
-	/* TODO: set correct mode, owner etc. */
+	lock_mtab_fd = open ("/media/.hal-mtab-lock", O_CREAT | O_RDWR, 0600);
 
 	if (lock_mtab_fd < 0)
 		return FALSE;


More information about the hal-commit mailing list