On Tue, Aug 5, 2014 at 1:42 PM, tito <farmat...@tiscali.it> wrote: > On Tuesday 05 August 2014 14:27:57 you wrote: > > I disagree. There is nothing to reject here. It fixes _my issue_ at hand. > > If you want to fix other bugs in your system that you are facing in > > reality, by all means, fix them in a separate change. > > > > I definitely do not agree with dynamically allocated buffer for the > simple > > reasons of: > > > > 1) Slow. > > 2) You would still need a maximum anyway. > > 3) Needless code complication. > > > > Let us keep things simple and good, you know the good old KISS principle. > > > > P.S.: please do not bikeshed. > > > > > > On Tue, Aug 5, 2014 at 1:17 PM, tito <farmat...@tiscali.it> wrote: > > > > > On Tuesday 05 August 2014 12:46:08 you wrote: > > > > Here is my tested fix without being to debug the busybox code, so > only > > > code > > > > reading and understanding were my friends: > > > > > > > > commit 9610650b6ce2a4c1904f78a2dcdb47cad3d2e3d1 > > > > Author: Laszlo Papp <lp...@kde.org> > > > > Date: Tue Aug 5 11:42:24 2014 +0100 > > > > > > > > Allow 256 bytes long usernames as per Unix standards (usually) > > > > > > > > diff --git a/libpwdgrp/pwd_grp.c b/libpwdgrp/pwd_grp.c > > > > index 2060d78..368c252 100644 > > > > --- a/libpwdgrp/pwd_grp.c > > > > +++ b/libpwdgrp/pwd_grp.c > > > > @@ -23,8 +23,8 @@ > > > > > /**********************************************************************/ > > > > /* Sizes for statically allocated buffers. */ > > > > > > > > -#define PWD_BUFFER_SIZE 256 > > > > -#define GRP_BUFFER_SIZE 256 > > > > +#define PWD_BUFFER_SIZE LOGIN_NAME_MAX+256 > > > > +#define GRP_BUFFER_SIZE LOGIN_NAME_MAX+256 > > > > > > > > > /**********************************************************************/ > > > > /* Prototypes for internal functions. */ > > > > > > > > > > Hi, > > > yes I've thought also about this solution but rejected it because: > > > > > > 1) there will not be enough space to hold the home dir if named the > same > > > as the user > > > so you need at least: > > > > > > #define PWD_BUFFER_SIZE LOGIN_NAME_MAX+ 6 (for home) + LOGIN_NAME_MAX > (or > > > PATH_MAX?) + 256 (for the rest) > > > > > > yet this might not be enough as in my passwd file I see a 101 char > long > > > passwd. > > > Add the gecos fields (arbitrary lenght) to it and the default > shell > > > and we are short on space again. > > > > > > 2) same for #define GRP_BUFFER_SIZE LOGIN_NAME_MAX+256: > > > here you need to take into account the group member list with > > > an arbitrary number of members: > > > > > > > > > #define GRP_BUFFER_SIZE LOGIN_NAME_MAX+256+ LOGIN_NAME_MAX*n: > > > > > > The only clean solution to fix it forever is dynamically allocated > buffers > > > created at the time you read in the line from the passwd files. > > > > > > Ciao, > > > Tito > > > > > > > > > P.S.: please don't top post. > > > > > > > > > > > > > > On Tue, Aug 5, 2014 at 11:22 AM, Laszlo Papp <lp...@kde.org> wrote: > > > > > > > > > Yeah, mayhaps... Thanks for the prompt reply. I tried to debug the > > > code, > > > > > but the busybox code here is a bit messy heavily abusing macros in > C > > > and > > > > > all that. It ain't easy I must confess! > > > > > > > > > > For instance, see this section: > > > > > > > > > > http://git.busybox.net/busybox/tree/libpwdgrp/pwd_grp.c#n227 > > > > > > > > > > The _source_ is included several times always getting a new meaning > > > based > > > > > on some defines... Now, check this function: > > > > > > > > > > > http://git.busybox.net/busybox/tree/libpwdgrp/pwd_grp_internal.c#n20 > > > > > > > > > > "resultbuf" will be always different depending on which include it > is. > > > > > Since it is failing at the pw_name check in there, I wanted to > print it > > > > > out, but no easy joy there as printing it like that will yield > > > compilation > > > > > error when the file is being included next time from above. > > > > > > > > > > Right, I thought instead of doing some "#if a == b" hackery, > debugging > > > > > would be easier, BUT: > > > > > > > > > > 1) The default build is stripped, yuck! > > > > > > > > > > 2) The unstripped build cannot locate the symbols (*). > > > > > > > > > > So, I am giving up on this for now; this is not the type of source > code > > > > > that is so pleasant to work with. ;-) > > > > > > > > > > Cheers, L. > > > > > > > > > > * (gdb) b main > > > > > > > > > > Breakpoint 2 at 0x407c71 > > > > > (gdb) r > > > > > Starting program: /home/lpapp/Projects/busybox/busybox_unstripped > > > deluser > > > > fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff > > > > > warning: Could not load shared library symbols for linux-vdso.so.1. > > > > > Do you need "set solib-search-path" or "set sysroot"? > > > > > > > > > > Breakpoint 2, 0x0000000000407c71 in main () > > > > > (gdb) list > > > > > No symbol table is loaded. Use the "file" command. > > > > > (gdb) > > > > > > > > > > > > > > > > > > > > On Mon, Aug 4, 2014 at 10:40 PM, tito <farmat...@tiscali.it> > wrote: > > > > > > > > > >> On Monday 04 August 2014 19:06:39 Laszlo Papp wrote: > > > > >> > Hi, > > > > >> > > > > > >> > sudo busybox adduser > > > > >> > > > > > >> > > > > fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff > > > > >> > passwd: unknown user > > > > >> > > > > > >> > > > > fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff > > > > >> > > > > > >> > Yet, the user is created in /etc/shadow: > > > > >> > > > > > >> > > > > > >> > > > > fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff:!:16286:0:99999:7::: > > > > >> > > > > > >> > This is at least one issue, but there is another here: > > > > >> > > > > > >> > sudo busybox deluser > > > > >> > > > > > >> > > > > fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff > > > > >> > deluser: unknown user > > > > >> > > > > > >> > > > > fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff > > > > >> > > > > > >> > So, basically, once you create that long username, you cannot > > > remove it > > > > >> > anymore with busybox and it keeps polluting your system. > > > > >> > > > > > >> > You have to gain other means to clean up! I am using this > version > > > over > > > > >> here: > > > > >> > > > > > >> > BusyBox v1.22.1 (2014-06-02 14:47:37 MSK) multi-call binary. > > > > >> > > > > > >> > Could you please look into this and potentially fix it? Thanks > in > > > > >> advance. > > > > >> > > > > > >> > Cheers, L. > > > > >> > > > > > >> Hi, > > > > >> if disabling libb's internal pwd/grp lib and by jumping through > some > > > hops > > > > >> it > > > > >> works for me: > > > > >> > > > > >> I need to add the group first as my system's groupadd command > called > > > by > > > > >> bb's adduser > > > > >> supports only groupnames of max 32 chars. > > > > >> > > > > >> ./busybox addgroup > > > > >> > > > > fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff > > > > >> ./busybox adduser > > > > >> > > > > fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff > > > > >> -G > > > > >> > > > > fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff > > > > >> Adding user > > > > >> > > > > `fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff' > > > > >> to group > > > > >> > > > > `fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff' > > > > >> ... > > > > >> Adding user > > > > >> > > > > fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff > > > > >> to group > > > > >> > > > > fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff > > > > >> Done. > > > > >> Enter new UNIX password: > > > > >> Retype new UNIX password: > > > > >> passwd: password updated successfully > > > > >> > > > > >> and then I could also delete it: > > > > >> > > > > >> ./busybox deluser > > > > >> > > > > fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff > > > > >> ./busybox delgroup > > > > >> > > > > fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff > > > > >> delgroup: unknown group > > > > >> > > > > fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff > > > > >> > > > > >> (deluser correctly deleted also the group) > > > > >> > > > > >> I suspect that the buffer size used in libbpwdgrp/pwd_grp.c is to > > > small: > > > > >> > > > > >> #define PWD_BUFFER_SIZE 256 > > > > >> #define GRP_BUFFER_SIZE 256 > > > > >> > > > > >> as it is meant for the whole struct pw passwd (or struct gr group) > > > fields > > > > >> which could be easily be bigger: > > > > >> > > > > >> grep ffff* /etc/shadow | wc > > > > >> 1 1 240 > > > > >> grep ffff* /etc/passwd | wc > > > > >> 1 2 286 > > > > >> > > > > >> I think that the buffers' size must be increased for example to: > > > > >> > > > > >> #define PWD_BUFFER_SIZE LOGIN_NAME_MAX+LOGIN_NAME_MAX+256 > > > > >> > > > > >> we need one LOGIN_NAME_MAX size for the login and one more for the > > > home > > > > >> dir > > > > >> if same as login, plus 256 for the remaining data (passwd, gecos, > > > shell, > > > > >> etc). > > > > >> > > > > >> #define GRP_BUFFER_SIZE LOGIN_NAME_MAX+256+LOGIN_NAME_MAX*n (?) > > > > >> > > > > >> we need one LOGIN_NAME_MAX size for the group name (for which we > > > > >> actually enforce the same size as for the username) plus 256 for > the > > > > >> remaining data > > > > >> plus LOGIN_NAME_MAX * n group member names. > > > > >> > > > > >> So for my limited understanding using static buffers here is > rather > > > > >> difficult > > > > >> as the size of data is not easily predictable. > > > > >> I don't know how real libc manages it (maybe realloc on ERANGE?) > > > > >> > > > > >> Your particular example for me is fixed by using. > > > > >> > > > > >> #define PWD_BUFFER_SIZE 1024 > > > > >> > > > > >> #define GRP_BUFFER_SIZE 1024 > > > > >> > > > > >> But to me it seems not an optimal solution, > > > > >> so other more experienced developers should > > > > >> take a look at it. > > > > >> > > > > >> Hope this helps. > > > > >> Ciao, > > > > >> Tito > > > > >> > > > > >> PS: in libbbpwdgrp functions we need to check for errors: > > > > >> > > > > >> 0 or ENOENT or ESRCH or EBADF or EPERM or ... > > > > >> The given name or gid was not found. > > > > >> > > > > >> EINTR A signal was caught. > > > > >> > > > > >> EIO I/O error. > > > > >> > > > > >> EMFILE The maximum number (OPEN_MAX) of files was open > already > > > in > > > > >> the calling process. > > > > >> > > > > >> ENFILE The maximum number of files was open already in the > > > system. > > > > >> > > > > >> ENOMEM Insufficient memory to allocate group structure. > > > > >> > > > > >> ERANGE Insufficient buffer space supplied. > > > > >> > > > > >> as for example the xgroup_study function in the addgroup applet > > > > >> assigns a wrong gid if getgrgid fails for example for ERANGE > > > > >> > > > > >> /* Check if the desired gid is free > > > > >> * or find the first free one */ > > > > >> while (1) { > > > > >> printf("gid %d\n", g->gr_gid); > > > > >> if (!getgrgid(g->gr_gid)) { > > > > >> return; /* found free group: return */ > > > > >> } > > > > >> > > > > >> > Hi, > I doubt it fixes your issue
What you are basically telling me here that I submitted untested changes for my use cases, directly or indirectly. Either way, I refuse this claim since I _did_ test the change for my use case. > because I tested the same fix and it did in fact fail. Please test: > > 1) ./busybox addgroup LONGUSERNAME (this fails in subtle ways as a > already in use gid is assigned) > 2) ./busybox adduser LONGUSERNAME -G LONGUSERNAME > 3) ./busybox deluser LONGUSERNAME > 4) ./busybox delgroup LONGUSERNAME > I do not see how this is any relevant to my initial steps. This is a completely different test step with a different issue. As I said earlier, please welcome contributors rather than being blocker just because they fix one particular and real bug, and not every "imaginary" use case that can just exist in the world out there.
_______________________________________________ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
