From owner-cvs-all@FreeBSD.ORG Thu Mar 4 18:40:22 2004 Return-Path: Delivered-To: cvs-all@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 61ADF16A4CE; Thu, 4 Mar 2004 18:40:22 -0800 (PST) Received: from sana.init-main.com (104.194.138.210.bn.2iij.net [210.138.194.104]) by mx1.FreeBSD.org (Postfix) with ESMTP id B21F743D1D; Thu, 4 Mar 2004 18:40:21 -0800 (PST) (envelope-from takawata@init-main.com) Received: from init-main.com (localhost [127.0.0.1]) by sana.init-main.com (8.12.10/8.12.9) with ESMTP id i252dI0u007320; Fri, 5 Mar 2004 11:39:18 +0900 (JST) (envelope-from takawata@init-main.com) Message-Id: <200403050239.i252dI0u007320@sana.init-main.com> To: Nate Lawson , imp@bsdimp.com In-reply-to: Your message of "Thu, 04 Mar 2004 10:12:16 PST." <20040304100856.Y24532@root.org> Date: Fri, 05 Mar 2004 11:39:18 +0900 From: Takanori Watanabe cc: cvs-src@FreeBSD.org cc: src-committers@FreeBSD.org cc: cvs-all@FreeBSD.org Subject: Re: cvs commit: src/usr.sbin/acpi/acpiconf acpiconf.c X-BeenThere: cvs-all@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: CVS commit messages for the entire tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 05 Mar 2004 02:40:22 -0000 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.