On 10/19/14 18:13, Andy Lutomirski wrote: > On Sun, Oct 19, 2014 at 2:48 PM, Rob Landley <[email protected]> wrote: >> I imported the first one, but applied the second as a patch because new >> commands go in the "pending" directory so I don't lose track of what >> I've fully reviewed yet. >> >> On 10/17/14 22:01, Andy Lutomirski wrote: >>> nsenter: A tool to use setns(2) >> >> I don't have this command on my host system, and it's not even in the >> python "install this package if you want this command" thing. > > It's in util-linux. I bet you're using Ubuntu or Debian :) Except > for very new Debians (IIRC), they're both quite a few years behind on > util-linux updates.
Possibly I should have a third directory for entries where the standard is the Linux man page maintained by Michael Kerrisk: http://man7.org/linux/man-pages/man1/nsenter.1.html That said, there isn't a good way to snapshot a version of that, or point to a specific release. With posix I could still point to the 2001 spec after 2008 came out. (When the 2013 spec went up they replaced the 2008 pages in situ, which is obnoxious, but _mostly_ it didn't change. Still, I'd probably be referring to it as posix-2013 and not still using my old local 2008 snapshot if they _hadn't_ done that. The easy way to get me to reject an upgrade is to try to force it down my throat...) I should poke Michael and see if there's some way of getting LTS versions of this... >>> This implements all of the namespace parts of nsenter, but UID and GID >>> switching are missing, as are -r and -w (both because they're not strictly >>> necessary and because the nsenter manpage has an insufficient >>> description of how they work). It looks like -r is just chroot and -w is just chdir. Except that it's not chroot, it's pivot_root in the new namespace: http://landley.net/notes-2011.html#02-06-2011 But still: doable. >>> diff -r 5330d3f88fa3 -r 5520248c82b2 toys/other/nsenter.c >>> --- /dev/null Thu Jan 01 00:00:00 1970 +0000 >>> +++ b/toys/other/nsenter.c Fri Oct 17 20:00:58 2014 -0700 >>> @@ -0,0 +1,104 @@ >>> +/* nsenter.c - Enter existing namespaces >>> + * >>> + * Copyright 2014 andy Lutomirski <[email protected]> >>> + >>> +USE_NSENTER(NEWTOY(nsenter, >>> "<1F(no-fork)t#(target)i:(ipc);m:(mount);n:(net);p:(pid);u:(uts);U:(user);", >>> TOYFLAG_USR|TOYFLAG_BIN)) >> >> All the ";" characters mean you can only supply the option to a longopt >> with =, as in "--ipc=abc" but "--ipc abc" are considered two separate >> arguments (as in "--ipc= abc"). >> >> (Because ls --color works that way. Because gnu, apparently.) >> >> I'm guessing this is _not_ the behavior you want? I don't even remember >> what I made the short options do in that case, I designed it to apply to >> a bare longopt that needed it. > > I think this is the behavior I want. I haven't checked exactly for > longopts, but, for the short form, the behavior seems to match > util-linux's. Quite possibly I implemented "accept next argument unless it's another option". If not, I can make it do something like that. (The hard part's not implementing the correct behavior, it's defining what the correct behavior should _be_, and this was long enough ago I'd have to investigate to remember the details...) >> That said, if unhare already has an array of these symbols we can reuse >> that and then ahve the names just be a "user\0uts\0pid\0net\0mnt\ipc" >> string. >> >> Also, I try not to define global data outside the GLOBALS block. If it >> needs to be initialized, I do that in the function's main(). (There are >> times when having static global data is sufficiently cheaper to be worth >> it, but it's not my first choice. Finding a way to avoid needing it is >> my first choice.) > > That should have been const, in which case I think it makes sense to > have it outside GLOBALS. But I'll defer to your proposed tinier > version. Eh, there's aways a funny little dance of "this function serves two masters but can only have one flag context #defined at any given time". There are ways around that (such as have the headers generate FLAGS_cmdname_a that are always enabled and then #define FLAG_a FLAGS_cmdname_a in the FOR_cmdname context) but "ability to do more stuff" vs "complexity" is a tradeoff... >> (P.S. I can do this cleanup, just explaining conceptually how I'd >> simplify it.) >> >>> +GLOBALS( >>> + char *nsnames[6]; >>> + long targetpid; >>> +) >> >> You have a macro above, and a hardwired 6 here. (I know the macro >> doesn't apply to globals, but if you're willing to hardwire 6 here >> having the macro at all is not a net win.) > > At least it reduces the number of them. :-/ At the expense of having a place that LOOKS like it's trying to be the one and only place you #define that information, except it isn't really. Usually those sorts of macros exist so there's only one place you have to change it. If there's more than one place, having the macro may be a net loss. However, we can have the array length in there BE the place and then use ARRAY_LEN(TT.nsnames) for the value everywhere else... >> >>> +static void enter_by_name(int idx) >>> +{ >> >> This function has exactly one caller, and can be inlined at that >> callsite. Doing so often reveals opportunities for further simplification. > > I wrote it this way because I might want to split the loop if I ever > implement the setuid stuff. But, on second thought, that might not be > necessary. > >> >>> + int fd, rc; >>> + char buf[64]; >> >> We have "toybuf", a static 4k global char array, available to all >> commands. (And there's another one "libbuf" for library code.) >> >>> + char *filename = TT.nsnames[idx]; >>> + >>> + if (!(toys.optflags & (1<<idx))) return; >>> + >>> + if (!filename || !*filename) { >> >> Ok, I'll bite: where did TT.nsnames[] get set to anything other than >> NULL, so that filename would not be NULL here? (The GLOBALS block starts >> zeroed.) By the option parsing logic. My bad, not used to seeing that as an array instead of individual variables. (Makes sense in this case though.) > I think that happened when I did toybox nsenter -t PID -U command. > This should be interpreted as -U with no argument followed by a > command, and filename ended up being the empty string. If the arguments are optional, they can be null. Makes perfect sense, I just left off reading because I had to run and didn't read enough to answer my own question. :) Rob _______________________________________________ Toybox mailing list [email protected] http://lists.landley.net/listinfo.cgi/toybox-landley.net
