Fred78290 / nct6687d

Linux kernel module for Nuvoton NCT6687-R
GNU General Public License v2.0
215 stars 42 forks source link

Access to PWM control without sudo #93

Open shvedes opened 8 months ago

shvedes commented 8 months ago

To begin with, I wanted to start with the fact that everything works fine on the MAG B550 TOMAHAWK MAX WIFI, thanks for this module!

PWM signal control works as expected, but it's a little annoying to use sudo every time. I would like to know if it is possible to control the PWM signal without sudo? I understand that this no longer applies to this driver, but to /sys in general, but I did not find the necessary information. I tried adding a regular user to the root group, but that didn't help. I have no idea how this can be implemented, or whether it is possible at all. I would be grateful for any answer, thanks!

xaizek commented 8 months ago

Code-wise root can write to files thanks to this line:

https://github.com/Fred78290/nct6687d/blob/0ee35ed9541bde22fe219305d1647b51ed010c5e/nct6687.c#L917

You can add | S_IWGRP to enable writes by members of root group and | S_IWOTH for writes by all the rest.

You can also chmod/chown files in /sys.

shvedes commented 8 months ago

I don't quite understand what exactly you are talking about. Can you explain more about this please?

xaizek commented 8 months ago

I told you which changes you can do to code to have different permissions.

Alternatively, run chmod 666 /sys/devices/platform/nct6687.2592/hwmon/hwmon*/pwm* in some system initialization script.

shvedes commented 8 months ago

Thanks. I will give a try

SergeyMy commented 8 months ago

It's better this way, but!! it is not safe for production systems

sudo chmod 777 /sys/devices/platform/nct6*/hwmon/hwmon*/*

xaizek commented 8 months ago

It's also possible to do it via udev (say, in /etc/udev/rules.d/nct6687.rules):

SUBSYSTEM=="hwmon", DEVPATH=="/devices/platform/nct6687.*/hwmon/hwmon*", ACTION=="add", RUN+="/bin/chmod 666 %S%p/pwm*"

This has the advantage of being run after manual modprobe as well.

shvedes commented 8 months ago

You can add | S_IWGRP to enable writes by members of root group and | S_IWOTH for writes by all the rest.

So i tried as you said by changing the source code and compiling again, but it doesn't work. I also tried the udev rule and systemd service, but it still doesn't work. Only manually changing rights solves the problem.

la | grep 'pwm[0-9]\+$'

.rw-r--r--  4.0Ki root root   6 Mar 23:05  pwm1
.rw-r--r--  4.0Ki root root   6 Mar 23:05  pwm2
.rw-r--r--  4.0Ki root root   6 Mar 23:05  pwm3
.rw-r--r--  4.0Ki root root   6 Mar 23:05  pwm4
.rw-r--r--  4.0Ki root root   6 Mar 23:05  pwm5
.rw-r--r--  4.0Ki root root   6 Mar 23:05  pwm6
.rw-r--r--  4.0Ki root root   6 Mar 23:05  pwm7
.rw-r--r--  4.0Ki root root   6 Mar 23:05  pwm8

realpath .

/sys/devices/platform/nct6687.2592/hwmon/hwmon4

What else can I do?

uname -r

6.7.6-zen1-1-zen
shvedes commented 8 months ago

Ok, now it works with S_IWGRP, but not with S_IWOTH.

I would like it to work as a regular user, but for now I’ll leave it that way. If you have any ideas why S_IWOTH does not work in my case, I would be glad to hear

SergeyMy commented 8 months ago

everything works great 2024-03-07 08-15-25

shvedes commented 8 months ago

everything works great

Doesn't work for me 😕 image_2024-03-07_04-54-27

SergeyMy commented 8 months ago

Maybe the previous version of the module was not unloaded from memory? but it just has to work

shvedes commented 8 months ago

Maybe the previous version of the module was not unloaded from memory? but it just has to work

Nope, i rebooted my PC several times. Still doesn't working

SergeyMy commented 8 months ago

module loaded via insmod nct6687.ko or modprobe nct6687 ? his may be important udev can override permissions. https://github.com/Fred78290/nct6687d/issues/93#issuecomment-1981145437

... make;sudo rmmod nct6687;sudo insmod $(uname -r)/nct6*.ko;ls -la /sys/devices/platform/nct6*/hwmon/hwmon*/pwm*

shvedes commented 8 months ago

Loaded via modprobe, on boot as well

image_2024-03-07_04-54-27

Installed from PKGBUILD. I just added sed -i '917c\ return attr->mode | S_IWUSR | S_IWGRP | S_IWOTH;' $srcdir/nct6687d-dkms-git/nct6687.c to package() function

SergeyMy commented 8 months ago

make;sudo rmmod $(lsmod|grep nct6);sudo insmod $(uname -r)/nct6*.ko;ls -la /sys/devices/platform/nct6*/hwmon/hwmon*/pwm* this must be done in the source code directory!, make rmmod insmod ...

shvedes commented 8 months ago

this must be done in the source code directory!,

My bad. image_2024-03-07_04-54-27

SergeyMy commented 8 months ago

I've run out of ideas, well... it could be the machinations of selinux. protects your safety :)

SergeyMy commented 8 months ago

My bad.

this is a version issue linux **kernel 6.** in kernel 4. this was still available

in version 6* this was certainly limited!

Linux probably refuses to make module parameters world-writable for security reasons. You should be able to use narrower permissions such as S_IWUSR | S_IWGRP.

:(:(:(

xaizek commented 8 months ago

Oups, sorry for advising something that can't work:

https://github.com/torvalds/linux/blob/67be068d31d423b857ffd8c34dbcc093f8dfff76/fs/sysfs/group.c#L63

Didn't know Linux limits permissions there. One can probably modify group, but using udev rule from above might be easier.

SergeyMy commented 8 months ago

I was also looking for how to change the group, for example to whel :) until I learned how

shvedes commented 8 months ago

but using udev rule from above might be easier

As i said above, udev rule doesn't work for me as well for some reason

SergeyMy commented 8 months ago

most likely for the same reason... //access for everyone should be given by root, of course and not automatically

need to see how to change the group

xaizek commented 8 months ago

As i said above, udev rule doesn't work for me as well for some reason

I missed that when catching up. Glob doesn't expand there and of course I tested without it... Try corrected version:

SUBSYSTEM=="hwmon", DEVPATH=="/devices/platform/nct6687.*/hwmon/hwmon*", ACTION=="add", RUN+="/usr/bin/find %S%p -name pwm* -exec chmod 666 {} ;"

I was also looking for how to change the group, for example to whel :) until I learned how

I think this is the API:

/**
 *  sysfs_file_change_owner - change owner of a sysfs file.
 *  @kobj:  object.
 *  @name:  name of the file to change.
 *  @kuid:  new owner's kuid
 *  @kgid:  new owner's kgid
 *
 * This function looks up the sysfs entry @name under @kobj and changes the
 * ownership to @kuid/@kgid.
 *
 * Returns 0 on success or error code on failure.
 */
shvedes commented 8 months ago

Try corrected version:

It WORKS!!! Thank you so much! image