All of lore.kernel.org
 help / color / mirror / Atom feed
* avc_open() and netlink_loop()
@ 2010-02-03  6:19 KaiGai Kohei
  2010-02-26 19:14 ` Eamon Walsh
  0 siblings, 1 reply; 8+ messages in thread
From: KaiGai Kohei @ 2010-02-03  6:19 UTC (permalink / raw)
  To: Eamon Walsh; +Cc: selinux

When we initialize userspace avc using avc_open(3), it internally calls
avc_init(3) without any callback functions. The avc_init() is introduced
as a deprecated interface from application code, so it is recommended to
use avc_open() instead for new applications.

The avc_init() internally calls avc_netlink_open(). If no thread callback
is not given, the 'blocking' argument shall be 0, then avc_netlink_open()
set O_NONBLOCK flag on the socket file descriptor.

Next, application will create a thread to receive messages via netlink
socket to invalidate userspace avc, using avc_netlink_loop().
However, if userspace avc of libselinux is already initialized,
the avc_netlink_loop() immediately returns with EWOULDBLOCK, because the
netlink socket is not blocked and avc_netlink_receive() does not expect
recvfrom() returns error.

It seems to me O_NONBLOCK is a wrong strategy in this case, and select(2)
should be checked in avc_netlink_check_nb() instead.

Eamon, what is your opinion?
-- 
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai@ak.jp.nec.com>

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

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

* Re: avc_open() and netlink_loop()
  2010-02-03  6:19 avc_open() and netlink_loop() KaiGai Kohei
@ 2010-02-26 19:14 ` Eamon Walsh
  2010-02-26 20:49   ` Eamon Walsh
  0 siblings, 1 reply; 8+ messages in thread
From: Eamon Walsh @ 2010-02-26 19:14 UTC (permalink / raw)
  To: KaiGai Kohei; +Cc: selinux

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

On 02/03/2010 01:19 AM, KaiGai Kohei wrote:
> When we initialize userspace avc using avc_open(3), it internally calls
> avc_init(3) without any callback functions. The avc_init() is introduced
> as a deprecated interface from application code, so it is recommended to
> use avc_open() instead for new applications.
>
> The avc_init() internally calls avc_netlink_open(). If no thread callback
> is not given, the 'blocking' argument shall be 0, then avc_netlink_open()
> set O_NONBLOCK flag on the socket file descriptor.
>
> Next, application will create a thread to receive messages via netlink
> socket to invalidate userspace avc, using avc_netlink_loop().
> However, if userspace avc of libselinux is already initialized,
> the avc_netlink_loop() immediately returns with EWOULDBLOCK, because the
> netlink socket is not blocked and avc_netlink_receive() does not expect
> recvfrom() returns error.
>
> It seems to me O_NONBLOCK is a wrong strategy in this case, and select(2)
> should be checked in avc_netlink_check_nb() instead.
>
> Eamon, what is your opinion?
>   


Hi, my apologies for the delayed response. Yes it appears as though this
is a bug. I think the solution is to configure the file descriptor in
blocking mode at the start of avc_netlink_loop(). When
avc_netlink_loop() is called, we always want blocking behavior. See the
attached patch.


-- 

Eamon Walsh 
National Security Agency


[-- Attachment #2: avc_blocking_bug.patch --]
[-- Type: text/x-patch, Size: 802 bytes --]

diff --git a/libselinux/src/avc.c b/libselinux/src/avc.c
index 881b915..5c8def0 100644
--- a/libselinux/src/avc.c
+++ b/libselinux/src/avc.c
@@ -222,7 +222,7 @@ int avc_init(const char *prefix,
 		avc_enforcing = rc;
 	}
 
-	rc = avc_netlink_open(avc_using_threads);
+	rc = avc_netlink_open(0);
 	if (rc < 0) {
 		avc_log(SELINUX_ERROR,
 			"%s:  can't open netlink socket: %d (%s)\n",
diff --git a/libselinux/src/avc_internal.c b/libselinux/src/avc_internal.c
index 8372f52..90dfa51 100644
--- a/libselinux/src/avc_internal.c
+++ b/libselinux/src/avc_internal.c
@@ -233,6 +233,8 @@ void avc_netlink_loop(void)
 	int rc;
 	char buf[1024] __attribute__ ((aligned));
 
+	fcntl(fd, F_SETFL, fcntl(fd, F_GETFL, 0) & ~O_NONBLOCK)
+
 	while (1) {
 		errno = 0;
 		rc = avc_netlink_receive(buf, sizeof(buf));

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

* Re: avc_open() and netlink_loop()
  2010-02-26 19:14 ` Eamon Walsh
@ 2010-02-26 20:49   ` Eamon Walsh
  2010-02-26 20:56     ` [PATCH] libselinux: fix avc_netlink_loop() error caused by nonblocking mode Eamon Walsh
  0 siblings, 1 reply; 8+ messages in thread
From: Eamon Walsh @ 2010-02-26 20:49 UTC (permalink / raw)
  To: KaiGai Kohei; +Cc: selinux

On 02/26/2010 02:14 PM, Eamon Walsh wrote:
> On 02/03/2010 01:19 AM, KaiGai Kohei wrote:
>> When we initialize userspace avc using avc_open(3), it internally calls
>> avc_init(3) without any callback functions. The avc_init() is introduced
>> as a deprecated interface from application code, so it is recommended to
>> use avc_open() instead for new applications.
>>
>> The avc_init() internally calls avc_netlink_open(). If no thread callback
>> is not given, the 'blocking' argument shall be 0, then avc_netlink_open()
>> set O_NONBLOCK flag on the socket file descriptor.
>>
>> Next, application will create a thread to receive messages via netlink
>> socket to invalidate userspace avc, using avc_netlink_loop().
>> However, if userspace avc of libselinux is already initialized,
>> the avc_netlink_loop() immediately returns with EWOULDBLOCK, because the
>> netlink socket is not blocked and avc_netlink_receive() does not expect
>> recvfrom() returns error.
>>
>> It seems to me O_NONBLOCK is a wrong strategy in this case, and select(2)
>> should be checked in avc_netlink_check_nb() instead.
>>
>> Eamon, what is your opinion?
>>   
> 
> 
> Hi, my apologies for the delayed response. Yes it appears as though this
> is a bug. I think the solution is to configure the file descriptor in
> blocking mode at the start of avc_netlink_loop(). When
> avc_netlink_loop() is called, we always want blocking behavior. See the
> attached patch.
> 
> 

Also, before you launch a thread to run avc_netlink_loop(), you need to call avc_netlink_acquire_fd().  Otherwise, the avc will internally make calls to avc_netlink_check_nb() which will not work properly.


-- 
Eamon Walsh 
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

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

* [PATCH] libselinux: fix avc_netlink_loop() error caused by nonblocking mode.
  2010-02-26 20:49   ` Eamon Walsh
@ 2010-02-26 20:56     ` Eamon Walsh
  2010-03-02  2:48       ` KaiGai Kohei
  0 siblings, 1 reply; 8+ messages in thread
From: Eamon Walsh @ 2010-02-26 20:56 UTC (permalink / raw)
  To: KaiGai Kohei; +Cc: selinux, Joshua Brindle

avc_open() creates the netlink socket in nonblocking mode.  If the
application later takes control of the netlink socket with
avc_netlink_acquire_fd() and then calls avc_netlink_loop(), it
will fail with EWOULDBLOCK.
    
To remedy this, remove the O_NONBLOCK flag from the netlink socket
at the start of avc_netlink_loop().  Also, with this fix, there is
no need for avc_open() to ever create a blocking socket, so change
that and update the man page.
    
Signed-off-by: Eamon Walsh <ewalsh@tycho.nsa.gov>
---

 man/man3/avc_netlink_loop.3 |    5 +----
 src/avc.c                   |    2 +-
 src/avc_internal.c          |    2 ++
 3 files changed, 4 insertions(+), 5 deletions(-)


diff --git a/libselinux/man/man3/avc_netlink_loop.3 b/libselinux/man/man3/avc_netlink_loop.3
index 67df6e4..7d8c9a4 100644
--- a/libselinux/man/man3/avc_netlink_loop.3
+++ b/libselinux/man/man3/avc_netlink_loop.3
@@ -43,10 +43,7 @@ to take ownership of it in application code.  The
 .I blocking
 argument specifies whether read operations on the socket will block.
 .BR avc_open (3)
-calls this function internally, specifying non-blocking behavior (unless
-threading callbacks were explicitly set using the deprecated
-.BR avc_init (3)
-interface, in which case blocking behavior is set).
+calls this function internally, specifying non-blocking behavior.
 
 .B avc_netlink_close
 closes the netlink socket.  This function is called automatically by
diff --git a/libselinux/src/avc.c b/libselinux/src/avc.c
index 881b915..5c8def0 100644
--- a/libselinux/src/avc.c
+++ b/libselinux/src/avc.c
@@ -222,7 +222,7 @@ int avc_init(const char *prefix,
 		avc_enforcing = rc;
 	}
 
-	rc = avc_netlink_open(avc_using_threads);
+	rc = avc_netlink_open(0);
 	if (rc < 0) {
 		avc_log(SELINUX_ERROR,
 			"%s:  can't open netlink socket: %d (%s)\n",
diff --git a/libselinux/src/avc_internal.c b/libselinux/src/avc_internal.c
index 8372f52..90dfa51 100644
--- a/libselinux/src/avc_internal.c
+++ b/libselinux/src/avc_internal.c
@@ -233,6 +233,8 @@ void avc_netlink_loop(void)
 	int rc;
 	char buf[1024] __attribute__ ((aligned));
 
+	fcntl(fd, F_SETFL, fcntl(fd, F_GETFL, 0) & ~O_NONBLOCK);
+
 	while (1) {
 		errno = 0;
 		rc = avc_netlink_receive(buf, sizeof(buf));



--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

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

* Re: [PATCH] libselinux: fix avc_netlink_loop() error caused by nonblocking mode.
  2010-02-26 20:56     ` [PATCH] libselinux: fix avc_netlink_loop() error caused by nonblocking mode Eamon Walsh
@ 2010-03-02  2:48       ` KaiGai Kohei
  2010-03-02 17:29         ` Eamon Walsh
  0 siblings, 1 reply; 8+ messages in thread
From: KaiGai Kohei @ 2010-03-02  2:48 UTC (permalink / raw)
  To: Eamon Walsh; +Cc: selinux, Joshua Brindle

(2010/02/27 5:56), Eamon Walsh wrote:
> avc_open() creates the netlink socket in nonblocking mode.  If the
> application later takes control of the netlink socket with
> avc_netlink_acquire_fd() and then calls avc_netlink_loop(), it
> will fail with EWOULDBLOCK.
> 
> To remedy this, remove the O_NONBLOCK flag from the netlink socket
> at the start of avc_netlink_loop().  Also, with this fix, there is
> no need for avc_open() to ever create a blocking socket, so change
> that and update the man page.

Apart from this design is sane, the libselinux allows applications to
call avc_netlink_check_nb() directly.
If we call the function although its worker thread calls avc_netlink_loop(),
the invocation of avc_netlink_check_nb() can be blocked, because it
assumes the internal netlink socket is non-blocking mode.
(Of course, we will not see the matter in most of sane applications.)

I think a better approach is to put select(2) with zero or infinite
timeout just before avc_netlink_receive() calls to check whether the
unread message is in the netlink socket, or not.
And the 'blocking' argument of avc_netlink_open() shall be obsoleted,
because this design makes nonsense whether the socket has O_NONBLOCK
flag, or not.

The point is we should not assume the netlink socket has a certain
state when we call avc_netlink_loop() and avc_netlink_check_nb().

What is your opinion?

> Signed-off-by: Eamon Walsh<ewalsh@tycho.nsa.gov>
> ---
> 
>   man/man3/avc_netlink_loop.3 |    5 +----
>   src/avc.c                   |    2 +-
>   src/avc_internal.c          |    2 ++
>   3 files changed, 4 insertions(+), 5 deletions(-)
> 
> 
> diff --git a/libselinux/man/man3/avc_netlink_loop.3 b/libselinux/man/man3/avc_netlink_loop.3
> index 67df6e4..7d8c9a4 100644
> --- a/libselinux/man/man3/avc_netlink_loop.3
> +++ b/libselinux/man/man3/avc_netlink_loop.3
> @@ -43,10 +43,7 @@ to take ownership of it in application code.  The
>   .I blocking
>   argument specifies whether read operations on the socket will block.
>   .BR avc_open (3)
> -calls this function internally, specifying non-blocking behavior (unless
> -threading callbacks were explicitly set using the deprecated
> -.BR avc_init (3)
> -interface, in which case blocking behavior is set).
> +calls this function internally, specifying non-blocking behavior.
> 
>   .B avc_netlink_close
>   closes the netlink socket.  This function is called automatically by
> diff --git a/libselinux/src/avc.c b/libselinux/src/avc.c
> index 881b915..5c8def0 100644
> --- a/libselinux/src/avc.c
> +++ b/libselinux/src/avc.c
> @@ -222,7 +222,7 @@ int avc_init(const char *prefix,
>   		avc_enforcing = rc;
>   	}
> 
> -	rc = avc_netlink_open(avc_using_threads);
> +	rc = avc_netlink_open(0);
>   	if (rc<  0) {
>   		avc_log(SELINUX_ERROR,
>   			"%s:  can't open netlink socket: %d (%s)\n",
> diff --git a/libselinux/src/avc_internal.c b/libselinux/src/avc_internal.c
> index 8372f52..90dfa51 100644
> --- a/libselinux/src/avc_internal.c
> +++ b/libselinux/src/avc_internal.c
> @@ -233,6 +233,8 @@ void avc_netlink_loop(void)
>   	int rc;
>   	char buf[1024] __attribute__ ((aligned));
> 
> +	fcntl(fd, F_SETFL, fcntl(fd, F_GETFL, 0)&  ~O_NONBLOCK);
> +
>   	while (1) {
>   		errno = 0;
>   		rc = avc_netlink_receive(buf, sizeof(buf));
> 
> 
> 
> --
> This message was distributed to subscribers of the selinux mailing list.
> If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
> the words "unsubscribe selinux" without quotes as the message.
> 


-- 
KaiGai Kohei <kaigai@ak.jp.nec.com>

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

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

* Re: [PATCH] libselinux: fix avc_netlink_loop() error caused by nonblocking mode.
  2010-03-02  2:48       ` KaiGai Kohei
@ 2010-03-02 17:29         ` Eamon Walsh
  2010-03-03  0:30           ` KaiGai Kohei
  0 siblings, 1 reply; 8+ messages in thread
From: Eamon Walsh @ 2010-03-02 17:29 UTC (permalink / raw)
  To: KaiGai Kohei; +Cc: selinux, Joshua Brindle

On 03/01/2010 09:48 PM, KaiGai Kohei wrote:
> (2010/02/27 5:56), Eamon Walsh wrote:
>   
>> avc_open() creates the netlink socket in nonblocking mode.  If the
>> application later takes control of the netlink socket with
>> avc_netlink_acquire_fd() and then calls avc_netlink_loop(), it
>> will fail with EWOULDBLOCK.
>>
>> To remedy this, remove the O_NONBLOCK flag from the netlink socket
>> at the start of avc_netlink_loop().  Also, with this fix, there is
>> no need for avc_open() to ever create a blocking socket, so change
>> that and update the man page.
>>     
> Apart from this design is sane, the libselinux allows applications to
> call avc_netlink_check_nb() directly.
> If we call the function although its worker thread calls avc_netlink_loop(),
> the invocation of avc_netlink_check_nb() can be blocked, because it
> assumes the internal netlink socket is non-blocking mode.
> (Of course, we will not see the matter in most of sane applications.)
>
> I think a better approach is to put select(2) with zero or infinite
> timeout just before avc_netlink_receive() calls to check whether the
> unread message is in the netlink socket, or not.
> And the 'blocking' argument of avc_netlink_open() shall be obsoleted,
> because this design makes nonsense whether the socket has O_NONBLOCK
> flag, or not.
>   

OK I see your point. How about the following:

libselinux: fix avc_netlink_loop() error caused by nonblocking mode.

avc_open() creates the netlink socket in nonblocking mode.  If the
application later takes control of the netlink socket with
avc_netlink_acquire_fd() and then calls avc_netlink_loop(), it
will fail with EWOULDBLOCK.

To remedy this, remove the O_NONBLOCK flag from the netlink socket
at the start of avc_netlink_loop().  Also, with this fix, there is
no need for avc_open() to ever create a blocking socket, so change
that and update the man page.

-v2: use poll() in avc_netlink_check_nb().  This makes both
avc_netlink_loop() and avc_netlink_check_nb() independent of the
O_NONBLOCK flag.

Signed-off-by: Eamon Walsh <ewalsh@tycho.nsa.gov>
--

 man/man3/avc_netlink_loop.3 |   11 +++--------
 src/avc.c                   |    2 +-
 src/avc_internal.c          |   13 +++++++++++++
 3 files changed, 17 insertions(+), 9 deletions(-)


diff --git a/libselinux/man/man3/avc_netlink_loop.3 b/libselinux/man/man3/avc_netlink_loop.3
index 67df6e4..785be4c 100644
--- a/libselinux/man/man3/avc_netlink_loop.3
+++ b/libselinux/man/man3/avc_netlink_loop.3
@@ -41,12 +41,9 @@ descriptor is stored internally; use
 .BR avc_netlink_acquire_fd (3)
 to take ownership of it in application code.  The
 .I blocking
-argument specifies whether read operations on the socket will block.
+argument controls whether the O_NONBLOCK flag is set on the socket descriptor.
 .BR avc_open (3)
-calls this function internally, specifying non-blocking behavior (unless
-threading callbacks were explicitly set using the deprecated
-.BR avc_init (3)
-interface, in which case blocking behavior is set).
+calls this function internally, specifying non-blocking behavior.
 
 .B avc_netlink_close
 closes the netlink socket.  This function is called automatically by
@@ -66,9 +63,7 @@ checks the netlink socket for pending messages and processes them.
 Callbacks for policyload and enforcing changes will be called;
 see
 .BR selinux_set_callback (3).
-This function does not block unless
-.BR avc_netlink_open (3)
-specified blocking behavior.
+This function does not block.
 
 .B avc_netlink_loop
 enters a loop blocking on the netlink socket and processing messages as they
diff --git a/libselinux/src/avc.c b/libselinux/src/avc.c
index 881b915..5c8def0 100644
--- a/libselinux/src/avc.c
+++ b/libselinux/src/avc.c
@@ -222,7 +222,7 @@ int avc_init(const char *prefix,
 		avc_enforcing = rc;
 	}
 
-	rc = avc_netlink_open(avc_using_threads);
+	rc = avc_netlink_open(0);
 	if (rc < 0) {
 		avc_log(SELINUX_ERROR,
 			"%s:  can't open netlink socket: %d (%s)\n",
diff --git a/libselinux/src/avc_internal.c b/libselinux/src/avc_internal.c
index 8372f52..7a39c61 100644
--- a/libselinux/src/avc_internal.c
+++ b/libselinux/src/avc_internal.c
@@ -15,6 +15,7 @@
 #include <unistd.h>
 #include <fcntl.h>
 #include <string.h>
+#include <poll.h>
 #include <sys/types.h>
 #include <sys/socket.h>
 #include <linux/types.h>
@@ -205,8 +206,18 @@ int avc_netlink_check_nb(void)
 {
 	int rc;
 	char buf[1024] __attribute__ ((aligned));
+	struct pollfd pfd = { fd, POLLIN | POLLPRI, 0 };
 
 	while (1) {
+		rc = poll(&pfd, 1, 0);
+		if (rc == 0)
+			return 0;
+		if (rc < 0) {
+			avc_log(SELINUX_ERROR, "%s:  netlink poll: error %d\n",
+				avc_prefix, errno);
+			return rc;
+		}
+
 		errno = 0;
 		rc = avc_netlink_receive(buf, sizeof(buf));
 		if (rc < 0) {
@@ -233,6 +244,8 @@ void avc_netlink_loop(void)
 	int rc;
 	char buf[1024] __attribute__ ((aligned));
 
+	fcntl(fd, F_SETFL, fcntl(fd, F_GETFL, 0) & ~O_NONBLOCK);
+
 	while (1) {
 		errno = 0;
 		rc = avc_netlink_receive(buf, sizeof(buf));



-- 

Eamon Walsh 
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

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

* Re: [PATCH] libselinux: fix avc_netlink_loop() error caused by nonblocking mode.
  2010-03-02 17:29         ` Eamon Walsh
@ 2010-03-03  0:30           ` KaiGai Kohei
  2010-03-08 23:19             ` Eamon Walsh
  0 siblings, 1 reply; 8+ messages in thread
From: KaiGai Kohei @ 2010-03-03  0:30 UTC (permalink / raw)
  To: Eamon Walsh; +Cc: selinux, Joshua Brindle

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

(2010/03/03 2:29), Eamon Walsh wrote:
> On 03/01/2010 09:48 PM, KaiGai Kohei wrote:
>> (2010/02/27 5:56), Eamon Walsh wrote:
>>
>>> avc_open() creates the netlink socket in nonblocking mode.  If the
>>> application later takes control of the netlink socket with
>>> avc_netlink_acquire_fd() and then calls avc_netlink_loop(), it
>>> will fail with EWOULDBLOCK.
>>>
>>> To remedy this, remove the O_NONBLOCK flag from the netlink socket
>>> at the start of avc_netlink_loop().  Also, with this fix, there is
>>> no need for avc_open() to ever create a blocking socket, so change
>>> that and update the man page.
>>>
>> Apart from this design is sane, the libselinux allows applications to
>> call avc_netlink_check_nb() directly.
>> If we call the function although its worker thread calls avc_netlink_loop(),
>> the invocation of avc_netlink_check_nb() can be blocked, because it
>> assumes the internal netlink socket is non-blocking mode.
>> (Of course, we will not see the matter in most of sane applications.)
>>
>> I think a better approach is to put select(2) with zero or infinite
>> timeout just before avc_netlink_receive() calls to check whether the
>> unread message is in the netlink socket, or not.
>> And the 'blocking' argument of avc_netlink_open() shall be obsoleted,
>> because this design makes nonsense whether the socket has O_NONBLOCK
>> flag, or not.
>>
> 
> OK I see your point. How about the following:
> 
> libselinux: fix avc_netlink_loop() error caused by nonblocking mode.
> 
> avc_open() creates the netlink socket in nonblocking mode.  If the
> application later takes control of the netlink socket with
> avc_netlink_acquire_fd() and then calls avc_netlink_loop(), it
> will fail with EWOULDBLOCK.
> 
> To remedy this, remove the O_NONBLOCK flag from the netlink socket
> at the start of avc_netlink_loop().  Also, with this fix, there is
> no need for avc_open() to ever create a blocking socket, so change
> that and update the man page.
> 
> -v2: use poll() in avc_netlink_check_nb().  This makes both
> avc_netlink_loop() and avc_netlink_check_nb() independent of the
> O_NONBLOCK flag.

It seems to me ok basically.

It is just my preference. Can you move the poll(2) at the head of
the avc_netlink_receive() call, like as the attachment?
How should I say...it seems to me itchy to change non-blocking mode
into blocking mode implicitly at avc_netlink_loop(), although we can
specify the mode at creation time (avc_netlink_open()).

Thanks,

> Signed-off-by: Eamon Walsh<ewalsh@tycho.nsa.gov>
> --
> 
>   man/man3/avc_netlink_loop.3 |   11 +++--------
>   src/avc.c                   |    2 +-
>   src/avc_internal.c          |   13 +++++++++++++
>   3 files changed, 17 insertions(+), 9 deletions(-)
> 
> 
> diff --git a/libselinux/man/man3/avc_netlink_loop.3 b/libselinux/man/man3/avc_netlink_loop.3
> index 67df6e4..785be4c 100644
> --- a/libselinux/man/man3/avc_netlink_loop.3
> +++ b/libselinux/man/man3/avc_netlink_loop.3
> @@ -41,12 +41,9 @@ descriptor is stored internally; use
>   .BR avc_netlink_acquire_fd (3)
>   to take ownership of it in application code.  The
>   .I blocking
> -argument specifies whether read operations on the socket will block.
> +argument controls whether the O_NONBLOCK flag is set on the socket descriptor.
>   .BR avc_open (3)
> -calls this function internally, specifying non-blocking behavior (unless
> -threading callbacks were explicitly set using the deprecated
> -.BR avc_init (3)
> -interface, in which case blocking behavior is set).
> +calls this function internally, specifying non-blocking behavior.
> 
>   .B avc_netlink_close
>   closes the netlink socket.  This function is called automatically by
> @@ -66,9 +63,7 @@ checks the netlink socket for pending messages and processes them.
>   Callbacks for policyload and enforcing changes will be called;
>   see
>   .BR selinux_set_callback (3).
> -This function does not block unless
> -.BR avc_netlink_open (3)
> -specified blocking behavior.
> +This function does not block.
> 
>   .B avc_netlink_loop
>   enters a loop blocking on the netlink socket and processing messages as they
> diff --git a/libselinux/src/avc.c b/libselinux/src/avc.c
> index 881b915..5c8def0 100644
> --- a/libselinux/src/avc.c
> +++ b/libselinux/src/avc.c
> @@ -222,7 +222,7 @@ int avc_init(const char *prefix,
>   		avc_enforcing = rc;
>   	}
> 
> -	rc = avc_netlink_open(avc_using_threads);
> +	rc = avc_netlink_open(0);
>   	if (rc<  0) {
>   		avc_log(SELINUX_ERROR,
>   			"%s:  can't open netlink socket: %d (%s)\n",
> diff --git a/libselinux/src/avc_internal.c b/libselinux/src/avc_internal.c
> index 8372f52..7a39c61 100644
> --- a/libselinux/src/avc_internal.c
> +++ b/libselinux/src/avc_internal.c
> @@ -15,6 +15,7 @@
>   #include<unistd.h>
>   #include<fcntl.h>
>   #include<string.h>
> +#include<poll.h>
>   #include<sys/types.h>
>   #include<sys/socket.h>
>   #include<linux/types.h>
> @@ -205,8 +206,18 @@ int avc_netlink_check_nb(void)
>   {
>   	int rc;
>   	char buf[1024] __attribute__ ((aligned));
> +	struct pollfd pfd = { fd, POLLIN | POLLPRI, 0 };
> 
>   	while (1) {
> +		rc = poll(&pfd, 1, 0);
> +		if (rc == 0)
> +			return 0;
> +		if (rc<  0) {
> +			avc_log(SELINUX_ERROR, "%s:  netlink poll: error %d\n",
> +				avc_prefix, errno);
> +			return rc;
> +		}
> +
>   		errno = 0;
>   		rc = avc_netlink_receive(buf, sizeof(buf));
>   		if (rc<  0) {
> @@ -233,6 +244,8 @@ void avc_netlink_loop(void)
>   	int rc;
>   	char buf[1024] __attribute__ ((aligned));
> 
> +	fcntl(fd, F_SETFL, fcntl(fd, F_GETFL, 0)&  ~O_NONBLOCK);
> +
>   	while (1) {
>   		errno = 0;
>   		rc = avc_netlink_receive(buf, sizeof(buf));
> 
> 
> 


-- 
KaiGai Kohei <kaigai@ak.jp.nec.com>

[-- Attachment #2: avc-netlink-poll.2.patch --]
[-- Type: application/octect-stream, Size: 3488 bytes --]

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

* Re: [PATCH] libselinux: fix avc_netlink_loop() error caused by nonblocking mode.
  2010-03-03  0:30           ` KaiGai Kohei
@ 2010-03-08 23:19             ` Eamon Walsh
  0 siblings, 0 replies; 8+ messages in thread
From: Eamon Walsh @ 2010-03-08 23:19 UTC (permalink / raw)
  To: KaiGai Kohei; +Cc: selinux, Joshua Brindle

On 03/02/2010 07:30 PM, KaiGai Kohei wrote:
>
> It seems to me ok basically.
>
> It is just my preference. Can you move the poll(2) at the head of
> the avc_netlink_receive() call, like as the attachment?
> How should I say...it seems to me itchy to change non-blocking mode
> into blocking mode implicitly at avc_netlink_loop(), although we can
> specify the mode at creation time (avc_netlink_open()).
>
> Thanks,
>   


I have pushed this patch, thanks.



-- 

Eamon Walsh 
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

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

end of thread, other threads:[~2010-03-08 23:19 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-02-03  6:19 avc_open() and netlink_loop() KaiGai Kohei
2010-02-26 19:14 ` Eamon Walsh
2010-02-26 20:49   ` Eamon Walsh
2010-02-26 20:56     ` [PATCH] libselinux: fix avc_netlink_loop() error caused by nonblocking mode Eamon Walsh
2010-03-02  2:48       ` KaiGai Kohei
2010-03-02 17:29         ` Eamon Walsh
2010-03-03  0:30           ` KaiGai Kohei
2010-03-08 23:19             ` Eamon Walsh

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.