On Tuesday 06 November 2007 19:55, Paul Fox wrote:
> is there any reason that passwd and chpasswd shouldn't follow
> symlinks to find /etc/passwd?
>
> i have a patch to allow this, and although i've written it with a
> CONFIG item to protect the change, i'm wondering if the
> configuration option is necessary. the reason things break,
> currently, is that update_passwd.c creates a new copy of
> /etc/passwd and renames it as a final step. this renaming breaks
> any existing symlink. all other passwd-using utilities are
> perfectly happy with the link, so it's arguably just a bug that
> updates don't work.
I'd say having a symlinked /etc/passwd means that you probably
try to live with RO /etc.
You can probably achieve it in embedded setup, but on desktop
it's just too hard. (Yes I did it). It's far easier to just
make entire /etc writable.
Having said that, I certainly don't want our passwd be an
(yet another) obstacle for those who try to have RO /etc.
This should be fixed.
> should i commit my patch with, or without, the config variable?
I think it doesn't need to be configurable.
> (can i also hear comments on putting the Config.in change near
> passwd/chpasswd, rather than in libbb -- is that okay?)
I don't like realpath for two reasons:
* it requires PATH_MAX size buffer (which is BIG).
* it eats tons of stack internally:
make stksizes hall of shame (uclibc static build):
0x080de75d realpath [busybox_unstripped]: 12296 <=== !!?!
0x080c4d35 input_tab [busybox_unstripped]: 10428
0x080d9013 __GI_clntudp_create [busybox_unstripped]: 9016
0x0809851b umount_main [busybox_unstripped]: 8252
0x0806bae0 rtnl_talk [busybox_unstripped]: 8240
0x0806bd7f xrtnl_dump_filter [busybox_unstripped]: 8240
0x08099996 sendMTFValues [busybox_unstripped]: 5316
0x0809a28e mainSort [busybox_unstripped]: 4700
0x08095a72 mkfs_minix_main [busybox_unstripped]: 4288
It will be great if we can avoid using it.
OTOH, xmalloc_readlink has a problem that
* it fails if the file is not a link,
* it does not fully resolve a link to a link (to a link).
> +#if ENABLE_PASSWD_FOLLOW_SYMLINKS
> + char resolved_filename[PATH_MAX+1];
> +
> + if (realpath(filename, resolved_filename)) {
Thus I propose introducing xmalloc_readlink_recursive()
which does not suffer from those two problems,
and using it here (and elsewhere: insmod, syslogd -
grep for xmalloc_readlink).
> + filename = resolved_filename;
> + } else {
> + return -1;
> + }
> +#endif
> +
--
vda
_______________________________________________
busybox mailing list
[email protected]
http://busybox.net/cgi-bin/mailman/listinfo/busybox