From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qt0-f178.google.com ([209.85.216.178]:33890 "EHLO mail-qt0-f178.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752435AbcHQSVR (ORCPT ); Wed, 17 Aug 2016 14:21:17 -0400 Received: by mail-qt0-f178.google.com with SMTP id u25so52871496qtb.1 for ; Wed, 17 Aug 2016 11:21:16 -0700 (PDT) Message-ID: <1471458074.3196.67.camel@redhat.com> Subject: Re: [glibc PATCH] fcntl: put F_OFD_* constants under #ifdef __USE_FILE_OFFSET64 From: Jeff Layton To: Florian Weimer , libc-alpha@sourceware.org Cc: linux-fsdevel@vger.kernel.org, Michael Kerrisk , Carlos O'Donell , Yuriy Kolerov Date: Wed, 17 Aug 2016 14:21:14 -0400 In-Reply-To: References: <1471445251-2450-1-git-send-email-jlayton@redhat.com> <024779d0-2800-8e43-b65c-180eca70cc8b@redhat.com> <1471455596.3196.36.camel@redhat.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: linux-fsdevel-owner@vger.kernel.org List-ID: 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. 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. > > 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. -- Jeff Layton