[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20130720202944.GA7154@openwall.com>
Date: Sun, 21 Jul 2013 00:29:44 +0400
From: Solar Designer <solar@...nwall.com>
To: john-dev@...ts.openwall.com
Subject: Re: cryptocurrency mining fallback in JtR
Rafael,
On Tue, Jul 16, 2013 at 12:05:01AM +0100, Rafael Waldo Delgado Doblas wrote:
> I have pushed the changes to my github repo. Now is working on linux.
Great. Are there any usage instructions for an advanced end-user, so
that we could ask someone to try this out? If not yet, then please
write some.
> In order to build the project, cgminer and dtach, I made a bash script
> called build.sh. To compile you need to provide the correct architecture as
> parameter.
OK, although it could be cleaner to turn this into an extra Makefile
(then invoke with "make -f Makefile.miner TARGET=...") or extra targets
inside the main Makefile (then invoke it with "make TARGET=...") or even
have cgminer and dtach built by default after a normal build, without
any unusual syntax on make's command line (this is what we discussed on
IRC originally). Do you find the latter hard to implement?
> This are the new options in the config file:
>
> MinerAfterEnd -> launchs a cgminer instance at the end of JtR if there are
> not a cgminer instance running.
> MinerPoolURL -> pool url.
> MinerPoolUSR -> pool user.
> MinerPoolPWD -> pool password.
> MinerPlatform -> cgminer platform to use.
> MinerAPIPort -> port to check where the CGminer instance (if there are any)
> is listening.
Shouldn't most of these (all but MinerAfterEnd) be combined into one
option called simply Options (where cgminer's command-line options
would be specified) and MinerAfterEnd renamed to simply AfterEnd (or we
may invent a better name) since you already put them into their new
section called Miner, making the Miner* prefix redundant?
I understand that with your use of execve() you'd have to parse the
command-line options into individual argv[] elements, yet I think you
can do that.
For ease of use it is sufficient to provide some sample default Options
string. It will illustrate proper usage just as well as the multiple
separate options do.
> If any cgminer instance is found JtR will stop the gpus used by JtR and
> after it finish it will start to mining again.
Great. Where are the corresponding changes to cgminer code (for the API
enhancement you mentioned)? I tried looking at every commit you made on
the bleeding-jumbo branch in https://github.com/LordRafa/JohnTheRipper
and there doesn't appear to be a commit with just your cgminer changes.
There must have been a commit like that, separate from pulling
upstream's cgminer code in.
We need to review those changes and then likely propose them upstream.
(For that, we also need to forward-port them to the latest cgminer.)
> I'm having problems with dtach. When I do this:
>
> char *envp[] = {"TERM=xterm", NULL};
> char *argv[] = {"./cgminer",
> "-o", miner_pool_url,
> "-u", miner_pool_usr,
> "-p", miner_pool_pwd,
> "--api-port", miner_api_port,
> "--api-listen", "--api-allow", "W:127.0.0.1/24",
> miner_platform,
> NULL
> };
> execve("./cgminer", argv, envp);
>
> cgminer works perfectly but when I try to do this:
>
>
> char *envp[] = {"TERM=xterm", NULL};
> char *argv[] = {"./dtach", "-c", "/tmp/cgminer",
> "./cgminer",
> "-o", miner_pool_url,
> "-u", miner_pool_usr,
> "-p", miner_pool_pwd,
> "--api-port", miner_api_port,
> "--api-listen", "--api-allow", "W:127.0.0.1/24",
> miner_platform,
> NULL
> };
> execve("./dtach", argv, envp);
>
> cgminer says that there no devices to use.
You're resetting the environment. You should preserve it. At least the
XAUTHORITY variable needs to be preserved for access to AMD GPUs to work.
Why do you set TERM=xterm? Perhaps you should not.
Why "-c /tmp/cgminer"? In general, explicit use of /tmp is considered a
security hole, with relatively few exceptions.
Some other assorted comments:
Is src/cgminer/.deps (and the files in this directory), which got into
your git tree, part of cgminer distribution, or did you commit it by
mistake?
Please add copyright and license statements to your added source files
(src/miner*).
You're assuming that cgminer and dtach are in the current directory, but
what if john is invoked with a path to it specified? Maybe you should
call path_expand() on "$JOHN/cgminer" and "$JOHN/dtach", and put these
strings in params.h along with other similar ones.
Your source code indentation is consistently inconsistent with that of
the rest of JtR. We use 8 char tabs (and 4 chars for half-indentation
on line-wrapped statements). You use 3 char indentation. Can you adopt
ours?
Why all the "#ifdef __linux__"? I understand that you have not tested
on other platforms yet, but let's try to keep the code available on all
platforms anyway - and only deactivate it on platforms where it's known
not to work.
Your multiple mentions of 4096 in miner.c is an example of how not to do
it. ;-) Use a #define or a global constant instead, and consider
passing sizeof(buf) as a parameter to get_data().
Why the /24 netmask on 127.0.0.1? Why not /32 or /8? /24 seems
entirely arbitrary (/32 or /8 would not), unless there's some reason.
miner.c unnecessarily exports multiple symbols to other source files.
Please use "static" on all global symbols that are not to be exported.
There should be very few globals without the "static" keyword on them.
You're not checking for error returns from many of the calls, e.g. from
fork(). Please add such error checking, and please use pexit() from
misc.c on fatal errors and fprintf(stderr, "...\n") for warnings.
When you reply to e-mails on this mailing list, please quote inline if
you don't mind:
http://www.complang.tuwien.ac.at/anton/mail-news-errors.html
http://www.netmeister.org/news/learn2quote.html
Your recent reply to Lukas did not quote any portion of his message at
all, even though it was a reply to his specific requests.
You use blocking I/O and no timeouts for communication over the socket.
This is fine for testing, but eventually it'd be nice to make this more
robust. (This is not to be worked on yet.)
Alexander
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
