From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com ([209.132.183.28]:55208 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752626AbcHQSvq (ORCPT ); Wed, 17 Aug 2016 14:51:46 -0400 Subject: Re: [glibc PATCH] fcntl: put F_OFD_* constants under #ifdef __USE_FILE_OFFSET64 To: Jeff Layton , libc-alpha@sourceware.org References: <1471445251-2450-1-git-send-email-jlayton@redhat.com> <024779d0-2800-8e43-b65c-180eca70cc8b@redhat.com> <1471455596.3196.36.camel@redhat.com> <1471458074.3196.67.camel@redhat.com> Cc: linux-fsdevel@vger.kernel.org, Michael Kerrisk , "Carlos O'Donell" , Yuriy Kolerov From: Florian Weimer Message-ID: <9a96f830-ad56-91fe-1293-1d38d9195e49@redhat.com> Date: Wed, 17 Aug 2016 20:51:42 +0200 MIME-Version: 1.0 In-Reply-To: <1471458074.3196.67.camel@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On 08/17/2016 08:21 PM, Jeff Layton wrote: > On Wed, 2016-08-17 at 20:02 +0200, Florian Weimer wrote: >> On 08/17/2016 07:39 PM, Jeff Layton wrote: >>> >>> On Wed, 2016-08-17 at 19:34 +0200, Florian Weimer wrote: >>>> >>>> On 08/17/2016 04:47 PM, Jeff Layton wrote: >>>>> >>>>> >>>>> The Linux kernel expects a flock64 structure whenever you use >>>>> OFD locks >>>>> with fcntl64. Unfortunately, you can currently build a 32-bit >>>>> program >>>>> that passes in a struct flock when it calls fcntl64. >>>>> >>>>> Only define the F_OFD_* constants when __USE_FILE_OFFSET64 is >>>>> also >>>>> defined, so that the build fails in this situation rather than >>>>> producing a broken binary. >>>> >>>> Doesn't this affect legacy POSIX-style locks as well, under very >>>> similar >>>> circumstances? >>>> >>>> >>> >>> No. The kernel will decide which type of struct it is based on >>> whether >>> userland passes in F_SETLK or F_SETLK64. >> >> Let me see if I can sort this out. Is the situation like this? >> >> _FILE_OFFSET_… …BITS == 32 …BITS == 64 >> struct … flock flock64 flock flock64 >> fcntl (F_SETLK) ok BAD ok BAD >> fcntl (F_SETLK64) BAD ok ok ok >> fcntl (F_OFD_SETLK) BAD ok¹ ok ok >> >> ¹ is broken by your patch, right? > > Not sure I 100% understand your chart, but if I do then I think it's > more like: > > _FILE_OFFSET_… …BITS == 32 …BITS == 64 > struct … flock flock64 flock flock64 > fcntl (F_SETLK) ok BAD ok ok > fcntl (F_SETLK64) BAD ok ok ok > fcntl (F_OFD_SETLK) BAD ok¹ ok ok > > struct flock and struct flock64 are generally equivalent when > _FILE_OFFSET_BITS==64. Why would the F_SETLK operation work with a struct flock64 in _FILE_OFFSET_BITS == 64 mode? I think the kernel still expects a 32-bit struct. glibc does not look at O_LARGEFILE and alters size expectations. Neither does the kernel. > I don't quite understand how ¹ would be broken by this patch. The idea > with the patch is to ensure that if you haven't defined > _FILE_OFFSET_BITS=64 on a 32 bit arch, that it's broken at compile time > instead of at runtime. Compile time breakage is still breakage. I want to avoid another strerror_r situation where it's very hard to get the job done due to the way the preprocessor conditionals work out. >> Looking at the definition of struct flock and struct flock64, the >> risk >> is that application silently succeed in locking the wrong thing when >> using struct flock64 with a 32-it interface. >> > > Yes. The basic problem is that the kernel will expect a struct flock64, > but if you don't set _FILE_OFFSET_BITS=64 glibc will pass in a legacy > struct flock instead. The kernel can then read beyond the end of the > struct. > > The bytes in l_start and l_len will be slurped into the kernel's > l_start field. The pid and whatever junk is beyond the struct will be > in the l_len and pid fields. > > It's also possible the program will get back EFAULT as well if > copy_from_user fails. I was mainly worried about the reverse case (calling 32-bit fcntl with struct flock64). But this cannot happen because glibc always calls fcntl64 on 32-bit architectures. Florian