[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Tue, 23 Jul 2013 04:57:56 +0100
From: Rafael Waldo Delgado Doblas <lord.rafa@...il.com>
To: john-dev@...ts.openwall.com
Subject: Re: cryptocurrency mining fallback in JtR
Alexander,
> 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.
I have written a draft here:
http://openwall.info/wiki/john/cgminer_integration
> 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?
Acturally yes, the first thing that I tried to do was add to the Makefile
../run/cgminer: ../run/john
cd cgminer; ./configure; make;.....
But the configure scrypt didn't work. Maybe there are a problem with the
make env variables.
> 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.
OK now there 3 options:
APIPort -> Port used by JtR to connect with an existing CGMiner API.
AfterEnd -> Used to tell JtR if must to start a new CGMiner instance or not
(only starts a new one if there are none running)
AfterEndOptions -> Used to set the new CGMiner instance parameters.
> 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.)
As I told in my last mail, those changes were made in the repo
https://github.com/LordRafa/cgminer on branch 2.10
> 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.
As we want to keep the environment I'm using execv instead of execve. Now
dtach works perfectly.
> Why "-c /tmp/cgminer"? In general, explicit use of /tmp is considered a
> security hole, with relatively few exceptions.
I move it to run folder.
> 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?
Was a mistake fixed.
> 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?
Done.
> 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.
I made some changes to allow work in windows (I didn't test it) and I
removed this directive.
> 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().
Improved in the new code.
> 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.
I copy-paste from the CGMiner Readme.
> 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.
Done in the new code.
> 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.)
To do.
> If you simply omit the option, would there really be a dependency?
> Wouldn't cgminer simply build without ncurses if the ncurses development
> library is not installed?
> As to preference, I think we shouldn't override cgminer default without
> reason.
Done.
Rafael.
2013/7/20 Solar Designer <solar@...nwall.com>
> 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
>
Content of type "text/html" skipped
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.
