Re: ld.so fix for empty LD_PRELOAD
On Sat, Dec 18, 2010 at 01:22:22AM +0100, Han Boetes wrote: Mark Kettenis wrote: From: Marco Peereboom sl...@peereboom.us I kind of disagree with you mark and I think that the diff makes sense. FWIW, I feel too strongly about this. If you want to check it check it properly. For example use stat to see if LD_PRELOAD contains an existing file or use file to see if it's really a library. But checking if LD_PRELOAD only contains an empty string seems like a non check. Would you also check if it contained a space or two spaces? Actually there is no need to check since if the file is not a valid library the result would be an error which the user has to debug. Something about giving users enough rope... I was just writting to ask what happens if there's a space or a tab :-) I think Han phrased it quite well and I agree with him. There's no need for this check. And the fact that you encountered an empty LD_PRELOAD was so obviously a case of bad scripting that when you wrote the second mail and confirmed it I decided to reply and say no to this.
Re: ld.so fix for empty LD_PRELOAD
I kind of disagree with you mark and I think that the diff makes sense. On Fri, Dec 17, 2010 at 11:48:06AM +0100, Mark Kettenis wrote: Date: Thu, 16 Dec 2010 22:43:04 +0100 From: Stefan Sperling s...@openbsd.org $ export LD_PRELOAD='' $ sed sed: can't load library '' $ env env: can't load library '' $ vim /usr/local/bin/vim: can't load library '' $ Is this the right way to fix it? I'd say it works just fine without your fix. If you really don't want to preload stuff, make sure LD_PRELOAD isn't set at all. Index: loader.c === RCS file: /cvs/src/libexec/ld.so/loader.c,v retrieving revision 1.120 diff -u -p -r1.120 loader.c --- loader.c25 Oct 2010 20:34:44 - 1.120 +++ loader.c16 Dec 2010 21:40:07 - @@ -493,7 +493,7 @@ _dl_boot(const char **argv, char **envp, TAILQ_INSERT_TAIL(_dlopened_child_list, n, next_sib); exe_obj-opencount++; - if (_dl_preload != NULL) + if (_dl_preload != NULL _dl_preload[0] != '\0') _dl_dopreload(_dl_preload); _dl_load_dep_libs(exe_obj, exe_obj-obj_flags, 1);
Re: ld.so fix for empty LD_PRELOAD
Date: Fri, 17 Dec 2010 08:21:05 -0600 From: Marco Peereboom sl...@peereboom.us I kind of disagree with you mark and I think that the diff makes sense. FWIW, I feel too strongly about this. On Fri, Dec 17, 2010 at 11:48:06AM +0100, Mark Kettenis wrote: Date: Thu, 16 Dec 2010 22:43:04 +0100 From: Stefan Sperling s...@openbsd.org $ export LD_PRELOAD='' $ sed sed: can't load library '' $ env env: can't load library '' $ vim /usr/local/bin/vim: can't load library '' $ Is this the right way to fix it? I'd say it works just fine without your fix. If you really don't want to preload stuff, make sure LD_PRELOAD isn't set at all. Index: loader.c === RCS file: /cvs/src/libexec/ld.so/loader.c,v retrieving revision 1.120 diff -u -p -r1.120 loader.c --- loader.c 25 Oct 2010 20:34:44 - 1.120 +++ loader.c 16 Dec 2010 21:40:07 - @@ -493,7 +493,7 @@ _dl_boot(const char **argv, char **envp, TAILQ_INSERT_TAIL(_dlopened_child_list, n, next_sib); exe_obj-opencount++; - if (_dl_preload != NULL) + if (_dl_preload != NULL _dl_preload[0] != '\0') _dl_dopreload(_dl_preload); _dl_load_dep_libs(exe_obj, exe_obj-obj_flags, 1);
Re: ld.so fix for empty LD_PRELOAD
Mark Kettenis wrote: From: Marco Peereboom sl...@peereboom.us I kind of disagree with you mark and I think that the diff makes sense. FWIW, I feel too strongly about this. If you want to check it check it properly. For example use stat to see if LD_PRELOAD contains an existing file or use file to see if it's really a library. But checking if LD_PRELOAD only contains an empty string seems like a non check. Would you also check if it contained a space or two spaces? Actually there is no need to check since if the file is not a valid library the result would be an error which the user has to debug. Something about giving users enough rope... # Han
