Bug 31483

On Thu, Oct 25, 2001 at 03:17:55AM +0300, Giorgos Keramidas wrote:
> 
> >Number:         31483
> >Category:       bin
> >Synopsis:       minor correction in error message printed by acpiconf.8
> >Originator:     Giorgos Keramidas
> >Release:        FreeBSD 5.0-CURRENT i386

[snip]

> -		err(1, NULL);
> +		err(1, ACPIDEV);

Just as an idea.. err(1, "%s", ACPIDEV)? :)

G'luck,
Peter

-- 
You have, of course, just begun reading the sentence that you have just finished reading.

On Thu, Oct 25, 2001 at 03:50:02AM -0700, Peter Pentchev wrote:
> The following reply was made to PR bin/31483; it has been noted by GNATS.
> 
> From: Peter Pentchev <roam@ringlet.net>
> To: Giorgos Keramidas <charon@labs.gr>
> Cc: FreeBSD-gnats-submit@freebsd.org
> Subject: Re: bin/31483: minor correction in error message printed by acpiconf.8
> Date: Thu, 25 Oct 2001 13:35:02 +0300
> 
>  On Thu, Oct 25, 2001 at 03:17:55AM +0300, Giorgos Keramidas wrote:
>  > 
>  > >Number:         31483
>  > >Category:       bin
>  > >Synopsis:       minor correction in error message printed by acpiconf.8
>  > >Originator:     Giorgos Keramidas
>  > >Release:        FreeBSD 5.0-CURRENT i386
>  
>  [snip]
>  
>  > -		err(1, NULL);
>  > +		err(1, ACPIDEV);
>  
>  Just as an idea.. err(1, "%s", ACPIDEV)? :)

ACPIDEV is a #define'd constant string. It is not a security
vulnerability to use it as the original poster did. The original
poster's code is,

  err(1, "/dev/acpi");

By the time the compiler gets to it. No format string vulnerability.

Anyway, here is a general err.h and sysexits.h cleanup of the program
that I'll commit if msmith and iwasaki don't mind,

Index: usr.sbin/acpi/acpiconf/acpiconf.c
===================================================================
RCS file: /export/ncvs/src/usr.sbin/acpi/acpiconf/acpiconf.c,v
retrieving revision 1.6
diff -u -r1.6 acpiconf.c
--- usr.sbin/acpi/acpiconf/acpiconf.c	2001/07/21 21:51:44	1.6
+++ usr.sbin/acpi/acpiconf/acpiconf.c	2001/10/25 11:30:33
@@ -33,6 +33,7 @@
 #include <fcntl.h>
 #include <stdio.h>
 #include <sys/ioctl.h>
+#include <sysexits.h>
 #include <unistd.h>
 
 #include <dev/acpica/acpiio.h>
@@ -49,10 +50,10 @@
 
 	fd  = open(ACPIDEV, O_RDWR);
 	if (fd == -1) {
-		err(1, NULL);
+		err(EX_OSFILE, ACPIDEV);
 	}
 	if (ioctl(fd, enable, NULL) == -1) {
-		err(1, NULL);
+		err(EX_IOERR, NULL);
 	}
 	close(fd);
 
@@ -66,10 +67,10 @@
 
 	fd  = open(ACPIDEV, O_RDWR);
 	if (fd == -1) {
-		err(1, NULL);
+		err(EX_OSFILE, ACPIDEV);
 	}
 	if (ioctl(fd, ACPIIO_SETSLPSTATE, &sleep_type) == -1) {
-		err(1, NULL);
+		err(EX_IOERR, NULL);
 	}
 	close(fd);
 
@@ -107,11 +108,9 @@
 
 		case 's':
 			sleep_type = optarg[0] - '0';
-			if (sleep_type < 0 || sleep_type > 5) {
-				fprintf(stderr, "%s: invalid sleep type (%d)\n",
-				    argv[0], sleep_type);
-				return -1;
-			}
+			if (sleep_type < 0 || sleep_type > 5)
+				errx(EX_USAGE, "invalid sleep type (%d)",
+				    sleep_type);
 			break;
 		default:
 			argc -= optind;

-- 
Crist J. Clark                     |     cjclark@alum.mit.edu
                                   |     cjclark@jhu.edu
http://people.freebsd.org/~cjc/    |     cjc@freebsd.org

Hi,

> >  On Thu, Oct 25, 2001 at 03:17:55AM +0300, Giorgos Keramidas wrote:
> >  > 
> >  > >Number:         31483
> >  > >Category:       bin
> >  > >Synopsis:       minor correction in error message printed by acpiconf.8
> >  > >Originator:     Giorgos Keramidas
> >  > >Release:        FreeBSD 5.0-CURRENT i386
> >  
> >  [snip]
> >  
> >  > -		err(1, NULL);
> >  > +		err(1, ACPIDEV);
> >  
> >  Just as an idea.. err(1, "%s", ACPIDEV)? :)
> 
> ACPIDEV is a #define'd constant string. It is not a security
> vulnerability to use it as the original poster did. The original
> poster's code is,
> 
>   err(1, "/dev/acpi");
> 
> By the time the compiler gets to it. No format string vulnerability.
> 
> Anyway, here is a general err.h and sysexits.h cleanup of the program
> that I'll commit if msmith and iwasaki don't mind,

Thanks taking care of this, I don't mind so please commit :-)

Thanks again

On Thu, 25 Oct 2001, Mitsuru IWASAKI wrote:

> To: cjclark@alum.mit.edu, cristjc@earthlink.net
>  > ACPIDEV is a #define'd constant string. It is not a security
>  > vulnerability to use it as the original poster did. The original
>  > poster's code is,
>  >
>  >   err(1, "/dev/acpi");
>  >
>  > By the time the compiler gets to it. No format string vulnerability.
>  >
>  > Anyway, here is a general err.h and sysexits.h cleanup of the program
>  > that I'll commit if msmith and iwasaki don't mind,
>
>  Thanks taking care of this, I don't mind so please commit :-)

Use non-cryptic error messages, not sysexits.h.  sysexits.h is only
understood by a a couple of programs (mainly sendmail).

Bruce

Comment 5 Crist J. Clark 2001-11-30 11:35:15 UTC

State Changed
From-To: open->analyzed

Changes committed to CURRENT. MFC 4 days. 

Comment 6 Crist J. Clark 2001-11-30 11:35:15 UTC

Responsible Changed
From-To: freebsd-bugs->cjc

Made change to CURRENT, waiting for MFC.

Comment 7 Crist J. Clark 2001-11-30 11:58:45 UTC

State Changed
From-To: analyzed->closed

On second thought, not much reason to wait for the MFC to close this 
since acpiconf(8) doesn't exist in STABLE. (Guess I shouldn't be 
messing with PRs at 4 AM.)