linux-kernel - Re: [PATCH] ipc/shm: properly return EIDRM in 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]

Date:   Tue, 25 Sep 2018 14:00:03 +0200
From:   Arnd Bergmann <arnd@...db.de>
To:     Davidlohr Bueso <dave@...olabs.net>
Cc:     rong.a.chen@...el.com, Stephen Rothwell <sfr@...b.auug.org.au>,
        dbueso@...e.de, manfred@...orfullife.com,
        Dmitry Vyukov <dvyukov@...gle.com>,
        Herbert Xu <herbert@...dor.apana.org.au>,
        Kees Cook <keescook@...omium.org>,
        Michael Kerrisk <mtk.manpages@...il.com>,
        Michal Hocko <mhocko@...e.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        LKP <lkp@...org>, ltp@...ts.linux.it
Subject: Re: [PATCH] ipc/shm: properly return EIDRM in shm_lock()

On Fri, Aug 24, 2018 at 5:09 AM Davidlohr Bueso <dave@...olabs.net> wrote:
>
> When getting rid of the general ipc_lock(), this was missed
> furthermore, making the comment around the ipc object validity
> check bogus. Under EIDRM conditions, callers will in turn not
> see the error and continue with the operation.
>
> Fixes: 82061c57ce9 (ipc: drop ipc_lock())
> Signed-off-by: Davidlohr Bueso <dbueso@...e.de>
> ---

Oddly, this change introduces a gcc warning in some configurations
(i.e. with randstruct enabled):

ipc/shm.c: In function 'shm_lock':
ipc/shm.c:209:9: note: randstruct: casting between randomized
structure pointer types (ssa): 'struct shmid_kernel' and 'struct
kern_ipc_perm'
  return (void *)ipcp;
         ^~~~~~~~~~~~

Not sure why we didn't see that warning before, probably
it ended up making its own thing when the return code
was uninitialized.

The change below gets rid of the warning, but is a bit ugly.

       Arnd

diff --git a/ipc/shm.c b/ipc/shm.c
index fe3c42e66a48..922012a745e5 100644
--- a/ipc/shm.c
+++ b/ipc/shm.c
@@ -180,10 +180,12 @@ static inline struct shmid_kernel
*shm_obtain_object_check(struct ipc_namespace
 static inline struct shmid_kernel *shm_lock(struct ipc_namespace *ns, int id)
 {
        struct kern_ipc_perm *ipcp;
+       int ret;

        rcu_read_lock();
        ipcp = ipc_obtain_object_idr(&shm_ids(ns), id);
-       if (IS_ERR(ipcp))
+       ret = PTR_ERR_OR_ZERO(ipcp);
+       if (ret)
                goto err;

        ipc_lock_object(ipcp);
@@ -199,14 +201,14 @@ static inline struct shmid_kernel
*shm_lock(struct ipc_namespace *ns, int id)
        }

        ipc_unlock_object(ipcp);
-       ipcp = ERR_PTR(-EIDRM);
+       ret = -EIDRM;
 err:
        rcu_read_unlock();
        /*
         * Callers of shm_lock() must validate the status of the returned ipc
         * object pointer and error out as appropriate.
         */
-       return (void *)ipcp;
+       return ERR_PTR(ret);
 }

 static inline void shm_lock_by_ptr(struct shmid_kernel *ipcp)