lkml.org 
[lkml]   [2018]   [Jan]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
Date
SubjectRe: [PATCH] Documentation: security/credentials.rst: explain need to sort group_list
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 <neilb@suse.com>
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 <neilb@suse.com>
---
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)
{
--
2.14.0.rc0.dirty
[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2018-01-14 23:16    [W:0.122 / U:0.692 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site