Re: [glibc PATCH] fcntl: put F_OFD_* constants under #ifdef __USE_FILE_OFFSET64

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]
  Powered by Linux