On 2015-03-06 15:48:25, Tyler Hicks wrote: > From: John Johansen <[email protected]> > > Signed-off-by: John Johansen <[email protected]> > [tyhicks: Handle inverted return from find_subdomainfs_mountpoint()] > [tyhicks: Link test progs to libapparmor to fix make check build fail] > [tyhicks: Migrate from opendir() to open() for opening apparmorfs] > [tyhicks: Make some of the split out functions static] > Signed-off-by: Tyler Hicks <[email protected]> > ---
Re-reviewed this one and it still looks good.
Tyler
> parser/Makefile | 12 ++-
> parser/kernel_interface.c | 206
> ++++++++++++++++++++++++++++++++++++++++++++++
> parser/kernel_interface.h | 25 ++++++
> parser/lib.c | 2 +-
> parser/lib.h | 2 +-
> parser/parser_interface.c | 83 -------------------
> parser/parser_main.c | 24 +++---
> 7 files changed, 252 insertions(+), 102 deletions(-)
> create mode 100644 parser/kernel_interface.c
> create mode 100644 parser/kernel_interface.h
>
> diff --git a/parser/Makefile b/parser/Makefile
> index 9175a30..9735ea4 100644
> --- a/parser/Makefile
> +++ b/parser/Makefile
> @@ -75,10 +75,10 @@ SRCS = parser_common.c parser_include.c
> parser_interface.c parser_lex.c \
> parser_yacc.c parser_regex.c parser_variable.c parser_policy.c \
> parser_alias.c common_optarg.c lib.c network.c \
> mount.cc dbus.cc profile.cc rule.cc signal.cc ptrace.cc \
> - af_rule.cc af_unix.cc features.c policy_cache.c
> + af_rule.cc af_unix.cc features.c policy_cache.c kernel_interface.c
> HDRS = parser.h parser_include.h immunix.h mount.h dbus.h lib.h profile.h \
> rule.h common_optarg.h signal.h ptrace.h network.h af_rule.h
> af_unix.h \
> - features.h policy_cache.h
> + features.h policy_cache.h kernel_interface.h
> TOOLS = apparmor_parser
>
> OBJECTS = $(patsubst %.cc, %.o, $(SRCS:.c=.o))
> @@ -119,6 +119,7 @@ TEST_OBJECTS = $(filter-out \
> policy_cache.o, ${OBJECTS}) \
> $(AAREOBJECTS)
> TEST_LDFLAGS = $(AARE_LDFLAGS)
> +TEST_LDLIBS = $(AALIB)
>
> ifdef V
> VERBOSE = 1
> @@ -242,6 +243,9 @@ features.o: features.c features.h parser.h
> libapparmor_re/apparmor_re.h
> policy_cache.o: policy_cache.c policy_cache.h parser.h
> $(CXX) $(EXTRA_CFLAGS) -c -o $@ $<
>
> +kernel_interface.o: kernel_interface.c kernel_interface.h
> + $(CXX) $(EXTRA_CFLAGS) -c -o $@ $<
> +
> lib.o: lib.c lib.h parser.h
> $(CXX) $(EXTRA_CFLAGS) -c -o $@ $<
>
> @@ -287,9 +291,9 @@ cap_names.h: /usr/include/linux/capability.h
> echo "$(CAPABILITIES)" | LC_ALL=C sed -n -e "s/[
> \\t]\\?CAP_\\([A-Z0-9_]\\+\\)/\{\"\\L\\1\", \\UCAP_\\1\},\\n/pg" > $@
>
> tst_lib: lib.c parser.h $(filter-out lib.o, ${TEST_OBJECTS})
> - $(CXX) $(TEST_CFLAGS) -o $@ $< $(filter-out $(<:.c=.o),
> ${TEST_OBJECTS}) $(TEST_LDFLAGS)
> + $(CXX) $(TEST_CFLAGS) -o $@ $< $(filter-out $(<:.c=.o),
> ${TEST_OBJECTS}) $(TEST_LDFLAGS) $(TEST_LDLIBS)
> tst_%: parser_%.c parser.h $(filter-out parser_%.o, ${TEST_OBJECTS})
> - $(CXX) $(TEST_CFLAGS) -o $@ $< $(filter-out $(<:.c=.o),
> ${TEST_OBJECTS}) $(TEST_LDFLAGS)
> + $(CXX) $(TEST_CFLAGS) -o $@ $< $(filter-out $(<:.c=.o),
> ${TEST_OBJECTS}) $(TEST_LDFLAGS) $(TEST_LDLIBS)
>
> .SILENT: check
> .PHONY: check
> diff --git a/parser/kernel_interface.c b/parser/kernel_interface.c
> new file mode 100644
> index 0000000..5585249
> --- /dev/null
> +++ b/parser/kernel_interface.c
> @@ -0,0 +1,206 @@
> +/*
> + * Copyright (c) 2014
> + * Canonical, Ltd. (All rights reserved)
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of version 2 of the GNU General Public
> + * License published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, contact Novell, Inc. or Canonical
> + * Ltd.
> + */
> +
> +#include <errno.h>
> +#include <fcntl.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <sys/stat.h>
> +#include <sys/types.h>
> +#include <sys/apparmor.h>
> +#include <unistd.h>
> +
> +#include "kernel_interface.h"
> +#include "lib.h"
> +#include "parser.h"
> +
> +#define DEFAULT_APPARMORFS "/sys/kernel/security/apparmor"
> +
> +/**
> + * aa_find_iface_dir - find where the apparmor interface is located
> + * @dir - RETURNs: stored location of interface director
> + *
> + * Returns: 0 on success, -1 with errno set if there is an error
> + */
> +int aa_find_iface_dir(char **dir)
> +{
> + if (aa_find_mountpoint(dir) == -1) {
> + struct stat buf;
> + if (stat(DEFAULT_APPARMORFS, &buf) == -1) {
> + return -1;
> + } else {
> + *dir = strdup(DEFAULT_APPARMORFS);
> + if (*dir == NULL)
> + return -1;
> + }
> + }
> +
> + return 0;
> +}
> +
> +/**
> + * open_iface_dir - open the apparmor interface dir
> + *
> + * Returns: opened file descriptor, or -1 with errno on error
> + */
> +static int open_iface_dir(void)
> +{
> + autofree char *dir = NULL;
> +
> + if (aa_find_iface_dir(&dir) == -1)
> + return -1;
> +
> + return open(dir, O_RDONLY | O_CLOEXEC | O_DIRECTORY);
> +}
> +
> +
> +/* bleah the kernel should just loop and do multiple load, but to support
> + * older systems we need to do this
> + */
> +#define PROFILE_HEADER_SIZE
> +static char header_version[] = "\x04\x08\x00version";
> +
> +static const char *next_profile_buffer(const char *buffer, int size)
> +{
> + const char *b = buffer;
> +
> + for (; size - sizeof(header_version); b++, size--) {
> + if (memcmp(b, header_version, sizeof(header_version)) == 0) {
> + return b;
> + }
> + }
> + return NULL;
> +}
> +
> +static int write_buffer(int fd, const char *buffer, int size, int set)
> +{
> + const char *err_str = set ? "profile set" : "profile";
> + int wsize = write(fd, buffer, size);
> + if (wsize < 0) {
> + PERROR(_("%s: Unable to write %s\n"), progname, err_str);
> + return -1;
> + } else if (wsize < size) {
> + PERROR(_("%s: Unable to write %s\n"), progname, err_str);
> + errno = EPROTO;
> + return -1;
> + }
> + return 0;
> +}
> +
> +/**
> + * write_policy_buffer - load compiled policy into the kernel
> + * @fd: kernel iterface to write to
> + * @atomic: whether to load all policy in buffer atomically (true)
> + * @buffer: buffer of policy to load
> + * @size: the size of the data in the buffer
> + *
> + * Returns: 0 if the buffer loaded correctly
> + * -1 if the load failed with errno set to the error
> + *
> + * @atomic should only be set to true if the kernel supports atomic profile
> + * set loads, otherwise only the 1st profile in the buffer will be loaded
> + * (older kernels only support loading one profile at a time).
> + */
> +static int write_policy_buffer(int fd, int atomic,
> + const char *buffer, size_t size)
> +{
> + size_t bsize;
> + int rc;
> +
> + if (atomic) {
> + rc = write_buffer(fd, buffer, size, true);
> + } else {
> + const char *b, *next;
> +
> + rc = 0; /* in case there are no profiles */
> + for (b = buffer; b; b = next, size -= bsize) {
> + next = next_profile_buffer(b + sizeof(header_version),
> + size);
> + if (next)
> + bsize = next - b;
> + else
> + bsize = size;
> + if (write_buffer(fd, b, bsize, false) == -1)
> + return -1;
> + }
> + }
> +
> + if (rc)
> + return -1;
> +
> + return 0;
> +}
> +
> +/**
> + * open_option_iface - open the interface file for @option
> + * @aadir: apparmorfs dir
> + * @option: load command option
> + *
> + * Returns: fd to interface or -1 on error, with errno set.
> + */
> +static int open_option_iface(int aadir, int option)
> +{
> + const char *name;
> +
> + switch (option) {
> + case OPTION_ADD:
> + name = ".load";
> + break;
> + case OPTION_REPLACE:
> + name = ".replace";
> + break;
> + case OPTION_REMOVE:
> + name = ".remove";
> + break;
> + default:
> + errno = EINVAL;
> + return -1;
> + }
> +
> + return openat(aadir, name, O_WRONLY);
> +
> + /* TODO: push up */
> + /*
> + if (fd < 0) {
> + PERROR(_("Unable to open %s - %s\n"), filename,
> + strerror(errno));
> + return -errno;
> + }
> + */
> +}
> +
> +int aa_load_buffer(int option, char *buffer, int size)
> +{
> + autoclose int dirfd = -1;
> + autoclose int fd = -1;
> +
> + /* TODO: push backup into caller */
> + if (!kernel_load)
> + return 0;
> +
> + dirfd = open_iface_dir();
> + if (dirfd == -1)
> + return -1;
> +
> + fd = open_option_iface(dirfd, option);
> + if (fd == -1)
> + return -1;
> +
> + return write_policy_buffer(fd, kernel_supports_setload, buffer, size);
> +}
> diff --git a/parser/kernel_interface.h b/parser/kernel_interface.h
> new file mode 100644
> index 0000000..13e5996
> --- /dev/null
> +++ b/parser/kernel_interface.h
> @@ -0,0 +1,25 @@
> +/*
> + * Copyright (c) 2014
> + * Canonical, Ltd. (All rights reserved)
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of version 2 of the GNU General Public
> + * License published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, contact Novell, Inc. or Canonical
> + * Ltd.
> + */
> +
> +#ifndef __AA_KERNEL_INTERFACE_H
> +#define __AA_KERNEL_INTERFACE_H
> +
> +int aa_find_iface_dir(char **dir);
> +int aa_load_buffer(int option, char *buffer, int size);
> +
> +#endif /* __AA_KERNEL_INTERFACE_H */
> diff --git a/parser/lib.c b/parser/lib.c
> index 5340971..9dbc154 100644
> --- a/parser/lib.c
> +++ b/parser/lib.c
> @@ -174,7 +174,7 @@ fail:
> *
> * Returns: true if an octal digit, else false
> */
> -bool isodigit(char c)
> +int isodigit(char c)
> {
> return (c >= '0' && c <= '7') ? true : false;
> }
> diff --git a/parser/lib.h b/parser/lib.h
> index f047fd8..4cdf428 100644
> --- a/parser/lib.h
> +++ b/parser/lib.h
> @@ -13,7 +13,7 @@ void __autofclose(FILE **f);
> int dirat_for_each(DIR *dir, const char *name, void *data,
> int (* cb)(DIR *, const char *, struct stat *, void *));
>
> -bool isodigit(char c);
> +int isodigit(char c);
> long strntol(const char *str, const char **endptr, int base, long maxval,
> size_t n);
> int strn_escseq(const char **pos, const char *chrs, size_t n);
> diff --git a/parser/parser_interface.c b/parser/parser_interface.c
> index 7829b22..895b2bc 100644
> --- a/parser/parser_interface.c
> +++ b/parser/parser_interface.c
> @@ -604,86 +604,3 @@ exit:
> return error;
> }
>
> -/* bleah the kernel should just loop and do multiple load, but to support
> - * older systems we need to do this
> - */
> -#define PROFILE_HEADER_SIZE
> -static char header_version[] = "\x04\x08\x00version";
> -
> -static char *next_profile_buffer(char *buffer, int size)
> -{
> - char *b = buffer;
> -
> - for (; size - sizeof(header_version); b++, size--) {
> - if (memcmp(b, header_version, sizeof(header_version)) == 0) {
> - return b;
> - }
> - }
> - return NULL;
> -}
> -
> -static int write_buffer(int fd, char *buffer, int size, bool set)
> -{
> - const char *err_str = set ? "profile set" : "profile";
> - int wsize = write(fd, buffer, size);
> - if (wsize < 0) {
> - PERROR(_("%s: Unable to write %s\n"), progname, err_str);
> - return -errno;
> - } else if (wsize < size) {
> - PERROR(_("%s: Unable to write %s\n"), progname, err_str);
> - return -EPROTO;
> - }
> - return 0;
> -}
> -
> -int sd_load_buffer(int option, char *buffer, int size)
> -{
> - autoclose int fd = -1;
> - int error, bsize;
> - autofree char *filename = NULL;
> -
> - /* TODO: push backup into caller */
> - if (!kernel_load)
> - return 0;
> -
> - switch (option) {
> - case OPTION_ADD:
> - if (asprintf(&filename, "%s/.load", subdomainbase) == -1)
> - return -ENOMEM;
> - break;
> - case OPTION_REPLACE:
> - if (asprintf(&filename, "%s/.replace", subdomainbase) == -1)
> - return -ENOMEM;
> - break;
> - default:
> - return -EINVAL;
> - }
> -
> - fd = open(filename, O_WRONLY);
> - if (fd < 0) {
> - PERROR(_("Unable to open %s - %s\n"), filename,
> - strerror(errno));
> - return -errno;
> - }
> -
> - if (kernel_supports_setload) {
> - error = write_buffer(fd, buffer, size, true);
> - } else {
> - char *b, *next;
> -
> - error = 0; /* in case there are no profiles */
> - for (b = buffer; b; b = next, size -= bsize) {
> - next = next_profile_buffer(b + sizeof(header_version),
> - size);
> - if (next)
> - bsize = next - b;
> - else
> - bsize = size;
> - error = write_buffer(fd, b, bsize, false);
> - if (error)
> - break;
> - }
> - }
> -
> - return error;
> -}
> diff --git a/parser/parser_main.c b/parser/parser_main.c
> index 6850086..3358373 100644
> --- a/parser/parser_main.c
> +++ b/parser/parser_main.c
> @@ -41,6 +41,7 @@
>
> #include "lib.h"
> #include "features.h"
> +#include "kernel_interface.h"
> #include "parser.h"
> #include "parser_version.h"
> #include "parser_include.h"
> @@ -50,7 +51,6 @@
>
> #define OLD_MODULE_NAME "subdomain"
> #define PROC_MODULES "/proc/modules"
> -#define DEFAULT_APPARMORFS "/sys/kernel/security/" MODULE_NAME
> #define MATCH_FILE "/sys/kernel/security/" MODULE_NAME "/matching"
> #define MOUNTED_FS "/proc/mounts"
> #define AADFA "pattern=aadfa"
> @@ -513,18 +513,14 @@ static int process_config_file(const char *name)
>
> int find_subdomainfs_mountpoint(void)
> {
> - if (aa_find_mountpoint(&subdomainbase) == -1) {
> - struct stat buf;
> - if (stat(DEFAULT_APPARMORFS, &buf) == -1) {
> + if (aa_find_iface_dir(&subdomainbase) == -1) {
> PERROR(_("Warning: unable to find a suitable fs in %s, is it "
> "mounted?\nUse --subdomainfs to override.\n"),
> MOUNTED_FS);
> - } else {
> - subdomainbase = strdup(DEFAULT_APPARMORFS);
> - }
> + return false;
> }
>
> - return (subdomainbase == NULL);
> + return true;
> }
>
> int have_enough_privilege(void)
> @@ -643,9 +639,11 @@ int process_binary(int option, const char *profilename)
> size += rsize;
> } while (rsize > 0);
>
> - if (rsize == 0)
> - retval = sd_load_buffer(option, buffer, size);
> - else
> + if (rsize == 0) {
> + retval = aa_load_buffer(option, buffer, size);
> + if (retval == -1)
> + retval = -errno;
> + } else
> retval = rsize;
>
> if (conf_verbose) {
> @@ -988,8 +986,8 @@ int main(int argc, char *argv[])
>
> /* Check to make sure there is an interface to load policy */
> if (!(UNPRIVILEGED_OPS) && (subdomainbase == NULL) &&
> - (retval = find_subdomainfs_mountpoint())) {
> - return retval;
> + !find_subdomainfs_mountpoint()) {
> + return 1;
> }
>
> if (!binary_input) parse_default_paths();
> --
> 2.1.4
>
>
> --
> AppArmor mailing list
> [email protected]
> Modify settings or unsubscribe at:
> https://lists.ubuntu.com/mailman/listinfo/apparmor
signature.asc
Description: Digital signature
-- AppArmor mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
