From userv-maint@chiark.greenend.org.uk Tue, 20 Jan 98 18:55 GMT Date: Tue, 20 Jan 98 18:55 GMT From: Ian Jackson userv-maint@chiark.greenend.org.uk Subject: userv and group id of service rjk@greenend.org.uk writes ("userv and group id of service"): > And another thing - I was surprised to discover that the daemon > executes the service without changing GID. (This is again in 0.57.) Thanks for that and your previous messages. Majordomo bounced them; I think I've forwarded them OK now. I'll put out a new release with your fixes shortly. Ian. -- To remove yourself from this mailing list, send the word UNSUBSCRIBE to userv-discuss-REQUEST@chiark.greenend.org.uk. Info is at: . From rjk@greenend.org.uk Tue, 20 Jan 1998 19:53:13 GMT Date: Tue, 20 Jan 1998 19:53:13 GMT From: rjk@greenend.org.uk rjk@greenend.org.uk Subject: patch against userv 0.57 --06UGXDsrox Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Actually IIRC Ian is away for a few days so I'll send this to userv-discuss as well. ttfn/rjk --06UGXDsrox Content-Type: message/rfc822 Content-Transfer-Encoding: 7bit MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="hl3LVGUb3R" Message-Id: X-Mailer: VM 6.34 under 20.3 "Vatican City" XEmacs Lucid From: To: userv-maint@chiark.greenend.org.uk Subject: bug in user 0.57 Date: Sun, 11 Jan 98 20:12:37 +0000 (GMT) --hl3LVGUb3R Content-Type: text/plain; charset=us-ascii Hi, parser.c:parse_file() fails to copy the filename somewhere where it won't get mangled, with the result that syntax errors can give garbled error messages. Patch attached. ttfn/rjk --hl3LVGUb3R Content-Type: text/plain Content-Disposition: inline; filename="parse_file.diff" --- userv-0.57.orig/parser.c Thu Sep 18 02:58:56 1997 +++ userv-0.57/parser.c Sun Jan 11 20:10:51 1998 @@ -1267,6 +1267,7 @@ int r; FILE *file; struct stat newstab; + char *filename; if (fileparselevel >= MAX_INCLUDE_NEST) { parseerrprint("too many nested levels of included files"); @@ -1296,7 +1297,8 @@ ybuf= yy_create_buffer(file,YY_BUF_SIZE); if (!ybuf) syscallerror("unable to create flex buffer for file"); - parser_push(&usestate,string,&newstab,ybuf,0); + filename = xstrsave(string); + parser_push(&usestate,filename,&newstab,ybuf,0); fileparselevel++; r= parser(0); @@ -1307,6 +1309,7 @@ fileparselevel--; parser_pop(); + free(filename); fclose(file); if (r == tokv_eof) r= 0; return r; --hl3LVGUb3R-- --06UGXDsrox-- -- To remove yourself from this mailing list, send the word UNSUBSCRIBE to userv-discuss-REQUEST@chiark.greenend.org.uk. Info is at: . From rjk@greenend.org.uk Tue, 20 Jan 1998 19:53:14 GMT Date: Tue, 20 Jan 1998 19:53:14 GMT From: rjk@greenend.org.uk rjk@greenend.org.uk Subject: userv and group id of service And another thing - I was surprised to discover that the daemon executes the service without changing GID. (This is again in 0.57.) Why is setreuid() called twice? ttfn/rjk --- process.c~ Tue Oct 14 02:05:19 1997 +++ process.c Sun Jan 11 22:23:07 1998 @@ -508,6 +508,7 @@ serviceuser_uid= pw->pw_uid; if (initgroups(pw->pw_name,pw->pw_gid)) syscallerror("initgroups"); + if (setregid(pw->pw_gid,pw->pw_gid)) syscallerror("setregid"); if (setreuid(pw->pw_uid,pw->pw_uid)) syscallerror("setreuid 1"); if (setreuid(pw->pw_uid,pw->pw_uid)) syscallerror("setreuid 2"); if (pw->pw_uid) { -- To remove yourself from this mailing list, send the word UNSUBSCRIBE to userv-discuss-REQUEST@chiark.greenend.org.uk. Info is at: . From rjk@greenend.org.uk Tue, 20 Jan 1998 19:53:17 GMT Date: Tue, 20 Jan 1998 19:53:17 GMT From: rjk@greenend.org.uk rjk@greenend.org.uk Subject: another userv bug The patch below contains both the previous reported fixes, plus an additional nasty which has taken me the best part of two evenings to track down. Having done all that work I feel compelled to describe it in gory detail. Suppose you have specified `ignore-fd 0' for a service and then redirect the client's stdin from /dev/null. The client will launch a `cat' to pipe the null stdin across, but this will terminate immediately and throw the client a SIGCHLD, which will send a et_closereadfd message to the server. Now the first half of the bug is that the fd isn't filled in correctly, so it'd always be stdin which gets closed in the server (since the buffer was memset to 0 before doing anything with it). The second half of the bug is a bit more complicated, though still a one-line fix. Suppose the server has already got as far as calling makenonexistentfd() (which it won't always have done, resulting in the somewhat random occurence of this bug). Then it'll try and close whatever the hold fd is (in getevent()). Unfortunately makenonexistentfd() has (i) already closed this and (ii) neglected to squash it with -1, so getevent() tries to close the fd again ... and gets EBADF and of course it's downhill from there on in. Anyway: with the following patch applied all this and all other bugs in 0.57 I've so far discovered go away. It can also be found at this URL: http://staff.elmail.co.uk/~richard/userv-0.57rjk.patch in case it gets mangled in email. I'll update this with any other fixes I generate (though I'm not aware of any further problems right now). Hope I've got all this right... ttfn/rjk diff -ruN userv-0.57.orig/client.c userv-0.57/client.c --- userv-0.57.orig/client.c Tue Oct 14 02:05:15 1997 +++ userv-0.57/client.c Mon Jan 12 23:35:20 1998 @@ -378,6 +378,7 @@ memset(&event_mbuf,0,sizeof(event_mbuf)); event_mbuf.magic= EVENT_MAGIC; event_mbuf.type= et_closereadfd; + event_mbuf.data.closereadfd.fd = fd; r= fwrite(&event_mbuf,1,sizeof(event_mbuf),swfile); if (r != sizeof(event_mbuf) || fflush(swfile)) if (errno != EPIPE) syscallerror("inform service of closed read fd"); diff -ruN userv-0.57.orig/parser.c userv-0.57/parser.c --- userv-0.57.orig/parser.c Thu Sep 18 02:58:56 1997 +++ userv-0.57/parser.c Mon Jan 12 23:35:59 1998 @@ -1267,6 +1267,7 @@ int r; FILE *file; struct stat newstab; + char *filename; if (fileparselevel >= MAX_INCLUDE_NEST) { parseerrprint("too many nested levels of included files"); @@ -1296,7 +1297,8 @@ ybuf= yy_create_buffer(file,YY_BUF_SIZE); if (!ybuf) syscallerror("unable to create flex buffer for file"); - parser_push(&usestate,string,&newstab,ybuf,0); + filename = xstrsave(string); + parser_push(&usestate,filename,&newstab,ybuf,0); fileparselevel++; r= parser(0); @@ -1307,6 +1309,7 @@ fileparselevel--; parser_pop(); + free(filename); fclose(file); if (r == tokv_eof) r= 0; return r; diff -ruN userv-0.57.orig/process.c userv-0.57/process.c --- userv-0.57.orig/process.c Tue Oct 14 02:05:19 1997 +++ userv-0.57/process.c Mon Jan 12 23:36:06 1998 @@ -508,6 +508,7 @@ serviceuser_uid= pw->pw_uid; if (initgroups(pw->pw_name,pw->pw_gid)) syscallerror("initgroups"); + if (setregid(pw->pw_gid,pw->pw_gid)) syscallerror("setregid"); if (setreuid(pw->pw_uid,pw->pw_uid)) syscallerror("setreuid 1"); if (setreuid(pw->pw_uid,pw->pw_uid)) syscallerror("setreuid 2"); if (pw->pw_uid) { @@ -597,6 +598,7 @@ if (fdarray[fd].holdfd != -1) { if (close(fdarray[fd].holdfd)) syscallfailure("close unwanted hold descriptor for %d",fd); + fdarray[fd].holdfd = -1; } } } -- To remove yourself from this mailing list, send the word UNSUBSCRIBE to userv-discuss-REQUEST@chiark.greenend.org.uk. Info is at: . From userv-maint at chiark.greenend.org.uk Tue Jan 20 18:55:00 1998 From: userv-maint at chiark.greenend.org.uk (Ian Jackson) Date: Tue, 20 Jan 98 18:55 GMT Subject: userv and group id of service In-Reply-To: References: Message-ID: rjk@greenend.org.uk writes ("userv and group id of service"): > And another thing - I was surprised to discover that the daemon > executes the service without changing GID. (This is again in 0.57.) Thanks for that and your previous messages. Majordomo bounced them; I think I've forwarded them OK now. I'll put out a new release with your fixes shortly. Ian. -- To remove yourself from this mailing list, send the word UNSUBSCRIBE to userv-discuss-REQUEST@chiark.greenend.org.uk. Info is at: . From rjk at greenend.org.uk Tue Jan 20 19:53:13 1998 From: rjk at greenend.org.uk (rjk@greenend.org.uk) Date: Tue, 20 Jan 1998 19:53:13 GMT Subject: patch against userv 0.57 Message-ID: --06UGXDsrox Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Actually IIRC Ian is away for a few days so I'll send this to userv-discuss as well. ttfn/rjk --06UGXDsrox Content-Type: message/rfc822 Content-Transfer-Encoding: 7bit MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="hl3LVGUb3R" Message-Id: X-Mailer: VM 6.34 under 20.3 "Vatican City" XEmacs Lucid From: To: userv-maint@chiark.greenend.org.uk Subject: bug in user 0.57 Date: Sun, 11 Jan 98 20:12:37 +0000 (GMT) --hl3LVGUb3R Content-Type: text/plain; charset=us-ascii Hi, parser.c:parse_file() fails to copy the filename somewhere where it won't get mangled, with the result that syntax errors can give garbled error messages. Patch attached. ttfn/rjk --hl3LVGUb3R Content-Type: text/plain Content-Disposition: inline; filename="parse_file.diff" --- userv-0.57.orig/parser.c Thu Sep 18 02:58:56 1997 +++ userv-0.57/parser.c Sun Jan 11 20:10:51 1998 @@ -1267,6 +1267,7 @@ int r; FILE *file; struct stat newstab; + char *filename; if (fileparselevel >= MAX_INCLUDE_NEST) { parseerrprint("too many nested levels of included files"); @@ -1296,7 +1297,8 @@ ybuf= yy_create_buffer(file,YY_BUF_SIZE); if (!ybuf) syscallerror("unable to create flex buffer for file"); - parser_push(&usestate,string,&newstab,ybuf,0); + filename = xstrsave(string); + parser_push(&usestate,filename,&newstab,ybuf,0); fileparselevel++; r= parser(0); @@ -1307,6 +1309,7 @@ fileparselevel--; parser_pop(); + free(filename); fclose(file); if (r == tokv_eof) r= 0; return r; --hl3LVGUb3R-- --06UGXDsrox-- -- To remove yourself from this mailing list, send the word UNSUBSCRIBE to userv-discuss-REQUEST@chiark.greenend.org.uk. Info is at: . From rjk at greenend.org.uk Tue Jan 20 19:53:14 1998 From: rjk at greenend.org.uk (rjk@greenend.org.uk) Date: Tue, 20 Jan 1998 19:53:14 GMT Subject: userv and group id of service Message-ID: And another thing - I was surprised to discover that the daemon executes the service without changing GID. (This is again in 0.57.) Why is setreuid() called twice? ttfn/rjk --- process.c~ Tue Oct 14 02:05:19 1997 +++ process.c Sun Jan 11 22:23:07 1998 @@ -508,6 +508,7 @@ serviceuser_uid= pw->pw_uid; if (initgroups(pw->pw_name,pw->pw_gid)) syscallerror("initgroups"); + if (setregid(pw->pw_gid,pw->pw_gid)) syscallerror("setregid"); if (setreuid(pw->pw_uid,pw->pw_uid)) syscallerror("setreuid 1"); if (setreuid(pw->pw_uid,pw->pw_uid)) syscallerror("setreuid 2"); if (pw->pw_uid) { -- To remove yourself from this mailing list, send the word UNSUBSCRIBE to userv-discuss-REQUEST@chiark.greenend.org.uk. Info is at: . From rjk at greenend.org.uk Tue Jan 20 19:53:17 1998 From: rjk at greenend.org.uk (rjk@greenend.org.uk) Date: Tue, 20 Jan 1998 19:53:17 GMT Subject: another userv bug Message-ID: The patch below contains both the previous reported fixes, plus an additional nasty which has taken me the best part of two evenings to track down. Having done all that work I feel compelled to describe it in gory detail. Suppose you have specified `ignore-fd 0' for a service and then redirect the client's stdin from /dev/null. The client will launch a `cat' to pipe the null stdin across, but this will terminate immediately and throw the client a SIGCHLD, which will send a et_closereadfd message to the server. Now the first half of the bug is that the fd isn't filled in correctly, so it'd always be stdin which gets closed in the server (since the buffer was memset to 0 before doing anything with it). The second half of the bug is a bit more complicated, though still a one-line fix. Suppose the server has already got as far as calling makenonexistentfd() (which it won't always have done, resulting in the somewhat random occurence of this bug). Then it'll try and close whatever the hold fd is (in getevent()). Unfortunately makenonexistentfd() has (i) already closed this and (ii) neglected to squash it with -1, so getevent() tries to close the fd again ... and gets EBADF and of course it's downhill from there on in. Anyway: with the following patch applied all this and all other bugs in 0.57 I've so far discovered go away. It can also be found at this URL: http://staff.elmail.co.uk/~richard/userv-0.57rjk.patch in case it gets mangled in email. I'll update this with any other fixes I generate (though I'm not aware of any further problems right now). Hope I've got all this right... ttfn/rjk diff -ruN userv-0.57.orig/client.c userv-0.57/client.c --- userv-0.57.orig/client.c Tue Oct 14 02:05:15 1997 +++ userv-0.57/client.c Mon Jan 12 23:35:20 1998 @@ -378,6 +378,7 @@ memset(&event_mbuf,0,sizeof(event_mbuf)); event_mbuf.magic= EVENT_MAGIC; event_mbuf.type= et_closereadfd; + event_mbuf.data.closereadfd.fd = fd; r= fwrite(&event_mbuf,1,sizeof(event_mbuf),swfile); if (r != sizeof(event_mbuf) || fflush(swfile)) if (errno != EPIPE) syscallerror("inform service of closed read fd"); diff -ruN userv-0.57.orig/parser.c userv-0.57/parser.c --- userv-0.57.orig/parser.c Thu Sep 18 02:58:56 1997 +++ userv-0.57/parser.c Mon Jan 12 23:35:59 1998 @@ -1267,6 +1267,7 @@ int r; FILE *file; struct stat newstab; + char *filename; if (fileparselevel >= MAX_INCLUDE_NEST) { parseerrprint("too many nested levels of included files"); @@ -1296,7 +1297,8 @@ ybuf= yy_create_buffer(file,YY_BUF_SIZE); if (!ybuf) syscallerror("unable to create flex buffer for file"); - parser_push(&usestate,string,&newstab,ybuf,0); + filename = xstrsave(string); + parser_push(&usestate,filename,&newstab,ybuf,0); fileparselevel++; r= parser(0); @@ -1307,6 +1309,7 @@ fileparselevel--; parser_pop(); + free(filename); fclose(file); if (r == tokv_eof) r= 0; return r; diff -ruN userv-0.57.orig/process.c userv-0.57/process.c --- userv-0.57.orig/process.c Tue Oct 14 02:05:19 1997 +++ userv-0.57/process.c Mon Jan 12 23:36:06 1998 @@ -508,6 +508,7 @@ serviceuser_uid= pw->pw_uid; if (initgroups(pw->pw_name,pw->pw_gid)) syscallerror("initgroups"); + if (setregid(pw->pw_gid,pw->pw_gid)) syscallerror("setregid"); if (setreuid(pw->pw_uid,pw->pw_uid)) syscallerror("setreuid 1"); if (setreuid(pw->pw_uid,pw->pw_uid)) syscallerror("setreuid 2"); if (pw->pw_uid) { @@ -597,6 +598,7 @@ if (fdarray[fd].holdfd != -1) { if (close(fdarray[fd].holdfd)) syscallfailure("close unwanted hold descriptor for %d",fd); + fdarray[fd].holdfd = -1; } } } -- To remove yourself from this mailing list, send the word UNSUBSCRIBE to userv-discuss-REQUEST@chiark.greenend.org.uk. Info is at: .