Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wireless.js kill() function #312

Open
xipmix opened this issue Mar 13, 2018 · 2 comments
Open

wireless.js kill() function #312

xipmix opened this issue Mar 13, 2018 · 2 comments

Comments

@xipmix
Copy link
Contributor

xipmix commented Mar 13, 2018

I want to raise a concern about this function, it may not be valid.
Consider what happens when you feed it with the process string '/usr/bin/sudo /sbin/dhcpcd'

function kill(process, callback) {
    var all = process.split(" ");
    var process = all[0];
    var command = 'kill `pgrep -f "^' + process + '"` || true';
    logger("killing: " + command);
    return thus.exec(command, callback);
}

Splitting on spaces sets process to the string /usr/bin/sudo. It seems there is at least a small chance of killing the wrong sudo call. What is wanted is to kill the dhcpcd process.

To be more sure of hitting the target, I think something like this is needed

 function kill(process, callback) {
    target = process.replace(/^[^\s]*\/sudo\s+/,'');  // remove prefixed sudo invocation
    var all = target.split(" ");
    target = all[0]; // separate command from arguments
    var command = 'kill `pgrep -f "^' + target + '"` || true';
    logger("killing: " + command);
    return thus.exec(command, callback);
}

There's a further issue; both ethernet and wireless interfaces may have a dhcpcd process listening to them. At present the former appears in the process table as the command dhcpcd eth0, not /sbin/dhcpcd eth0, so it won't get shot down; but this is more by good luck than good management. I'm not sure how to resolve that one, possibly requiring the process argument to be a suitable regex and not splitting up the string.

@volumio
Copy link
Owner

volumio commented Mar 13, 2018

You're right on this one. Anyhow, I don't see this causing any effect (luck, as you say).
Are you able to develop your changes and then test them?

@xipmix
Copy link
Contributor Author

xipmix commented Mar 15, 2018

Sure, will take a look.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants