From owner-svn-soc-all@FreeBSD.ORG Thu Jun 7 15:08:07 2012 Return-Path: Delivered-To: svn-soc-all@FreeBSD.org Received: from socsvn.FreeBSD.org (unknown [IPv6:2001:4f8:fff6::2f]) by hub.freebsd.org (Postfix) with SMTP id 4B041106564A for ; Thu, 7 Jun 2012 15:08:05 +0000 (UTC) (envelope-from gpf@FreeBSD.org) Received: by socsvn.FreeBSD.org (sSMTP sendmail emulation); Thu, 07 Jun 2012 15:08:05 +0000 Date: Thu, 07 Jun 2012 15:08:05 +0000 From: gpf@FreeBSD.org To: svn-soc-all@FreeBSD.org MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Message-Id: <20120607150805.4B041106564A@hub.freebsd.org> Cc: Subject: socsvn commit: r237264 - soc2012/gpf/pefs_kmod/sbin/pefs X-BeenThere: svn-soc-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: SVN commit messages for the entire Summer of Code repository List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 07 Jun 2012 15:08:07 -0000 Author: gpf Date: Thu Jun 7 15:08:05 2012 New Revision: 237264 URL: http://svnweb.FreeBSD.org/socsvn/?view=rev&rev=237264 Log: - use dirname/basename instead of homegrown solution - open(2) fhp->path as well as fhp->dirpath of parent dir so as to eliminate most race conditions. file descriptors are kept in struct file_header. This is done alongside semantic checks for each file in the now renamed pefs_open_semantic_check(). Modified: soc2012/gpf/pefs_kmod/sbin/pefs/pefs_checksum.c Modified: soc2012/gpf/pefs_kmod/sbin/pefs/pefs_checksum.c ============================================================================== --- soc2012/gpf/pefs_kmod/sbin/pefs/pefs_checksum.c Thu Jun 7 14:38:43 2012 (r237263) +++ soc2012/gpf/pefs_kmod/sbin/pefs/pefs_checksum.c Thu Jun 7 15:08:05 2012 (r237264) @@ -28,6 +28,7 @@ #include __FBSDID("$FreeBSD$"); +#include #include #include #include @@ -40,6 +41,7 @@ #include #include #include +#include #include #include #include @@ -84,8 +86,6 @@ RB_ENTRY(hardlink_counter) hardlink_entries; }; -/* XXXgpf: [TODO] check pathname string lengths. Some are MAXPATHLEN + 1, some MAXPATHLEN */ - /* XXXgpf: unions for on disk structs and move to a different header? */ struct checksum_file_header { uint8_t version; @@ -105,9 +105,12 @@ struct file_header { uint32_t nhashes; uint64_t file_id; - char path[MAXPATHLEN]; + char path[MAXPATHLEN + 1]; + char dirpath[MAXPATHLEN + 1]; + char filename[MAXNAMLEN + 1]; char *target_path; uint32_t offset_to_checksums; + int fd, pfd; struct checksum_head checksums; TAILQ_ENTRY(file_header) file_header_entries; TAILQ_ENTRY(file_header) fh_hardlink_entries; @@ -165,41 +168,34 @@ return 0; } +static void +pefs_close_file(struct file_header *fhp) +{ + if (fhp->fd >= 0) + close(fhp->fd); + if (fhp->pfd >= 0) + close(fhp->pfd); +} + static int pefs_compute_symlink_checksum(struct file_header *fhp, const EVP_MD *md, uint8_t hash_len) { struct pefs_xslink_ctext xsl; - char parent_dir[MAXPATHLEN]; EVP_MD_CTX mdctx; - int error, i, fd, md_len; + int error, i, md_len; struct checksum *csp; - char *pch; + TAILQ_INIT(&(fhp->checksums)); fhp->nhashes = 0; /* feed parent directory to ioctl() */ - strlcpy(parent_dir, fhp->path, sizeof(parent_dir)); - pch = strrchr(parent_dir, '/'); - if (pch == NULL) { - pefs_warn("error retrieving parent dir of %s", fhp->path); - return (PEFS_ERR_NOENT); - } - *pch = '\0'; - - strlcpy(xsl.pxsl_filename, pch + 1, sizeof(xsl.pxsl_filename)); + strlcpy(xsl.pxsl_filename, fhp->filename, sizeof(xsl.pxsl_filename)); xsl.pxsl_namelen = strnlen(xsl.pxsl_filename, sizeof(xsl.pxsl_filename)); - fd = open(parent_dir, O_RDONLY); - if (fd < 0) { - warn("failed to open file: %s", parent_dir); - return (PEFS_ERR_IO); - } - - error = ioctl(fd, PEFS_GETSLINKCTEXT, &xsl); + error = ioctl(fhp->pfd, PEFS_GETSLINKCTEXT, &xsl); if (error != 0) { pefs_warn("error retrieving symlink's ciphertext of %s", fhp->path); - close(fd); return (PEFS_ERR_IO); } @@ -215,14 +211,12 @@ csp = malloc(sizeof(struct checksum)); if (csp == NULL) { pefs_warn("memory allocation error"); - close(fd); return (PEFS_ERR_SYS); } csp->hash = malloc(hash_len); if (csp->hash == NULL) { pefs_warn("memory allocation error"); free(csp); - close(fd); return (PEFS_ERR_SYS); } @@ -236,7 +230,6 @@ TAILQ_INSERT_TAIL(&(fhp->checksums), csp, checksum_entries); fhp->nhashes++; - close(fd); return (0); } @@ -249,32 +242,26 @@ struct stat sb; off_t resid; uint32_t bytes_to_read; - int error, i, fd, md_len; + int error, i, md_len; struct checksum *csp; + if (fhp->target_path != NULL) + return (pefs_compute_symlink_checksum(fhp, md, hash_len)); + TAILQ_INIT(&(fhp->checksums)); /* XXXgpf: what happens if file size > 2^64? */ - if (lstat(fhp->path, &sb) != 0) { + if (fstat(fhp->fd, &sb) != 0) { warn("cannot stat file %s", fhp->path); return (PEFS_ERR_SYS); } - if (S_ISLNK(sb.st_mode) != 0) - return (pefs_compute_symlink_checksum(fhp, md, hash_len)); - resid = sb.st_size; if (resid == 0) { pefs_warn("empty files are not allowed: %s", fhp->path); return (PEFS_ERR_INVALID); } - fd = open(fhp->path, O_RDONLY | O_NOFOLLOW); - if (fd < 0) { - warn("failed to open file: %s", fhp->path); - return (PEFS_ERR_IO); - } - fhp->nhashes = 0; xsct.pxsct_offset = 0; while (resid > 0) { @@ -285,10 +272,9 @@ resid-=bytes_to_read; xsct.pxsct_ctext_len = bytes_to_read; - error = ioctl(fd, PEFS_GETSECTORCTEXT, &xsct); + error = ioctl(fhp->fd, PEFS_GETSECTORCTEXT, &xsct); if (error != 0) { pefs_warn("error retrieving ciphertext of %s", fhp->path); - close(fd); return (PEFS_ERR_IO); } xsct.pxsct_offset+= xsct.pxsct_ctext_len; @@ -305,14 +291,12 @@ csp = malloc(sizeof(struct checksum)); if (csp == NULL) { pefs_warn("memory allocation error"); - close(fd); return (PEFS_ERR_SYS); } csp->hash = malloc(hash_len); if (csp->hash == NULL) { pefs_warn("memory allocation error"); free(csp); - close(fd); return (PEFS_ERR_SYS); } @@ -328,7 +312,6 @@ fhp->nhashes++; } - close(fd); return (0); } @@ -586,33 +569,15 @@ static int pefs_get_file_id(struct file_header *fhp) { - char parent_dir[MAXPATHLEN]; struct pefs_xnamecsum xncs; - char *pch; uint64_t temp; - int error, fd; - - /* feed parent directory to ioctl() */ - strlcpy(parent_dir, fhp->path, sizeof(parent_dir)); - pch = strrchr(parent_dir, '/'); - if (pch == NULL) { - pefs_warn("error retrieving parent dir of %s", fhp->path); - return (PEFS_ERR_NOENT); - } - *pch = '\0'; - - fd = open(parent_dir, O_RDONLY); - if (fd < 0) { - warn("unable to open file %s", parent_dir); - return (PEFS_ERR_SYS); - } + int error; - pch = strrchr(fhp->path, '/'); - pch++; - strlcpy(xncs.pxnc_filename, pch, sizeof(xncs.pxnc_filename)); + strlcpy(xncs.pxnc_filename, fhp->filename, sizeof(xncs.pxnc_filename)); xncs.pxnc_namelen = strnlen(xncs.pxnc_filename, sizeof(xncs.pxnc_filename)); - error = ioctl(fd, PEFS_GETNAMECSUM, &xncs); + /* feed parent directory to ioctl() */ + error = ioctl(fhp->pfd, PEFS_GETNAMECSUM, &xncs); if (error == 0) { /* XXXgpf: Is this correct? */ @@ -622,7 +587,6 @@ else pefs_warn("failed to fetch file id from kernel"); - close(fd); return (error); } @@ -631,7 +595,9 @@ { struct stat sb; struct file_header targetfh; + char dirbuf[MAXPATHLEN + 1], namebuf[MAXNAMLEN + 1]; struct file_header *fhp, *res; + char *dirnamep, *namep; int error; TAILQ_FOREACH(fhp, fhhp, file_header_entries) { @@ -655,7 +621,32 @@ if (S_ISLNK(sb.st_mode) == 0 && S_ISREG(sb.st_mode) == 0) continue; + /* retrieve dirpath & filaname */ strlcpy(targetfh.path, fhp->target_path, sizeof(targetfh.path)); + strlcpy(dirbuf, targetfh.path, sizeof(dirbuf)); + strlcpy(namebuf, targetfh.path, sizeof(namebuf)); + + dirnamep = dirname(dirbuf); + if (dirnamep == NULL) { + pefs_warn("failed to extract dirname of %s", targetfh.path); + continue; + } + strlcpy(targetfh.dirpath, dirnamep, sizeof(targetfh.dirpath)); + + namep = basename(namebuf); + if (namep == NULL) { + pefs_warn("failed to extract filename of %s", targetfh.path); + continue; + } + strlcpy(targetfh.filename, namep, sizeof(targetfh.filename)); + + targetfh.pfd = -1; + targetfh.pfd = open(targetfh.dirpath, O_RDONLY); + if (targetfh.pfd < 0) { + warn("cannot open %s", targetfh.dirpath); + continue; + } + error = pefs_get_file_id(&targetfh); if (error == 0) { res = pefs_cuckoo_lookup(chtp, &targetfh); @@ -663,6 +654,7 @@ pefs_warn("target file %s of symlink %s was not found in inputlist", targetfh.path, fhp->path); } + pefs_close_file(&targetfh); } } } @@ -746,16 +738,41 @@ } static int -pefs_file_semantic_checks(struct file_header *fhp, struct statfs *fsp, struct hardlink_head *hlc_headp) +pefs_open_semantic_checks(struct file_header *fhp, struct statfs *fsp, struct hardlink_head *hlc_headp) { - char parent_dir[MAXPATHLEN]; - char sbuf[MAXPATHLEN]; + char dirbuf[MAXPATHLEN + 1], namebuf[MAXNAMLEN + 1]; + char sbuf[MAXPATHLEN + 1]; struct stat sb; struct statfs this_fs; - char *pch; + char *dirnamep, *namep; size_t target_path_size; int nchars; + /* initialize file descriptors in case error occurs */ + fhp->fd = -1; + fhp->pfd = -1; + + /* retrieve dirpath & filename */ + strlcpy(dirbuf, fhp->path, sizeof(dirbuf)); + strlcpy(namebuf, fhp->path, sizeof(namebuf)); + + dirnamep = dirname(dirbuf); + if (dirnamep == NULL) { + pefs_warn("failed to extract dirname of %s", fhp->path); + return (PEFS_ERR_GENERIC); + } + strlcpy(fhp->dirpath, dirnamep, sizeof(fhp->dirpath)); + + + namep = basename(namebuf); + if (namep == NULL) { + pefs_warn("failed to extract filename of %s", fhp->path); + return (PEFS_ERR_GENERIC); + } + strlcpy(fhp->filename, namep, sizeof(fhp->filename)); + + dprintf(("path = %s!\ndirname = %s!\nbasename =%s!\n", fhp->path, fhp->dirpath, fhp->filename)); + if (lstat(fhp->path, &sb) != 0) { warn("cannot stat file %s", fhp->path); return (PEFS_ERR_SYS); @@ -764,6 +781,12 @@ if (S_ISLNK(sb.st_mode) != 0) { fhp->target_path = NULL; + fhp->pfd = open(fhp->dirpath, O_RDONLY); + if (fhp->pfd < 0) { + warn("cannot open %s", fhp->dirpath); + return (PEFS_ERR_IO); + } + nchars = readlink(fhp->path, sbuf, sizeof(sbuf)); if (nchars == -1) { warn("readlink failed: %s", fhp->path); @@ -780,25 +803,17 @@ * Target referes to the file immediately pointed to by our symlink, not * the final target of a possible symlink chain. */ - target_path_size = MAXPATHLEN; + target_path_size = MAXPATHLEN + 1; fhp->target_path = malloc(target_path_size); if (fhp->target_path == NULL) { warn("memory allocation error"); return (PEFS_ERR_SYS); } - sbuf[nchars] = '\0'; - /* turn relative paths to absolute paths which are needed for ioctl() */ - if (sbuf[0] != '/') { - strlcpy(parent_dir, fhp->path, sizeof(parent_dir)); - pch = strrchr(parent_dir, '/'); - if (pch == NULL) { - pefs_warn("error retrieving parent dir of %s", fhp->path); - return (PEFS_ERR_NOENT); - } - *pch = '\0'; - snprintf(fhp->target_path, target_path_size, "%s/%s", parent_dir, sbuf); - } + + /* turn relative paths to absolute paths */ + if (sbuf[0] != '/') + snprintf(fhp->target_path, target_path_size, "%s/%s", fhp->dirpath, sbuf); else strlcpy(fhp->target_path, sbuf, target_path_size); @@ -828,12 +843,29 @@ else fhp->target_path = NULL; + fhp->fd = open(fhp->path, O_RDONLY | O_NOFOLLOW); + if (fhp->fd < 0) { + warn("cannot open %s", fhp->path); + return (PEFS_ERR_IO); + } + + fhp->pfd = open(fhp->dirpath, O_RDONLY); + if (fhp->pfd < 0) { + warn("cannot open %s", fhp->dirpath); + return (PEFS_ERR_IO); + } + + if (fstat(fhp->fd, &sb) != 0) { + warn("cannot stat file %s", fhp->path); + return (PEFS_ERR_SYS); + } + if (S_ISREG(sb.st_mode) == 0) { pefs_warn("filename: %s is not a regular file", fhp->path); return (PEFS_ERR_INVALID); } - if (statfs(fhp->path, &this_fs) == -1) { + if (fstatfs(fhp->fd, &this_fs) == -1) { pefs_warn("statfs failed: %s: %s", fhp->path, strerror(errno)); return (PEFS_ERR_SYS); } @@ -888,19 +920,20 @@ /* * This function creates the in memory database that will be later written to * the checksum file. - * A) The total sum of entries is gathered so that the hash tables are allocated. - * B) For each file entry: - * B1) semantic checks: - * B1a) file should reside in pefs filesystem & file should be regular file. - * B1b) if symlink, acquire and save the absolute path of the symlink's + * A) For each file entry: + * A1) semantic checks: + * A1a) file should reside in pefs filesystem & file should be regular file. + * A1b) if symlink, acquire and save the absolute path of the symlink's * target. Try to stat() the target but don't do anything else. - * B1c) If hardlink, save a reference to this file entry in our rb tree. + * A1c) If hardlink, save a reference to this file entry in our rb tree. * rb-tree uses inodes as keys and is used in part C to print warnings. - * B2) the file_id is retrieved. - * B3) list of checksums is computed for the file's 4k blocks. - * B4) file entry is added to universal fh_head. - * C) Print warnings for hardlinks if the number of links found in inputlist isn't + * A1d) Open and store file descriptors to file & parent_directory. + * A2) the file_id is retrieved. + * A3) list of checksums is computed for the file's 4k blocks. + * A4) file entry is added to universal fh_head. + * B) Print warnings for hardlinks if the number of links found in inputlist isn't * equal to the number of total inode links. + * C) Hash tables are allocated. * D) Cuckoo insertion: * We try to populate our hash tables using the cuckoo algorithm. Should we fall * into an infinite loop during insertion, we re-allocate larger hash tables @@ -929,19 +962,26 @@ TAILQ_INIT(&fh_head); RB_INIT(&hlc_head); while((fhp = pefs_next_file(fpin, &error, &nfiles)) != NULL) { - error = pefs_file_semantic_checks(fhp, &fs, &hlc_head); - if (error != 0) + error = pefs_open_semantic_checks(fhp, &fs, &hlc_head); + if (error != 0) { + pefs_close_file(fhp); return (error); + } error = pefs_get_file_id(fhp); - if (error != 0) + if (error != 0) { + pefs_close_file(fhp); return (error); + } error = pefs_compute_file_checksums(fhp, md, hash_len); - if (error != 0) + if (error != 0) { + pefs_close_file(fhp); return (error); + } TAILQ_INSERT_TAIL(&fh_head, fhp, file_header_entries); + pefs_close_file(fhp); } /* checking I/O error with pefs_next_file()*/