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

Installing fake_vcgencmd for Pine64 #5095

Merged
merged 1 commit into from
Apr 24, 2023

Conversation

PanderMusubi
Copy link
Contributor

Description

Fake vcgencmd allows requesting information from a non-Raspberry Pi board in the same way that can be done via an executable on there. Since this command is now available, the Armbian board can also be monitored, rebooted etc via eht FOSS Android app RasPi Check.

See also:

Jira reference number [AR-9999]

How Has This Been Tested?

Yes, made a build and it worked, also via the Android app.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • Any dependent changes have been merged and published in downstream modules

@igorpecovnik igorpecovnik added the Ready to merge Reviewed, tested and ready for merge label Apr 23, 2023
@PanderMusubi PanderMusubi merged commit 420f16c into armbian:main Apr 24, 2023
@juanesf
Copy link
Collaborator

juanesf commented Apr 24, 2023

Also works on Banana Pi M2 Ultra

@ThomasKaiser
Copy link
Contributor

Introduces another temp file vulnerability (usual practice with Armbian these days it seems)

@PanderMusubi
Copy link
Contributor Author

Introduces another temp file vulnerability (usual practice with Armbian these days it seems)

That part of the code is not executed. Would it be getter that the line with exit is removed and the lines for logging are commented out?

@PanderMusubi
Copy link
Contributor Author

Second question, is using mktemp not triggering temp file vulnerabilities?

@ThomasKaiser
Copy link
Contributor

is using mktemp not triggering temp file vulnerabilities?

It's only purpose is to avoid temp file vulnerabilities in scripts. You pull in code from somewhere that's not under your own control and the exit statement might vanish at any time without anyone noticing. That's almost recipe for disaster :)

Why not let the script be part of this repo, use mktemp or delete the whole logging stuff entirely? The last two variants are of course useless as long as you pull in random code from the Internet that's not under your own control.

@PanderMusubi
Copy link
Contributor Author

Thanks, the author is very helpful and responsive so we'll fix this soon.

@clach04
Copy link

clach04 commented Apr 25, 2023

is using mktemp not triggering temp file vulnerabilities?

It's only purpose is to avoid temp file vulnerabilities in scripts. You pull in code from somewhere that's not under your own control and the exit statement might vanish at any time without anyone noticing. That's almost recipe for disaster :)

Why not let the script be part of this repo, use mktemp or delete the whole logging stuff entirely? The last two variants are of course useless as long as you pull in random code from the Internet that's not under your own control.

Agreed, original code was quickly put together without thinking about anyone else using this ;-) Tracking via clach04/fake_vcgencmd#5

@clach04
Copy link

clach04 commented Apr 25, 2023

@PanderMusubi fixed, with a bunch of other minor cleanups (with no actual code change).

@clach04
Copy link

clach04 commented Apr 26, 2023

@rpardini
Copy link
Member

Please revert this, see discussion in #5100 -- this does not belong in Armbian core

This was referenced Apr 28, 2023
@PanderMusubi PanderMusubi deleted the fake_vcgencmd branch August 31, 2023 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready to merge Reviewed, tested and ready for merge
Development

Successfully merging this pull request may close these issues.

None yet

6 participants