Re: Patch to the context lookup change

Re: Patch to the context lookup change

[Stephen Smalley]

On 3/11/19 3:20 PM, Tianjie Xu wrote:
> Hi there,
> I recently made a change in android's selinux repo. The change 
> implements a new lookup helper function for context matches, and it aims 
> to skip unnecessary restorecon if the corresponding context doesn't 
> change. (Wasn't able to attach the URL to the CL, otherwise it's blocked 
> by spam detector).
> 
> Though I haven't done the work to implement this change selinux 
> upstream. I did a local merge and the code compiles. So I wonder if it's 
> possible to merge in this change to the lookup functions first? It will 
> make the upstream & android repo more consistent and also save some work 
> for future merges.

Please, send your patch via git send-email or similar, or otherwise 
follow Linux kernel instructions for submitting patches.  If using a 
mail client, use one that you can configure to send plaintext-only mail.

As part of the patch description, please include the full rationale for 
the change, e.g. the motivation for why these changes are being 
introduced and what they do.

Re: Patch to the context lookup change

[Tianjie Xu]

[-- Attachment #1: Type: text/plain, Size: 1310 bytes --]

Hi there,
I recently made a change in android's selinux repo. The change
implements a new lookup helper function for context matches, and it
aims to skip unnecessary restorecon if the corresponding context
doesn't change.

Quote from the commit message. "We used to hash the file_context and
skip the restorecon on the top level directory if the hash doesn't
change. But the file_context might change after an OTA update; and
some users experienced long restorecon time as they have lots of files
under directories like /data/media. This CL tries to hash all the
partial match entries in the file_context for each directory; and
skips the restorecon if that digest stays the same, regardless of the
changes to the other parts of file_context."
CL in: https://android-review.googlesource.com/c/platform/external/selinux/+/918713

I haven't done the work in upstream code to fully implement this
change. But this CL merges and compiles. It factors out a lookup
helper function and returns an array of matched pointers instead of a
single one. The old loopup_common function is then modified to take
the first element in the array. I wonder if it's possible to merge in
this change to the lookup functions first? It will make the upstream &
android repo more consistent and also save some work for future
merges.

[-- Attachment #2: f69947c.diff --]
[-- Type: application/x-patch, Size: 6860 bytes --]

Re: Patch to the context lookup change

[Stephen Smalley]

On 3/11/19 3:36 PM, Tianjie Xu wrote:
> Hi there,
> I recently made a change in android's selinux repo. The change
> implements a new lookup helper function for context matches, and it
> aims to skip unnecessary restorecon if the corresponding context
> doesn't change.
> 
> Quote from the commit message. "We used to hash the file_context and
> skip the restorecon on the top level directory if the hash doesn't
> change. But the file_context might change after an OTA update; and
> some users experienced long restorecon time as they have lots of files
> under directories like /data/media. This CL tries to hash all the
> partial match entries in the file_context for each directory; and
> skips the restorecon if that digest stays the same, regardless of the
> changes to the other parts of file_context."
> CL in: https://android-review.googlesource.com/c/platform/external/selinux/+/918713
> 
> I haven't done the work in upstream code to fully implement this
> change. But this CL merges and compiles. It factors out a lookup
> helper function and returns an array of matched pointers instead of a
> single one. The old loopup_common function is then modified to take
> the first element in the array. I wonder if it's possible to merge in
> this change to the lookup functions first? It will make the upstream &
> android repo more consistent and also save some work for future
> merges.

Just like Linux kernel submitting patches [1][2], we want patches to be 
inline in the message for easier review and inline commenting.  Best way 
to do it is to use git send-email after first using git commit --amend 
to work some of the rationale above into the commit message.  See also:

[1] 
https://www.kernel.org/doc/html/latest/process/submitting-patches.html#submittingpatches

[2] https://www.ozlabs.org/~akpm/stuff/tpp.txt