From patchwork Sun Jan 7 23:39:14 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: NeilBrown X-Patchwork-Id: 10148531 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id B63A6601A1 for ; Sun, 7 Jan 2018 23:39:43 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id AAF2728606 for ; Sun, 7 Jan 2018 23:39:43 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 9EA54287E9; Sun, 7 Jan 2018 23:39:43 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=unavailable version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id C1DDC28606 for ; Sun, 7 Jan 2018 23:39:42 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754788AbeAGXj2 (ORCPT ); Sun, 7 Jan 2018 18:39:28 -0500 Received: from mx2.suse.de ([195.135.220.15]:44774 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754649AbeAGXj1 (ORCPT ); Sun, 7 Jan 2018 18:39:27 -0500 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay1.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id 51810AAC1; Sun, 7 Jan 2018 23:39:25 +0000 (UTC) From: NeilBrown To: Jonathan Corbet , Matthew Wilcox Date: Mon, 08 Jan 2018 10:39:14 +1100 Cc: linux-doc@vger.kernel.org, dhowells@redhat.com, Thiago Rafael Becker , viro@zeniv.linux.org.uk, schwidefsky@de.ibm.com, bfields@fieldses.org, linux-nfs@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] Documentation: security/credentials.rst: explain need to sort group_list In-Reply-To: <20180106110908.0adc1be2@lwn.net> References: <878te9os81.fsf@notabene.neil.brown.name> <20171211142708.GA23284@bombadil.infradead.org> <20171211151420.18655-1-thiago.becker@gmail.com> <20742.1514904840@warthog.procyon.org.uk> <87wp10dlgk.fsf@notabene.neil.brown.name> <20180102210431.GA20405@bombadil.infradead.org> <20180106110908.0adc1be2@lwn.net> Message-ID: <87608dck7x.fsf@notabene.neil.brown.name> MIME-Version: 1.0 Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Sat, Jan 06 2018, Jonathan Corbet wrote: > There is value in using the c:func syntax, as it will generate > cross-references to the kerneldoc comments for those functions. In this > case, it would appear that these comments exist, but nobody has pulled > them into the docs yet. I took the liberty of adding :c:func: references > to this patch on its way into docs-next so that things will Just Work on > that happy day when somebody adds the function documentation. Is this the sort of thing that might result in that happy day? I didn't spend tooo much time on it, in case including the kernel-doc in credentials.rst like this was the wrong direction. Thanks, NeilBrown ------------------------------8<------------------------- From: NeilBrown Subject: [PATCH] Documentation: include kernel-doc in credentials.rst - kernel-doc from include/linux/cred.h, kernel/cred.c, and kernel/group.c is included in credentials.rst. - Various function references in credentials.rst are changed to link to this documentation. - Various minor improvements to the documentation. Signed-off-by: NeilBrown --- Documentation/security/credentials.rst | 55 ++++++++++++++++++---------------- kernel/groups.c | 19 +++++++++++- 2 files changed, 47 insertions(+), 27 deletions(-) diff --git a/Documentation/security/credentials.rst b/Documentation/security/credentials.rst index 66a2e24939d8..fd1b7d3b2a78 100644 --- a/Documentation/security/credentials.rst +++ b/Documentation/security/credentials.rst @@ -296,7 +296,7 @@ for example), it must be considered immutable, barring two exceptions: To catch accidental credential alteration at compile time, struct task_struct has _const_ pointers to its credential sets, as does struct file. Furthermore, -certain functions such as ``get_cred()`` and ``put_cred()`` operate on const +certain functions such as :c:func:`get_cred` and :c:func:`put_cred` operate on const pointers, thus rendering casts unnecessary, but require to temporarily ditch the const qualification to be able to alter the reference count. @@ -391,8 +391,8 @@ This does all the RCU magic inside of it. The caller must call put_cred() on the credentials so obtained when they're finished with. .. note:: - The result of ``__task_cred()`` should not be passed directly to - ``get_cred()`` as this may race with ``commit_cred()``. + The result of :c:func:`__task_cred()` should not be passed directly to + :c:func:`get_cred` as this may race with :c:func:`commit_cred`. There are a couple of convenience functions to access bits of another task's credentials, hiding the RCU magic from the caller:: @@ -406,7 +406,7 @@ If the caller is holding the RCU read lock at the time anyway, then:: __task_cred(task)->euid should be used instead. Similarly, if multiple aspects of a task's credentials -need to be accessed, RCU read lock should be used, ``__task_cred()`` called, +need to be accessed, RCU read lock should be used, :c:func:`__task_cred` called, the result stored in a temporary pointer and then the credential aspects called from that before dropping the lock. This prevents the potentially expensive RCU magic from being invoked multiple times. @@ -433,11 +433,8 @@ alter those of another task. This means that it doesn't need to use any locking to alter its own credentials. To alter the current process's credentials, a function should first prepare a -new set of credentials by calling:: - - struct cred *prepare_creds(void); - -this locks current->cred_replace_mutex and then allocates and constructs a +new set of credentials by calling :c:func:`prepare_creds`. +This locks ``current->cred_replace_mutex`` and then allocates and constructs a duplicate of the current process's credentials, returning with the mutex still held if successful. It returns NULL if not successful (out of memory). @@ -453,10 +450,7 @@ still at this point. When the credential set is ready, it should be committed to the current process -by calling:: - - int commit_creds(struct cred *new); - +by calling :c:func:`commit_creds`. This will alter various aspects of the credentials and the process, giving the LSM a chance to do likewise, then it will use ``rcu_assign_pointer()`` to actually commit the new credentials to ``current->cred``, it will release @@ -467,20 +461,17 @@ This function is guaranteed to return 0, so that it can be tail-called at the end of such functions as ``sys_setresuid()``. Note that this function consumes the caller's reference to the new credentials. -The caller should _not_ call ``put_cred()`` on the new credentials afterwards. +The caller should _not_ call :c:func:`put_cred` on the new credentials afterwards. Furthermore, once this function has been called on a new set of credentials, those credentials may _not_ be changed further. Should the security checks fail or some other error occur after -``prepare_creds()`` has been called, then the following function should be -invoked:: - - void abort_creds(struct cred *new); - +:c:func:`prepare_creds` has been called, then the function +:c:func:`abort_creds` should be invoked. This releases the lock on ``current->cred_replace_mutex`` that -``prepare_creds()`` got and then releases the new credentials. +:c:func:`prepare_creds` got and then releases the new credentials. A typical credentials alteration function would look something like this:: @@ -512,19 +503,20 @@ There are some functions to help manage credentials: - ``void put_cred(const struct cred *cred);`` - This releases a reference to the given set of credentials. If the + :c:func:`put_cred` releases a reference to the given set of credentials. If the reference count reaches zero, the credentials will be scheduled for destruction by the RCU system. - ``const struct cred *get_cred(const struct cred *cred);`` - This gets a reference on a live set of credentials, returning a pointer to - that set of credentials. + :c:func:`get_cred` gets a reference on a live set of credentials, + returning a pointer to that set of credentials. - ``struct cred *get_new_cred(struct cred *cred);`` - This gets a reference on a set of credentials that is under construction - and is thus still mutable, returning a pointer to that set of credentials. + :c:func:`get_new_cred` gets a reference on a set of credentials + that is under construction and is thus still mutable, returning a + pointer to that set of credentials. Open File Credentials @@ -546,9 +538,20 @@ Overriding the VFS's Use of Credentials ======================================= Under some circumstances it is desirable to override the credentials used by -the VFS, and that can be done by calling into such as ``vfs_mkdir()`` with a +the VFS, and that can be done by calling into such as :c:func:`vfs_mkdir` with a different set of credentials. This is done in the following places: * ``sys_faccessat()``. * ``do_coredump()``. * nfs4recover.c. + +List of functions for managing credentials +========================================== + +.. kernel-doc:: include/linux/cred.h + +.. kernel-doc:: kernel/cred.c + :export: + +.. kernel-doc:: kernel/groups.c + :export: diff --git a/kernel/groups.c b/kernel/groups.c index daae2f2dc6d4..dddbb52f9035 100644 --- a/kernel/groups.c +++ b/kernel/groups.c @@ -86,6 +86,19 @@ static int gid_cmp(const void *_a, const void *_b) return gid_gt(a, b) - gid_lt(a, b); } +/** + * groups_sort - sort supplementary group list numerically + * @group_info: The group list + * + * groups_search() uses a binary search to see if a + * given gid is a member of a group list, so the list must be + * sorted. This can be achieved by calling groups_sort(). + * As a &struct group_info is often shared, and as this function + * can temporary permute elements even of a sorted list, + * groups_sort() must be called *before* a @struct group_info + * is added to a credential, typically using set_groups() or + * set_current_groups(). + */ void groups_sort(struct group_info *group_info) { sort(group_info->gid, group_info->ngroups, sizeof(*group_info->gid), @@ -119,6 +132,10 @@ int groups_search(const struct group_info *group_info, kgid_t grp) * set_groups - Change a group subscription in a set of credentials * @new: The newly prepared set of credentials to alter * @group_info: The group list to install + * + * The group list must already be sorted (by groups_sort()) + * and the credential must not be in active use. To change the + * groups list of an active credential, use set_current_groups(). */ void set_groups(struct cred *new, struct group_info *group_info) { @@ -134,7 +151,7 @@ EXPORT_SYMBOL(set_groups); * @group_info: The group list to impose * * Validate a group subscription and, if valid, impose it upon current's task - * security record. + * security record. The group list must already be sorted. */ int set_current_groups(struct group_info *group_info) {