Mailing-List: contact cygwin-developers-help AT cygwin DOT com; run by ezmlm List-Subscribe: List-Archive: List-Post: List-Help: , Sender: cygwin-developers-owner AT cygwin DOT com Delivered-To: mailing list cygwin-developers AT cygwin DOT com Message-ID: <3DCBD52C.A1F794FD@ieee.org> Date: Fri, 08 Nov 2002 10:15:56 -0500 From: "Pierre A. Humblet" X-Accept-Language: en,pdf MIME-Version: 1.0 To: "cygwin-developers AT cygwin DOT com" Subject: ntsec patch #4: passwd and group Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Corinna, This is a heads up on a coming patch affecting security.h, passwd.cc, grp.cc, sec_helper.cc and uinfo.cc. The ChangeLog of the patch is detailed below. No diffs are attached for now as some of these files were affected by a patch that's under review. This patch has two objectives: 1) Fix the bug reported on Tuesday in http://cygwin.com/ml/cygwin/2002-11/msg00166.html and evidenced by: /usr/sbin> strace sshd | fgrep 'Read /etc/passwd' 10761 101398 [main] sshd 272 read_etc_passwd: Read /etc/passwd, 305 lines after touching /etc/passwd and running ssh, sshd outputs 10682 32079255 [main] sshd 270 read_etc_passwd: Read /etc/passwd, 610 lines 2) Operate better with incomplete passwd and group files. Each of these two files can be in one of 4 states: 1) missing or unreadable 2) readable but without entry for the current user 3) readable with an entry, but without sid 4) correct The goal is to be able to start Cygwin in any state (16 combinations), to have "id" and "ls -l" display correct information (as much as possible), and to keep doing so consistently even when the state changes to any other value. The main changes are in the passwd/group emulation code, which has been totally redone to cover the four cases. Additionally two new functions have been created: getpwsid and getgrsid. There is one design decision: when the group entry is missing, the code calls LookupAccountSidA to get the current group name. That will cause delays for domain users starting Cygwin while disconnected from the PDC (but only if /etc/group is incomplete). The alternative is to always use "unknown". I have also detected 4 outstanding issue: 1) memory leak in grp.cc: grp.gr_mem is calloc'ed but never freed. 2) I thought that the passwd/group files where only read "for the first cygwin process that start up on a given console", to use Chris' words in http://cygwin.com/ml/cygwin-patches/2002-q4/msg00024.html Now that I have my nose in the code, I see that they reside in malloc'ed structures and that consequently they are reread when accessing passwd/group after execs (but not after forks). 3) Why is pthread_testcancel called after read_etc_passwd in some (but not all) passwd.cc functions, but never after read_etc_group in grp.cc ? 4) Altough I see some mutual exclusion stuff, I don't see what prevents two threads from simultaneously updating the internal copies, or one thread to free them while they are used by another. An alternative would be to only update the passwd/group internal copies on process startup (fork or exec), if needed. That would be simple, safe and fast. Do you know applications that really benefit from reading the files every time they change? In fact applications such as sshd would benefit from rereading the files (if needed) *before* forks or execs, so that a single reread can serve all future children, but that approach does not help with thread issues. Fixing 3 and 4 is beyond me for now (pthread learning curve). Do you think it would be a good idea to improve 2 by placing the copies on the cygheap? If so, I would fix 1 at the same time. Pierre 2002/11/XX Pierre Humblet * security.h: Declare getpwsid and getgrsid. Undeclare internal_getpwent. * passwd.cc (getpwsid): Create. (internal_getpwent): Suppress. (read_etc_passwd): Rewrite the code for the completion line. Reset curr_lines after reading /etc/passwd. (parse_pwd): Change type to static int. Return 0 for short lines. (add_pwd_line): Pay attention to the value of parse_pwd. (search_for): Do not look for nor return the DEFAULT_UID. * grp.cc (read_etc_group): Rewrite the code for the completion line. Reset curr_lines after reading /etc/group. Always call add_pwd_line. (parse_grp): Never NULL gr_passwd. (getgrgid32): Only return the default if ntsec is off and the gid is ILLEGAL_GID. (parse_grp): If grp.gr_mem is empty, set it to &null_ptr. * sec_helper.cc (cygsid::get_id): Use getpwsid and getgrsid. (cygsid_getfrompw): Clean up last line. (cygsid_getfromgr): Ditto. * uinfo.cc (internal_getlogin): Set DEFAULT_GID at start. Use getpwsid. Move the read of /etc/group after the second access of /etc/passwd. Change some debug_printf.