lpereira / hardinfo

System profiler and benchmark tool for Linux systems
http://hardinfo.org
GNU General Public License v2.0
772 stars 130 forks source link

Usability: Load missing kernel modules if needed #658

Open hamishmb opened 2 years ago

hamishmb commented 2 years ago

Originally reported by https://github.com/coffeecrisp

OS: Kubuntu 22.04 Version: 0.6-alpha

In Devices -> Memory SPD, it says "Please load the eeprom module to obtain information about memory SPD".

We should offer to load the eeprom module, or at least indicate what it is and how to do it.

Also, the Memory SPD option should be below, or maybe integrated into, the Memory section.

hamishmb commented 2 years ago

In current git version, Memory SPD is merged with Memory Devices. We should be using pkexec to load the kernel modules, though.

lpereira commented 2 years ago

Newer systems require another module that needs configuration, so automatically loading modules isn't really going to work. Also, the lm-sensors package has some detecting code for sensors; for some older sensors it doesn't do an automatic probe because that's been known to brick computers -- and I don't want that responsibility.

In any case, HardInfo is in a really bad shape internally -- I started writing it 20 years ago without knowing much about C or programming, so it's probably filled with buffer overflows and other nasty things -- so I'm always uneasy when it comes requiring it to be ran as root, even if it's just some part of its code.

There are many other parts of HardInfo where, say, reading a file in /proc as root would be useful, but I'd rather avoid this entirely.

On Fri, Aug 5, 2022, at 6:44 AM, Hamish Mcintyre-Bhatty wrote:

In current git version, Memory SPD is merged with Memory Devices. We should be using pkexec to load the kernel modules, though.

— Reply to this email directly, view it on GitHub https://github.com/lpereira/hardinfo/issues/658#issuecomment-1206475710, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAADVGJ35QPHI6S4OGJEBILVXULFDANCNFSM55WBP67A. You are receiving this because you are subscribed to this thread.Message ID: @.***>

hamishmb commented 2 years ago

Fair enough. In that case, though, it'd still be much better to just run a pkexec script to do things like this, requiring a user to okay it, than run the whole GUI as root as is currently required for some things, especially if you're not sure about the quality of the code. Still not great, but safer.

EDIT: Agreed though, automatic sensor probe is definitely not a good idea.

hamishmb commented 2 years ago

I don't have much C experience at the moment, and am interested in contributing to this project as a useful way of learning more. Would it potentially be better to rewrite the UI, especially seeing as it's GTK2?

lpereira commented 2 years ago

Rewriting the UI is definitely on the list of things that I want to eventually see happen. The next version should still be based off of GTK+2, but I don't see any problem in making it a Gtk4 app after that. (By the way, HardInfo builds with Gtk3 right now, which is essential to make it work well on HiDPI screens.)

There are many architectural issues in HardInfo that I'd like to fix if it's ever have the UI rewritten. Maybe even rewrite it in another language that makes it easier/safer to parse text, such as Rust. I'm sure there are buffer overflows everywhere in HardInfo right now, which hasn't been much of a problem so far because we read from what the system gives us, but it makes me uneasy nonetheless.

On Tue, Aug 16, 2022, at 11:41 AM, Hamish Mcintyre-Bhatty wrote:

I don't have much C experience at the moment, and am interested in contributing to this project as a useful way of learning more. Would it potentially be better to rewrite the UI, especially seeing as it's GTK2?

— Reply to this email directly, view it on GitHub https://github.com/lpereira/hardinfo/issues/658#issuecomment-1217010237, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAADVGJ6GD6MR7J3KLN22XDVZPOEPANCNFSM55WBP67A. You are receiving this because you commented.Message ID: @.***>

hamishmb commented 2 years ago

I'm pretty proficient in Python GUIs with wxPython at this point in time, but I don't think that supports GTK4, at least not yet. It tends to lag behind a bit on new functionality.

GitHub also has a security analysis feature as of late, so that might be useful in finding security issues. I haven't used it, so I don't know how well it works, but maybe I'll enable it on a fork of this repo and see if it finds any issues.

lpereira commented 2 years ago

Rewriting the UI is definitely on the list of things that I want to eventually see happen. The next version should still be based off of GTK+2, but I don't see any problem in making it a Gtk4 app after that. (By the way, HardInfo builds with Gtk3 right now, which is essential to make it work well on HiDPI screens.)

There are many architectural issues in HardInfo that I'd like to fix if it's ever have the UI rewritten. Maybe even rewrite it in another language that makes it easier/safer to parse text, such as Rust. I'm sure there are buffer overflows everywhere in HardInfo right now, which hasn't been much of a problem so far because we read from what the system gives us, but it makes me uneasy nonetheless.

On Tue, Aug 16, 2022, at 11:41 AM, Hamish Mcintyre-Bhatty wrote:

I don't have much C experience at the moment, and am interested in contributing to this project as a useful way of learning more. Would it potentially be better to rewrite the UI, especially seeing as it's GTK2?

— Reply to this email directly, view it on GitHub https://github.com/lpereira/hardinfo/issues/658#issuecomment-1217010237, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAADVGJ6GD6MR7J3KLN22XDVZPOEPANCNFSM55WBP67A. You are receiving this because you commented.Message ID: @.***>

andreamoro commented 2 years ago

Any clue on how the module reported at the top can be loaded... in laymen terms please :)

hamishmb commented 2 years ago

You should be able to run:

sudo modprobe eeprom

To do that.