On Mon, Sep 15, 2014 at 02:55:58PM -0500, Tyler Hicks wrote:
> Update unix_socket and unix_socket_client to use setsockopt() in order
> to set send and receive timeouts for socket IO operations. This takes
> the place of poll(). Poll() was not being used for all potentially
> blocking socket operations which could have resulted in test cases
> blocking infinitely.
> 
> This also has the nice side effect of using getsockopt() and
> setsockopt(). These are AppArmor mediation points in kernel ABI v7 so it
> is worthwhile to test the calls while under confinement.
> 
> This patch updates the existing v7 policy generation to allow the getopt
> and setopt accesses.
> 
> Signed-off-by: Tyler Hicks <tyhi...@canonical.com>
> Acked-by: Seth Arnold <seth.arn...@canonical.com>

Acked-by: Steve Beattie <st...@nxnw.org>

Not necessary before committing, but can you shove
get_set_sock_io_timeo() into a socket_common.h header or somesuch, to
reduce code duplication?

> ---
>  tests/regression/apparmor/unix_socket.c           | 43 
> +++++++++++++++++------
>  tests/regression/apparmor/unix_socket_client.c    | 34 ++++++++++++++++++
>  tests/regression/apparmor/unix_socket_pathname.sh |  4 ++-
>  3 files changed, 69 insertions(+), 12 deletions(-)
> 
> diff --git a/tests/regression/apparmor/unix_socket.c 
> b/tests/regression/apparmor/unix_socket.c
> index cd492e3..34d1b9e 100644
> --- a/tests/regression/apparmor/unix_socket.c
> +++ b/tests/regression/apparmor/unix_socket.c
> @@ -14,7 +14,6 @@
>   * along with this program; if not, contact Canonical Ltd.
>   */
>  
> -#include <poll.h>
>  #include <stdio.h>
>  #include <stdlib.h>
>  #include <string.h>
> @@ -81,10 +80,39 @@ static int connectionless_messaging(int sock, char 
> *msg_buf, size_t msg_buf_len)
>       return 0;
>  }
>  
> +static int get_set_sock_io_timeo(int sock)
> +{
> +     struct timeval tv;
> +     socklen_t tv_len = sizeof(tv);
> +     int rc;
> +
> +     rc = getsockopt(sock, SOL_SOCKET, SO_RCVTIMEO, &tv, &tv_len);
> +     if (rc == -1) {
> +             perror("FAIL - getsockopt");
> +             return 1;
> +     }
> +
> +     tv.tv_sec = 1;
> +     tv.tv_usec = 0;
> +
> +     rc = setsockopt(sock, SOL_SOCKET, SO_RCVTIMEO, &tv, tv_len);
> +     if (rc == -1) {
> +             perror("FAIL - setsockopt (SO_RCVTIMEO)");
> +             return 1;
> +     }
> +
> +     rc = setsockopt(sock, SOL_SOCKET, SO_SNDTIMEO, &tv, tv_len);
> +     if (rc == -1) {
> +             perror("FAIL - setsockopt (SO_SNDTIMEO)");
> +             return 1;
> +     }
> +
> +     return 0;
> +}
> +
>  int main (int argc, char *argv[])
>  {
>       struct sockaddr_un addr;
> -     struct pollfd pfd;
>       char msg_buf[MSG_BUF_MAX];
>       size_t msg_buf_len;
>       const char *sun_path;
> @@ -172,16 +200,9 @@ int main (int argc, char *argv[])
>               exit(1);
>       }
>  
> -     pfd.fd = sock;
> -     pfd.events = POLLIN;
> -     rc = poll(&pfd, 1, 500);
> -     if (rc < 0) {
> -             perror("FAIL - poll");
> -             exit(1);
> -     } else if (!rc) {
> -             fprintf(stderr, "FAIL - poll timed out\n");
> +     rc = get_set_sock_io_timeo(sock);
> +     if (rc)
>               exit(1);
> -     }
>  
>       rc = (type & SOCK_STREAM || type & SOCK_SEQPACKET) ?
>               connection_based_messaging(sock, msg_buf, msg_buf_len) :
> diff --git a/tests/regression/apparmor/unix_socket_client.c 
> b/tests/regression/apparmor/unix_socket_client.c
> index d7d5510..bda7db6 100644
> --- a/tests/regression/apparmor/unix_socket_client.c
> +++ b/tests/regression/apparmor/unix_socket_client.c
> @@ -78,6 +78,36 @@ static int connectionless_messaging(int sock)
>       return 0;
>  }
>  
> +static int get_set_sock_io_timeo(int sock)
> +{
> +     struct timeval tv;
> +     socklen_t tv_len = sizeof(tv);
> +     int rc;
> +
> +     rc = getsockopt(sock, SOL_SOCKET, SO_RCVTIMEO, &tv, &tv_len);
> +     if (rc == -1) {
> +             perror("FAIL - getsockopt");
> +             return 1;
> +     }
> +
> +     tv.tv_sec = 1;
> +     tv.tv_usec = 0;
> +
> +     rc = setsockopt(sock, SOL_SOCKET, SO_RCVTIMEO, &tv, tv_len);
> +     if (rc == -1) {
> +             perror("FAIL - setsockopt (SO_RCVTIMEO)");
> +             return 1;
> +     }
> +
> +     rc = setsockopt(sock, SOL_SOCKET, SO_SNDTIMEO, &tv, tv_len);
> +     if (rc == -1) {
> +             perror("FAIL - setsockopt (SO_SNDTIMEO)");
> +             return 1;
> +     }
> +
> +     return 0;
> +}
> +
>  int main(int argc, char *argv[])
>  {
>       struct sockaddr_un peer_addr;
> @@ -131,6 +161,10 @@ int main(int argc, char *argv[])
>               exit(1);
>       }
>  
> +     rc = get_set_sock_io_timeo(sock);
> +     if (rc)
> +             exit(1);
> +
>       rc = connect(sock, (struct sockaddr *)&peer_addr,
>                    sun_path_len + sizeof(peer_addr.sun_family));
>       if (rc < 0) {
> diff --git a/tests/regression/apparmor/unix_socket_pathname.sh 
> b/tests/regression/apparmor/unix_socket_pathname.sh
> index 45d74b9..17ed855 100755
> --- a/tests/regression/apparmor/unix_socket_pathname.sh
> +++ b/tests/regression/apparmor/unix_socket_pathname.sh
> @@ -46,9 +46,11 @@ if [ "$(have_features policy/versions/v7)" == "true" ] ; 
> then
>  fi
>  
>  # af_unix support requires 'unix create' to call socket()
> +# af_unix support requires 'unix getopt' to call getsockopt()
> +# af_unix support requires 'unix setopt' to call setsockopt()
>  af_unix=
>  if [ "$(have_features network/af_unix)" == "true" ] ; then
> -     af_unix="unix:create"
> +     af_unix="unix:(create,getopt,setopt)"
>  fi
>  
>  okclient=rw

-- 
Steve Beattie
<sbeat...@ubuntu.com>
http://NxNW.org/~steve/

Attachment: signature.asc
Description: Digital signature

-- 
AppArmor mailing list
AppArmor@lists.ubuntu.com
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/apparmor

Reply via email to