From owner-svn-src-all@freebsd.org Thu Jan 19 16:46:06 2017 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 6DF2ACB852A; Thu, 19 Jan 2017 16:46:06 +0000 (UTC) (envelope-from cem@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 4810B1252; Thu, 19 Jan 2017 16:46:06 +0000 (UTC) (envelope-from cem@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id v0JGk59J028457; Thu, 19 Jan 2017 16:46:05 GMT (envelope-from cem@FreeBSD.org) Received: (from cem@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id v0JGk5p1028456; Thu, 19 Jan 2017 16:46:05 GMT (envelope-from cem@FreeBSD.org) Message-Id: <201701191646.v0JGk5p1028456@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: cem set sender to cem@FreeBSD.org using -f From: "Conrad E. Meyer" Date: Thu, 19 Jan 2017 16:46:05 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r312416 - head/sys/ufs/ffs X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 19 Jan 2017 16:46:06 -0000 Author: cem Date: Thu Jan 19 16:46:05 2017 New Revision: 312416 URL: https://svnweb.freebsd.org/changeset/base/312416 Log: ffs_vnops: Simplify extattr access As suggested in r167010, use the structure type and macros to access and modify UFS2 extended attributes. Add assertions that pointers are aligned in places where we now access the data through a structure pointer, instead of character-by-character. PR: 216127 Reported by: dewayne at heuristicsystems.com.au Reviewed by: kib@ Sponsored by: Dell EMC Isilon Differential Revision: https://reviews.freebsd.org/D9225 Modified: head/sys/ufs/ffs/ffs_vnops.c Modified: head/sys/ufs/ffs/ffs_vnops.c ============================================================================== --- head/sys/ufs/ffs/ffs_vnops.c Thu Jan 19 16:07:52 2017 (r312415) +++ head/sys/ufs/ffs/ffs_vnops.c Thu Jan 19 16:46:05 2017 (r312416) @@ -100,6 +100,9 @@ __FBSDID("$FreeBSD$"); #include "opt_directio.h" #include "opt_ffs.h" +#define ALIGNED_TO(ptr, s) \ + (((uintptr_t)(ptr) & (_Alignof(s) - 1)) == 0) + #ifdef DIRECTIO extern int ffs_rawread(struct vnode *vp, struct uio *uio, int *workdone); #endif @@ -1100,46 +1103,30 @@ ffs_extwrite(struct vnode *vp, struct ui * the length of the EA, and possibly the pointer to the entry and to the data. */ static int -ffs_findextattr(u_char *ptr, u_int length, int nspace, const char *name, u_char **eap, u_char **eac) +ffs_findextattr(u_char *ptr, u_int length, int nspace, const char *name, + struct extattr **eapp, u_char **eac) { - u_char *p, *pe, *pn, *p0; - int eapad1, eapad2, ealength, ealen, nlen; - uint32_t ul; + struct extattr *eap, *eaend; + size_t nlen; - pe = ptr + length; nlen = strlen(name); - - for (p = ptr; p < pe; p = pn) { - p0 = p; - bcopy(p, &ul, sizeof(ul)); - pn = p + ul; + KASSERT(ALIGNED_TO(ptr, struct extattr), ("unaligned")); + eap = (struct extattr *)ptr; + eaend = (struct extattr *)(ptr + length); + for (; eap < eaend; eap = EXTATTR_NEXT(eap)) { /* make sure this entry is complete */ - if (pn > pe) + if (EXTATTR_NEXT(eap) > eaend) break; - p += sizeof(uint32_t); - if (*p != nspace) - continue; - p++; - eapad2 = *p++; - if (*p != nlen) - continue; - p++; - if (bcmp(p, name, nlen)) + if (eap->ea_namespace != nspace || eap->ea_namelength != nlen + || memcmp(eap->ea_name, name, nlen) != 0) continue; - ealength = sizeof(uint32_t) + 3 + nlen; - eapad1 = 8 - (ealength % 8); - if (eapad1 == 8) - eapad1 = 0; - ealength += eapad1; - ealen = ul - ealength - eapad2; - p += nlen + eapad1; - if (eap != NULL) - *eap = p0; + if (eapp != NULL) + *eapp = eap; if (eac != NULL) - *eac = p; - return (ealen); + *eac = EXTATTR_CONTENT(eap); + return (EXTATTR_CONTENT_SIZE(eap)); } - return(-1); + return (-1); } static int @@ -1380,9 +1367,11 @@ vop_deleteextattr { { struct inode *ip; struct fs *fs; - uint32_t ealength, ul; - int ealen, olen, eapad1, eapad2, error, i, easize; - u_char *eae, *p; + struct extattr *eap; + uint32_t ul; + int olen, error, i, easize; + u_char *eae; + void *tmp; ip = VTOI(ap->a_vp); fs = ITOFS(ip); @@ -1413,39 +1402,30 @@ vop_deleteextattr { if (error) return (error); - ealength = eapad1 = ealen = eapad2 = 0; - + /* CEM: delete could be done in-place instead */ eae = malloc(ip->i_ea_len, M_TEMP, M_WAITOK); bcopy(ip->i_ea_area, eae, ip->i_ea_len); easize = ip->i_ea_len; olen = ffs_findextattr(eae, easize, ap->a_attrnamespace, ap->a_name, - &p, NULL); + &eap, NULL); if (olen == -1) { /* delete but nonexistent */ free(eae, M_TEMP); ffs_close_ea(ap->a_vp, 0, ap->a_cred, ap->a_td); - return(ENOATTR); + return (ENOATTR); } - bcopy(p, &ul, sizeof ul); - i = p - eae + ul; - if (ul != ealength) { - bcopy(p + ul, p + ealength, easize - i); - easize += (ealength - ul); - } - if (easize > NXADDR * fs->fs_bsize) { - free(eae, M_TEMP); - ffs_close_ea(ap->a_vp, 0, ap->a_cred, ap->a_td); - if (ip->i_ea_area != NULL && ip->i_ea_error == 0) - ip->i_ea_error = ENOSPC; - return(ENOSPC); - } - p = ip->i_ea_area; + ul = eap->ea_length; + i = (u_char *)EXTATTR_NEXT(eap) - eae; + bcopy(EXTATTR_NEXT(eap), eap, easize - i); + easize -= ul; + + tmp = ip->i_ea_area; ip->i_ea_area = eae; ip->i_ea_len = easize; - free(p, M_TEMP); + free(tmp, M_TEMP); error = ffs_close_ea(ap->a_vp, 1, ap->a_cred, ap->a_td); - return(error); + return (error); } /* @@ -1499,7 +1479,7 @@ vop_getextattr { error = ENOATTR; ffs_close_ea(ap->a_vp, 0, ap->a_cred, ap->a_td); - return(error); + return (error); } /* @@ -1519,9 +1499,7 @@ vop_listextattr { */ { struct inode *ip; - u_char *eae, *p, *pe, *pn; - unsigned easize; - uint32_t ul; + struct extattr *eap, *eaend; int error, ealen; ip = VTOI(ap->a_vp); @@ -1537,31 +1515,31 @@ vop_listextattr { error = ffs_open_ea(ap->a_vp, ap->a_cred, ap->a_td); if (error) return (error); - eae = ip->i_ea_area; - easize = ip->i_ea_len; error = 0; if (ap->a_size != NULL) *ap->a_size = 0; - pe = eae + easize; - for(p = eae; error == 0 && p < pe; p = pn) { - bcopy(p, &ul, sizeof(ul)); - pn = p + ul; - if (pn > pe) + + KASSERT(ALIGNED_TO(ip->i_ea_area, struct extattr), ("unaligned")); + eap = (struct extattr *)ip->i_ea_area; + eaend = (struct extattr *)(ip->i_ea_area + ip->i_ea_len); + for (; error == 0 && eap < eaend; eap = EXTATTR_NEXT(eap)) { + /* make sure this entry is complete */ + if (EXTATTR_NEXT(eap) > eaend) break; - p += sizeof(ul); - if (*p++ != ap->a_attrnamespace) + if (eap->ea_namespace != ap->a_attrnamespace) continue; - p++; /* pad2 */ - ealen = *p; - if (ap->a_size != NULL) { + + ealen = eap->ea_namelength; + if (ap->a_size != NULL) *ap->a_size += ealen + 1; - } else if (ap->a_uio != NULL) { - error = uiomove(p, ealen + 1, ap->a_uio); - } + else if (ap->a_uio != NULL) + error = uiomove(&eap->ea_namelength, ealen + 1, + ap->a_uio); } + ffs_close_ea(ap->a_vp, 0, ap->a_cred, ap->a_td); - return(error); + return (error); } /* @@ -1582,10 +1560,12 @@ vop_setextattr { { struct inode *ip; struct fs *fs; + struct extattr *eap; uint32_t ealength, ul; ssize_t ealen; int olen, eapad1, eapad2, error, i, easize; - u_char *eae, *p; + u_char *eae; + void *tmp; ip = VTOI(ap->a_vp); fs = ITOFS(ip); @@ -1625,29 +1605,33 @@ vop_setextattr { return (error); ealength = sizeof(uint32_t) + 3 + strlen(ap->a_name); - eapad1 = 8 - (ealength % 8); - if (eapad1 == 8) - eapad1 = 0; - eapad2 = 8 - (ealen % 8); - if (eapad2 == 8) - eapad2 = 0; + eapad1 = roundup2(ealength, 8) - ealength; + eapad2 = roundup2(ealen, 8) - ealen; ealength += eapad1 + ealen + eapad2; + /* + * CEM: rewrites of the same size or smaller could be done in-place + * instead. (We don't acquire any fine-grained locks in here either, + * so we could also do bigger writes in-place.) + */ eae = malloc(ip->i_ea_len + ealength, M_TEMP, M_WAITOK); bcopy(ip->i_ea_area, eae, ip->i_ea_len); easize = ip->i_ea_len; - olen = ffs_findextattr(eae, easize, - ap->a_attrnamespace, ap->a_name, &p, NULL); + olen = ffs_findextattr(eae, easize, ap->a_attrnamespace, ap->a_name, + &eap, NULL); if (olen == -1) { /* new, append at end */ - p = eae + easize; + KASSERT(ALIGNED_TO(eae + easize, struct extattr), + ("unaligned")); + eap = (struct extattr *)(eae + easize); easize += ealength; } else { - bcopy(p, &ul, sizeof ul); - i = p - eae + ul; + ul = eap->ea_length; + i = (u_char *)EXTATTR_NEXT(eap) - eae; if (ul != ealength) { - bcopy(p + ul, p + ealength, easize - i); + bcopy(EXTATTR_NEXT(eap), (u_char *)eap + ealength, + easize - i); easize += (ealength - ul); } } @@ -1656,34 +1640,30 @@ vop_setextattr { ffs_close_ea(ap->a_vp, 0, ap->a_cred, ap->a_td); if (ip->i_ea_area != NULL && ip->i_ea_error == 0) ip->i_ea_error = ENOSPC; - return(ENOSPC); + return (ENOSPC); } - bcopy(&ealength, p, sizeof(ealength)); - p += sizeof(ealength); - *p++ = ap->a_attrnamespace; - *p++ = eapad2; - *p++ = strlen(ap->a_name); - strcpy(p, ap->a_name); - p += strlen(ap->a_name); - bzero(p, eapad1); - p += eapad1; - error = uiomove(p, ealen, ap->a_uio); + eap->ea_length = ealength; + eap->ea_namespace = ap->a_attrnamespace; + eap->ea_contentpadlen = eapad2; + eap->ea_namelength = strlen(ap->a_name); + memcpy(eap->ea_name, ap->a_name, strlen(ap->a_name)); + bzero(&eap->ea_name[strlen(ap->a_name)], eapad1); + error = uiomove(EXTATTR_CONTENT(eap), ealen, ap->a_uio); if (error) { free(eae, M_TEMP); ffs_close_ea(ap->a_vp, 0, ap->a_cred, ap->a_td); if (ip->i_ea_area != NULL && ip->i_ea_error == 0) ip->i_ea_error = error; - return(error); + return (error); } - p += ealen; - bzero(p, eapad2); + bzero((u_char *)EXTATTR_CONTENT(eap) + ealen, eapad2); - p = ip->i_ea_area; + tmp = ip->i_ea_area; ip->i_ea_area = eae; ip->i_ea_len = easize; - free(p, M_TEMP); + free(tmp, M_TEMP); error = ffs_close_ea(ap->a_vp, 1, ap->a_cred, ap->a_td); - return(error); + return (error); } /*