Date: Fri, 05 Mar 2004 11:39:18 +0900 From: Takanori Watanabe <takawata@init-main.com> To: Nate Lawson <nate@root.org>, imp@bsdimp.com Cc: cvs-all@FreeBSD.org Subject: Re: cvs commit: src/usr.sbin/acpi/acpiconf acpiconf.c Message-ID: <200403050239.i252dI0u007320@sana.init-main.com> In-Reply-To: Your message of "Thu, 04 Mar 2004 10:12:16 PST." <20040304100856.Y24532@root.org>
next in thread | previous in thread | raw e-mail | index | archive | help
In message <20040304100856.Y24532@root.org>, Nate Lawson wrote:
>On Thu, 4 Mar 2004, Takanori Watanabe wrote:
>> FreeBSD src repository
>>
>> Modified files:
>> usr.sbin/acpi/acpiconf acpiconf.c
>> Log:
>> Make unprivilaged user can see battery info.
>>
>> Revision Changes Path
>> 1.13 +5 -2 src/usr.sbin/acpi/acpiconf/acpiconf.c
>
>Hi, welcome back! A few comments...
>
>> --- src/usr.sbin/acpi/acpiconf/acpiconf.c:1.12 Tue Dec 30 09:28:06 200
>3
>> +++ src/usr.sbin/acpi/acpiconf/acpiconf.c Thu Mar 4 09:03:49 2004
>> @@ -44,13 +44,16 @@
>> #define RC_RESUME_PATH "/etc/rc.resume"
>>
>> static int acpifd;
>> -
>> static int
>> acpi_init()
>
>I think that newline should be left for style(9)
This is my mistake. I'll fix this.
>> {
>> acpifd = open(ACPIDEV, O_RDWR);
>> - if (acpifd == -1)
>> + if (acpifd == -1){
>> + acpifd = open(ACPIDEV, O_RDONLY);
>> + }
>> + if (acpifd == -1){
>> err(EX_OSFILE, ACPIDEV);
>> + }
>> }
>>
>> static int
>
>I am concerned about this change since being able to open a descriptor
>read-only implies they can do ioctl, which means they can also suspend the
>system. If we have to do this, we might have to go the setgid route
State transition ioctls have security check for write permisson.
I added this feature exactly for this reason.
http://www.freebsd.org/cgi/cvsweb.cgi/src/sys/dev/acpica/acpi.c.diff?r1=1.83&r2=1.84
http://www.freebsd.org/cgi/cvsweb.cgi/src/sys/dev/acpica/acpi_battery.c.diff?r1=1.6&r2=1.7
http://www.freebsd.org/cgi/cvsweb.cgi/src/sys/dev/acpica/acpi_cmbat.c.diff?r1=1.19&r2=1.20
http://www.freebsd.org/cgi/cvsweb.cgi/src/sys/dev/acpica/acpi_acad.c.diff?r1=1.13&r2=1.14
>although I don't necessarily want to deal with the security implications
>of that. Users can get battery status through apm(8) but not battery
>info.
apm(8) information is only emulated apm status.
The acpiconf -i informations are not available from apm(8) interface.
Certainly, some people invoke -s by mistake and execute rc.suspend, but
rc.suspend will stop because they have enough permission to write pid
file.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200403050239.i252dI0u007320>