[Toybox] Issues with 'xexec()', 'losetup' and 'cp'

[Rob Landley]

Sorry for the delay answering, still working out work/life balance at  
my new contract.

On 01/30/2013 03:11:58 AM, Ashwini Sharma wrote:
> Hi Rob,
>    1. I was using 'xexec()' function. It gives me the coredump
>       from function toy_init() in main.c at line #82.
> 
> My use case is as follows.
> 
> I have to run a program from my feature under development. For the  
> purpose
> I am using xexec.
> 
> The arguments passed to it has members from 'toys.optargs'  array. As  
> soon
> as it reaches line #82
> 
> if (toys.optargs != toys.argv+1) free(toys.optargs); it dumps.

Where did toys.optargs get set in the command calling xexec()? In  
theory, toys.optargs gets set in two places:

1) lib/args.c will malloc() an array during argument parsing to hold  
the leftover arguments after flags and such are removed by  
get_optflags(). (The strings the array points to are environment data,  
we don't free those.)

2) if the option string is NULL, toys.optargs is set to argv+1, in  
toy_init() (currently line 89).

Those are the only two places that set optargs, and it starts NULL. So  
either it's still NULL when this is called (that's ok because posix  
says free(NULL) is a NOP), or it's a malloc() chunk of memory that  
free() handles, or it's argv+1 which we specifically test for. (And  
since the test for argv+1 and the assignment of argv+1 are only 7 lines  
apart in the same function, having that be a magic case isn't too bad).

It sounds like your code, somewhere, set optargs to something _other_  
then a freeable block of memory. And then when you called xexec() and  
it recognized an internal command it called toy_init() to clear the  
global variable state, which saw that your optarg block wasn't argv+1  
so tried to free it, and segfaulted.

> Can you please have a look at it.
> 
> 2. In the implementation of losetup, you have used /dev/loop-control  
> and
> ioctls related to it. Which doesn't compile for me. I am using kernel
> 
> Linux AshwiniPC 2.6.33.3-85.fc13.i686.PAE #1 SMP Thu May 6 18:27:11  
> UTC
> 2010 i686 i686 i386 GNU/Linux
> 
> Compiler complains of LOOP_CTL_GET_FREE - not present
> 
> 3. Same is the case with cp, here also it complains for O_NOFOLLOW
> 
> Please provide your inputs on the same.

Ah, sorry.

In theory I have tests for this in lib/portability.h (there's a check  
for glibc < version 2.10), but I didn't run my regression test for 4  
year old build environments recently.

In both cases the functionality should cleanly drop out even if the  
kernel doesn't support it (/dev/loop-control won't be there, and the  
O_NOFOLLOW flag should be ignored). But the build environment needs to  
know about it. Let's see...

The cleanest thing to do for the LOOP_* stuff is substitute the  
constants in losetup.c. (so losetup -c can become an unknown ioctl at  
runtime if necessary, no big deal). I'll add an #ifndef check to  
portability.h for O_NOFOLLOW.

My old build environment (ubuntu 08.04) is also giving a bunch of  
warnings about implicit declaration of all the openat() family  
functions, but the result seems to work. I can try to shoehorn them  
into the old glibc stanza in portability.h, but that won't help if  
you're building against bionic or something. (musl should have them.  
It's all posix-2008 stuff, but ubuntu 08.04 is so old it predates that  
standard.)

Rob
 1359983919.0