From 36796bbb83af2650a872234fdb5cf7124bf6cfa8 Mon Sep 17 00:00:00 2001 From: Michael Meffie Date: Mon, 23 Aug 2021 14:23:01 -0400 Subject: [PATCH] ubik: Fix ubeacon_updateUbikNetworkAddress() mismatched array parameter warning The ubeacon_updateUbikNetworkAddress() prototype does not match the function definition. The ubik_host parameter is declared as an unbounded array in the prototype but is defined as a bounded array. As of GCC 12, a warning is issued for the mismatch: error: argument 1 of type ‘afs_uint32[256]’ {aka ‘unsigned int[256]’} with mismatched bound [-Werror=array-parameter=] ubeacon_updateUbikNetworkAddress( afs_uint32 ubik_host[UBIK_MAX_INTERFACE_ADDR]) note: previously declared as ‘afs_uint32[]’ {aka ‘unsigned int[]’} extern int ubeacon_updateUbikNetworkAddress(afs_uint32 ubik_host[]); Restore the ubik_host array length in the function prototype, which was dropped in commit 9020e6e2f0357b1082705dcaa6626573433969ec (ubik: Defer updateUbikNetworkAddress until after RX startup). Change-Id: I8189effc5b68ef8c1b45b4107f5e22e44ecf59fd Reviewed-on: https://gerrit.openafs.org/14767 Tested-by: BuildBot Reviewed-by: Benjamin Kaduk --- diff --git a/src/ubik/ubik.p.h b/src/ubik/ubik.p.h index cfd12f7..52869ad 100644 --- a/src/ubik/ubik.p.h +++ b/src/ubik/ubik.p.h @@ -492,7 +492,7 @@ char clones[]); extern int ubeacon_InitServerList(afs_uint32 ame, afs_uint32 aservers[]); extern void *ubeacon_Interact(void *); -extern int ubeacon_updateUbikNetworkAddress(afs_uint32 ubik_host[]); +extern int ubeacon_updateUbikNetworkAddress(afs_uint32 ubik_host[UBIK_MAX_INTERFACE_ADDR]); extern struct beacon_data beacon_globals; extern struct addr_data addr_globals; From 4a8d0c4089078fb3df9cc06b595c80c9b4c2ca7f Mon Sep 17 00:00:00 2001 From: Michael Meffie Date: Mon, 23 Aug 2021 15:42:52 -0400 Subject: [PATCH] libadmin: Fix isAlias may be uninitialized warning The cfgutil_HostNameIsAlias() function has an output parameter called isAlias, which is used when cfgutil_HostIsAlias() returns non-zero. However, it possible for isAlias to not be set before returning. GCC 12 issues a warning about the possible use of the uninitialized isAlias variable: cfginternal.c:366:32: error: ‘isAlias’ may be used uninitialized [-Werror=maybe-uninitialized] Initialize the cfgutil_HostNameIsAlias() isAlias output flag to false. Also, fix the misleading code indentation around the cfgutil_HostNameIsAlias() call. Change-Id: I68e66ae5f9019a613187321bb792d0505959ed30 Reviewed-on: https://gerrit.openafs.org/14772 Reviewed-by: Cheyenne Wills Tested-by: BuildBot Reviewed-by: Benjamin Kaduk --- diff --git a/src/libadmin/cfg/cfginternal.c b/src/libadmin/cfg/cfginternal.c index 1f581b8..f1abacb 100644 --- a/src/libadmin/cfg/cfginternal.c +++ b/src/libadmin/cfg/cfginternal.c @@ -348,7 +348,7 @@ short dbhostFound = 0; while (!dbhostDone) { - short isAlias; + short isAlias = 0; if (!bos_HostGetNext(dbIter, hostNameAlias, &tst2)) { /* no more entries (or failure) */ @@ -357,15 +357,15 @@ } dbhostDone = 1; - } else - if (!cfgutil_HostNameIsAlias - (hostName, hostNameAlias, &isAlias, &tst2)) { - tst = tst2; - dbhostDone = 1; - - } else if (isAlias) { - dbhostFound = 1; - dbhostDone = 1; + } else { + if (!cfgutil_HostNameIsAlias(hostName, hostNameAlias, + &isAlias, &tst2)) { + tst = tst2; + dbhostDone = 1; + } else if (isAlias) { + dbhostFound = 1; + dbhostDone = 1; + } } } From 7924aecf95bf4918a485a041f2426bd1fa407ac8 Mon Sep 17 00:00:00 2001 From: Michael Meffie Date: Fri, 03 Sep 2021 07:05:36 -0400 Subject: [PATCH] ptserver: Fix CreateEntry() stringop-overflow warnings The CreateEntry() prototype has been fixed to match the function definition, so callers are expected to provide bounded arrays for the user or group name. Fix the InitialGroup() macro which is used to set the built-in names using string literal to avoid stringop-overflow warnings. error: ‘CreateEntry’ accessing 64 bytes in a region of size 22 [-Werror=stringop-overflow=] code = CreateEntry(tt, (name), &temp, /*idflag*/1, flag, SYSADMINID, SYSADMINID); \ note: in expansion of macro ‘InitialGroup’ InitialGroup(SYSADMINID, "system:administrators"); note: referencing argument 2 of type ‘char *’ note: in a call to function ‘CreateEntry’ CreateEntry(struct ubik_trans *at, char aname[PR_MAXNAMELEN], ... (Repeated for "system:backup", "system:anyuser", "system:authuser", "system:ptsviewers", and "anonymous".) Change-Id: I7a37d4c8e191ffff52c2fdc1ed3783f4c3592b11 Reviewed-on: https://gerrit.openafs.org/14789 Reviewed-by: Benjamin Kaduk Tested-by: Benjamin Kaduk --- diff --git a/src/ptserver/ptutils.c b/src/ptserver/ptutils.c index dfa54ee..07fd220 100644 --- a/src/ptserver/ptutils.c +++ b/src/ptserver/ptutils.c @@ -1847,8 +1847,15 @@ #define InitialGroup(id,name) do { \ afs_int32 temp = (id); \ afs_int32 flag = (id) < 0 ? PRGRP : 0; \ + char tname[PR_MAXNAMELEN]; \ + if (strlcpy(tname, (name), sizeof(tname)) >= sizeof(tname)) { \ + code = PRBADNAM; \ + afs_com_err (whoami, code, "name too long %s", (name)); \ + ubik_AbortTrans(tt); \ + return code; \ + } \ code = CreateEntry \ - (tt, (name), &temp, /*idflag*/1, flag, SYSADMINID, SYSADMINID); \ + (tt, tname, &temp, /*idflag*/1, flag, SYSADMINID, SYSADMINID); \ if (code) { \ afs_com_err (whoami, code, "couldn't create %s with id %di.", \ (name), (id)); \ From 92a6242de2d8ea280debc283a7c089f97c1670bc Mon Sep 17 00:00:00 2001 From: Michael Meffie Date: Mon, 23 Aug 2021 15:37:13 -0400 Subject: [PATCH] bucoord: Fix doDispatch() array-parameter gcc warning The doDispatch() prototype does not match the function definition. The targv parameter is declared as an unbounded array in the prototype, but is defined as a bounded array. As of GCC 12, a warning is issued for the mismatch. main.c:346:18: error: argument 2 of type ‘char *[100]’ with mismatched bound [-Werror=array-parameter=] bucoord_internal.h:123:40: note: previously declared as ‘char *[]’ Within doDispatch(), the targv argument is just passed to cmd_Dispatch() (this is the only use of targv). Since cmd_Displatch() expects an unbounded array, update the doDispatch() definition to match the prototype. Change-Id: I50a170b3490d0d4e5d971b9ccb483cccb6833686 Reviewed-on: https://gerrit.openafs.org/14771 Reviewed-by: Michael Meffie Reviewed-by: Cheyenne Wills Tested-by: BuildBot Reviewed-by: Benjamin Kaduk --- diff --git a/src/bucoord/main.c b/src/bucoord/main.c index a453cc9..972ac2d 100644 --- a/src/bucoord/main.c +++ b/src/bucoord/main.c @@ -343,7 +343,7 @@ afs_int32 doDispatch(afs_int32 targc, - char *targv[MAXV], + char *targv[], afs_int32 dispatchCount) /* to prevent infinite recursion */ { char *sargv[MAXV]; From a1e57d2e42b6d01e5ece93d5d49a4b9f3ecd3edc Mon Sep 17 00:00:00 2001 From: Michael Meffie Date: Mon, 23 Aug 2021 15:33:19 -0400 Subject: [PATCH] Fix PrintInode() mismatched array parameter warnings The PrintInode() prototypes do not match the function definitions. When AFS_64BIT_IOPS_ENV is defined (which is the common case and is required for namei), the buffer parameter is declared as a bounded character array (afs_ino_str_t) in the prototype, but is defined as an unbounded character pointer. When AFS_64BIT_IOPS_ENV is not defined (for legacy 32-bit inode vice partitions), PrintInode() is declared with no specified parameters. A static buffer is used to hold the formatted string when a NULL is passed as the first argument to PrintInode(). However, this method is only used by the volinfo and iopen utility programs. Fix the mismatch function prototypes and definitions to use the bounded char array (afs_ino_str_t) in all cases. Remove the deprecated function declaration with no specified parameters. Update vol-info and iopen to pass an afs_ino_str_t buffer and remove the now unused static buffer. Update the duplicated PrintInode() function definition in namei_ops.c. (This duplicated code could be removed in a future commit.) Change-Id: I5c0128bb0d572dab0df637289daad0e648ad8a8f Reviewed-on: https://gerrit.openafs.org/14770 Tested-by: BuildBot Reviewed-by: Cheyenne Wills Reviewed-by: Benjamin Kaduk --- diff --git a/src/sys/afssyscalls.c b/src/sys/afssyscalls.c index 8291a46..b248e69 100644 --- a/src/sys/afssyscalls.c +++ b/src/sys/afssyscalls.c @@ -281,23 +281,16 @@ } -/* PrintInode +/** + * Format a string to print either 32 or 64 bit inode numbers. * - * returns a static string used to print either 32 or 64 bit inode numbers. + * @param[out] s string buffer + * @param[in] ino inode number + * @returns pointer to formatted inode number string */ -#ifdef AFS_64BIT_IOPS_ENV -char * -PrintInode(char *s, Inode ino) -#else char * PrintInode(afs_ino_str_t s, Inode ino) -#endif { - static afs_ino_str_t result; - - if (!s) - s = result; - #ifdef AFS_64BIT_IOPS_ENV (void)sprintf((char *)s, "%llu", ino); #else diff --git a/src/sys/afssyscalls.h b/src/sys/afssyscalls.h index df37c42..ea74bcd 100644 --- a/src/sys/afssyscalls.h +++ b/src/sys/afssyscalls.h @@ -88,14 +88,9 @@ #define AFS_INO_STR_LENGTH 32 typedef char afs_ino_str_t[AFS_INO_STR_LENGTH]; -/* Print either 32 or 64 bit inode numbers. char * may be NULL. In which case - * a local statis is returned. +/* Format either 32 or 64 bit inode numbers. */ -#ifdef AFS_64BIT_IOPS_ENV -extern char *PrintInode(afs_ino_str_t, Inode); -#else -extern char *PrintInode(); -#endif +extern char *PrintInode(afs_ino_str_t s, Inode ino) AFS_NONNULL((1)); /* Some places in the code assume icreate can return 0 when there's * an error. diff --git a/src/sys/iopen.c b/src/sys/iopen.c index 7c1a961..b9cd3d6 100644 --- a/src/sys/iopen.c +++ b/src/sys/iopen.c @@ -37,6 +37,7 @@ int fd, n; struct stat status; Inode ino; + afs_ino_str_t inode_str; if (argc != 3) Usage(); @@ -54,7 +55,7 @@ } printf("ino=%" AFS_INT64_FMT "\n", ino); printf("About to iopen(dev=(%d,%d), inode=%s, mode=%d\n", - major(status.st_dev), minor(status.st_dev), PrintInode(NULL, ino), + major(status.st_dev), minor(status.st_dev), PrintInode(inode_str, ino), O_RDONLY); fflush(stdout); fd = IOPEN(status.st_dev, ino, O_RDONLY); diff --git a/src/vol/namei_ops.c b/src/vol/namei_ops.c index 463b66b..d2f94bb 100644 --- a/src/vol/namei_ops.c +++ b/src/vol/namei_ops.c @@ -3275,19 +3275,17 @@ return code; } -/* PrintInode +/** + * Format a string to print inode numbers. * - * returns a static string used to print either 32 or 64 bit inode numbers. + * @param[out] s string buffer + * @param[in] ino inode number + * @returns pointer to formatted inode number string */ char * -PrintInode(char *s, Inode ino) +PrintInode(afs_ino_str_t s, Inode ino) { - static afs_ino_str_t result; - if (!s) - s = result; - snprintf(s, sizeof(afs_ino_str_t), "%llu", (afs_uintmax_t) ino); - return s; } diff --git a/src/vol/vol-info.c b/src/vol/vol-info.c index 22ac94a..6423fa8 100644 --- a/src/vol/vol-info.c +++ b/src/vol/vol-info.c @@ -331,6 +331,7 @@ struct versionStamp *vsn; int bad = 0; int code; + afs_ino_str_t inode_str; vsn = (struct versionStamp *)to; @@ -341,7 +342,7 @@ if (vsn->magic != magic) { bad++; fprintf(stderr, "%s: Inode %s: Bad magic %x (%x): IGNORED\n", - progname, PrintInode(NULL, ih->ih_ino), vsn->magic, magic); + progname, PrintInode(inode_str, ih->ih_ino), vsn->magic, magic); } /* Check is conditional, in case caller wants to inspect version himself */ @@ -349,23 +350,23 @@ bad++; fprintf(stderr, "%s: Inode %s: Bad version %x (%x): IGNORED\n", progname, - PrintInode(NULL, ih->ih_ino), vsn->version, version); + PrintInode(inode_str, ih->ih_ino), vsn->version, version); } if (bad && opt->fixHeader) { vsn->magic = magic; vsn->version = version; printf("Special index inode %s has a bad header. Reconstructing...\n", - PrintInode(NULL, ih->ih_ino)); + PrintInode(inode_str, ih->ih_ino)); code = IH_IWRITE(ih, 0, to, size); if (code != size) { fprintf(stderr, "%s: Write failed for inode %s; header left in damaged state\n", - progname, PrintInode(NULL, ih->ih_ino)); + progname, PrintInode(inode_str, ih->ih_ino)); } } if (!bad && opt->dumpInfo) { printf("Inode %s: Good magic %x and version %x\n", - PrintInode(NULL, ih->ih_ino), magic, version); + PrintInode(inode_str, ih->ih_ino), magic, version); } return 0; } @@ -887,6 +888,7 @@ afs_sfsize_t size = -1; IHandle_t *ih = NULL; FdHandle_t *fdP = NULL; + afs_ino_str_t inode_str; #ifdef AFS_NAMEI_ENV namei_t filename; #endif /* AFS_NAMEI_ENV */ @@ -912,7 +914,7 @@ error: if (opt->dumpInfo) { - printf("\t%s inode\t= %s (size = ", name, PrintInode(NULL, inode)); + printf("\t%s inode\t= %s (size = ", name, PrintInode(inode_str, inode)); if (size != -1) { printf("%lld)\n", size); } else { @@ -1241,6 +1243,7 @@ afs_foff_t total; ssize_t len; Inode ino = VNDISK_GET_INO(vdp->vnode); + afs_ino_str_t inode_str; if (!VALID_INO(ino)) { return; @@ -1251,10 +1254,10 @@ if (fdP == NULL) { fprintf(stderr, "%s: Can't open inode %s error %d (ignored)\n", - progname, PrintInode(NULL, ino), errno); + progname, PrintInode(inode_str, ino), errno); return; } - snprintf(nfile, sizeof nfile, "TmpInode.%s", PrintInode(NULL, ino)); + snprintf(nfile, sizeof nfile, "TmpInode.%s", PrintInode(inode_str, ino)); ofd = afs_open(nfile, O_CREAT | O_RDWR | O_TRUNC, 0600); if (ofd < 0) { fprintf(stderr, @@ -1276,7 +1279,7 @@ unlink(nfile); fprintf(stderr, "%s: Error while reading from inode %s (%d)\n", - progname, PrintInode(NULL, ino), errno); + progname, PrintInode(inode_str, ino), errno); return; } if (len == 0) @@ -1299,7 +1302,7 @@ IH_RELEASE(ih); close(ofd); printf("... Copied inode %s to file %s (%lu bytes)\n", - PrintInode(NULL, ino), nfile, (unsigned long)total); + PrintInode(inode_str, ino), nfile, (unsigned long)total); } /** @@ -1812,6 +1815,7 @@ VnodeDiskObject *vnode = vdp->vnode; afs_fsize_t fileLength; Inode ino; + afs_ino_str_t inode_str; ino = VNDISK_GET_INO(vnode); VNDISK_GET_LEN(fileLength, vnode); @@ -1828,7 +1832,7 @@ vnode->dataVersion, vnode->cloned, (afs_uintmax_t) fileLength, vnode->linkCount, vnode->parent); if (opt->dumpInodeNumber) - printf(" inode: %s", PrintInode(NULL, ino)); + printf(" inode: %s", PrintInode(inode_str, ino)); if (opt->dumpDate) printf(" ServerModTime: %s", date(vnode->serverModifyTime)); #if defined(AFS_NAMEI_ENV) From a3aac5106beddc5a6f7a09c2d21c2524342aca01 Mon Sep 17 00:00:00 2001 From: Michael Meffie Date: Mon, 23 Aug 2021 19:43:45 -0400 Subject: [PATCH] pts: Fix stringop-overflow warnings The ptutil functions are defined to accept bounded character arrays for user and group names. As of GCC 11, callers which provide the names as string literals now trigger the stringop-overflow warning, since the regions provided by the string literals are smaller than the bounded areas. error: ‘pr_ChangeEntry’ accessing 64 bytes in a region of size 1 [-Werror=stringop-overflow=] note: referencing argument 4 of type ‘char *’ error: ‘pr_IsAMemberOf’ accessing 64 bytes in a region of size 22 [-Werror=stringop-overflow=] note: referencing argument 2 of type ‘char *’ error: ‘pr_CreateUser’ accessing 64 bytes in a region of size 16 [-Werror=stringop-overflow=] note: referencing argument 1 of type ‘char *’ error: ‘pr_Delete’ accessing 64 bytes in a region of size 16 [-Werror=stringop-overflow=] note: referencing argument 1 of type ‘char *’ Update the callers in pts and testpt which pass literal strings. Instead of passing char pointers to literal strings, assign the strings to prname buffers and pass the prname buffers to the pr utility functions. Change-Id: I7d8c67aa28d21bb6889ef92a2193a77b54c83cb1 Reviewed-on: https://gerrit.openafs.org/14769 Reviewed-by: Cheyenne Wills Tested-by: BuildBot Reviewed-by: Benjamin Kaduk --- diff --git a/src/ptserver/pts.c b/src/ptserver/pts.c index e749714..90d6f16 100644 --- a/src/ptserver/pts.c +++ b/src/ptserver/pts.c @@ -663,6 +663,7 @@ idlist ids; idlist lids; struct prcheckentry aentry; + prname admins = "system:administrators"; if (GetNameOrId(as, &ids, &names)) return PRBADARG; @@ -728,7 +729,7 @@ } if (aentry.id == SYSADMINID) admin = 1; - else if (!pr_IsAMemberOf(aentry.name, "system:administrators", &flag)) { + else if (!pr_IsAMemberOf(aentry.name, admins, &flag)) { if (flag) admin = 1; } @@ -791,11 +792,12 @@ { afs_int32 code; char *name; + prname newname = ""; char *owner; name = as->parms[0].items->data; owner = as->parms[1].items->data; - code = pr_ChangeEntry(name, "", 0, owner); + code = pr_ChangeEntry(name, newname, 0, owner); if (code) afs_com_err(whoami, code, "; unable to change owner of %s to %s", name, owner); @@ -808,10 +810,11 @@ afs_int32 code; char *oldname; char *newname; + prname owner = ""; oldname = as->parms[0].items->data; newname = as->parms[1].items->data; - code = pr_ChangeEntry(oldname, newname, 0, ""); + code = pr_ChangeEntry(oldname, newname, 0, owner); if (code) afs_com_err(whoami, code, "; unable to change name of %s to %s", oldname, newname); diff --git a/src/ptserver/testpt.c b/src/ptserver/testpt.c index eaaee48..c359f8f 100644 --- a/src/ptserver/testpt.c +++ b/src/ptserver/testpt.c @@ -217,7 +217,7 @@ CreateUser(int u) { afs_int32 code; - char name[16]; + prname name; afs_int32 id; sprintf(name, "%s%d", createPrefix, u); From fe64ddd3b49bf15222d32d443ff226dd4c2b899e Mon Sep 17 00:00:00 2001 From: Michael Meffie Date: Tue, 24 Aug 2021 16:40:22 -0400 Subject: [PATCH] ptserver: Fix CreateEntry() mismatched array parameter warning The CreateEntry() prototype does not match the function definition. The aname parameter is declared as an unbounded array in the prototype but is defined as a bounded array. As of GCC 12, a warning is issued for the mismatch. error: argument 2 of type ‘char[64]’ with mismatched bound [-Werror=array-parameter=] CreateEntry(struct ubik_trans *at, char aname[PR_MAXNAMELEN], ... note: previously declared as ‘char[]’ extern afs_int32 CreateEntry(struct ubik_trans *at, char aname[], ... Fix the prototype to declare the 'aname' parameter as a bounded array as expected for this function. Change-Id: I6d4dadcdd3f80c2b6f1b17670bbbc1e9e6076559 Reviewed-on: https://gerrit.openafs.org/14768 Tested-by: BuildBot Reviewed-by: Benjamin Kaduk --- diff --git a/src/ptserver/ptprototypes.h b/src/ptserver/ptprototypes.h index c6a9dab..bb21b6b 100644 --- a/src/ptserver/ptprototypes.h +++ b/src/ptserver/ptprototypes.h @@ -72,7 +72,7 @@ afs_int32 loc, afs_int32 aid); extern int AccessOK(struct ubik_trans *ut, afs_int32 cid, struct prentry *tentry, int mem, int any); -extern afs_int32 CreateEntry(struct ubik_trans *at, char aname[], +extern afs_int32 CreateEntry(struct ubik_trans *at, char aname[PR_MAXNAMELEN], afs_int32 *aid, afs_int32 idflag, afs_int32 flag, afs_int32 oid, afs_int32 creator); extern afs_int32 RemoveFromEntry(struct ubik_trans *at, afs_int32 aid,