| Message ID | 20180824030920.GD3677@linux-r8p5 |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | ipc/shm: properly return EIDRM in shm_lock() | expand |
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)
On Tue, Sep 25, 2018 at 5:00 AM, Arnd Bergmann <arnd@arndb.de> wrote: > 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 fix is already queued up in mmotm: https://www.ozlabs.org/~akpm/mmotm/broken-out/ipc-shm-use-err_cast-for-shm_lock-error-return.patch randstruct stays quiet about ERR_PTR-family casts since they're not "real" casts to a functional struct. -Kees -Kees
diff --git a/ipc/shm.c b/ipc/shm.c index b0eb3757ab89..4cd402e4cfeb 100644 --- a/ipc/shm.c +++ b/ipc/shm.c @@ -199,6 +199,7 @@ static inline struct shmid_kernel *shm_lock(struct ipc_namespace *ns, int id) } ipc_unlock_object(ipcp); + ipcp = ERR_PTR(-EIDRM); err: rcu_read_unlock(); /*
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> --- ipc/shm.c | 1 + 1 file changed, 1 insertion(+)