From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ross Zwisler Subject: Re: [PATCH 01/17] mm: introduce MAP_SHARED_VALIDATE, a mechanism to safely define new mmap flags Date: Tue, 24 Oct 2017 15:21:19 -0600 Message-ID: <20171024212119.GB1611@linux.intel.com> References: <20171024152415.22864-1-jack@suse.cz> <20171024152415.22864-2-jack@suse.cz> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <20171024152415.22864-2-jack-AlSwsSmVLrQ@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: linux-nvdimm-bounces-hn68Rpc1hR1g9hUCZPvPmw@public.gmane.org Sender: "Linux-nvdimm" To: Jan Kara Cc: linux-xfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Andrew Morton , Arnd Bergmann , linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw@public.gmane.org, linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Christoph Hellwig , linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org, Andy Lutomirski , linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-ext4-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-Id: linux-api@vger.kernel.org On Tue, Oct 24, 2017 at 05:23:58PM +0200, Jan Kara wrote: > From: Dan Williams > > The mmap(2) syscall suffers from the ABI anti-pattern of not validating > unknown flags. However, proposals like MAP_SYNC need a mechanism to > define new behavior that is known to fail on older kernels without the > support. Define a new MAP_SHARED_VALIDATE flag pattern that is > guaranteed to fail on all legacy mmap implementations. > > It is worth noting that the original proposal was for a standalone > MAP_VALIDATE flag. However, when that could not be supported by all > archs Linus observed: > > I see why you *think* you want a bitmap. You think you want > a bitmap because you want to make MAP_VALIDATE be part of MAP_SYNC > etc, so that people can do > > ret = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED > | MAP_SYNC, fd, 0); > > and "know" that MAP_SYNC actually takes. > > And I'm saying that whole wish is bogus. You're fundamentally > depending on special semantics, just make it explicit. It's already > not portable, so don't try to make it so. > > Rename that MAP_VALIDATE as MAP_SHARED_VALIDATE, make it have a value > of 0x3, and make people do > > ret = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED_VALIDATE > | MAP_SYNC, fd, 0); > > and then the kernel side is easier too (none of that random garbage > playing games with looking at the "MAP_VALIDATE bit", but just another > case statement in that map type thing. > > Boom. Done. > > Similar to ->fallocate() we also want the ability to validate the > support for new flags on a per ->mmap() 'struct file_operations' > instance basis. Towards that end arrange for flags to be generically > validated against a mmap_supported_flags exported by 'struct > file_operations'. By default all existing flags are implicitly > supported, but new flags require MAP_SHARED_VALIDATE and > per-instance-opt-in. > > Cc: Jan Kara > Cc: Arnd Bergmann > Cc: Andy Lutomirski > Cc: Andrew Morton > Suggested-by: Christoph Hellwig > Suggested-by: Linus Torvalds > Signed-off-by: Dan Williams > Signed-off-by: Jan Kara Looks great. Reviewed-by: Ross Zwisler From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 25D4821CEB153 for ; Tue, 24 Oct 2017 14:17:36 -0700 (PDT) Date: Tue, 24 Oct 2017 15:21:19 -0600 From: Ross Zwisler Subject: Re: [PATCH 01/17] mm: introduce MAP_SHARED_VALIDATE, a mechanism to safely define new mmap flags Message-ID: <20171024212119.GB1611@linux.intel.com> References: <20171024152415.22864-1-jack@suse.cz> <20171024152415.22864-2-jack@suse.cz> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20171024152415.22864-2-jack@suse.cz> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: linux-nvdimm-bounces@lists.01.org Sender: "Linux-nvdimm" To: Jan Kara Cc: linux-xfs@vger.kernel.org, Andrew Morton , Arnd Bergmann , linux-nvdimm@lists.01.org, linux-api@vger.kernel.org, Christoph Hellwig , linux-mm@kvack.org, Andy Lutomirski , linux-fsdevel@vger.kernel.org, linux-ext4@vger.kernel.org List-ID: On Tue, Oct 24, 2017 at 05:23:58PM +0200, Jan Kara wrote: > From: Dan Williams > > The mmap(2) syscall suffers from the ABI anti-pattern of not validating > unknown flags. However, proposals like MAP_SYNC need a mechanism to > define new behavior that is known to fail on older kernels without the > support. Define a new MAP_SHARED_VALIDATE flag pattern that is > guaranteed to fail on all legacy mmap implementations. > > It is worth noting that the original proposal was for a standalone > MAP_VALIDATE flag. However, when that could not be supported by all > archs Linus observed: > > I see why you *think* you want a bitmap. You think you want > a bitmap because you want to make MAP_VALIDATE be part of MAP_SYNC > etc, so that people can do > > ret = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED > | MAP_SYNC, fd, 0); > > and "know" that MAP_SYNC actually takes. > > And I'm saying that whole wish is bogus. You're fundamentally > depending on special semantics, just make it explicit. It's already > not portable, so don't try to make it so. > > Rename that MAP_VALIDATE as MAP_SHARED_VALIDATE, make it have a value > of 0x3, and make people do > > ret = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED_VALIDATE > | MAP_SYNC, fd, 0); > > and then the kernel side is easier too (none of that random garbage > playing games with looking at the "MAP_VALIDATE bit", but just another > case statement in that map type thing. > > Boom. Done. > > Similar to ->fallocate() we also want the ability to validate the > support for new flags on a per ->mmap() 'struct file_operations' > instance basis. Towards that end arrange for flags to be generically > validated against a mmap_supported_flags exported by 'struct > file_operations'. By default all existing flags are implicitly > supported, but new flags require MAP_SHARED_VALIDATE and > per-instance-opt-in. > > Cc: Jan Kara > Cc: Arnd Bergmann > Cc: Andy Lutomirski > Cc: Andrew Morton > Suggested-by: Christoph Hellwig > Suggested-by: Linus Torvalds > Signed-off-by: Dan Williams > Signed-off-by: Jan Kara Looks great. Reviewed-by: Ross Zwisler _______________________________________________ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga09.intel.com ([134.134.136.24]:9667 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751267AbdJXVVZ (ORCPT ); Tue, 24 Oct 2017 17:21:25 -0400 Date: Tue, 24 Oct 2017 15:21:19 -0600 From: Ross Zwisler Subject: Re: [PATCH 01/17] mm: introduce MAP_SHARED_VALIDATE, a mechanism to safely define new mmap flags Message-ID: <20171024212119.GB1611@linux.intel.com> References: <20171024152415.22864-1-jack@suse.cz> <20171024152415.22864-2-jack@suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20171024152415.22864-2-jack@suse.cz> Sender: linux-xfs-owner@vger.kernel.org List-ID: List-Id: xfs To: Jan Kara Cc: Dan Williams , Ross Zwisler , Christoph Hellwig , linux-ext4@vger.kernel.org, linux-nvdimm@lists.01.org, linux-fsdevel@vger.kernel.org, linux-xfs@vger.kernel.org, linux-api@vger.kernel.org, linux-mm@kvack.org, Arnd Bergmann , Andy Lutomirski , Andrew Morton On Tue, Oct 24, 2017 at 05:23:58PM +0200, Jan Kara wrote: > From: Dan Williams > > The mmap(2) syscall suffers from the ABI anti-pattern of not validating > unknown flags. However, proposals like MAP_SYNC need a mechanism to > define new behavior that is known to fail on older kernels without the > support. Define a new MAP_SHARED_VALIDATE flag pattern that is > guaranteed to fail on all legacy mmap implementations. > > It is worth noting that the original proposal was for a standalone > MAP_VALIDATE flag. However, when that could not be supported by all > archs Linus observed: > > I see why you *think* you want a bitmap. You think you want > a bitmap because you want to make MAP_VALIDATE be part of MAP_SYNC > etc, so that people can do > > ret = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED > | MAP_SYNC, fd, 0); > > and "know" that MAP_SYNC actually takes. > > And I'm saying that whole wish is bogus. You're fundamentally > depending on special semantics, just make it explicit. It's already > not portable, so don't try to make it so. > > Rename that MAP_VALIDATE as MAP_SHARED_VALIDATE, make it have a value > of 0x3, and make people do > > ret = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED_VALIDATE > | MAP_SYNC, fd, 0); > > and then the kernel side is easier too (none of that random garbage > playing games with looking at the "MAP_VALIDATE bit", but just another > case statement in that map type thing. > > Boom. Done. > > Similar to ->fallocate() we also want the ability to validate the > support for new flags on a per ->mmap() 'struct file_operations' > instance basis. Towards that end arrange for flags to be generically > validated against a mmap_supported_flags exported by 'struct > file_operations'. By default all existing flags are implicitly > supported, but new flags require MAP_SHARED_VALIDATE and > per-instance-opt-in. > > Cc: Jan Kara > Cc: Arnd Bergmann > Cc: Andy Lutomirski > Cc: Andrew Morton > Suggested-by: Christoph Hellwig > Suggested-by: Linus Torvalds > Signed-off-by: Dan Williams > Signed-off-by: Jan Kara Looks great. Reviewed-by: Ross Zwisler From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Tue, 24 Oct 2017 15:21:19 -0600 From: Ross Zwisler To: Jan Kara Cc: Dan Williams , Ross Zwisler , Christoph Hellwig , linux-ext4@vger.kernel.org, linux-nvdimm@lists.01.org, linux-fsdevel@vger.kernel.org, linux-xfs@vger.kernel.org, linux-api@vger.kernel.org, linux-mm@kvack.org, Arnd Bergmann , Andy Lutomirski , Andrew Morton Subject: Re: [PATCH 01/17] mm: introduce MAP_SHARED_VALIDATE, a mechanism to safely define new mmap flags Message-ID: <20171024212119.GB1611@linux.intel.com> References: <20171024152415.22864-1-jack@suse.cz> <20171024152415.22864-2-jack@suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20171024152415.22864-2-jack@suse.cz> Sender: owner-linux-mm@kvack.org List-ID: On Tue, Oct 24, 2017 at 05:23:58PM +0200, Jan Kara wrote: > From: Dan Williams > > The mmap(2) syscall suffers from the ABI anti-pattern of not validating > unknown flags. However, proposals like MAP_SYNC need a mechanism to > define new behavior that is known to fail on older kernels without the > support. Define a new MAP_SHARED_VALIDATE flag pattern that is > guaranteed to fail on all legacy mmap implementations. > > It is worth noting that the original proposal was for a standalone > MAP_VALIDATE flag. However, when that could not be supported by all > archs Linus observed: > > I see why you *think* you want a bitmap. You think you want > a bitmap because you want to make MAP_VALIDATE be part of MAP_SYNC > etc, so that people can do > > ret = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED > | MAP_SYNC, fd, 0); > > and "know" that MAP_SYNC actually takes. > > And I'm saying that whole wish is bogus. You're fundamentally > depending on special semantics, just make it explicit. It's already > not portable, so don't try to make it so. > > Rename that MAP_VALIDATE as MAP_SHARED_VALIDATE, make it have a value > of 0x3, and make people do > > ret = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED_VALIDATE > | MAP_SYNC, fd, 0); > > and then the kernel side is easier too (none of that random garbage > playing games with looking at the "MAP_VALIDATE bit", but just another > case statement in that map type thing. > > Boom. Done. > > Similar to ->fallocate() we also want the ability to validate the > support for new flags on a per ->mmap() 'struct file_operations' > instance basis. Towards that end arrange for flags to be generically > validated against a mmap_supported_flags exported by 'struct > file_operations'. By default all existing flags are implicitly > supported, but new flags require MAP_SHARED_VALIDATE and > per-instance-opt-in. > > Cc: Jan Kara > Cc: Arnd Bergmann > Cc: Andy Lutomirski > Cc: Andrew Morton > Suggested-by: Christoph Hellwig > Suggested-by: Linus Torvalds > Signed-off-by: Dan Williams > Signed-off-by: Jan Kara Looks great. Reviewed-by: Ross Zwisler -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org