On 2015-03-06 15:48:26, Tyler Hicks wrote: > From: John Johansen <[email protected]> > > Signed-off-by: John Johansen <[email protected]> > [tyhicks: Forward ported patch to trunk] > [tyhicks: remove commented out code] > Signed-off-by: Tyler Hicks <[email protected]> > ---
Re-reviewed this one and it still looks good with the exception of one
minor issue noted below.
> parser/kernel_interface.c | 23 +++++++++++++++++++++++
> parser/kernel_interface.h | 1 +
> parser/parser_interface.c | 46 +++-------------------------------------------
> parser/parser_variable.c | 4 +---
> parser/parser_yacc.y | 9 ++++++++-
> parser/profile.h | 35 ++++++++++++++++++++++-------------
> 6 files changed, 58 insertions(+), 60 deletions(-)
>
> diff --git a/parser/kernel_interface.c b/parser/kernel_interface.c
> index 5585249..a33a6be 100644
> --- a/parser/kernel_interface.c
> +++ b/parser/kernel_interface.c
> @@ -204,3 +204,26 @@ int aa_load_buffer(int option, char *buffer, int size)
>
> return write_policy_buffer(fd, kernel_supports_setload, buffer, size);
> }
> +
> +/**
> + * aa_remove_profile - remove a profile from the kernel
> + * @fqname: the fully qualified name of the profile to remove
> + *
> + * Returns: 0 on success, -1 on error with errno set
> + */
> +int aa_remove_profile(const char *fqname)
> +{
> + autoclose int dirfd = -1;
> + autoclose int fd = -1;
> +
> + dirfd = open_iface_dir();
> + if (dirfd == -1)
> + return -1;
> +
> + fd = open_option_iface(dirfd, OPTION_REMOVE);
> + if (fd == -1)
> + return -1;
> +
> + /* include trailing \0 in buffer write */
> + return write_buffer(fd, fqname, strlen(fqname) + 1, 0);
> +}
> diff --git a/parser/kernel_interface.h b/parser/kernel_interface.h
> index 13e5996..3d53238 100644
> --- a/parser/kernel_interface.h
> +++ b/parser/kernel_interface.h
> @@ -21,5 +21,6 @@
>
> int aa_find_iface_dir(char **dir);
> int aa_load_buffer(int option, char *buffer, int size);
> +int aa_remove_profile(const char *fqname);
>
> #endif /* __AA_KERNEL_INTERFACE_H */
> diff --git a/parser/parser_interface.c b/parser/parser_interface.c
> index 895b2bc..6a9c918 100644
> --- a/parser/parser_interface.c
> +++ b/parser/parser_interface.c
> @@ -29,6 +29,7 @@
> #include <sstream>
>
> #include "lib.h"
> +#include "kernel_interface.h"
> #include "parser.h"
> #include "profile.h"
> #include "libapparmor_re/apparmor_re.h"
> @@ -467,9 +468,7 @@ void sd_serialize_top_profile(std::ostringstream &buf,
> Profile *profile)
> sd_write_name(buf, "version");
> sd_write_uint32(buf, version);
>
> - if (profile_ns) {
> - sd_write_string(buf, profile_ns, "namespace");
> - } else if (profile->ns) {
> + if (profile->ns) {
> sd_write_string(buf, profile->ns, "namespace");
> }
>
> @@ -523,49 +522,10 @@ int __sd_serialize_profile(int option, Profile *prof)
> error = 0;
>
> if (option == OPTION_REMOVE) {
> - char *name, *ns = NULL;
> - int len = 0;
> -
> - if (profile_ns) {
> - len += strlen(profile_ns) + 2;
> - ns = profile_ns;
> - } else if (prof->ns) {
> - len += strlen(prof->ns) + 2;
> - ns = prof->ns;
> - }
> - if (prof->parent) {
> - name = (char *) malloc(strlen(prof->name) + 3 +
> - strlen(prof->parent->name) + len);
> - if (!name) {
> - PERROR(_("Memory Allocation Error: Unable to
> remove ^%s\n"), prof->name);
> - error = -errno;
> - goto exit;
> - }
> - if (ns)
> - sprintf(name, ":%s:%s//%s", ns,
> - prof->parent->name, prof->name);
> - else
> - sprintf(name, "%s//%s", prof->parent->name,
> - prof->name);
> - } else if (ns) {
> - name = (char *) malloc(len + strlen(prof->name) + 1);
> - if (!name) {
> - PERROR(_("Memory Allocation Error: Unable to
> remove %s:%s."), ns, prof->name);
> - error = -errno;
> - goto exit;
> - }
> - sprintf(name, ":%s:%s", ns, prof->name);
> - } else {
> - name = prof->name;
> - }
> - size = strlen(name) + 1;
> if (kernel_load) {
> - wsize = write(fd, name, size);
> - if (wsize < 0)
> + if (aa_remove_profile(prof->fqname().c_str()) == -1)
> error = -errno;
> }
> - if (prof->parent || ns)
> - free(name);
> } else {
> sd_serialize_top_profile(work_area, prof);
>
> diff --git a/parser/parser_variable.c b/parser/parser_variable.c
> index 00807b7..e1f6543 100644
> --- a/parser/parser_variable.c
> +++ b/parser/parser_variable.c
> @@ -274,10 +274,8 @@ static int process_variables_in_rules(Profile &prof)
> int process_profile_variables(Profile *prof)
> {
> int error = 0, rc;
> - std::string *buf = prof->get_name(true);
>
> - error = new_set_var(PROFILE_NAME_VARIABLE, buf->c_str());
> - delete buf;
> + error = new_set_var(PROFILE_NAME_VARIABLE,
> prof->get_name(true).c_str());
>
> if (!error)
> error = process_variables_in_entries(prof->entries);
> diff --git a/parser/parser_yacc.y b/parser/parser_yacc.y
> index bec68ca..634e200 100644
> --- a/parser/parser_yacc.y
> +++ b/parser/parser_yacc.y
> @@ -347,7 +347,14 @@ profile: opt_profile_flag opt_ns profile_base
> if ($3->name[0] != '/' && !($1 || $2))
> yyerror(_("Profile names must begin with a '/',
> namespace or keyword 'profile' or 'hat'."));
>
> - prof->ns = $2;
> + if ($2 && profile_ns) {
> + free($2);
> + pwarn("%s: -n %s overriding policy specified namespace
> :%s:\n", progname, profile_ns, $2);
There's a use after free here. I'll just make the change in my local
branch instead of sending out another patch.
Tyler
> + prof->ns = strdup(profile_ns);
> + if (!prof->ns)
> + yyerror(_("Memory allocation error."));
> + } else
> + prof->ns = $2;
> if ($1 == 2)
> prof->flags.hat = 1;
> $$ = prof;
> diff --git a/parser/profile.h b/parser/profile.h
> index 063ab17..5adbbcf 100644
> --- a/parser/profile.h
> +++ b/parser/profile.h
> @@ -213,25 +213,34 @@ public:
>
> bool alloc_net_table();
>
> - std::string* get_name(bool fqp)
> + std::string hname(void)
> {
> - std::string *buf;
> - if (fqp && parent) {
> - buf = parent->get_name(fqp);
> - buf->append("//");
> - buf->append(name);
> - } else {
> - return new std::string(name);
> - }
> + if (!parent)
> + return name;
> +
> + return parent->hname() + "//" + name;
> + }
>
> - return buf;
> + /* assumes ns is set as part of profile creation */
> + std::string fqname(void)
> + {
> + if (parent)
> + return parent->fqname() + "://" + name;
> + else if (!ns)
> + return hname();
> + return ":" + std::string(ns) + "://" + hname();
> + }
> +
> + std::string get_name(bool fqp)
> + {
> + if (fqp)
> + return fqname();
> + return hname();
> }
>
> void dump_name(bool fqp)
> {
> - std::string *buf = get_name(fqp);;
> - cout << *buf;
> - delete buf;
> + cout << get_name(fqp);;
> }
> };
>
> --
> 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
