Re: Fixing the delete queue security
On Mon, Sep 15, 2003 at 09:18:08PM -0400, Pierre A. Humblet wrote: 2003-09-15 Pierre Humblet [EMAIL PROTECTED] * shared_info.h (class user_info): New. (cygwin_user_h): New. (user_shared): New. (enum shared_locations): Replace SH_MOUNT_TABLE by SH_USER_SHARED; (mount_table): Change from variable to macro. * shared.cc: Use sizeof(user_info) in offsets. (user_shared_initialize): Add reinit argument to indicate need to reinitialize the mapping. Replace mount_table by user_shared throughout. Call user_shared-mountinfo.init and user_shared-delqueue.init. (shared_info::initialize): Do not call delqueue.init. (memory_init): Add argument to user_shared_initialize. * child_info.h (child_info::mount_h): Delete. (child_info::user_h): New. * sigpproc.cc (init_child_info): Use user_h instead of mount_h. * dcrt0.cc (_dll_crt0): Ditto. * fhandler_disk_file.cc (fhandler_disk_file::close): Use user_shared-delqueue instead of cygwin_shared-delqueue. * fhandler_virtual.cc (fhandler_virtual::close): Ditto. * syscalls.cc (close_all_files): Ditto. (unlink): Ditto. (seteuid32): Add argument to user_shared_initialize. This is ok. Please check in. You'll have to accommodate the new layout after my checkin but it should apply with only minor problems. cgf
Re: Fixing the delete queue security
On Wed, Sep 24, 2003 at 08:43:55PM -0400, Christopher Faylor wrote: Please check in. You'll have to accommodate the new layout after my checkin but it should apply with only minor problems. I just built cygwin with these changes and noticed the warnings due to MOUNT_MAGIC changing. I should have mentioned that you really do need to update the MOUNT_MAGIC value prior to checkin. All you have to do is edit the shared_info.h file. You shouldn't check things in with warnings. cgf
Re: Fixing the delete queue security
This is the second part of the patch, class cleanup and Makefile update. Pierre 2003-09-25 Pierre Humblet [EMAIL PROTECTED] * shared_info.h: Update CURR_USER_MAGIC, CURR_SHARED_MAGIC and SHARED_INFO_CB. (mount_info::cb): Delete. (mount_info::version): Delete. (shared_info::delqueue): Delete. * Makefile.in: Do magic for USER_MAGIC, class user_info, instead of for mount_info.Index: shared_info.h === RCS file: /cvs/src/src/winsup/cygwin/shared_info.h,v retrieving revision 1.36 diff -u -p -r1.36 shared_info.h --- shared_info.h 25 Sep 2003 03:06:36 - 1.36 +++ shared_info.h 25 Sep 2003 03:32:49 - @@ -43,8 +43,8 @@ class mount_item #define MAX_MOUNTS 30 #define USER_VERSION 1 // increment when mount table changes and -#define USER_VERSION_MAGIC CYGWIN_VERSION_MAGIC (MOUNT_MAGIC, USER_VERSION) -#define CURR_MOUNT_MAGIC 0x6dd73a3fU +#define USER_VERSION_MAGIC CYGWIN_VERSION_MAGIC (USER_MAGIC, USER_VERSION) +#define CURR_USER_MAGIC 0x8dc7b1d5U class reg_key; struct device; @@ -55,8 +55,6 @@ struct device; class mount_info { public: - DWORD version; - unsigned cb; DWORD sys_mount_table_counter; int nmounts; mount_item mount[MAX_MOUNTS]; @@ -147,9 +145,9 @@ public: cygwin_version.api_minor) #define SHARED_VERSION_MAGIC CYGWIN_VERSION_MAGIC (SHARED_MAGIC, SHARED_VERSION) -#define SHARED_INFO_CB 47112 +#define SHARED_INFO_CB 21008 -#define CURR_SHARED_MAGIC 0x359218a2U +#define CURR_SHARED_MAGIC 0x818f75beU /* NOTE: Do not make gratuitous changes to the names or organization of the below class. The layout is checksummed to determine compatibility between @@ -163,7 +161,6 @@ class shared_info DWORD sys_mount_table_counter; tty_list tty; - delqueue_list delqueue; void initialize (); unsigned heap_chunk_size (); }; Index: Makefile.in === RCS file: /cvs/src/src/winsup/cygwin/Makefile.in,v retrieving revision 1.138 diff -u -p -r1.138 Makefile.in --- Makefile.in 25 Sep 2003 00:37:16 - 1.138 +++ Makefile.in 25 Sep 2003 03:32:50 - @@ -377,7 +377,7 @@ version.cc winver.o: winver_stamp @ : shared_info_magic.h: cygmagic shared_info.h - /bin/sh ${word 1,$^} $@ $(CC) -x c ${word 2,$^} MOUNT_MAGIC 'class mount_info' SHARED_MAGIC 'class shared_info' + /bin/sh ${word 1,$^} $@ $(CC) -x c ${word 2,$^} USER_MAGIC 'class user_info' SHARED_MAGIC 'class shared_info' child_info_magic.h: cygmagic child_info.h /bin/sh ${word 1,$^} $@ $(CC) -x c ${word 2,$^} CHILD_INFO_MAGIC 'class child_info'
RE: Fixing the delete queue security
You are a *God*, Pierre. ;-) -- Gary R. Van Sickle Brewer. Patriot. Cygwin uses a delete queue in a shared file mapping to hold the names of files that could not be deleted on unlink, usually because they were still opened. The queue is scanned by all processes so that the files eventually get deleted after they are closed. Because Everyone has write access to the file mapping, any user can add names to the delete queue, and thus any user can trick other processes into deleting any and all files on a PC where a cygwin daemon is running as SYSTEM. The solution is simple: create per user delete queues. They are placed in the same mapping as the mount table. So the change is extremely straightforward. The length of the change log comes from renaming many variable to have names reflect functions. There will be a follow up patch with the following cleanup: remove now unneeded fields from the mount_info and shared_info and run the magic on the new/modified structures. Pierre 2003-09-15 Pierre Humblet [EMAIL PROTECTED] * shared_info.h (class user_info): New. (cygwin_user_h): New. (user_shared): New. (enum shared_locations): Replace SH_MOUNT_TABLE by SH_USER_SHARED; (mount_table): Change from variable to macro. * shared.cc: Use sizeof(user_info) in offsets. (user_shared_initialize): Add reinit argument to indicate need to reinitialize the mapping. Replace mount_table by user_shared throughout. Call user_shared-mountinfo.init and user_shared-delqueue.init. (shared_info::initialize): Do not call delqueue.init. (memory_init): Add argument to user_shared_initialize. * child_info.h (child_info::mount_h): Delete. (child_info::user_h): New. * sigpproc.cc (init_child_info): Use user_h instead of mount_h. * dcrt0.cc (_dll_crt0): Ditto. * fhandler_disk_file.cc (fhandler_disk_file::close): Use user_shared-delqueue instead of cygwin_shared-delqueue. * fhandler_virtual.cc (fhandler_virtual::close): Ditto. * syscalls.cc (close_all_files): Ditto. (unlink): Ditto. (seteuid32): Add argument to user_shared_initialize.