Date: Thu, 4 Mar 2004 10:12:16 -0800 (PST) From: Nate Lawson <nate@root.org> To: Takanori Watanabe <takawata@FreeBSD.org> Cc: cvs-all@FreeBSD.org Subject: Re: cvs commit: src/usr.sbin/acpi/acpiconf acpiconf.c Message-ID: <20040304100856.Y24532@root.org> In-Reply-To: <20040304170353.64D4F16A4D4@hub.freebsd.org> References: <20040304170353.64D4F16A4D4@hub.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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 2003
> +++ 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)
> {
> 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
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.
-Nate
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20040304100856.Y24532>