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

Reply via email to