Re: [PATCH] libselinux: clean up process file

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Sep 6, 2016 at 1:22 PM, Stephen Smalley <sds@xxxxxxxxxxxxx> wrote:
> On 09/06/2016 04:06 PM, William Roberts wrote:
>> On Sep 6, 2016 13:01, "Stephen Smalley" <sds@xxxxxxxxxxxxx
>> <mailto:sds@xxxxxxxxxxxxx>> wrote:
>>>
>>> On 09/06/2016 11:51 AM, william.c.roberts@xxxxxxxxx
>> <mailto:william.c.roberts@xxxxxxxxx> wrote:
>>> > From: William Roberts <william.c.roberts@xxxxxxxxx
>> <mailto:william.c.roberts@xxxxxxxxx>>
>>> >
>>> > The current process_file() code will open the file
>>> > twice on the case of a binary file, correct this.
>>> >
>>> > The general flow through process_file() was a bit
>>> > difficult to read, streamline the routine to be
>>> > more readable.
>>> >
>>> > Detailed statistics of before and after:
>>> >
>>> > Source lines of code reported by cloc on modified files:
>>> > before: 1090
>>> > after: 1102
>>> >
>>> > Object size difference:
>>> > before: 195530 bytes
>>> > after:  195563 bytes
>>>
>>> Old logic would use the .bin file as long as it is not older than the
>>> base file; new logic will only use the .bin file if it is newer.  The
>>> end result on my system was that it was using the text file instead.
>>
>> I'm not following here. I spent a lot of time with strace comparing old
>> vs new behavior here. If x and x.bin exist, it should use x.bin if it's
>> newer or the same age as x? Is that correct?
>
> Yes.  strace before shows that it opens the text file and the bin file,
> and then proceeds to map the bin file and close the text file without
> processing it.  strace after shows that it stats them both and only
> opens the text file.  The st_mtime values are the same (possibly we
> ought to be comparing the full st_mtim with nanosecond precision
> instead, but regardless, if equal, we ought to use the bin file) because
> we now generate the bin file in the sandbox and then copy them both to
> /etc/selinux at the same time (commit
> a7334eb0de98af11ec38b6263536fa01bc2a606c).  You wouldn't see that if
> your policy was last built/updated with the older selinux userspace
> prior to that commit; if you run semodule -B after installing the
> current userspace, you will get that behavior.

Ok this should be simple enough the > should go to >= for the mtime comparison.
We can change the granularity in a separate patch, if you'd like.

>
>>
>>>
>>> Also, there are some memory leaks in there; run it under valgrind, e.g.
>>> valgrind --leak-check=full matchpathcon /etc
>>
>> OK I'll run that test.

I cant reproduce:


<snip>
_______________________________________________
Selinux mailing list
Selinux@xxxxxxxxxxxxx
To unsubscribe, send email to Selinux-leave@xxxxxxxxxxxxx.
To get help, send an email containing "help" to Selinux-request@xxxxxxxxxxxxx.



[Index of Archives]     [Selinux Refpolicy]     [Linux SGX]     [Fedora Users]     [Fedora Desktop]     [Yosemite Photos]     [Yosemite Camping]     [Yosemite Campsites]     [KDE Users]     [Gnome Users]

  Powered by Linux