All of lore.kernel.org
 help / color / mirror / Atom feed
* RE: unlocked stdio
@ 2016-09-21 19:57 Roberts, William C
  2016-09-21 20:08 ` Stephen Smalley
  0 siblings, 1 reply; 9+ messages in thread
From: Roberts, William C @ 2016-09-21 19:57 UTC (permalink / raw)
  To: 'seandroid-list@tycho.nsa.gov',
	'selinux@tycho.nsa.gov', 'sds@tycho.nsa.gov'

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

Correction, it's just fgets_unlocked, it appears to support the others.

From: Roberts, William C
Sent: Wednesday, September 21, 2016 12:53 PM
To: 'seandroid-list@tycho.nsa.gov' <seandroid-list@tycho.nsa.gov>; 'selinux@tycho.nsa.gov' <selinux@tycho.nsa.gov>; 'sds@tycho.nsa.gov' <sds@tycho.nsa.gov>
Subject: unlocked stdio

What was the purpose of using unlocked stdio in libselinux? Bionic doesn't support it, is it really necessary?

Bill


[-- Attachment #2: Type: text/html, Size: 2835 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: unlocked stdio
  2016-09-21 19:57 unlocked stdio Roberts, William C
@ 2016-09-21 20:08 ` Stephen Smalley
  2016-09-21 20:11   ` William Roberts
  0 siblings, 1 reply; 9+ messages in thread
From: Stephen Smalley @ 2016-09-21 20:08 UTC (permalink / raw)
  To: Roberts, William C, 'seandroid-list@tycho.nsa.gov',
	'selinux@tycho.nsa.gov'

On 09/21/2016 03:57 PM, Roberts, William C wrote:
> Correction, it’s just fgets_unlocked, it appears to support the others.

Seems like a bug in bionic, but we can work around it by:
#ifdef ANDROID
#define fgets_unlocked(x) fgets(x)
#endif

in selinux_internal.h or some similar internal header.

It avoids unnecessary locking overheads when dealing with FILE
descriptors that are only used locally and guaranteed to not be shared
by multiple threads.

> 
>  
> 
> *From:* Roberts, William C
> *Sent:* Wednesday, September 21, 2016 12:53 PM
> *To:* 'seandroid-list@tycho.nsa.gov' <seandroid-list@tycho.nsa.gov>;
> 'selinux@tycho.nsa.gov' <selinux@tycho.nsa.gov>; 'sds@tycho.nsa.gov'
> <sds@tycho.nsa.gov>
> *Subject:* unlocked stdio
> 
>  
> 
> What was the purpose of using unlocked stdio in libselinux? Bionic
> doesn’t support it, is it really necessary?
> 
>  
> 
> Bill
> 
>  
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: unlocked stdio
  2016-09-21 20:08 ` Stephen Smalley
@ 2016-09-21 20:11   ` William Roberts
  2016-09-21 20:18     ` Stephen Smalley
  0 siblings, 1 reply; 9+ messages in thread
From: William Roberts @ 2016-09-21 20:11 UTC (permalink / raw)
  To: Stephen Smalley; +Cc: seandroid-list, selinux, Roberts, William C

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

On Sep 21, 2016 13:06, "Stephen Smalley" <sds@tycho.nsa.gov> wrote:
>
> On 09/21/2016 03:57 PM, Roberts, William C wrote:
> > Correction, it’s just fgets_unlocked, it appears to support the others.
>
> Seems like a bug in bionic, but we can work around it by:
> #ifdef ANDROID
> #define fgets_unlocked(x) fgets(x)
> #endif
>
> in selinux_internal.h or some similar internal header.
>
> It avoids unnecessary locking overheads when dealing with FILE
> descriptors that are only used locally and guaranteed to not be shared
> by multiple threads.

I know what it does and why, but was it really that necessary?

>
> >
> >
> >
> > *From:* Roberts, William C
> > *Sent:* Wednesday, September 21, 2016 12:53 PM
> > *To:* 'seandroid-list@tycho.nsa.gov' <seandroid-list@tycho.nsa.gov>;
> > 'selinux@tycho.nsa.gov' <selinux@tycho.nsa.gov>; 'sds@tycho.nsa.gov'
> > <sds@tycho.nsa.gov>
> > *Subject:* unlocked stdio
> >
> >
> >
> > What was the purpose of using unlocked stdio in libselinux? Bionic
> > doesn’t support it, is it really necessary?
> >
> >
> >
> > Bill
> >
> >
> >
>
> _______________________________________________
> Selinux mailing list
> Selinux@tycho.nsa.gov
> To unsubscribe, send email to Selinux-leave@tycho.nsa.gov.
> To get help, send an email containing "help" to
Selinux-request@tycho.nsa.gov.

[-- Attachment #2: Type: text/html, Size: 2301 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: unlocked stdio
  2016-09-21 20:11   ` William Roberts
@ 2016-09-21 20:18     ` Stephen Smalley
  2016-09-21 21:48       ` William Roberts
  0 siblings, 1 reply; 9+ messages in thread
From: Stephen Smalley @ 2016-09-21 20:18 UTC (permalink / raw)
  To: William Roberts; +Cc: seandroid-list, selinux, Roberts, William C

On 09/21/2016 04:11 PM, William Roberts wrote:
> On Sep 21, 2016 13:06, "Stephen Smalley" <sds@tycho.nsa.gov
> <mailto:sds@tycho.nsa.gov>> wrote:
>>
>> On 09/21/2016 03:57 PM, Roberts, William C wrote:
>> > Correction, it’s just fgets_unlocked, it appears to support the others.
>>
>> Seems like a bug in bionic, but we can work around it by:
>> #ifdef ANDROID
>> #define fgets_unlocked(x) fgets(x)
>> #endif
>>
>> in selinux_internal.h or some similar internal header.
>>
>> It avoids unnecessary locking overheads when dealing with FILE
>> descriptors that are only used locally and guaranteed to not be shared
>> by multiple threads.
> 
> I know what it does and why, but was it really that necessary?

The patch came from Red Hat.  Anyway, we use the _unlocked functions
throughout, and the fact that bionic supports all of them except that
one function suggests that we should just use a fix like the above
rather than dropping it.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: unlocked stdio
  2016-09-21 20:18     ` Stephen Smalley
@ 2016-09-21 21:48       ` William Roberts
  2016-09-21 21:53         ` William Roberts
  0 siblings, 1 reply; 9+ messages in thread
From: William Roberts @ 2016-09-21 21:48 UTC (permalink / raw)
  To: Stephen Smalley; +Cc: William Roberts, selinux, seandroid-list

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

On Sep 21, 2016 13:16, "Stephen Smalley" <sds@tycho.nsa.gov> wrote:
>
> On 09/21/2016 04:11 PM, William Roberts wrote:
> > On Sep 21, 2016 13:06, "Stephen Smalley" <sds@tycho.nsa.gov
> > <mailto:sds@tycho.nsa.gov>> wrote:
> >>
> >> On 09/21/2016 03:57 PM, Roberts, William C wrote:
> >> > Correction, it’s just fgets_unlocked, it appears to support the
others.
> >>
> >> Seems like a bug in bionic, but we can work around it by:
> >> #ifdef ANDROID
> >> #define fgets_unlocked(x) fgets(x)
> >> #endif
> >>
> >> in selinux_internal.h or some similar internal header.
> >>
> >> It avoids unnecessary locking overheads when dealing with FILE
> >> descriptors that are only used locally and guaranteed to not be shared
> >> by multiple threads.
> >
> > I know what it does and why, but was it really that necessary?
>
> The patch came from Red Hat.  Anyway, we use the _unlocked functions
> throughout, and the fact that bionic supports all of them except that
> one function suggests that we should just use a fix like the above
> rather than dropping it.

I'm not arguing the fix, just wondering if it was blazingly fast. I didn't
find the definition or a define in bionic for fgets_unlocked, but when I
set _GNUC define, it seemed to build OK.

>
>

[-- Attachment #2: Type: text/html, Size: 1795 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: unlocked stdio
  2016-09-21 21:48       ` William Roberts
@ 2016-09-21 21:53         ` William Roberts
  2016-09-21 23:47           ` William Roberts
  0 siblings, 1 reply; 9+ messages in thread
From: William Roberts @ 2016-09-21 21:53 UTC (permalink / raw)
  To: Stephen Smalley; +Cc: William Roberts, selinux, seandroid-list

On Wed, Sep 21, 2016 at 2:48 PM, William Roberts
<bill.c.roberts@gmail.com> wrote:
> On Sep 21, 2016 13:16, "Stephen Smalley" <sds@tycho.nsa.gov> wrote:
>>
>> On 09/21/2016 04:11 PM, William Roberts wrote:
>> > On Sep 21, 2016 13:06, "Stephen Smalley" <sds@tycho.nsa.gov
>> > <mailto:sds@tycho.nsa.gov>> wrote:
>> >>
>> >> On 09/21/2016 03:57 PM, Roberts, William C wrote:
>> >> > Correction, it’s just fgets_unlocked, it appears to support the
>> >> > others.
>> >>
>> >> Seems like a bug in bionic, but we can work around it by:
>> >> #ifdef ANDROID
>> >> #define fgets_unlocked(x) fgets(x)
>> >> #endif
>> >>
>> >> in selinux_internal.h or some similar internal header.
>> >>
>> >> It avoids unnecessary locking overheads when dealing with FILE
>> >> descriptors that are only used locally and guaranteed to not be shared
>> >> by multiple threads.
>> >
>> > I know what it does and why, but was it really that necessary?
>>
>> The patch came from Red Hat.  Anyway, we use the _unlocked functions
>> throughout, and the fact that bionic supports all of them except that
>> one function suggests that we should just use a fix like the above
>> rather than dropping it.
>
> I'm not arguing the fix, just wondering if it was blazingly fast. I didn't
> find the definition or a define in bionic for fgets_unlocked, but when I set
> _GNUC define, it seemed to build OK.

That's an incorrect statement, that was only for the host built
against glibc where
_GNU_SOURCE needs to be defined. For Android, yes the simplest way is as
you suggest.


-- 
Respectfully,

William C Roberts

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: unlocked stdio
  2016-09-21 21:53         ` William Roberts
@ 2016-09-21 23:47           ` William Roberts
  2016-09-22 12:20             ` Stephen Smalley
  0 siblings, 1 reply; 9+ messages in thread
From: William Roberts @ 2016-09-21 23:47 UTC (permalink / raw)
  To: Stephen Smalley; +Cc: William Roberts, selinux, seandroid-list

Another thing I noticed rectifying the Android tree is that the
selinux/Android.mk upstream is empty, but the secondary levels are
present, any reason that hasn't been pushed?

On Wed, Sep 21, 2016 at 2:53 PM, William Roberts
<bill.c.roberts@gmail.com> wrote:
> On Wed, Sep 21, 2016 at 2:48 PM, William Roberts
> <bill.c.roberts@gmail.com> wrote:
>> On Sep 21, 2016 13:16, "Stephen Smalley" <sds@tycho.nsa.gov> wrote:
>>>
>>> On 09/21/2016 04:11 PM, William Roberts wrote:
>>> > On Sep 21, 2016 13:06, "Stephen Smalley" <sds@tycho.nsa.gov
>>> > <mailto:sds@tycho.nsa.gov>> wrote:
>>> >>
>>> >> On 09/21/2016 03:57 PM, Roberts, William C wrote:
>>> >> > Correction, it’s just fgets_unlocked, it appears to support the
>>> >> > others.
>>> >>
>>> >> Seems like a bug in bionic, but we can work around it by:
>>> >> #ifdef ANDROID
>>> >> #define fgets_unlocked(x) fgets(x)
>>> >> #endif
>>> >>
>>> >> in selinux_internal.h or some similar internal header.
>>> >>
>>> >> It avoids unnecessary locking overheads when dealing with FILE
>>> >> descriptors that are only used locally and guaranteed to not be shared
>>> >> by multiple threads.
>>> >
>>> > I know what it does and why, but was it really that necessary?
>>>
>>> The patch came from Red Hat.  Anyway, we use the _unlocked functions
>>> throughout, and the fact that bionic supports all of them except that
>>> one function suggests that we should just use a fix like the above
>>> rather than dropping it.
>>
>> I'm not arguing the fix, just wondering if it was blazingly fast. I didn't
>> find the definition or a define in bionic for fgets_unlocked, but when I set
>> _GNUC define, it seemed to build OK.
>
> That's an incorrect statement, that was only for the host built
> against glibc where
> _GNU_SOURCE needs to be defined. For Android, yes the simplest way is as
> you suggest.
>
>
> --
> Respectfully,
>
> William C Roberts



-- 
Respectfully,

William C Roberts

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: unlocked stdio
  2016-09-21 23:47           ` William Roberts
@ 2016-09-22 12:20             ` Stephen Smalley
  0 siblings, 0 replies; 9+ messages in thread
From: Stephen Smalley @ 2016-09-22 12:20 UTC (permalink / raw)
  To: William Roberts; +Cc: William Roberts, selinux, seandroid-list

On 09/21/2016 07:47 PM, William Roberts wrote:
> Another thing I noticed rectifying the Android tree is that the
> selinux/Android.mk upstream is empty, but the secondary levels are
> present, any reason that hasn't been pushed?

No, just that no one has submitted them upstream.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* unlocked stdio
@ 2016-09-21 19:53 Roberts, William C
  0 siblings, 0 replies; 9+ messages in thread
From: Roberts, William C @ 2016-09-21 19:53 UTC (permalink / raw)
  To: 'seandroid-list@tycho.nsa.gov',
	'selinux@tycho.nsa.gov', 'sds@tycho.nsa.gov'

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

What was the purpose of using unlocked stdio in libselinux? Bionic doesn't support it, is it really necessary?

Bill


[-- Attachment #2: Type: text/html, Size: 1898 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2016-09-22 12:20 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-21 19:57 unlocked stdio Roberts, William C
2016-09-21 20:08 ` Stephen Smalley
2016-09-21 20:11   ` William Roberts
2016-09-21 20:18     ` Stephen Smalley
2016-09-21 21:48       ` William Roberts
2016-09-21 21:53         ` William Roberts
2016-09-21 23:47           ` William Roberts
2016-09-22 12:20             ` Stephen Smalley
  -- strict thread matches above, loose matches on Subject: below --
2016-09-21 19:53 Roberts, William C

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.