Date: Sat, 11 Jul 2009 22:56:47 GMT From: "Yair K." <cesium2@gmail.com> To: freebsd-gnats-submit@FreeBSD.org Subject: misc/136669: [patch] setmode() should always set errno on error Message-ID: <200907112256.n6BMulde000236@www.freebsd.org> Resent-Message-ID: <200907112300.n6BN068G087523@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
>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 <sys/stat.h> #include <ctype.h> +#include <errno.h> #include <signal.h> #include <stddef.h> #include <stdlib.h> @@ -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:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200907112256.n6BMulde000236>