linux-kernel - Re: [PATCH] mm: fix hugetlb bug due to user_shm

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <8bd0f97a0909110703o4d496a45jddc0d7d6fd8674b4@mail.gmail.com>
Date:	Fri, 11 Sep 2009 10:03:28 -0400
From:	Mike Frysinger <vapier.adi@...il.com>
To:	Hugh Dickins <hugh.dickins@...cali.co.uk>
Cc:	Linus Torvalds <torvalds@...ux-foundation.org>,
	Stefan Huber <shuber2@...il.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Peter Meerwald <pmeerw@...y.sbg.ac.at>,
	James Morris <jmorris@...ei.org>,
	William Irwin <wli@...ementarian.org>,
	Mel Gorman <mel@....ul.ie>,
	Ravikiran G Thirumalai <kiran@...lex86.org>,
	linux-kernel@...r.kernel.org, linux-mm@...ck.org
Subject: Re: [PATCH] mm: fix hugetlb bug due to user_shm_unlock call

On Mon, Aug 24, 2009 at 11:30, Hugh Dickins wrote:
> --- 2.6.31-rc7/ipc/shm.c        2009-06-25 05:18:09.000000000 +0100
> +++ linux/ipc/shm.c     2009-08-24 16:06:30.000000000 +0100
> @@ -174,7 +174,7 @@ static void shm_destroy(struct ipc_names
>        shm_unlock(shp);
>        if (!is_file_hugepages(shp->shm_file))
>                shmem_lock(shp->shm_file, 0, shp->mlock_user);
> -       else
> +       else if (shp->mlock_user)
>                user_shm_unlock(shp->shm_file->f_path.dentry->d_inode->i_size,
>                                                shp->mlock_user);
>        fput (shp->shm_file);
> @@ -369,8 +369,8 @@ static int newseg(struct ipc_namespace *
>                /* hugetlb_file_setup applies strict accounting */
>                if (shmflg & SHM_NORESERVE)
>                        acctflag = VM_NORESERVE;
> -               file = hugetlb_file_setup(name, size, acctflag);
> -               shp->mlock_user = current_user();
> +               file = hugetlb_file_setup(name, size, acctflag,
> +                                                       &shp->mlock_user);
>        } else {
>                /*
>                 * Do not allow no accounting for OVERCOMMIT_NEVER, even
> @@ -410,6 +410,8 @@ static int newseg(struct ipc_namespace *
>        return error;
>
>  no_id:
> +       if (shp->mlock_user)    /* shmflg & SHM_HUGETLB case */
> +               user_shm_unlock(size, shp->mlock_user);
>        fput(file);
>  no_file:
>        security_shm_free(shp);

this breaks on no-mmu systems due to user_shm_unlock() being
mmu-specific.  normally gcc is smart enough to do dead code culling so
it hasnt caused problems, but not here.  hugetlb support is not
available on no-mmu systems, so the stubbed hugepage functions prevent
calls to user_shm_unlock() and such, but here gcc cant figure it out:

static int newseg(struct ipc_namespace *ns, struct ipc_params *params)
{
...
    shp->mlock_user = NULL;
...
    if (shmflg & SHM_HUGETLB) {
        /* hugetlb_file_setup applies strict accounting */
        if (shmflg & SHM_NORESERVE)
            acctflag = VM_NORESERVE;
        file = hugetlb_file_setup(name, size, acctflag,
                            &shp->mlock_user);
...
    id = ipc_addid(&shm_ids(ns), &shp->shm_perm, ns->shm_ctlmni);
    if (id < 0) {
        error = id;
        goto no_id;
    }
...
no_id:
    if (shp->mlock_user)    /* shmflg & SHM_HUGETLB case */
        user_shm_unlock(size, shp->mlock_user);
...

hugetlb_file_setup() expands to nothing and so mlock_user will never
come back from NULL, but gcc still emits a reference to
user_shm_unlock() in the error path.  perhaps the best thing here is
to just add an #ifdef ?
 no_id:
+#ifdef CONFIG_HUGETLB_PAGE
+    /* gcc isn't smart enough to see that mlock_user goes non-NULL
only by hugetlb */
    if (shp->mlock_user)    /* shmflg & SHM_HUGETLB case */
        user_shm_unlock(size, shp->mlock_user);
+#endif
-mike
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/