Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20180823024051.GC13343@shao2-debian> <20180823041434.GC3677@linux-r8p5>
 <20180824030920.GD3677@linux-r8p5>
In-Reply-To: <20180824030920.GD3677@linux-r8p5>
From:   Arnd Bergmann <arnd@arndb.de>
Date:   Tue, 25 Sep 2018 14:00:03 +0200
Message-ID: <CAK8P3a3TMUbqLtde-wJyvF+H31VTPQwCKCqPjf41dwGMK-vbQg@mail.gmail.com>
Subject: Re: [PATCH] ipc/shm: properly return EIDRM in shm_lock()
To:     Davidlohr Bueso <dave@stgolabs.net>
Cc:     rong.a.chen@intel.com, Stephen Rothwell <sfr@canb.auug.org.au>,
        dbueso@suse.de, manfred@colorfullife.com,
        Dmitry Vyukov <dvyukov@google.com>,
        Herbert Xu <herbert@gondor.apana.org.au>,
        Kees Cook <keescook@chromium.org>,
        Michael Kerrisk <mtk.manpages@gmail.com>,
        Michal Hocko <mhocko@suse.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        LKP <lkp@01.org>, ltp@lists.linux.it
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, Aug 24, 2018 at 5:09 AM Davidlohr Bueso <dave@stgolabs.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@suse.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)