Message-ID: <CAGXu5jJY2pATLcW6Mc9iigSgnayR-j0vH0Hs=uLsHKS+V6=7Kw@mail.gmail.com>
Date:   Mon, 15 May 2017 07:42:00 -0700
From:   Kees Cook <keescook@...omium.org>
To:     Casey Schaufler <casey@...aufler-ca.com>
Cc:     Jonathan Corbet <corbet@....net>,
        John Johansen <john.johansen@...onical.com>,
        Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>,
        Paul Moore <paul@...l-moore.com>,
        David Howells <dhowells@...hat.com>,
        Mimi Zohar <zohar@...ux.vnet.ibm.com>,
        James Morris <james.l.morris@...cle.com>,
        Tyler Hicks <tyhicks@...onical.com>,
        David Safford <safford@...ibm.com>,
        "linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
        linux-security-module <linux-security-module@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 06/17] doc: security: minor cleanups to build kernel-doc

On Sun, May 14, 2017 at 5:00 PM, Casey Schaufler <casey@...aufler-ca.com> wrote:
> On 5/13/2017 4:51 AM, Kees Cook wrote:
>> These fixes were needed to parse lsm_hooks.h kernel-doc. More work is
>> needed, but this is the first step.
>>
>> Cc: Casey Schaufler <casey@...aufler-ca.com>
>> Signed-off-by: Kees Cook <keescook@...omium.org>
>
> Acked_by: Casey Schaufler <casey@...aufler-ca.com>
>
> Tell me more about the additional work that's needed.

What I wanted to do was insert the kernel-doc from lsm_hooks.h into
the LSM kernel API documentation .rst file (via the special ReST
markup that includes structure documentation). There is, however,
free-form text in the existing union security_list_options kernel-doc
to announce related function groups which ReST just kind of skips over
and the collects all at the end in the HTML output. It also orders the
HTML doc output by the struct ordering, which makes things even
stranger to parse. And additionally, kernel-doc for fields that are
function pointers is especially hard to read.

So, while this patch fixes the kernel-doc to at least be parsed
without errors, it doesn't really fix the overall appearance, which
I'm not sure how to fix yet. It might be possible to do per-struct
"/** DOC:" markup, but I decided to leave that for another pass in the
future.

If there is a v2 of this series, I'll update the changelog to include
these details. :)

-Kees

>> ---
>>  include/linux/lsm_hooks.h | 25 ++++++++++++-------------
>>  1 file changed, 12 insertions(+), 13 deletions(-)
>>
>> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
>> index 080f34e66017..a1eeaf603d2f 100644
>> --- a/include/linux/lsm_hooks.h
>> +++ b/include/linux/lsm_hooks.h
>> @@ -29,6 +29,8 @@
>>  #include <linux/rculist.h>
>>
>>  /**
>> + * union security_list_options - Linux Security Module hook function list
>> + *
>>   * Security hooks for program execution operations.
>>   *
>>   * @bprm_set_creds:
>> @@ -193,8 +195,8 @@
>>   *   @value will be set to the allocated attribute value.
>>   *   @len will be set to the length of the value.
>>   *   Returns 0 if @name and @value have been successfully set,
>> - *           -EOPNOTSUPP if no security attribute is needed, or
>> - *           -ENOMEM on memory allocation failure.
>> + *   -EOPNOTSUPP if no security attribute is needed, or
>> + *   -ENOMEM on memory allocation failure.
>>   * @inode_create:
>>   *   Check permission to create a regular file.
>>   *   @dir contains inode structure of the parent of the new file.
>> @@ -510,8 +512,7 @@
>>   *   process @tsk.  Note that this hook is sometimes called from interrupt.
>>   *   Note that the fown_struct, @fown, is never outside the context of a
>>   *   struct file, so the file structure (and associated security information)
>> - *   can always be obtained:
>> - *           container_of(fown, struct file, f_owner)
>> + *   can always be obtained: container_of(fown, struct file, f_owner)
>>   *   @tsk contains the structure of task receiving signal.
>>   *   @fown contains the file owner information.
>>   *   @sig is the signal that will be sent.  When 0, kernel sends SIGIO.
>> @@ -521,7 +522,7 @@
>>   *   to receive an open file descriptor via socket IPC.
>>   *   @file contains the file structure being received.
>>   *   Return 0 if permission is granted.
>> - * @file_open
>> + * @file_open:
>>   *   Save open-time permission checking state for later use upon
>>   *   file_permission, and recheck access if anything has changed
>>   *   since inode_permission.
>> @@ -1143,7 +1144,7 @@
>>   *   @sma contains the semaphore structure.  May be NULL.
>>   *   @cmd contains the operation to be performed.
>>   *   Return 0 if permission is granted.
>> - * @sem_semop
>> + * @sem_semop:
>>   *   Check permissions before performing operations on members of the
>>   *   semaphore set @sma.  If the @alter flag is nonzero, the semaphore set
>>   *   may be modified.
>> @@ -1153,20 +1154,20 @@
>>   *   @alter contains the flag indicating whether changes are to be made.
>>   *   Return 0 if permission is granted.
>>   *
>> - * @binder_set_context_mgr
>> + * @binder_set_context_mgr:
>>   *   Check whether @mgr is allowed to be the binder context manager.
>>   *   @mgr contains the task_struct for the task being registered.
>>   *   Return 0 if permission is granted.
>> - * @binder_transaction
>> + * @binder_transaction:
>>   *   Check whether @from is allowed to invoke a binder transaction call
>>   *   to @to.
>>   *   @from contains the task_struct for the sending task.
>>   *   @to contains the task_struct for the receiving task.
>> - * @binder_transfer_binder
>> + * @binder_transfer_binder:
>>   *   Check whether @from is allowed to transfer a binder reference to @to.
>>   *   @from contains the task_struct for the sending task.
>>   *   @to contains the task_struct for the receiving task.
>> - * @binder_transfer_file
>> + * @binder_transfer_file:
>>   *   Check whether @from is allowed to transfer @file to @to.
>>   *   @from contains the task_struct for the sending task.
>>   *   @file contains the struct file being transferred.
>> @@ -1214,7 +1215,7 @@
>>   *   @cred contains the credentials to use.
>>   *   @ns contains the user namespace we want the capability in
>>   *   @cap contains the capability <include/linux/capability.h>.
>> - *   @audit: Whether to write an audit message or not
>> + *   @audit contains whether to write an audit message or not
>>   *   Return 0 if the capability is granted for @tsk.
>>   * @syslog:
>>   *   Check permission before accessing the kernel message ring or changing
>> @@ -1336,9 +1337,7 @@
>>   *   @inode we wish to get the security context of.
>>   *   @ctx is a pointer in which to place the allocated security context.
>>   *   @ctxlen points to the place to put the length of @ctx.
>> - * This is the main security structure.
>>   */
>> -
>>  union security_list_options {
>>       int (*binder_set_context_mgr)(struct task_struct *mgr);
>>       int (*binder_transaction)(struct task_struct *from,
>



-- 
Kees Cook
Pixel Security

Powered by blists - more mailing lists