[RFC] malloced getpw/grxxx/_r functions for bb

Wed Aug 20 20:08:01 UTC 2014

On Monday 18 August 2014 21:50:44 tito wrote:
> On Tuesday 05 August 2014 20:16:04 Denys Vlasenko wrote:
> > On Mon, Aug 4, 2014 at 7:06 PM, Laszlo Papp <lpapp at kde.org> wrote:
> > > 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
> > 
> > Both issues come from the same location in codebase:
> > bb__pgsreader() parser drops lines which are longer than its buffer.
> > 
> > Effectively, bbox ignores offending line in /etc/passwd.
> > 
> > > Could you please look into this and potentially fix it? Thanks in advance.
> > 
> > Anyone willing to rewrite getpwnam API to use variable-sized malloced buffer?
> 
> Hi,
> i couldn 't resist as I was sure there was a lot to learn and so I've started
> to reinvent the wheel. At first I've tried to modify the original bb implementation
> but the code with its preprocessor black magic made it difficult so I've started
> from scratch.
> At the moment I have a proof of concept implementation for:
> 
> getpwuid
> getgrgid
> getpwnam
> getgrnam
> getpwuid_r
> getgrgid_r
> getpwnam_r
> 
> The code is ugly and probably bloated for bb so look at it at your own risk :-)
> 
> Other functions still missing but used in bb:
> fgetpwent_r
> fgetgrent_r
> getspnam_r
> setpwent
> endpwent
> setgrent 
> endgrent
> getpwent_r
> getgrent_r
> initgroups
> 
> What it does:
> 1) the buffer for getpwuid, getgrgid, getpwnam, getgrnam is dynamically
>     allocated and reused by later calls. if ERANGE error pops up it is
>     reallocated to the _NEEDED_ size and reused for later calls.
> 
> 2) the fields in the passwd files are checked and if empty (in some cases) or containing
>     whitespace, etc. a error message about malformed line in file xxxxxx
>     is printed so that the user knows instead of being silently ignored.
> 
> 3) the _r functions use the user supplied buffers that are never reallocated
>     but use mostly the same common functions.
> 
> 4) there was something........
> 
> Before going further some review and improvements (size reduction, obvious errors, bugs
> and other horrors) by the list members is needed  and also some hints if this implementation
> makes sense at all or if I should forget about it.
> 
> My biggest problem at the moment is a memory leak in handling the members in
> group->gr_mem and I don't see a obvious way to fix it.
> 
> The proof of concept is developed out of tree in the pwdgrp.c file that contains also
> a test program to grossly check if everything works. The relevant libbb functions are
> copied over to the libbb.c file which is included automatically. You can compile it simply by:
> 
> gcc pwdgrp.c -o test -Wall
> 
> 
> valgrind of a test run shows:
> 
> =7229== HEAP SUMMARY:
> ==7229==     in use at exit: 272 bytes in 4 blocks
> ==7229==   total heap usage: 22 allocs, 18 frees, 4,012 bytes allocated
> ==7229== 
> ==7229== 68 bytes in 1 blocks are still reachable in loss record 1 of 4
> ==7229==    at 0x4028308: malloc (vg_replace_malloc.c:263)
> ==7229==    by 0x402849F: realloc (vg_replace_malloc.c:632)
> ==7229==    by 0x80489E4: xrealloc (in /home/tito/Desktop/test)
> ==7229==    by 0x8048A5C: xrealloc_vector_helper (in /home/tito/Desktop/test)
> ==7229==    by 0x8049584: convert_to_grp (in /home/tito/Desktop/test)
> ==7229==    by 0x80498AB: my_getgr_common (in /home/tito/Desktop/test)
> ==7229==    by 0x80498FE: my_getgrgid (in /home/tito/Desktop/test)
> ==7229==    by 0x8049EBD: main (in /home/tito/Desktop/test)
> ==7229== 
> ==7229== 68 bytes in 1 blocks are definitely lost in loss record 2 of 4
> ==7229==    at 0x4028308: malloc (vg_replace_malloc.c:263)
> ==7229==    by 0x402849F: realloc (vg_replace_malloc.c:632)
> ==7229==    by 0x80489E4: xrealloc (in /home/tito/Desktop/test)
> ==7229==    by 0x8048A5C: xrealloc_vector_helper (in /home/tito/Desktop/test)
> ==7229==    by 0x804953E: convert_to_grp (in /home/tito/Desktop/test)
> ==7229==    by 0x80498AB: my_getgr_common (in /home/tito/Desktop/test)
> ==7229==    by 0x80498CD: my_getgrnam (in /home/tito/Desktop/test)
> ==7229==    by 0x8049D43: main (in /home/tito/Desktop/test)
> ==7229== 
> ==7229== 68 bytes in 1 blocks are definitely lost in loss record 3 of 4
> ==7229==    at 0x4028308: malloc (vg_replace_malloc.c:263)
> ==7229==    by 0x402849F: realloc (vg_replace_malloc.c:632)
> ==7229==    by 0x80489E4: xrealloc (in /home/tito/Desktop/test)
> ==7229==    by 0x8048A5C: xrealloc_vector_helper (in /home/tito/Desktop/test)
> ==7229==    by 0x804953E: convert_to_grp (in /home/tito/Desktop/test)
> ==7229==    by 0x804976C: getgr_common_r (in /home/tito/Desktop/test)
> ==7229==    by 0x80497B9: my_getgrnam_r (in /home/tito/Desktop/test)
> ==7229==    by 0x8049E0A: main (in /home/tito/Desktop/test)
> ==7229== 
> ==7229== 68 bytes in 1 blocks are definitely lost in loss record 4 of 4
> ==7229==    at 0x4028308: malloc (vg_replace_malloc.c:263)
> ==7229==    by 0x402849F: realloc (vg_replace_malloc.c:632)
> ==7229==    by 0x80489E4: xrealloc (in /home/tito/Desktop/test)
> ==7229==    by 0x8048A5C: xrealloc_vector_helper (in /home/tito/Desktop/test)
> ==7229==    by 0x804953E: convert_to_grp (in /home/tito/Desktop/test)
> ==7229==    by 0x804976C: getgr_common_r (in /home/tito/Desktop/test)
> ==7229==    by 0x8049806: my_getgrgid_r (in /home/tito/Desktop/test)
> ==7229==    by 0x8049F84: main (in /home/tito/Desktop/test)
> ==7229== 
> ==7229== LEAK SUMMARY:
> ==7229==    definitely lost: 204 bytes in 3 blocks
> ==7229==    indirectly lost: 0 bytes in 0 blocks
> ==7229==      possibly lost: 0 bytes in 0 blocks
> ==7229==    still reachable: 68 bytes in 1 blocks
> ==7229==         suppressed: 0 bytes in 0 blocks
> 
> 
> Ciao,
> Tito
> 
> 
> 
> 
> 
Hi,
v 0.2 memory leaks reduced and better checking of leading/trailing whitespace and empty fields in records.

Ciao,
Tito

=9068== LEAK SUMMARY:
==9068==    definitely lost: 28 bytes in 2 blocks
==9068==    indirectly lost: 0 bytes in 0 blocks
==9068==      possibly lost: 12 bytes in 1 blocks
==9068==    still reachable: 0 bytes in 0 blocks
==9068==         suppressed: 0 bytes in 0 blocks
==9068== 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: pwdgrp.c
Type: text/x-csrc
Size: 11199 bytes
Desc: not available
URL: <http://lists.busybox.net/pipermail/busybox/attachments/20140820/293da8a4/attachment.c>