Mercurial > illumos > illumos-gate
changeset 13207:4f482eb481b3
269 Modload functions use unsafe operations with configuration files
Reviewed by: garrett@nexenta.com
Approved by: garrett@nexenta.com
author | Alexander Eremin <a.eremin@nexenta.com> |
---|---|
date | Tue, 12 Oct 2010 19:37:15 +0400 |
parents | e072bd4baed8 |
children | aecca69fdd0e |
files | usr/src/cmd/modload/addrem.h usr/src/cmd/modload/drvsubr.c usr/src/cmd/modload/errmsg.h |
diffstat | 3 files changed, 49 insertions(+), 59 deletions(-) [+] |
line wrap: on
line diff
--- a/usr/src/cmd/modload/addrem.h Mon Oct 11 16:24:58 2010 -0700 +++ b/usr/src/cmd/modload/addrem.h Tue Oct 12 19:37:15 2010 +0400 @@ -73,7 +73,6 @@ #define REM_NAM_TO_MAJ "/etc/rem_name_to_major" #define ADD_REM_LOCK "/var/run/AdDrEm.lck" -#define TMPHOLD "/etc/TmPhOlD" #if defined(__x86) #define DRVDIR64 "amd64"
--- a/usr/src/cmd/modload/drvsubr.c Mon Oct 11 16:24:58 2010 -0700 +++ b/usr/src/cmd/modload/drvsubr.c Tue Oct 12 19:37:15 2010 +0400 @@ -19,6 +19,10 @@ * CDDL HEADER END */ /* + * Copyright 2010 Nexenta Systems, Inc. All rights reserved. + * Use is subject to license terms. + */ +/* * Copyright 2009 Sun Microsystems, Inc. All rights reserved. * Use is subject to license terms. */ @@ -61,7 +65,6 @@ static char *add_rem_lock; /* lock file */ -static char *tmphold; /* temporary file for updating */ static int add_rem_lock_fd = -1; static int get_cached_n_to_m_file(char *filename, char ***cache); @@ -345,11 +348,13 @@ int status = NOERR; int drvr_found = 0; boolean_t nomatch = B_TRUE; - char *newfile, *tptr, *cp; + char newfile[MAXPATHLEN]; + char *cp; char line[MAX_DBFILE_ENTRY]; char drv[FILENAME_MAX + 1]; FILE *fp, *newfp; struct group *sysgrp; + int newfd; char *copy; /* same size as line */ char *match2 = NULL; /* match with quotes cleaned up */ @@ -389,18 +394,14 @@ } /* Space for defensive copy of input line */ - copy = calloc(sizeof (line), 1); - - /* Build filename for temporary file */ - tptr = calloc(strlen(oldfile) + strlen(XEND) + 1, 1); - if (tptr == NULL || copy == NULL) { + if ((copy = calloc(sizeof (line), 1)) == NULL) { perror(NULL); (void) fprintf(stderr, gettext(ERR_NO_MEM)); return (ERROR); } - (void) strcpy(tptr, oldfile); - (void) strcat(tptr, XEND); + /* Build filename for temporary file */ + (void) snprintf(newfile, sizeof (newfile), "%s%s", oldfile, ".hold"); /* * Set gid so we preserve group attribute. Ideally we wouldn't @@ -411,12 +412,23 @@ (void) setgid(sysgrp->gr_gid); } - newfile = mktemp(tptr); + if ((newfd = open(newfile, O_WRONLY | O_CREAT | O_EXCL, 0644)) < 0) { + if (errno == EEXIST) { + (void) fprintf(stderr, gettext(ERR_FILE_EXISTS), + newfile); + return (ERROR); + } else { + (void) fprintf(stderr, gettext(ERR_CREAT_LOCK), + newfile); + return (ERROR); + } + } - if ((newfp = fopen(newfile, "w")) == NULL) { + if ((newfp = fdopen(newfd, "w")) == NULL) { perror(NULL); (void) fprintf(stderr, gettext(ERR_CANT_ACCESS_FILE), newfile); + (void) close(newfd); return (ERROR); } @@ -495,7 +507,6 @@ } /* end of while */ (void) fclose(fp); - free(tptr); free(copy); if (match2) free(match2); @@ -521,24 +532,12 @@ */ if (status == NOERR) { - if (rename(oldfile, tmphold) == -1) { + if (rename(newfile, oldfile) == -1) { perror(NULL); (void) fprintf(stderr, gettext(ERR_UPDATE), oldfile); (void) unlink(newfile); return (ERROR); - } else if (rename(newfile, oldfile) == -1) { - perror(NULL); - (void) fprintf(stderr, gettext(ERR_UPDATE), oldfile); - (void) unlink(oldfile); - (void) unlink(newfile); - if (link(tmphold, oldfile) == -1) { - perror(NULL); - (void) fprintf(stderr, gettext(ERR_BAD_LINK), - oldfile, tmphold); - } - return (ERROR); } - (void) unlink(tmphold); } else { /* * since there's an error, leave file alone; remove @@ -1092,7 +1091,6 @@ int name_to_major_len; int rem_name_to_major_len; int add_rem_lock_len; - int tmphold_len; int devfs_root_len; int device_policy_len; int extra_privs_len; @@ -1104,7 +1102,6 @@ name_to_major = NAM_TO_MAJ; rem_name_to_major = REM_NAM_TO_MAJ; add_rem_lock = ADD_REM_LOCK; - tmphold = TMPHOLD; devfs_root = DEVFS_ROOT; device_policy = DEV_POLICY; extra_privs = EXTRA_PRIVS; @@ -1118,7 +1115,6 @@ name_to_major_len = len + sizeof (NAM_TO_MAJ); rem_name_to_major_len = len + sizeof (REM_NAM_TO_MAJ); add_rem_lock_len = len + sizeof (ADD_REM_LOCK); - tmphold_len = len + sizeof (TMPHOLD); devfs_root_len = len + sizeof (DEVFS_ROOT); device_policy_len = len + sizeof (DEV_POLICY); extra_privs_len = len + sizeof (EXTRA_PRIVS); @@ -1129,7 +1125,6 @@ name_to_major = malloc(name_to_major_len); rem_name_to_major = malloc(rem_name_to_major_len); add_rem_lock = malloc(add_rem_lock_len); - tmphold = malloc(tmphold_len); devfs_root = malloc(devfs_root_len); device_policy = malloc(device_policy_len); extra_privs = malloc(extra_privs_len); @@ -1140,7 +1135,6 @@ (name_to_major == NULL) || (rem_name_to_major == NULL) || (add_rem_lock == NULL) || - (tmphold == NULL) || (devfs_root == NULL) || (device_policy == NULL) || (extra_privs == NULL)) { @@ -1160,8 +1154,6 @@ "%s%s", basedir, REM_NAM_TO_MAJ); (void) snprintf(add_rem_lock, add_rem_lock_len, "%s%s", basedir, ADD_REM_LOCK); - (void) snprintf(tmphold, tmphold_len, - "%s%s", basedir, TMPHOLD); (void) snprintf(devfs_root, devfs_root_len, "%s%s", basedir, DEVFS_ROOT); (void) snprintf(device_policy, device_policy_len, @@ -1413,9 +1405,10 @@ char own[OPT_LEN + 1]; char grp[OPT_LEN + 1]; int status = NOERR, i; - char *newfile, *tptr; + char newfile[MAXPATHLEN]; char *cp, *dup, *drv_minor; - struct group *sysgrp; + struct group *sysgrp; + int newfd; if ((fp = fopen(minor_perm, "r")) == NULL) { perror(NULL); @@ -1425,15 +1418,8 @@ return (ERROR); } - /* - * Build filename for temporary file - */ - if ((tptr = calloc(strlen(minor_perm) + strlen(XEND) + 1, 1)) == NULL) { - perror(NULL); - (void) fprintf(stderr, gettext(ERR_NO_MEM)); - } - (void) strcpy(tptr, minor_perm); - (void) strcat(tptr, XEND); + /* Build filename for temporary file */ + (void) snprintf(newfile, sizeof (newfile), "%s%s", minor_perm, ".hold"); /* * Set gid so we preserve group attribute. Ideally we wouldn't @@ -1444,11 +1430,23 @@ (void) setgid(sysgrp->gr_gid); } - newfile = mktemp(tptr); - if ((newfp = fopen(newfile, "w")) == NULL) { + if ((newfd = open(newfile, O_WRONLY | O_CREAT | O_EXCL, 0644)) < 0) { + if (errno == EEXIST) { + (void) fprintf(stderr, gettext(ERR_FILE_EXISTS), + newfile); + return (ERROR); + } else { + (void) fprintf(stderr, gettext(ERR_CREAT_LOCK), + newfile); + return (ERROR); + } + } + + if ((newfp = fdopen(newfd, "w")) == NULL) { perror(NULL); (void) fprintf(stderr, gettext(ERR_CANT_ACCESS_FILE), newfile); + (void) close(newfd); return (ERROR); } @@ -1564,24 +1562,12 @@ * if noerr, replace original file with new file */ if (status == NOERR) { - if (rename(minor_perm, tmphold) == -1) { + if (rename(newfile, minor_perm) == -1) { perror(NULL); (void) fprintf(stderr, gettext(ERR_UPDATE), minor_perm); (void) unlink(newfile); return (ERROR); - } else if (rename(newfile, minor_perm) == -1) { - perror(NULL); - (void) fprintf(stderr, gettext(ERR_UPDATE), minor_perm); - (void) unlink(minor_perm); - (void) unlink(newfile); - if (link(tmphold, minor_perm) == -1) { - perror(NULL); - (void) fprintf(stderr, gettext(ERR_BAD_LINK), - minor_perm, tmphold); - } - return (ERROR); } - (void) unlink(tmphold); } else { /* * since there's an error, leave file alone; remove
--- a/usr/src/cmd/modload/errmsg.h Mon Oct 11 16:24:58 2010 -0700 +++ b/usr/src/cmd/modload/errmsg.h Tue Oct 12 19:37:15 2010 +0400 @@ -19,6 +19,10 @@ * CDDL HEADER END */ /* + * Copyright 2010 Nexenta Systems, Inc. All rights reserved. + * Use is subject to license terms. + */ +/* * Copyright 2009 Sun Microsystems, Inc. All rights reserved. * Use is subject to license terms. */ @@ -75,6 +79,7 @@ #define ERR_NO_UPDATE "Cannot update (%s)\n" #define ERR_CANT_RM "Cannot remove temporary file (%s); remove by hand.\n" #define ERR_BAD_LINK "(%s) exists as (%s); Please rename by hand.\n" +#define ERR_FILE_EXISTS "Temporary file (%s) exists; Please remove by hand.\n" #define ERR_NO_MEM "Not enough memory\n" #define ERR_DEL_ENTRY "Cannot delete entry for driver (%s) from file (%s).\n" #define ERR_NO_ENTRY "No entry found for driver (%s) in file (%s).\n"