Discussion:
[PATCH] netcat: make -l exit after handling a request
(too old to reply)
Josh Gao
2017-06-09 18:05:41 UTC
Permalink
Raw Message
Patch attached.
Rob Landley
2017-06-09 20:31:08 UTC
Permalink
Raw Message
Post by Josh Gao
Patch attached.
What specifically did you fix? (I.E. is this the same "netcat -L
zombies" issue I have on my todo list?)

Sigh, I have local netcat changes I need to merge this with, although
the most intrusive part of that is cleaning it up to use XVFORK() so it
doesn't have that weird liveness analysis workaround ala
http://lists.landley.net/pipermail/toybox-landley.net/2016-December/008796.html
which _might_ still be a musl bug with vfork not annotated
attribute(returns_twice)...

I'm -> <- this close to getting a release out, the only issue left after
dmesg is fixing the help text generator, lemme do that, cut a release,
and _then_ dive into this. :)

Thanks,

Rob
Josh Gao
2017-06-09 21:22:38 UTC
Permalink
Raw Message
Post by Rob Landley
Post by Josh Gao
Patch attached.
What specifically did you fix? (I.E. is this the same "netcat -L
zombies" issue I have on my todo list?)
This makes `nc -l` do what it says it does, and listen for exactly one
connection before exiting, instead of handling connections until a ^C.

terminal1$ echo foo | toybox nc -l -p 1234 > bar
terminal1$ cat bar
bar

terminal2$ echo bar | toybox nc localhost 1234 > foo
terminal2$ cat foo
foo

(Unrelated: is matching the BSD netcat's command line interface a goal? I
keep
accidentally doing `toybox nc -l 1234` and getting bitten by it.)

I'm -> <- this close to getting a release out, the only issue left after
Post by Rob Landley
dmesg is fixing the help text generator, lemme do that, cut a release,
and _then_ dive into this. :)
Sure, thanks :-)

-Josh
Rob Landley
2017-06-25 19:14:01 UTC
Permalink
Raw Message
Post by Rob Landley
Post by Josh Gao
Patch attached.
What specifically did you fix? (I.E. is this the same "netcat -L
zombies" issue I have on my todo list?)
This makes `nc -l` do what it says it does, and listen for exactly one
connection before exiting, instead of handling connections until a ^C.
Ok, getting around to looking at this one (sorry for the delay).

I have a longish todo list for netcat, the oldest entry in which is that
commit f492fccc9ceb was sort of awkward, and I'm pretty sure I POSTED
about why it was awkward but the second half of
http://lists.landley.net/pipermail/toybox-landley.net/2015-January/date.html
is of course missing from the archive because Dreamhost is incompetent.
(I think there are now four holes in the archive because of them? I'd
have to check my notes. http://landley.net/dreamhost.txt and
http://landley.net/dreamhost2.txt have plenty more I haven't bothered to
put online because what's the point?)

Anyway, the problem I have with that commit is now this:

netcat 127.0.0.1 80 -t

doesn't work. It treats -t as a third non-option argument because it hit
a non-option argument first and stopped parsing. But it ONLY does that
if you enable server mode! If you build without server mode, it works fine.)

Originally I had it so that hitting -l or -L would stop the option
parsing (there's an optstr "l^" syntax), but then somebody did (and I
quote from their email):

netcat -l ls -p 1234

And they were surprised it didn't work. (Sigh. It still doesn't, but now
it doesn't work in the xargs way, which I assume more people understand?
Except xargs doesn't parse any non-option arguments _other_ than the
command line to run, and netcat more or less requires them in dialout
mode...)

Other netcat todo items include:

1) switching it to use xconnect() which it predates, and which is hard
because the various users in tree all want slighty different things out
of the getaddrinfo() plumbing and I've made a couple attempts to
unify/genericize it but keep getting pulled alway by $DAYJOB crisis du
jour halfway through and forgetting what design problem details I was
halfway through solving and have to start over again...

2) adding UDP mode so it can handle netconsole, and then deleting
tcpsvd.c out of pending (which was a third party submission for
functionality I'd always planned to have netcat implement).

3) converting netcat to use xvfork() so the fiddly sequencing in commit
4e867b8a3527 isn't necessary anymore. (See the comment "gcc's insane
optimizer can overwrite this", and the comment about liveness analysis
below that. I posted about this to the list back in October, and also
blogged about it but I fell behind editing my blog and restarted in
january so that entry's not actually uploaded; that gap isn't
dreamhost's fault it's me not having enough hours in the day since my
startup ran out of money last summer and those of us left took on extra
responsibilities to cover the lost headcount. It's on the todo list...)

The vfork() issue may actually be a musl-libc bug because built against
musl vfork() hasn't got attribute returns_twice and thus gcc is
"optimizing" stack usage next time it makes a function call, not
protecting local variable values it thinks it won't be reused, and thus
when the parent resumes it has garbled local variables unless you hit it
with a rock like that commit did. But XVFORK() uses a wrapper function
that _is_ annotated returns_twice and thus it should work reliably
whatever musl's headers do?

(Ok, it doesn't really, the vfork() call is an argument to the
returns_twice function, so vfork is called and the result is passed into
that function call, twice. But that should be close enough? Ok, I can
imagine a pathological "optimizer" moving the vfork() call up to the
start of the outer function, calling it once, and cacheing its value. I
wouldn't put that kind of lunacy past modern gcc. But given I can't
#undef a function prototype to redo it, and can't return from a function
after vfork, this is the closest I can come up with to make it work?
This is half the reason the linux kernel craps memory barriers all over
the place, to block gcc's optimizer from doing stupid reordering of
straightforward C...)
Post by Rob Landley
terminal1$ echo foo | toybox nc -l -p 1234 > bar
terminal1$ cat bar
bar
Your call there wouldn't work with the "stop after -l" version either,
the -p would have to come before the -l. :P
Post by Rob Landley
terminal2$ echo bar | toybox nc localhost 1234 > foo
terminal2$ cat foo
foo
Um... yes? That's what it's supposed to do, and what it's doing without
this patch?

Your first hunk told it to run an instance of netcat in chat mode (no
command to run, so instead stdin and stdout of netcat get forwarded to
the connection). This will block until the connection is made, although
if you're redirecting the input you can background it if you want to.

The second hunk then made a connection. The "bar" you wrote to the
second instance got written to the first instance's output, and the
"foo" you wrote to the first instance got forwarded to the second
instance's output. When each instance gets EOF from stdin it calls
shutdown(fd) to let the other instance know, so both instances exit.

I just built netcat from clean toybox, and it did that. I don't see the
problem?
Post by Rob Landley
(Unrelated: is matching the BSD netcat's command line interface a goal?
I hadn't planned on it?

I wrote my first netcat in 2001 when there WAS no standard version, just
a source posting from somebody named "Frodo" I think? My old one's
probably still on the net somewhere... yup.

http://dvpn.sourceforge.net/old/netcat.c

I then ripped busybox's version a new one a few years later because it
just got stuff _wrong_ (for example it didn't use shutdown() on the
connection to pass along EOF info which meant if you did "echo GET / |
netcat cnn.com 80" it wouldn't work, which at the time was still
relevant). I also made server mode work and so on. At the time, linux
distros still didn't install a netcat by default, so the functionality
busybox implemented was new. (My busybox mount was also the first one to
autodetect when to use loop devices so you didn't have to say "-o loop".)

But people expected busybox implementations of commands to be inferior
rather than to ever be _more_ capable than others, so even though I
submitted my "count" program to busybox in 2003
(http://lists.busybox.net/pipermail/busybox/2003-October/043590.html
which got bikeshedded into pipe_progress) which I'd been using since I
wanted to know how long imaging disks too while working at boxxtech in
2000, that didn't a "pv" package from showing up later and being
considered the "real" one that busybox must obviously have been
retroactively copying.)

Sometimes I bow to this stuff. I added "touch -l" to set the length of a
file in 2007 (since there was no obvious way to do it), the FSF added a
"truncate" command in 2009, I noticed and implemented the new command in
2011:

http://landley.net/notes-2011.html#08-06-2011

But my ubuntu 14.04 command line goes:

$ netcat --help
netcat: invalid option -- '-'
This is nc from the netcat-openbsd package. An alternative nc is
available in the netcat-traditional package.

And I'm pretty sure I installed that netcat manually? (It says
"Automatically installed: no".) Which implies there _isn't_ a netcat in
the base os, or wasn't a while back.

Meanwhile, Denys Vlasenko made a different call back in 2009:

http://lists.busybox.net/pipermail/busybox/2009-July/069920.html

Yes, busybox has two versions of netcat now. Because nothing says small
and simple like having multiple implementations of the same command...

I note that the "netcat 1.10" he added was released January 2, 2007, and
the last commit I did to the busybox version before I left was
d921b2ecc0d2 on August 3 2006, so once again I get dinged for not being
compatible with something that wasn't released until after I wrote mine.

I'm aware there is a "gnu netcat". That is the extent of my knowledge on
that subject, and I'm pretty happy with that decision.
Post by Rob Landley
I keep accidentally doing `toybox nc -l 1234` and getting bitten by it.)
Um, what do you expect that to do?

What it's _trying_ to do is tell you the port number it auto-selected
before backgrounding the -l command ready to run "1234". Since you
didn't specify a -p the network layer will auto-select a port, printing
it lets you do PORT=$(netcat -l blah) and then use that port number in a
script.

It does -p and -s for consistency: you specify the local port and local
address the same way dialing out as in sever mode. And if you don't it
provides defaults (and announces them):

$ ./netcat -l echo hello
40351
$ pgrep netcat
2298
$ ./netcat 127.0.0.1 40351
hello
$ pgrep netcat
$
Post by Rob Landley
I'm -> <- this close to getting a release out, the only issue left after
dmesg is fixing the help text generator, lemme do that, cut a release,
and _then_ dive into this. :)
Sure, thanks :-)
I got the release out, and now it's the weekend I'm digging out some
backlog, but I'm still not quite sure what this patch fixes?

Having two pollinate() calls (and thus two alarm(0) calls) is fallout
from that stupid vfork weirdness, part of the XVFORK() work is unifying
that again (and testing it with llvm, current gcc, and older gcc, on
multiple architectures) to make sure my returns_twice theory holds up.
(Supporting nommu and non-nommu with the same code path is fiddly). The
need for that much testing is why I've been getting mkroot ready to be a
testing environment before revisiting this, and also why I'm reluctant
to apply this patch without a better idea what it's supposed to fix...

Hmmm... another issue, if xexec() doesn't actually exec sockfd() gets
inherited by the child. It should explicitly close it the way it's doing
in1...

Lemme clean up my pending issues and see if you still have yours?
Post by Rob Landley
-Josh
Thanks,

Rob
Josh Gao
2017-06-27 01:22:42 UTC
Permalink
Raw Message
Post by Rob Landley
Post by Josh Gao
terminal1$ echo foo | toybox nc -l -p 1234 > bar
terminal1$ cat bar
bar
Your call there wouldn't work with the "stop after -l" version either,
the -p would have to come before the -l. :P
Seems to work for me?
Post by Rob Landley
Post by Josh Gao
terminal2$ echo bar | toybox nc localhost 1234 > foo
terminal2$ cat foo
foo
Um... yes? That's what it's supposed to do, and what it's doing without
this patch?
Your first hunk told it to run an instance of netcat in chat mode (no
command to run, so instead stdin and stdout of netcat get forwarded to
the connection). This will block until the connection is made, although
if you're redirecting the input you can background it if you want to.
The second hunk then made a connection. The "bar" you wrote to the
second instance got written to the first instance's output, and the
"foo" you wrote to the first instance got forwarded to the second
instance's output. When each instance gets EOF from stdin it calls
shutdown(fd) to let the other instance know, so both instances exit.
I just built netcat from clean toybox, and it did that. I don't see the
problem?
That's not what I'm seeing. With a freshly compiled tip of tree master
(279eb227), the listener doesn't ever exit:

# Terminal 1
***@cyclops2:/android/upstream/toybox$ echo foo | ./toybox nc localhost
1234
foo
***@cyclops2:/android/upstream/toybox$ echo bar | ./toybox nc localhost
1234
***@cyclops2:/android/upstream/toybox$ echo baz | ./toybox nc localhost
1234
***@cyclops2:/android/upstream/toybox$

# Terminal 2
***@cyclops2:/android/upstream/toybox$ echo foo | ./toybox nc -l -p 1234
foo
bar
baz
^C
***@cyclops2:/android/upstream/toybox$

`toybox nc -l -p 1234 echo foo` works fine because the process execs
without forking.
Using -l without a command will fall through to pollinate, finish, and then
try to listen again.
Josh Gao
2017-06-28 22:47:30 UTC
Permalink
Raw Message
Post by Rob Landley
1) switching it to use xconnect() which it predates, and which is hard
because the various users in tree all want slighty different things out
of the getaddrinfo() plumbing and I've made a couple attempts to
unify/genericize it but keep getting pulled alway by $DAYJOB crisis du
jour halfway through and forgetting what design problem details I was
halfway through solving and have to start over again...
BTW, I took a quick look at this because we have users that want -4/-6 (and
IPv6 support in general). `nc -s` makes it so that you can't use xconnect
because you don't know what to bind to until after you've resolved the
target
address. Something like xbind_and_connect might work, but there's also
things
that we might want to do in between socket and bind (e.g. setting
SO_REUSEADDR).

The thing that everyone really wants is a way to iterate over getaddrinfo
results;
maybe that's what should be exposed? I have a rough proof of concept patch
attached that implements this and uses it in netcat.

(There's also another edge case with -s: what happens if the host you pass
in
resolves to multiple addresses? OpenBSD's netcat seems to just bind the
first
compatible address it resolves to, so we can maybe just ignore this.)

-Josh
enh
2017-06-30 01:59:55 UTC
Permalink
Raw Message
ping? various networking folks are looking to add tests that use
netcat, and i'd rather start them off on toybox netcat rather than BSD
and then have to move them across to toybox later. (obviously there's
other missing stuff in toybox, but these patches are the only things
they actually need right now.)
Post by Josh Gao
Post by Rob Landley
1) switching it to use xconnect() which it predates, and which is hard
because the various users in tree all want slighty different things out
of the getaddrinfo() plumbing and I've made a couple attempts to
unify/genericize it but keep getting pulled alway by $DAYJOB crisis du
jour halfway through and forgetting what design problem details I was
halfway through solving and have to start over again...
BTW, I took a quick look at this because we have users that want -4/-6 (and
IPv6 support in general). `nc -s` makes it so that you can't use xconnect
because you don't know what to bind to until after you've resolved the
target
address. Something like xbind_and_connect might work, but there's also
things
that we might want to do in between socket and bind (e.g. setting
SO_REUSEADDR).
The thing that everyone really wants is a way to iterate over getaddrinfo
results;
maybe that's what should be exposed? I have a rough proof of concept patch
attached that implements this and uses it in netcat.
(There's also another edge case with -s: what happens if the host you pass
in
resolves to multiple addresses? OpenBSD's netcat seems to just bind the
first
compatible address it resolves to, so we can maybe just ignore this.)
-Josh
_______________________________________________
Toybox mailing list
http://lists.landley.net/listinfo.cgi/toybox-landley.net
--
Elliott Hughes - http://who/enh - http://jessies.org/~enh/
Android native code/tools questions? Mail me/drop by/add me as a reviewer.
Josh Gao
2017-06-30 19:17:43 UTC
Permalink
Raw Message
Updated the original patch to just jump to cleanup, instead of rearranging
things.

Also, polished up the proof of concept xgetaddrinfo (this needs a better
name)
patch, and used it to implement -4, -6 (plus another patch to do -u).
Post by enh
ping? various networking folks are looking to add tests that use
netcat, and i'd rather start them off on toybox netcat rather than BSD
and then have to move them across to toybox later. (obviously there's
other missing stuff in toybox, but these patches are the only things
they actually need right now.)
Post by Josh Gao
Post by Rob Landley
1) switching it to use xconnect() which it predates, and which is hard
because the various users in tree all want slighty different things out
of the getaddrinfo() plumbing and I've made a couple attempts to
unify/genericize it but keep getting pulled alway by $DAYJOB crisis du
jour halfway through and forgetting what design problem details I was
halfway through solving and have to start over again...
BTW, I took a quick look at this because we have users that want -4/-6
(and
Post by Josh Gao
IPv6 support in general). `nc -s` makes it so that you can't use xconnect
because you don't know what to bind to until after you've resolved the
target
address. Something like xbind_and_connect might work, but there's also
things
that we might want to do in between socket and bind (e.g. setting
SO_REUSEADDR).
The thing that everyone really wants is a way to iterate over getaddrinfo
results;
maybe that's what should be exposed? I have a rough proof of concept
patch
Post by Josh Gao
attached that implements this and uses it in netcat.
(There's also another edge case with -s: what happens if the host you
pass
Post by Josh Gao
in
resolves to multiple addresses? OpenBSD's netcat seems to just bind the
first
compatible address it resolves to, so we can maybe just ignore this.)
-Josh
_______________________________________________
Toybox mailing list
http://lists.landley.net/listinfo.cgi/toybox-landley.net
--
Elliott Hughes - http://who/enh - http://jessies.org/~enh/
Android native code/tools questions? Mail me/drop by/add me as a reviewer.
Josh Gao
2017-07-18 18:44:19 UTC
Permalink
Raw Message
Sorry for pestering, but have you gotten a chance to take a look at these
yet? We're starting to grow dependencies on some of these patches (tests
that want to be checked in Soon are being written against a checkout with
the patches manually applied)
Post by Josh Gao
Updated the original patch to just jump to cleanup, instead of rearranging
things.
Also, polished up the proof of concept xgetaddrinfo (this needs a better
name)
patch, and used it to implement -4, -6 (plus another patch to do -u).
Post by enh
ping? various networking folks are looking to add tests that use
netcat, and i'd rather start them off on toybox netcat rather than BSD
and then have to move them across to toybox later. (obviously there's
other missing stuff in toybox, but these patches are the only things
they actually need right now.)
Post by Josh Gao
Post by Rob Landley
1) switching it to use xconnect() which it predates, and which is hard
because the various users in tree all want slighty different things out
of the getaddrinfo() plumbing and I've made a couple attempts to
unify/genericize it but keep getting pulled alway by $DAYJOB crisis du
jour halfway through and forgetting what design problem details I was
halfway through solving and have to start over again...
BTW, I took a quick look at this because we have users that want -4/-6
(and
Post by Josh Gao
IPv6 support in general). `nc -s` makes it so that you can't use
xconnect
Post by Josh Gao
because you don't know what to bind to until after you've resolved the
target
address. Something like xbind_and_connect might work, but there's also
things
that we might want to do in between socket and bind (e.g. setting
SO_REUSEADDR).
The thing that everyone really wants is a way to iterate over
getaddrinfo
Post by Josh Gao
results;
maybe that's what should be exposed? I have a rough proof of concept
patch
Post by Josh Gao
attached that implements this and uses it in netcat.
(There's also another edge case with -s: what happens if the host you
pass
Post by Josh Gao
in
resolves to multiple addresses? OpenBSD's netcat seems to just bind the
first
compatible address it resolves to, so we can maybe just ignore this.)
-Josh
_______________________________________________
Toybox mailing list
http://lists.landley.net/listinfo.cgi/toybox-landley.net
--
Elliott Hughes - http://who/enh - http://jessies.org/~enh/
Android native code/tools questions? Mail me/drop by/add me as a reviewer.
Rob Landley
2017-07-19 19:45:59 UTC
Permalink
Raw Message
Post by Josh Gao
Sorry for pestering,
Pester away. (General rule is if I haven't gotten back to you in a week,
it's my fault and I should be dinged.)
Post by Josh Gao
but have you gotten a chance to take a look at
these yet? We're starting to grow dependencies on some of these patches
(tests that want to be checked in Soon are being written against a
checkout with the patches manually applied)
I'll take a look tonight. Sorry for the delay.

Thanks,

Rob
Rob Landley
2017-07-21 21:45:53 UTC
Permalink
Raw Message
Post by Rob Landley
Post by Josh Gao
Sorry for pestering,
Pester away. (General rule is if I haven't gotten back to you in a week,
it's my fault and I should be dinged.)
My other general rule is if I don't get to it a couple days _after_ such
poking, I should revert my local changes and apply your patch, then redo
my stuff on top of yours, and worry about any cleanup I want to do after
it's no longer a blocking issue for the submitter.

Which I've now done.

(Sorry, hip deep in $DAYJOB stuff at the moment...)

Rob

Loading...