lkml.org 
[lkml]   [2024]   [Jan]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [6.8-rc1 Regression] Unable to exec apparmor_parser from virt-aa-helper
    On Wed, Jan 24, 2024 at 08:35:29AM -0800, Kees Cook wrote:
    > On Wed, Jan 24, 2024 at 09:19:54AM -0700, Kevin Locke wrote:
    > > Hello Linux developers,
    > >
    > > Using AppArmor 3.0.12 and libvirt 10.0.0 (from Debian packages) with
    > > Linux 6.8-rc1 (unpatched), I'm unable to start KVM domains due to
    > > AppArmor errors. Everything works fine on Linux 6.7. After attempting
    > > to start a domain, syslog contains:
    > >
    > > libvirtd[38705]: internal error: Child process (LIBVIRT_LOG_OUTPUTS=3:stderr /usr/lib/libvirt/virt-aa-helper -c -u libvirt-4fad83ef-4285-4cf5-953c-5c13d943c1fb) unexpected exit status 1: virt-aa-helper: error: apparmor_parser exited with error
    > > libvirtd[38705]: internal error: cannot load AppArmor profile 'libvirt-4fad83ef-4285-4cf5-953c-5c13d943c1fb'
    > >
    > > dmesg contains the additional message:
    > >
    > > audit: type=1400 audit(1706112657.438:74): apparmor="DENIED" operation="open" class="file" profile="virt-aa-helper" name="/usr/sbin/apparmor_parser" pid=6333 comm="virt-aa-helper" requested_mask="r" denied_mask="r" fsuid=0 ouid=0
    >
    > Oh, yikes. This means the LSM lost the knowledge that this open is an
    > _exec_, not a _read_.
    >
    > I will starting looking at this. John might be able to point me in the
    > right direction more quickly, though.

    Here's a possible patch for in_execve. Can you test this? I'm going to
    also examine switching to FMODE_EXEC ... I think I know why this wasn't
    done in the past, but I have to check the history...


    diff --git a/fs/exec.c b/fs/exec.c
    index 39d773021fff..ddd0fa2e84a7 100644
    --- a/fs/exec.c
    +++ b/fs/exec.c
    @@ -1505,7 +1505,7 @@ static int prepare_bprm_creds(struct linux_binprm *bprm)
    /* Matches do_open_execat() */
    static void do_close_execat(struct file *file)
    {
    - if (!file)
    + if (IS_ERR_OR_NULL(file))
    return;
    allow_write_access(file);
    fput(file);
    @@ -1530,23 +1530,30 @@ static void free_bprm(struct linux_binprm *bprm)
    kfree(bprm->interp);
    kfree(bprm->fdpath);
    kfree(bprm);
    + current->in_execve = 0;
    }

    static struct linux_binprm *alloc_bprm(int fd, struct filename *filename, int flags)
    {
    - struct linux_binprm *bprm;
    - struct file *file;
    + struct linux_binprm *bprm = NULL;
    + struct file *file = NULL;
    int retval = -ENOMEM;

    + /*
    + * Mark this "open" as an exec attempt for the LSMs. We reset
    + * it in bprm_free() (and our common error path below).
    + */
    + current->in_execve = 1;
    +
    file = do_open_execat(fd, filename, flags);
    - if (IS_ERR(file))
    - return ERR_CAST(file);
    + if (IS_ERR(file)) {
    + retval = PTR_ERR(file);
    + goto out_cleanup;
    + }

    bprm = kzalloc(sizeof(*bprm), GFP_KERNEL);
    - if (!bprm) {
    - do_close_execat(file);
    - return ERR_PTR(-ENOMEM);
    - }
    + if (!bprm)
    + goto out_cleanup;

    bprm->file = file;

    @@ -1559,7 +1566,7 @@ static struct linux_binprm *alloc_bprm(int fd, struct filename *filename, int fl
    bprm->fdpath = kasprintf(GFP_KERNEL, "/dev/fd/%d/%s",
    fd, filename->name);
    if (!bprm->fdpath)
    - goto out_free;
    + goto out_cleanup;

    /*
    * Record that a name derived from an O_CLOEXEC fd will be
    @@ -1581,8 +1588,11 @@ static struct linux_binprm *alloc_bprm(int fd, struct filename *filename, int fl
    if (!retval)
    return bprm;

    -out_free:
    - free_bprm(bprm);
    +out_cleanup:
    + if (bprm)
    + free_bprm(bprm);
    + do_close_execat(file);
    + current->in_execve = 0;
    return ERR_PTR(retval);
    }

    @@ -1633,6 +1643,7 @@ static void check_unsafe_exec(struct linux_binprm *bprm)
    }
    rcu_read_unlock();

    + /* "users" and "in_exec" locked for copy_fs() */
    if (p->fs->users > n_fs)
    bprm->unsafe |= LSM_UNSAFE_SHARE;
    else
    @@ -1863,7 +1874,6 @@ static int bprm_execve(struct linux_binprm *bprm)
    * where setuid-ness is evaluated.
    */
    check_unsafe_exec(bprm);
    - current->in_execve = 1;
    sched_mm_cid_before_execve(current);

    sched_exec();
    @@ -1880,7 +1890,6 @@ static int bprm_execve(struct linux_binprm *bprm)
    sched_mm_cid_after_execve(current);
    /* execve succeeded */
    current->fs->in_exec = 0;
    - current->in_execve = 0;
    rseq_execve(current);
    user_events_execve(current);
    acct_update_integrals(current);
    @@ -1899,7 +1908,6 @@ static int bprm_execve(struct linux_binprm *bprm)

    sched_mm_cid_after_execve(current);
    current->fs->in_exec = 0;
    - current->in_execve = 0;

    return retval;
    }
    diff --git a/kernel/fork.c b/kernel/fork.c
    index 47ff3b35352e..0d944e92a43f 100644
    --- a/kernel/fork.c
    +++ b/kernel/fork.c
    @@ -1748,6 +1748,7 @@ static int copy_fs(unsigned long clone_flags, struct task_struct *tsk)
    if (clone_flags & CLONE_FS) {
    /* tsk->fs is already what we want */
    spin_lock(&fs->lock);
    + /* "users" and "in_exec" locked for check_unsafe_exec() */
    if (fs->in_exec) {
    spin_unlock(&fs->lock);
    return -EAGAIN;
    --
    Kees Cook

    \
     
     \ /
      Last update: 2024-05-27 14:32    [W:5.941 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site