Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754904AbbKMT7F (ORCPT ); Fri, 13 Nov 2015 14:59:05 -0500 Received: from mx2.suse.de ([195.135.220.15]:42907 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754594AbbKMT7C (ORCPT ); Fri, 13 Nov 2015 14:59:02 -0500 Date: Fri, 13 Nov 2015 11:58:53 -0800 From: Davidlohr Bueso To: "Kirill A. Shutemov" , "Kirill A. Shutemov" , Andrew Morton , linux-kernel@vger.kernel.org, linux-mm@kvack.org, Dmitry Vyukov , Manfred Spraul Subject: Re: [PATCH, RESEND] ipc/shm: handle removed segments gracefully in shm_mmap() Message-ID: <20151113195853.GD3502@linux-uzut.site> Mail-Followup-To: "Kirill A. Shutemov" , "Kirill A. Shutemov" , Andrew Morton , linux-kernel@vger.kernel.org, linux-mm@kvack.org, Dmitry Vyukov , Manfred Spraul References: <1447232220-36879-1-git-send-email-kirill.shutemov@linux.intel.com> <20151111170347.GA3502@linux-uzut.site> <20151111195023.GA17310@node.shutemov.name> <20151113053137.GB3502@linux-uzut.site> <20151113091259.GB28904@node.shutemov.name> <20151113192310.GC3502@linux-uzut.site> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Disposition: inline In-Reply-To: <20151113192310.GC3502@linux-uzut.site> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2514 Lines: 73 On Fri, 13 Nov 2015, Bueso wrote: >So considering EINVAL, even your approach to bumping up nattach by calling >_shm_open earlier isn't enough. Races exposed to user called rmid can still >occur between dropping the lock and doing ->mmap(). Ultimately this leads to >all ipc_valid_object() checks, as we totally ignore SHM_DEST segments nowadays >since we forbid mapping previously removed segments. > >I think this is the first thing we must decide before going forward with this >mess. ipc currently defines invalid objects by merely checking the deleted flag. Particularly something like this, which we could then add to the vma validity check, thus saving the lookup as well. diff --git a/ipc/shm.c b/ipc/shm.c index 4178727..d9b2fb1 100644 --- a/ipc/shm.c +++ b/ipc/shm.c @@ -64,6 +64,20 @@ static const struct vm_operations_struct shm_vm_ops; #define shm_unlock(shp) \ ipc_unlock(&(shp)->shm_perm) +/* + * shm object validity is different than the rest of ipc + * as shm needs to deal with segments previously marked + * for deletion, which can occur at any time via user calls. + */ +static inline int shm_invalid_object(struct kern_ipc_perm *perm) +{ + if (perm->mode & SHM_DEST) + return -EINVAL; + if (ipc_valid_object(perm)) + return -EIDRM; + return 0; /* yay */ +} + static int newseg(struct ipc_namespace *, struct ipc_params *); static void shm_open(struct vm_area_struct *vma); static void shm_close(struct vm_area_struct *vma); @@ -985,11 +999,9 @@ SYSCALL_DEFINE3(shmctl, int, shmid, int, cmd, struct shmid_ds __user *, buf) ipc_lock_object(&shp->shm_perm); - /* check if shm_destroy() is tearing down shp */ - if (!ipc_valid_object(&shp->shm_perm)) { - err = -EIDRM; + err = shm_invalid_object(&shp->shm_perm); + if (err) goto out_unlock0; - } if (!ns_capable(ns->user_ns, CAP_IPC_LOCK)) { kuid_t euid = current_euid(); @@ -1124,10 +1136,9 @@ long do_shmat(int shmid, char __user *shmaddr, int shmflg, ulong *raddr, ipc_lock_object(&shp->shm_perm); - /* check if shm_destroy() is tearing down shp */ - if (!ipc_valid_object(&shp->shm_perm)) { + err = shm_invalid_object(&shp->shm_perm); + if (err) { ipc_unlock_object(&shp->shm_perm); - err = -EIDRM; goto out_unlock; } -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/