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@xxxxxxxxxxxxx>
--
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@xxxxxxxxxxxxx with
the words "unsubscribe selinux" without quotes as the message.
