Skip site navigation (1)Skip section navigation (2)
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>