| Message ID | 87wp10dlgk.fsf@notabene.neil.brown.name (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Jan 03, 2018 at 08:01:15AM +1100, NeilBrown wrote: > > +When replacing the group list, the new list must be sorted before it > +is added to the credential, as a binary search is used to test for > +membership. In practice, this means ``groups_sort()`` should be For a .rst file, shouldn't we be using :c:func:`groups_sort` instead of ``groups_sort()``? > +called before ``set_groups()`` or ``set_current_groups()``. > +``groups_sort()`` must not be called on a ``struct group_list`` which > +is shared as it may permute elements as part of the sorting process > +even if the array is already sorted. > > When the credential set is ready, it should be committed to the current process > by calling:: > -- > 2.14.0.rc0.dirty >
On Tue, 2 Jan 2018 13:04:31 -0800 Matthew Wilcox <willy@infradead.org> wrote: > > +When replacing the group list, the new list must be sorted before it > > +is added to the credential, as a binary search is used to test for > > +membership. In practice, this means ``groups_sort()`` should be > > For a .rst file, shouldn't we be using :c:func:`groups_sort` instead of > ``groups_sort()``? 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. Thanks, jon
On Sat, Jan 06, 2018 at 11:09:08AM -0700, Jonathan Corbet wrote: > On Tue, 2 Jan 2018 13:04:31 -0800 > Matthew Wilcox <willy@infradead.org> wrote: > > > > +When replacing the group list, the new list must be sorted before it > > > +is added to the credential, as a binary search is used to test for > > > +membership. In practice, this means ``groups_sort()`` should be > > > > For a .rst file, shouldn't we be using :c:func:`groups_sort` instead of > > ``groups_sort()``? > > 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. Thanks for making that substitution. I've been thinking about all the kernel-doc we have that's completely unincorporated. I've also been thinking about core-api/kernel-api.rst which to my mind is completely unreadable in its current form -- look at https://www.kernel.org/doc/html/latest/core-api/kernel-api.html and you wouldn't really know there's anything in it beyond the List Management Functions. I think the right path forward is to have kernel-api.rst be the dumping ground for all the files with kernel-doc but nothing more. That gives us somewhere to link to. Then we need little stories about how all the functions in a subsystem fit together. For example, we can create a list.rst which explains how this is a doubly-linked list that you use by embedding a list_head into your data structure, and has O(1) insertion/deletion, etc, etc. Then we would move all the list.h kernel-doc from kernel-api.rst into list.rst. Is this a reasonable strategy to follow? Does anyone have a better strategy? I mean ... you've written a book, you presumably have some idea about how to present the vast amount of information we've accumulated over the years :-)
On 01/06/18 12:20, Matthew Wilcox wrote: > > I've been thinking about all the kernel-doc we have that's completely > unincorporated. I've also been thinking about core-api/kernel-api.rst > which to my mind is completely unreadable in its current form -- look at > https://www.kernel.org/doc/html/latest/core-api/kernel-api.html and you > wouldn't really know there's anything in it beyond the List Management > Functions. The index is on the left side, but would be better (duplicated?) at the beginning of the chapter. The left side is still useful for navigation, but then it scrolls away too quickly when the right side text is scrolled. > I think the right path forward is to have kernel-api.rst be the dumping > ground for all the files with kernel-doc but nothing more. That gives > us somewhere to link to. FWFW, I have recently done firewire.rst, infiniband.rst, and some additions to scsi.rst. But the new firewire.rst and infiniband.rst could use some introductory material before just jumping into the API. > Then we need little stories about how all the functions in a subsystem > fit together. For example, we can create a list.rst which explains how > this is a doubly-linked list that you use by embedding a list_head into > your data structure, and has O(1) insertion/deletion, etc, etc. Then we > would move all the list.h kernel-doc from kernel-api.rst into list.rst.
On Sat, 6 Jan 2018 12:20:13 -0800 Matthew Wilcox <willy@infradead.org> wrote: > I've been thinking about all the kernel-doc we have that's completely > unincorporated. I've also been thinking about core-api/kernel-api.rst > which to my mind is completely unreadable in its current form -- look at > https://www.kernel.org/doc/html/latest/core-api/kernel-api.html and you > wouldn't really know there's anything in it beyond the List Management > Functions. Yes, it's a mess. > I think the right path forward is to have kernel-api.rst be the dumping > ground for all the files with kernel-doc but nothing more. That gives > us somewhere to link to. That's kind of what it is now :) Note that we have a script now (scripts/find-unused-docs.sh) that can seek out kerneldoc comments that aren't yet pulled in to the RST document tree. > Then we need little stories about how all the functions in a subsystem > fit together. For example, we can create a list.rst which explains how > this is a doubly-linked list that you use by embedding a list_head into > your data structure, and has O(1) insertion/deletion, etc, etc. Then we > would move all the list.h kernel-doc from kernel-api.rst into list.rst. > > Is this a reasonable strategy to follow? Does anyone have a better > strategy? That more-or-less corresponds to what I've been aiming at. All of Documentation/ currently is a bit of a dumping ground; I'd really like to knit it together into something coherent and useful. Progress on that front has been slower than I would have liked - I've been distracted by a number of other things. Funny, I've never heard of that happening before. The "little stories", incidentally, can also go into DOC: sections in the source itself. There's a bit of tension, though, between doing that and putting the stories in the RST files. The former approach puts the docs where they might be most useful and most likely to be updated, but it can also leave the RST files as a collection of kerneldoc directives that is rather unhelpful to read in unprocessed form. Since the whole idea is to not require any sort of processing to make the docs useful, I don't entirely like that outcome. Thanks, jon
diff --git a/Documentation/security/credentials.rst b/Documentation/security/credentials.rst index 66a2e24939d8..5d659e3e52ad 100644 --- a/Documentation/security/credentials.rst +++ b/Documentation/security/credentials.rst @@ -451,6 +451,13 @@ checks and hooks done. Both the current and the proposed sets of credentials are available for this purpose as current_cred() will return the current set still at this point. +When replacing the group list, the new list must be sorted before it +is added to the credential, as a binary search is used to test for +membership. In practice, this means ``groups_sort()`` should be +called before ``set_groups()`` or ``set_current_groups()``. +``groups_sort()`` must not be called on a ``struct group_list`` which +is shared as it may permute elements as part of the sorting process +even if the array is already sorted. When the credential set is ready, it should be committed to the current process by calling::
This patch updates the documentation with the observations that led to commit bdcf0a423ea1 ("kernel: make groups_sort calling a responsibility group_info allocators") and the new behaviour required. Specifically that groups_sort() should be called on a new group_list before set_groups() or set_current_groups() is called. Signed-off-by: NeilBrown <neilb@suse.com> --- Documentation/security/credentials.rst | 7 +++++++ 1 file changed, 7 insertions(+)