From owner-freebsd-bugs@FreeBSD.ORG Sat Jul 11 23:00:06 2009 Return-Path: Delivered-To: freebsd-bugs@hub.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id ECB2B1065672 for ; Sat, 11 Jul 2009 23:00:06 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id BD5F48FC17 for ; Sat, 11 Jul 2009 23:00:06 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (gnats@localhost [127.0.0.1]) by freefall.freebsd.org (8.14.3/8.14.3) with ESMTP id n6BN06So087525 for ; Sat, 11 Jul 2009 23:00:06 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.3/8.14.3/Submit) id n6BN068G087523; Sat, 11 Jul 2009 23:00:06 GMT (envelope-from gnats) Resent-Date: Sat, 11 Jul 2009 23:00:06 GMT Resent-Message-Id: <200907112300.n6BN068G087523@freefall.freebsd.org> Resent-From: FreeBSD-gnats-submit@FreeBSD.org (GNATS Filer) Resent-To: freebsd-bugs@FreeBSD.org Resent-Reply-To: FreeBSD-gnats-submit@FreeBSD.org, "Yair K." Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id B74021065670 for ; Sat, 11 Jul 2009 22:56:47 +0000 (UTC) (envelope-from nobody@FreeBSD.org) Received: from www.freebsd.org (www.freebsd.org [IPv6:2001:4f8:fff6::21]) by mx1.freebsd.org (Postfix) with ESMTP id 9653D8FC12 for ; Sat, 11 Jul 2009 22:56:47 +0000 (UTC) (envelope-from nobody@FreeBSD.org) Received: from www.freebsd.org (localhost [127.0.0.1]) by www.freebsd.org (8.14.3/8.14.3) with ESMTP id n6BMulLw000237 for ; Sat, 11 Jul 2009 22:56:47 GMT (envelope-from nobody@www.freebsd.org) Received: (from nobody@localhost) by www.freebsd.org (8.14.3/8.14.3/Submit) id n6BMulde000236; Sat, 11 Jul 2009 22:56:47 GMT (envelope-from nobody) Message-Id: <200907112256.n6BMulde000236@www.freebsd.org> Date: Sat, 11 Jul 2009 22:56:47 GMT From: "Yair K." To: freebsd-gnats-submit@FreeBSD.org X-Send-Pr-Version: www-3.1 Cc: Subject: misc/136669: [patch] setmode() should always set errno on error X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 11 Jul 2009 23:00:07 -0000 >Number: 136669 >Category: misc >Synopsis: [patch] setmode() should always set errno on error >Confidential: no >Severity: non-critical >Priority: low >Responsible: freebsd-bugs >State: open >Quarter: >Keywords: >Date-Required: >Class: sw-bug >Submitter-Id: current-users >Arrival-Date: Sat Jul 11 23:00:06 UTC 2009 >Closed-Date: >Last-Modified: >Originator: Yair K. >Release: >Organization: >Environment: >Description: The setmode() function doesn't always set errno on error. The attached patch pulls rev 1.31 from NetBSD[1] to fix this. Notes: A. OpenBSD[2] and NetBSD have slightly different error returns for octal mode errors. OpenBSD always returns ERANGE, while NetBSD returns EINVAL unless strtol returned ERANGE. The attached patch follows the OpenBSD policy. B. The patch uses a slightly different definition for addcmd from NetBSD: (BITCMD *, char, mode_t, mode_t, mode_t) instead of (BITCMD *, mode_t, mode_t, mode_t, mode_t). It's pretty obvious from the context and use that op is only a char. Also, we change the types of the local vars in addcmd() appropriately to match the function definition. C. Comments from rev 1.15[3] are added since they are more accurate after that change (which was imported to FreeBSD in the past), and reduce any future diffs. However, spelling error was corrected. [1] http://cvsweb.netbsd.org/bsdweb.cgi/src/lib/libc/gen/setmode.c.diff?r1=1.30&r2=1.31&only_with_tag=MAIN [2] http://www.openbsd.org/cgi-bin/cvsweb/src/lib/libc/gen/setmode.c?rev=1.20;content-type=text%2Fplain [3] http://cvsweb.netbsd.org/bsdweb.cgi/src/lib/libc/gen/setmode.c.diff?r1=1.14&r2=1.15&only_with_tag=MAIN >How-To-Repeat: >Fix: See attached patch. Patch attached with submission follows: --- setmode.c 2009-07-11 22:11:49.832306655 +0300 +++ setmode.c 2009-07-11 22:21:26.175049175 +0300 @@ -41,6 +41,7 @@ #include #include +#include #include #include #include @@ -66,7 +67,7 @@ #define CMD2_OBITS 0x08 #define CMD2_UBITS 0x10 -static BITCMD *addcmd(BITCMD *, int, int, int, u_int); +static BITCMD *addcmd(BITCMD *, char, mode_t, mode_t, mode_t); static void compress_mode(BITCMD *); #ifdef SETMODE_DEBUG static void dumpmode(BITCMD *); @@ -161,23 +162,24 @@ saveset = newset; \ endset = newset + (setlen - 2); \ } \ - set = addcmd(set, (a), (b), (c), (d)) + set = addcmd(set, (a), (b), (mode_t)(c), (d)) #define STANDARD_BITS (S_ISUID|S_ISGID|S_IRWXU|S_IRWXG|S_IRWXO) void * setmode(const char *p) { - int perm, who; char op, *ep; BITCMD *set, *saveset, *endset; sigset_t sigset, sigoset; - mode_t mask; - int equalopdone=0, permXbits, setlen; + mode_t mask, perm, permXbits, who; + int equalopdone=0, setlen; long perml; - if (!*p) + if (!*p) { + errno = EINVAL; return (NULL); + } /* * Get a copy of the mask for the permissions that are mask relative. @@ -206,10 +208,15 @@ perml = strtol(p, &ep, 8); if (*ep || perml < 0 || perml & ~(STANDARD_BITS|S_ISTXT)) { free(saveset); + /* + * This follows the OpenBSD behaviour. NetBSD returns + * EINVAL, except for the case when errno == ERANGE + * from strtol. + */ + errno = ERANGE; return (NULL); } - perm = (mode_t)perml; - ADDCMD('=', (STANDARD_BITS|S_ISTXT), perm, mask); + ADDCMD('=', (STANDARD_BITS|S_ISTXT), perml, mask); set->cmd = 0; return (saveset); } @@ -241,6 +248,7 @@ getop: if ((op = *p++) != '+' && op != '-' && op != '=') { free(saveset); + errno = EINVAL; return (NULL); } if (op == '=') @@ -253,12 +261,18 @@ perm |= S_IRUSR|S_IRGRP|S_IROTH; break; case 's': - /* If only "other" bits ignore set-id. */ + /* + * If specific bits were requested and + * only "other" bits ignore set-id. + */ if (!who || who & ~S_IRWXO) perm |= S_ISUID|S_ISGID; break; case 't': - /* If only "other" bits ignore sticky. */ + /* + * If specific bits were requested and + * only "other" bits ignore sticky. + */ if (!who || who & ~S_IRWXO) { who |= S_ISTXT; perm |= S_ISTXT; @@ -333,7 +347,7 @@ } static BITCMD * -addcmd(BITCMD *set, int op, int who, int oparg, u_int mask) +addcmd(BITCMD *set, char op, mode_t who, mode_t oparg, mode_t mask) { switch (op) { case '=': @@ -400,7 +414,8 @@ compress_mode(BITCMD *set) { BITCMD *nset; - int setbits, clrbits, Xbits, op; + char op; + mode_t setbits, clrbits, Xbits; for (nset = set;;) { /* Copy over any 'u', 'g' and 'o' commands. */ --- setmode.3 2009-07-12 01:42:34.938005607 +0300 +++ setmode.3 2009-07-12 01:42:55.277377686 +0300 @@ -97,9 +97,13 @@ The .Fn setmode function -may fail and set errno for any of the errors specified for the library -routine -.Xr malloc 3 . +may fail and set errno to +.Er ERANGE +if an invalid octal value was specified, or to +.Er EINVAL +if an invalid mode value was specified. It may also fail and set errno +to any of the errors specified for the library routine +.Xr malloc 3 .Sh SEE ALSO .Xr chmod 1 , .Xr stat 2 , >Release-Note: >Audit-Trail: >Unformatted: