Date: Tue, 18 Oct 2005 12:34:42 GMT From: Robert Watson <rwatson@FreeBSD.org> To: Perforce Change Reviews <perforce@freebsd.org> Subject: PERFORCE change 85491 for review Message-ID: <200510181234.j9ICYgnZ067553@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=85491 Change 85491 by rwatson@rwatson_zoo on 2005/10/18 12:33:41 Define ARG_SET_VALID() and ARG_IS_VALID(), which respectively set and test kernel record valid bits for various arguments. Use ARG_SET_VALID() when setting flags as a result of collecting an argument. Simplify or clarify some flag-related processing; annotate some additional places where simplification or clarification is called for but not yet present. Affected files ... .. //depot/projects/trustedbsd/audit3/sys/security/audit/audit_arg.c#5 edit .. //depot/projects/trustedbsd/audit3/sys/security/audit/audit_private.h#9 edit Differences ... ==== //depot/projects/trustedbsd/audit3/sys/security/audit/audit_arg.c#5 (text+ko) ==== @@ -60,7 +60,7 @@ return; ar->k_ar.ar_arg_addr = addr; - ar->k_ar.ar_valid_arg |= ARG_ADDR; + ARG_SET_VALID(ar, ARG_ADDR); } void @@ -74,7 +74,7 @@ ar->k_ar.ar_arg_exitstatus = status; ar->k_ar.ar_arg_exitretval = retval; - ar->k_ar.ar_valid_arg |= ARG_EXIT; + ARG_SET_VALID(ar, ARG_EXIT); } void @@ -87,7 +87,7 @@ return; ar->k_ar.ar_arg_len = len; - ar->k_ar.ar_valid_arg |= ARG_LEN; + ARG_SET_VALID(ar, ARG_LEN); } void @@ -100,7 +100,7 @@ return; ar->k_ar.ar_arg_fd = fd; - ar->k_ar.ar_valid_arg |= ARG_FD; + ARG_SET_VALID(ar, ARG_FD); } void @@ -113,7 +113,7 @@ return; ar->k_ar.ar_arg_fflags = fflags; - ar->k_ar.ar_valid_arg |= ARG_FFLAGS; + ARG_SET_VALID(ar, ARG_FFLAGS); } void @@ -126,7 +126,7 @@ return; ar->k_ar.ar_arg_gid = gid; - ar->k_ar.ar_valid_arg |= ARG_GID; + ARG_SET_VALID(ar, ARG_GID); } void @@ -139,7 +139,7 @@ return; ar->k_ar.ar_arg_uid = uid; - ar->k_ar.ar_valid_arg |= ARG_UID; + ARG_SET_VALID(ar, ARG_UID); } void @@ -152,7 +152,7 @@ return; ar->k_ar.ar_arg_egid = egid; - ar->k_ar.ar_valid_arg |= ARG_EGID; + ARG_SET_VALID(ar, ARG_EGID); } void @@ -165,7 +165,7 @@ return; ar->k_ar.ar_arg_euid = euid; - ar->k_ar.ar_valid_arg |= ARG_EUID; + ARG_SET_VALID(ar, ARG_EUID); } void @@ -178,7 +178,7 @@ return; ar->k_ar.ar_arg_rgid = rgid; - ar->k_ar.ar_valid_arg |= ARG_RGID; + ARG_SET_VALID(ar, ARG_RGID); } void @@ -191,7 +191,7 @@ return; ar->k_ar.ar_arg_ruid = ruid; - ar->k_ar.ar_valid_arg |= ARG_RUID; + ARG_SET_VALID(ar, ARG_RUID); } void @@ -204,7 +204,7 @@ return; ar->k_ar.ar_arg_sgid = sgid; - ar->k_ar.ar_valid_arg |= ARG_SGID; + ARG_SET_VALID(ar, ARG_SGID); } void @@ -217,7 +217,7 @@ return; ar->k_ar.ar_arg_suid = suid; - ar->k_ar.ar_valid_arg |= ARG_SUID; + ARG_SET_VALID(ar, ARG_SUID); } void @@ -233,7 +233,7 @@ for (i = 0; i < gidset_size; i++) ar->k_ar.ar_arg_groups.gidset[i] = gidset[i]; ar->k_ar.ar_arg_groups.gidset_size = gidset_size; - ar->k_ar.ar_valid_arg |= ARG_GROUPSET; + ARG_SET_VALID(ar, ARG_GROUPSET); } void @@ -245,16 +245,8 @@ if (ar == NULL) return; -#if 0 - /* - * XXX: Add strlcpy() to Darwin for improved safety. - */ strlcpy(ar->k_ar.ar_arg_login, login, MAXLOGNAME); -#else - strcpy(ar->k_ar.ar_arg_login, login); -#endif - - ar->k_ar.ar_valid_arg |= ARG_LOGIN; + ARG_SET_VALID(ar, ARG_LOGIN); } void @@ -268,7 +260,7 @@ bcopy(name, &ar->k_ar.ar_arg_ctlname, namelen * sizeof(int)); ar->k_ar.ar_arg_len = namelen; - ar->k_ar.ar_valid_arg |= (ARG_CTLNAME | ARG_LEN); + ARG_SET_VALID(ar, ARG_CTLNAME | ARG_LEN); } void @@ -281,7 +273,7 @@ return; ar->k_ar.ar_arg_mask = mask; - ar->k_ar.ar_valid_arg |= ARG_MASK; + ARG_SET_VALID(ar, ARG_MASK); } void @@ -294,7 +286,7 @@ return; ar->k_ar.ar_arg_mode = mode; - ar->k_ar.ar_valid_arg |= ARG_MODE; + ARG_SET_VALID(ar, ARG_MODE); } void @@ -307,7 +299,7 @@ return; ar->k_ar.ar_arg_dev = dev; - ar->k_ar.ar_valid_arg |= ARG_DEV; + ARG_SET_VALID(ar, ARG_DEV); } void @@ -320,7 +312,7 @@ return; ar->k_ar.ar_arg_value = value; - ar->k_ar.ar_valid_arg |= ARG_VALUE; + ARG_SET_VALID(ar, ARG_VALUE); } void @@ -334,7 +326,7 @@ ar->k_ar.ar_arg_uid = uid; ar->k_ar.ar_arg_gid = gid; - ar->k_ar.ar_valid_arg |= (ARG_UID | ARG_GID); + ARG_SET_VALID(ar, ARG_UID | ARG_GID); } void @@ -347,8 +339,7 @@ return; ar->k_ar.ar_arg_pid = pid; - ar->k_ar.ar_valid_arg |= ARG_PID; - + ARG_SET_VALID(ar, ARG_PID); } void @@ -369,11 +360,9 @@ ar->k_ar.ar_arg_ruid = p->p_ucred->cr_ruid; ar->k_ar.ar_arg_rgid = p->p_ucred->cr_rgid; ar->k_ar.ar_arg_asid = p->p_au->ai_asid; - ar->k_ar.ar_arg_termid = p->p_au->ai_termid; - - ar->k_ar.ar_valid_arg |= ARG_AUID | ARG_EUID | ARG_EGID | ARG_RUID | - ARG_RGID | ARG_ASID | ARG_TERMID | ARG_PROCESS; + ARG_SET_VALID(ar, ARG_AUID | ARG_EUID | ARG_EGID | ARG_RUID | + ARG_RGID | ARG_ASID | ARG_TERMID | ARG_PROCESS); } void @@ -386,13 +375,12 @@ return; ar->k_ar.ar_arg_signum = signum; - ar->k_ar.ar_valid_arg |= ARG_SIGNUM; + ARG_SET_VALID(ar, ARG_SIGNUM); } void audit_arg_socket(int sodomain, int sotype, int soprotocol) { - struct kaudit_record *ar; ar = currecord(); @@ -402,7 +390,7 @@ ar->k_ar.ar_arg_sockinfo.so_domain = sodomain; ar->k_ar.ar_arg_sockinfo.so_type = sotype; ar->k_ar.ar_arg_sockinfo.so_protocol = soprotocol; - ar->k_ar.ar_valid_arg |= ARG_SOCKINFO; + ARG_SET_VALID(ar, ARG_SOCKINFO); } /* @@ -421,15 +409,17 @@ bcopy(so, &ar->k_ar.ar_arg_sockaddr, sizeof(ar->k_ar.ar_arg_sockaddr)); switch (so->sa_family) { case AF_INET: - ar->k_ar.ar_valid_arg |= ARG_SADDRINET; + ARG_SET_VALID(ar, ARG_SADDRINET); break; + case AF_INET6: - ar->k_ar.ar_valid_arg |= ARG_SADDRINET6; + ARG_SET_VALID(ar, ARG_SADDRINET6); break; + case AF_UNIX: audit_arg_upath(td, ((struct sockaddr_un *)so)->sun_path, ARG_UPATH1); - ar->k_ar.ar_valid_arg |= ARG_SADDRUNIX; + ARG_SET_VALID(ar, ARG_SADDRUNIX); break; /* XXXAUDIT: default:? */ } @@ -445,7 +435,7 @@ return; ar->k_ar.ar_arg_auid = auid; - ar->k_ar.ar_valid_arg |= ARG_AUID; + ARG_SET_VALID(ar, ARG_AUID); } void @@ -463,7 +453,7 @@ ar->k_ar.ar_arg_amask.am_failure = au_info->ai_mask.am_failure; ar->k_ar.ar_arg_termid.port = au_info->ai_termid.port; ar->k_ar.ar_arg_termid.machine = au_info->ai_termid.machine; - ar->k_ar.ar_valid_arg |= ARG_AUID | ARG_ASID | ARG_AMASK | ARG_TERMID; + ARG_SET_VALID(ar, ARG_AUID | ARG_ASID | ARG_AMASK | ARG_TERMID); } void @@ -475,6 +465,9 @@ if (ar == NULL) return; + /* + * XXXAUDIT: Why do we accept a possibly NULL string here? + */ /* Invalidate the text string */ ar->k_ar.ar_valid_arg &= (ARG_ALL ^ ARG_TEXT); if (text == NULL) @@ -484,7 +477,7 @@ ar->k_ar.ar_arg_text = malloc(MAXPATHLEN, M_AUDIT, M_WAITOK); strncpy(ar->k_ar.ar_arg_text, text, MAXPATHLEN); - ar->k_ar.ar_valid_arg |= ARG_TEXT; + ARG_SET_VALID(ar, ARG_TEXT); } void @@ -497,7 +490,7 @@ return; ar->k_ar.ar_arg_cmd = cmd; - ar->k_ar.ar_valid_arg |= ARG_CMD; + ARG_SET_VALID(ar, ARG_CMD); } void @@ -510,7 +503,7 @@ return; ar->k_ar.ar_arg_svipc_cmd = cmd; - ar->k_ar.ar_valid_arg |= ARG_SVIPC_CMD; + ARG_SET_VALID(ar, ARG_SVIPC_CMD); } void @@ -524,7 +517,7 @@ bcopy(perm, &ar->k_ar.ar_arg_svipc_perm, sizeof(ar->k_ar.ar_arg_svipc_perm)); - ar->k_ar.ar_valid_arg |= ARG_SVIPC_PERM; + ARG_SET_VALID(ar, ARG_SVIPC_PERM); } void @@ -537,7 +530,7 @@ return; ar->k_ar.ar_arg_svipc_id = id; - ar->k_ar.ar_valid_arg |= ARG_SVIPC_ID; + ARG_SET_VALID(ar, ARG_SVIPC_ID); } void @@ -550,7 +543,7 @@ return; ar->k_ar.ar_arg_svipc_addr = addr; - ar->k_ar.ar_valid_arg |= ARG_SVIPC_ADDR; + ARG_SET_VALID(ar, ARG_SVIPC_ADDR); } void @@ -565,7 +558,7 @@ ar->k_ar.ar_arg_pipc_perm.pipc_uid = uid; ar->k_ar.ar_arg_pipc_perm.pipc_gid = gid; ar->k_ar.ar_arg_pipc_perm.pipc_mode = mode; - ar->k_ar.ar_valid_arg |= ARG_POSIX_IPC_PERM; + ARG_SET_VALID(ar, ARG_POSIX_IPC_PERM); } void @@ -579,7 +572,7 @@ bcopy((void *)udata, &ar->k_ar.ar_arg_auditon, sizeof(ar->k_ar.ar_arg_auditon)); - ar->k_ar.ar_valid_arg |= ARG_AUDITON; + ARG_SET_VALID(ar, ARG_AUDITON); } /* @@ -601,6 +594,9 @@ switch (fp->f_type) { case DTYPE_VNODE: case DTYPE_FIFO: + /* + * XXXAUDIT: Only possibly to record as first vnode? + */ vp = fp->f_vnode; vfslocked = VFS_LOCK_GIANT(vp->v_mount); vn_lock(vp, LK_EXCLUSIVE | LK_RETRY, curthread); @@ -636,7 +632,7 @@ pcb->inp_fport; ar->k_ar.ar_arg_sockinfo.so_lport = pcb->inp_lport; - ar->k_ar.ar_valid_arg |= ARG_SOCKINFO; + ARG_SET_VALID(ar, ARG_SOCKINFO); } break; @@ -656,7 +652,7 @@ * XXXAUDIT: Possibly assert that the memory isn't already allocated? */ void -audit_arg_upath(struct thread *td, char *upath, u_int64_t flags) +audit_arg_upath(struct thread *td, char *upath, u_int64_t flag) { struct kaudit_record *ar; char **pathp; @@ -667,33 +663,26 @@ /* * XXXAUDIT: Witness warning for possible sleep here? */ + KASSERT((flag == ARG_UPATH1) || (flag == ARG_UPATH2), + ("audit_arg_upath: flag %llu", flag)); + KASSERT((flag != ARG_UPATH1) || (flag != ARG_UPATH2), + ("audit_arg_upath: flag %llu", flag)); - /* - * XXXAUDIT: KASSERT argument validity? - */ - if ((flags & (ARG_UPATH1 | ARG_UPATH2)) == 0) - return; - ar = currecord(); if (ar == NULL) return; - if (flags & ARG_UPATH1) { - ar->k_ar.ar_valid_arg &= (ARG_ALL ^ ARG_UPATH1); + if (flag == ARG_UPATH1) pathp = &ar->k_ar.ar_arg_upath1; - } else { - ar->k_ar.ar_valid_arg &= (ARG_ALL ^ ARG_UPATH2); + else pathp = &ar->k_ar.ar_arg_upath2; - } if (*pathp == NULL) *pathp = malloc(MAXPATHLEN, M_AUDIT, M_WAITOK); canon_path(td, upath, *pathp); - if (flags & ARG_UPATH1) - ar->k_ar.ar_valid_arg |= ARG_UPATH1; - else - ar->k_ar.ar_valid_arg |= ARG_UPATH2; + + ARG_SET_VALID(ar, flag); } /* @@ -741,6 +730,10 @@ /* * XXXAUDIT: KASSERT argument validity instead? + * + * XXXAUDIT: The below clears, and then resets the flags for valid + * arguments. Ideally, either the new path is used, or the old one + * would be. */ if ((flags & (ARG_VNODE1 | ARG_VNODE2)) == 0) return; @@ -796,10 +789,9 @@ vnp->vn_fileid = vattr.va_fileid; vnp->vn_gen = vattr.va_gen; if (flags & ARG_VNODE1) - ar->k_ar.ar_valid_arg |= ARG_VNODE1; + ARG_SET_VALID(ar, ARG_VNODE1); else - ar->k_ar.ar_valid_arg |= ARG_VNODE2; - + ARG_SET_VALID(ar, ARG_VNODE2); } /* ==== //depot/projects/trustedbsd/audit3/sys/security/audit/audit_private.h#9 (text+ko) ==== @@ -188,6 +188,16 @@ }; /* + * Arguments in the audit record are initially not defined; flags are set to + * indicate if they are present so they can be included in the audit log + * stream only if defined. + */ +#define ARG_IS_VALID(kar, arg) ((kar)->k_ar.ar_valid_arg & (arg)) +#define ARG_SET_VALID(kar, arg) do { \ + (kar)->k_ar.ar_valid_arg |= (arg); \ +} while (0) + +/* * In-kernel version of audit record; the basic record plus queue meta-data. * This record can also have a pointer set to some opaque data that will * be passed through to the audit writing mechanism.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200510181234.j9ICYgnZ067553>