Re: [PATCH v9 3/8] IMA: define a hook to measure kernel integrity critical data

[Tushar Sugandhi]

On 2021-01-05 12:16 p.m., Mimi Zohar wrote:
> On Tue, 2021-01-05 at 12:01 -0800, Tushar Sugandhi wrote:
> > > > data. However, various data structures, policies, and states
> > >
> > > Here and everywhere else, there are two blanks after a period.
> > I checked this patch file in multiple text editors, but couldn’t find
> > any instance of period followed by two spaces. I will double check again
> > all the patches for multiple spaces, and remove them if any.
>
> There should be two blanks after a period, not one blank.
>
> <snip>
>
> > > > + *
> > > > + * Measure the kernel subsystem data, critical to the integrity of the kernel,
> > > > + * into the IMA log and extend the @pcr.
> > > > + *
> > > > + * Use @event_name to describe the state/buffer data change.
> > > > + * Examples of critical data (@buf) could be various data structures,
> > > > + * policies, and states stored in kernel memory that can impact the integrity
> > > > + * of the system.
> > > > + *
> > > > + * If @measure_buf_hash is set to true - measure hash of the buffer data,
> > > > + * else measure the buffer data itself.
> > > > + * @measure_buf_hash can be used to save space, if the data being measured
> > > > + * is too large.
> > > > + *
> > > > + * The data (@buf) can only be measured, not appraised.
> > >
> > > The "/**" is the start of kernel-doc.  Have you seen anywhere else in
> > My impression was the hooks in ima_main.c e.g. ima_file_free()
> > ima_file_mmap() required the double-asterisk ("/**"), and internal
> > functions like ima_rdwr_violation_check() require a single-asterisk
> > ("/*")
> >
> > kernel-doc.rst suggest the double-asterisk ("/**") for function comment
> > as well.
> >
> > Function documentation
> > ----------------------
> >
> > The general format of a function and function-like macro kernel-doc
> > comment is::
> >
> >     /**
> >      * function_name() - Brief description of function.
> >
> > Please let me know if you still want me to remove the double-asterisk
> > ("/**") here.
>
> Yes, of course this needs to be kernel-doc and requires "/**"
Thanks for confirming.
> > > the kernel using the @<variable name> in the longer function
> > > description?  Have you seen this style of longer   function
> > > description?  Refer to Documentation/doc-guide/kernel-doc.rst and other
> > > code for examples.
> > Thanks. I will remove the prefix "@" from <variable name> in the longer
> > function description.
>
> Removing the @<variable name> isn't sufficient.  Please look at other
> examples of longer function definitions before reposting.
Yes. Agreed. I will go as per the guidance in kernel-doc.rst

Thanks again,
Tushar


> thanks,
>
> Mimi