From 82e5d8cc768b0c7b03c551a9ab1f8f3f68d5f83f Mon Sep 17 00:00:00 2001 From: Arnd Bergmann Date: Mon, 22 Mar 2021 17:02:41 +0100 Subject: [PATCH 1/2] security: commoncap: fix -Wstringop-overread warning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit gcc-11 introdces a harmless warning for cap_inode_getsecurity: security/commoncap.c: In function ‘cap_inode_getsecurity’: security/commoncap.c:440:33: error: ‘memcpy’ reading 16 bytes from a region of size 0 [-Werror=stringop-overread] 440 | memcpy(&nscap->data, &cap->data, sizeof(__le32) * 2 * VFS_CAP_U32); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ The problem here is that tmpbuf is initialized to NULL, so gcc assumes it is not accessible unless it gets set by vfs_getxattr_alloc(). This is a legitimate warning as far as I can tell, but the code is correct since it correctly handles the error when that function fails. Add a separate NULL check to tell gcc about it as well. Signed-off-by: Arnd Bergmann Acked-by: Christian Brauner Signed-off-by: James Morris --- security/commoncap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/security/commoncap.c b/security/commoncap.c index 26c1cb725dcb..2bdeacd32e3f 100644 --- a/security/commoncap.c +++ b/security/commoncap.c @@ -391,7 +391,7 @@ int cap_inode_getsecurity(struct inode *inode, const char *name, void **buffer, &tmpbuf, size, GFP_NOFS); dput(dentry); - if (ret < 0) + if (ret < 0 || !tmpbuf) return ret; fs_ns = inode->i_sb->s_user_ns; From 049ae601f3fb3d5b1c1efdb434499770c96237f6 Mon Sep 17 00:00:00 2001 From: Randy Dunlap Date: Sun, 11 Apr 2021 17:55:28 -0700 Subject: [PATCH 2/2] security: commoncap: clean up kernel-doc comments Fix kernel-doc notation in commoncap.c. Use correct (matching) function name in comments as in code. Use correct function argument names in kernel-doc comments. Use kernel-doc's "Return:" format for function return values. Fixes these kernel-doc warnings: ../security/commoncap.c:1206: warning: expecting prototype for cap_task_ioprio(). Prototype was for cap_task_setioprio() instead ../security/commoncap.c:1219: warning: expecting prototype for cap_task_ioprio(). Prototype was for cap_task_setnice() instead Signed-off-by: Randy Dunlap Reviewed-by: Serge Hallyn Signed-off-by: James Morris --- security/commoncap.c | 50 +++++++++++++++++++++++++++++--------------- 1 file changed, 33 insertions(+), 17 deletions(-) diff --git a/security/commoncap.c b/security/commoncap.c index 2bdeacd32e3f..b088bf002db5 100644 --- a/security/commoncap.c +++ b/security/commoncap.c @@ -50,7 +50,7 @@ static void warn_setuid_and_fcaps_mixed(const char *fname) /** * cap_capable - Determine whether a task has a particular effective capability * @cred: The credentials to use - * @ns: The user namespace in which we need the capability + * @targ_ns: The user namespace in which we need the capability * @cap: The capability to check for * @opts: Bitmask of options defined in include/linux/security.h * @@ -289,7 +289,7 @@ int cap_capset(struct cred *new, * affects the security markings on that inode, and if it is, should * inode_killpriv() be invoked or the change rejected. * - * Returns 1 if security.capability has a value, meaning inode_killpriv() + * Return: 1 if security.capability has a value, meaning inode_killpriv() * is required, 0 otherwise, meaning inode_killpriv() is not required. */ int cap_inode_need_killpriv(struct dentry *dentry) @@ -307,7 +307,7 @@ int cap_inode_need_killpriv(struct dentry *dentry) * * Erase the privilege-enhancing security markings on an inode. * - * Returns 0 if successful, -ve on error. + * Return: 0 if successful, -ve on error. */ int cap_inode_killpriv(struct dentry *dentry) { @@ -490,7 +490,7 @@ static bool validheader(size_t size, const struct vfs_cap_data *cap) * User requested a write of security.capability. If needed, update the * xattr to change from v2 to v3, or to fixup the v3 rootid. * - * If all is ok, we return the new size, on error return < 0. + * Return: On success, return the new size; on error, return < 0. */ int cap_convert_nscap(struct dentry *dentry, const void **ivalue, size_t size) { @@ -822,7 +822,9 @@ static inline bool nonroot_raised_pE(struct cred *new, const struct cred *old, * * Set up the proposed credentials for a new execution context being * constructed by execve(). The proposed creds in @bprm->cred is altered, - * which won't take effect immediately. Returns 0 if successful, -ve on error. + * which won't take effect immediately. + * + * Return: 0 if successful, -ve on error. */ int cap_bprm_creds_from_file(struct linux_binprm *bprm, struct file *file) { @@ -1049,7 +1051,9 @@ static inline void cap_emulate_setxuid(struct cred *new, const struct cred *old) * @flags: Indications of what has changed * * Fix up the results of setuid() call before the credential changes are - * actually applied, returning 0 to grant the changes, -ve to deny them. + * actually applied. + * + * Return: 0 to grant the changes, -ve to deny them. */ int cap_task_fix_setuid(struct cred *new, const struct cred *old, int flags) { @@ -1119,7 +1123,9 @@ static int cap_safe_nice(struct task_struct *p) * @p: The task to affect * * Detemine if the requested scheduler policy change is permitted for the - * specified task, returning 0 if permission is granted, -ve if denied. + * specified task. + * + * Return: 0 if permission is granted, -ve if denied. */ int cap_task_setscheduler(struct task_struct *p) { @@ -1127,12 +1133,14 @@ int cap_task_setscheduler(struct task_struct *p) } /** - * cap_task_ioprio - Detemine if I/O priority change is permitted + * cap_task_setioprio - Detemine if I/O priority change is permitted * @p: The task to affect * @ioprio: The I/O priority to set * * Detemine if the requested I/O priority change is permitted for the specified - * task, returning 0 if permission is granted, -ve if denied. + * task. + * + * Return: 0 if permission is granted, -ve if denied. */ int cap_task_setioprio(struct task_struct *p, int ioprio) { @@ -1140,12 +1148,14 @@ int cap_task_setioprio(struct task_struct *p, int ioprio) } /** - * cap_task_ioprio - Detemine if task priority change is permitted + * cap_task_setnice - Detemine if task priority change is permitted * @p: The task to affect * @nice: The nice value to set * * Detemine if the requested task priority change is permitted for the - * specified task, returning 0 if permission is granted, -ve if denied. + * specified task. + * + * Return: 0 if permission is granted, -ve if denied. */ int cap_task_setnice(struct task_struct *p, int nice) { @@ -1175,12 +1185,15 @@ static int cap_prctl_drop(unsigned long cap) /** * cap_task_prctl - Implement process control functions for this security module * @option: The process control function requested - * @arg2, @arg3, @arg4, @arg5: The argument data for this function + * @arg2: The argument data for this function + * @arg3: The argument data for this function + * @arg4: The argument data for this function + * @arg5: The argument data for this function * * Allow process control functions (sys_prctl()) to alter capabilities; may * also deny access to other functions not otherwise implemented here. * - * Returns 0 or +ve on success, -ENOSYS if this function is not implemented + * Return: 0 or +ve on success, -ENOSYS if this function is not implemented * here, other -ve on error. If -ENOSYS is returned, sys_prctl() and other LSM * modules will consider performing the function. */ @@ -1315,7 +1328,9 @@ int cap_task_prctl(int option, unsigned long arg2, unsigned long arg3, * @pages: The size of the mapping * * Determine whether the allocation of a new virtual mapping by the current - * task is permitted, returning 1 if permission is granted, 0 if not. + * task is permitted. + * + * Return: 1 if permission is granted, 0 if not. */ int cap_vm_enough_memory(struct mm_struct *mm, long pages) { @@ -1328,14 +1343,15 @@ int cap_vm_enough_memory(struct mm_struct *mm, long pages) return cap_sys_admin; } -/* +/** * cap_mmap_addr - check if able to map given addr * @addr: address attempting to be mapped * * If the process is attempting to map memory below dac_mmap_min_addr they need * CAP_SYS_RAWIO. The other parameters to this function are unused by the - * capability security module. Returns 0 if this mapping should be allowed - * -EPERM if not. + * capability security module. + * + * Return: 0 if this mapping should be allowed or -EPERM if not. */ int cap_mmap_addr(unsigned long addr) {