Witko / nvidia-xrun

Utility to run separate X with discrete nvidia graphics with full performance
GNU General Public License v2.0
488 stars 69 forks source link

Find & use a free TTY automatically [already implemented in my fork] #61

Open Fincer opened 6 years ago

Fincer commented 6 years ago

Due to my laziness in picking up a free TTY and running nvidia-xrun blabla manually every damn time, I forked nvidia-xrun yesterday and added the following features:

I did quite radical changes to the original codebase. If someone finds this inconvenient...well, sorry for that.

Testing has not been so compherensive, so bugs may exist and implementation is not perfect (read: works for me). Feel free to improve, use in original nvidia-xrun repo or/and fork.

I use this personally only for nvidia-xrun openbox-session command.

andrebrait commented 6 years ago

By finding a free TTY, you mean only finding one, or finding and switching to it? Or even better, finding, switching and running the command on it? I don't even know if that's possible, but it's the first thing that came to my mind.

Does the user still need to type its credentials upon going to the new TTY?

Fincer commented 6 years ago

Yes, finding, switching, running a command in it - and when the GUI session terminates, it switches back to your original TTY. So, in a nutshell: the script looks for available TTYs on your system, checks which ones are already in use, and then uses the first free/non-reserved TTY it can find.

Of course, you can switch between TTYs during runtime like in the original nvidia-xrun.

I implemented my solution in a way so that your credentials are not required: the script uses systemd + agetty autologin feature to login as your current user. Afterwards, when you log out your Nvidia Xorg session, the script unloads Nvidia in the same way than the original nvidia-xrun does, and finally switches back to your Intel TTY.

I added some handling for situations where Ctrl + C (SIGINT) is received by nvidia-xrun, so that Nvidia should technically still unload. However, I found this somewhat buggy, and couldn't fix it properly as it has something to do with bbswitch. Fixes are welcome.

andrebrait commented 6 years ago

That sounds very nice. What did you do to handle the signals? Traps?

Witko commented 6 years ago

Hi @Fincer , i'd love to include this to nvidia-xrun if you are ok with it. I need to check it deeper what is there but i dont have too much time these days.

andrebrait commented 6 years ago

Other than opening the openbox-session, is there a way to make it run other apps, even if inside the openbox session?

Putting this in a better way, do you think it's possible?

And does it still require root/sudo?

Fincer commented 6 years ago

Sorry for being absent for a while, I've been busy with some mid-summer activities (Northern hemisphere).

@andrebrait

That sounds very nice. What did you do to handle the signals? Traps?

Signals...well. Hmm. Talking about exact POSIX/Unix signals here?

If user presses CTRL+C (gives SIGINT), the script executes function emergency (defined in the script) which handles turning off process of your Nvidia GPU, removes any possible systemd overrides and exits cleanly.

However, the script may fail to turn off the Nvidia GPU which gives a warning message to your terminal where nvidia-xrun is being executed. I've not had time to pin down the root cause, but I doubt it's caused by some bug in bbswitch.

I've not implemented signal handling for Ctrl+Z or killing the process, not sure if possible. I haven't had time to investigate this.


Key command (XRUN_COMMAND variable in my script fork) is completely run in the other TTY session, not in the Intel TTY session (where you execute nvidia-xrun command). XRUN_COMMAND below:

XRUN_COMMAND="xinit ${EXECL} -- $NEWDISP vt${NVIDIA_TTY_NUM} -nolisten tcp -br -config nvidia-xorg.conf -configdir nvidia-xorg.conf.d"

Values for variables EXECL, NEWDISP and NVIDIA_TTY_NUM seen above are being determined during the script runtime.


About script execution

@Witko

i'd love to include this to nvidia-xrun if you are ok with it. I need to check it deeper what is there but i dont have too much time these days.

Sure, feel free to implement it. If you need any explanations or you have any questions, feel free to ask.

I think this fork of mine requires more testing. My testing has been very narrow-scale.

@andrebrait

Other than opening the openbox-session, is there a way to make it run other apps, even if inside the openbox session?

This was very crucial to me, so I tested openbox-session + Windows Steam client with some D3D11 games (Wine + DXVK), and it works fine for me.

I have not directly tested any other desktops or programs than nvidia-xrun openbox-session. My key point is: once you-ve logged in, you can run your applications in the new TTY session like you normally always do.

And does it still require root/sudo?

Unfortunately, yes. Getting rid of any password prompts is in my priority list. I will take a look on it. You can take a look on execute_user and execute_root functions which are defined in the nvidia-xrun script file (my fork). Those functions are the only ones which include any use of sudo in the script.

I've thought about SUID & sticky bits here, not sure if any benefits.

andrebrait commented 6 years ago

Ok, so, for handling signals, yes, you can do that with signal traps in a shell script

https://www.shellscript.sh/trap.html

And for not having to use root, I used a sudoers file for PRIME Indicator Plus, which you can check here: https://github.com/andrebrait/prime-indicator/tree/master/etc/sudoers.d

The issue with this approach is that nvidia-xrun can, in theory, run any command. So that would be introduce a huge security flaw...

andrebrait commented 6 years ago

You can also handle the activation/deactivation of the card with a systemd service, like what bumblebeed does

Fincer commented 6 years ago

Yup! I will investigate & check out these bit later once I'll have some time for that. Thank you!

Fincer commented 6 years ago

Allright, I implemented suggested sudo solution. Other changes include implementation of '-u' // '-unload' argument which is for unloading attempt of Nvidia GPU without triggering any application launch process.

The script has some known bugs which are listed in header section of the script file as comments.

@andrebrait

The issue with this approach is that nvidia-xrun can, in theory, run any command. So that would be introduce a huge security flaw...

Yes, this is true. However, some security measures can be taken:

Some security issues include:

You can also handle the activation/deactivation of the card with a systemd service, like what bumblebeed does

Care to share more details about that?

Ok, so, for handling signals, yes, you can do that with signal traps in a shell script.

Yes, but according to the documentation, kill signal can't be customized (e.g. running a custom function when this signal is caught). This is the only signal type which gives me a headache, mostly because 1) Nvidia GPU won't be turned off 2) some getty@ttyN.service crap is left in '/etc/systemd/system/' folder, unlike with SIGINT//Ctrl + C which I can control in emergency function

andrebrait commented 6 years ago

Ok, one idea then is to make a process fork and watch for the child's death.

So that would demand two scripts/something that forked and then launched nvidia-xrun itself. This other thing could watch for nvidia-xrun's death and act accordingly.

Of course, this other thing could be killed and then nothing would help, but that would cover the use-case where nvidia-xrun ends up receiving a SIGKILL.

As for the service, I'll look into it a bit more to see if it's doable here. But basically having a systemd service that polls for the card being used and/or responds to nvidia-xrun running or not running and then takes actions accordingly is probably doable. Not elegant (I don't like polling, but polling every 10 seconds or so isn't really bad for the system, given how easy it is to poll for that).

Anyway, ideas that need to be evaluated and polished a lot.

As for the security solutions: those still would open-up the possibility of having stuff like nvidia-xrun runMyMalware, wouldn't it?

Fincer commented 6 years ago

nvidia-xrun + SIGKILL

Practically, two issues raise if we just kill nvidia-xrun process:

1) So in theory, as you said, one option could be:

Ok, one idea then is to make a process fork and watch for the child's death.

2) I'd still like to think other options, too.

nvidia-xrun + a new systemd service

As for the service, I'll look into it a bit more to see if it's doable here. But basically having a systemd service that polls for the card being used and/or responds to nvidia-xrun running or not running and then takes actions accordingly is probably doable. Not elegant (I don't like polling, but polling every 10 seconds or so isn't really bad for the system, given how easy it is to poll for that).

Sounds good. Let us know if you come up with something here! For meanwhile, I rely on bbswitch stuff.

nvidia-xrun + security/root issues

As for the security solutions: those still would open-up the possibility of having stuff like nvidia-xrun runMyMalware, wouldn't it?

Yes, I can't deny that. But I count on the fact that runMyMalware executable should be trusted beforehand, anyway. In the end, it is user's responsibility to trust the program he/she runs on the system, isn't it? There is also another thing... :

In my nvidia-xrun fork, the first script argument (in your case runMyMalware) is taken as input, and run as the current regular user. The logic goes as follows:

So, in the end, running sudo nvidia-xrun runMyMalware should result the program to be executed as the current regular user.

One thing,though: I haven't done proper security testing for the fork, so there may be security bugs/leaks present.

And another thing: If you gain root access in your current X session (by running su root command in user's bash), you can run nvidia-xrun runMyMalware though. However, runMyMalware in still run as the current regular user, not as root.

andrebrait commented 6 years ago

Ok, so here are some ideas (to which I apologize for not implementing myself but I don't have the time to do that right now)

I noticed sudo is needed for several parts of the script but not for the actual command running.

So, to address the security problem, I think you should:

  1. Split nvidia-xun (and nvidia-xinitrc) into the part for running the command (which now resides in nvidia-xinitrc, is that correct?) and the parts that need sudo, which should not interact with the command.
  2. Add those parts that need sudoto the nvidia-xrun-sudoers file.
  3. Make the unpriviledged script call the priviledged ones and then run the command after the switch to the new TTY. That way the arbitrary command won't run with super user permissions by default.

And for the power management, we'd still need bumblebee. But we'll find a way :)

andrebrait commented 6 years ago

Quick example:

And so

$ ./runner.sh echo
Printing this as 502
Printing this as 0
#There's a newline here because of the echo command
andrebrait commented 6 years ago

Ok, I got a even sillier idea.

Forget what I said above. That's problematic because it would involve calling those scripts in a shell that's not the current one.

Sice we're using the sudoers file, just... run everything that needs sudo with sudo!!

Like

sudo this #needs sudo sudo that #needs sudo too "$*" #to run the command as the use who called the script whoever it is.

andrebrait commented 6 years ago

Ok, I got a even sillier idea.

Forget what I said above. That's problematic because it would involve calling those scripts in a shell that's not the current one.

Sice we're using the sudoers file, just... run everything that needs sudo with sudo!!

Like

sudo this #needs sudo
sudo that #needs sudo too
"$*" #to run the command as the use who called the script whoever it is.
andrebrait commented 6 years ago

The only step needed to really account for bad usage of the script would be to not run it if the command is nvidia-xrun sudo foo

sudo nvidia-xrun foo should be fine. But the other way shouldn't because we'd potentially allow someone to run the script as a super user even if they don't have the password or something.

Fincer commented 6 years ago

Sorry for long delay, haven't been able to access my computer for a few days. Writing this message on my phone.

I will implement that parent-child script idea, quite trivial one and doesn't require .bashrc hassle from users.

It is hard for me to see it worthy to divide between root and user parts as you suggested. Almost all commands used by the fork require root permissions, excluding the actual Nvidia GPU execution part. However, I want to make sure that any program which is given as input to the script would not be run as root in any case because of obvious security reasons. I have done some test-runs for this.

1) Arguments in the previous paragraph 2) and the message where you indicated a possible security issue in case of sudo nvidia-xrun sudo malwareCommand made me aware of...well...I will block this hole, so the script will reject sudo or su as input arguments. Thank you for the hint!

So, two implementations on their way, once I'll get my computer back online!


Edit: Done. Modifications:

IngeniousDox commented 6 years ago

@Fincer I was going to open a feature request for Witko, but as he stated in here, he is busy. You on the other hand are happily coding away, and taking nvidia-xrun to the next level, and you don't have issues open on your github repo. And you are still looking into turning dGPU on/off. My request is about bbswitch, I'll explain:

Newer GPUs/laptops, together with new pm-suspend work, make it possible to enable / disable the nvidia dGPU simply by loading / unloading the nvidia modules. Using bbswitch is no longer needed. Actually kernel 4.17 (4.16.3+) leaves me unable to restart the dGPU if I disable it with bbswitch. So my request is:

The ability to disable the use of bbswitch, as in skipping the code that toggles ON / OFF.

Perhaps this could be done with flag, or perhaps an .nvidia-xrun config file where you can store standard options.

Witko commented 6 years ago

Hi @IngeniousDox , would you pls create an issue for this? I would like to integrate more of the things that were proposed and this seems like a right thing to do.

IngeniousDox commented 6 years ago

Right, done.

I fully hope for Fincers code to be incorporated. Switching to a free tty, and then running nvidia-xrun there is doable, but just being able to run an app with nvidia-xrun would be awesome. That way you can incorporate it into launchers aswell.